Skip to content

Subquery expressions in UDTF arguments are parsed but elided #17349

Closed

Description

@davisp

Describe the bug

I got nerd sniped by a question in the DataFusion Discord channel into seeing if I couldn't implement something similar to BigQuery's gap_fill function. I managed to get the table variant working with slightly different syntax, but the interesting bit was when I tried to pass a Subquery expression as an argument. The bad query looks like such:

SELECT * FROM gap_fill((SELECT * FROM my_table), ts, INTERVAL 1 SECOND);

When I print out the &[Exprs] received by the gap_fill UDTF I get:

exprs: [
    Column(
        Column {
            relation: None,
            name: "ts",
        },
    ),
    Literal(
        IntervalMonthDayNano("IntervalMonthDayNano { months: 0, days: 0, nanoseconds: 1000000000 }"),
        None,
    ),
]

I would have expected to either see a Subquery expression to be included or for some other parser/logical_plan error. Either way, having no error but the argument being elided seems like a bug.

To Reproduce

// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements.  See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership.  The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License.  You may obtain a copy of the License at
//
//   http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied.  See the License for the
// specific language governing permissions and limitations
// under the License.

use std::sync::Arc;

use async_trait::async_trait;
use datafusion::arrow::datatypes::SchemaRef;
use datafusion::arrow::record_batch::RecordBatch;
use datafusion::catalog::Session;
use datafusion::catalog::TableFunctionImpl;
use datafusion::common::arrow::datatypes::Schema;
use datafusion::datasource::memory::MemorySourceConfig;
use datafusion::datasource::TableProvider;
use datafusion::error::Result;
use datafusion::logical_expr::{Expr, TableType};
use datafusion::physical_plan::ExecutionPlan;
use datafusion::prelude::*;

#[tokio::main]
async fn main() -> Result<()> {
    // create local execution context
    let ctx = SessionContext::new();

    // register the table function that will be called in SQL statements by `read_csv`
    ctx.register_udtf("gap_fill", Arc::new(GapFillTableFunc {}));

    let df = ctx
        .sql("CREATE TABLE test(ts TIMESTAMP, value BIGINT);")
        .await?;
    df.show().await?;

    let df = ctx
        .sql("SELECT * FROM gap_fill(test.ts, INTERVAL 1 seconds);")
        .await?;
    df.show().await?;

    let df = ctx
        .sql("SELECT * FROM gap_fill((SELECT * FROM test), ts, INTERVAL 1 seconds);")
        .await?;
    df.show().await?;

    Ok(())
}

#[derive(Debug)]
struct GapFillTableFunc {}

impl TableFunctionImpl for GapFillTableFunc {
    fn call(&self, exprs: &[Expr]) -> Result<Arc<dyn TableProvider>> {
        eprintln!("exprs: {:#?}", exprs);

        let schema = Arc::new(Schema::empty());

        let table = MyTable {
            schema,
            batches: Default::default(),
        };
        Ok(Arc::new(table))
    }
}

#[derive(Debug)]
struct MyTable {
    schema: SchemaRef,
    batches: Vec<RecordBatch>,
}

#[async_trait]
impl TableProvider for MyTable {
    fn as_any(&self) -> &dyn std::any::Any {
        self
    }

    fn schema(&self) -> SchemaRef {
        self.schema.clone()
    }

    fn table_type(&self) -> TableType {
        TableType::Base
    }

    async fn scan(
        &self,
        _state: &dyn Session,
        projection: Option<&Vec<usize>>,
        _filters: &[Expr],
        _limit: Option<usize>,
    ) -> Result<Arc<dyn ExecutionPlan>> {
        Ok(MemorySourceConfig::try_new_exec(
            &[self.batches.clone()],
            TableProvider::schema(self),
            projection.cloned(),
        )?)
    }
}

Output:

++
++
exprs: [
    Column(
        Column {
            relation: Some(
                Bare {
                    table: "test",
                },
            ),
            name: "ts",
        },
    ),
    Literal(
        IntervalMonthDayNano("IntervalMonthDayNano { months: 0, days: 0, nanoseconds: 1000000000 }"),
        None,
    ),
]
++
++
exprs: [
    Column(
        Column {
            relation: None,
            name: "ts",
        },
    ),
    Literal(
        IntervalMonthDayNano("IntervalMonthDayNano { months: 0, days: 0, nanoseconds: 1000000000 }"),
        None,
    ),
]
++
++

Expected behavior

I would expect to either see some sort of error or to have that Subquery expression be part of the expression list passed to the function. Silently discarding it seems like a bug. My personal preference would be allow Subquery expressions in arguments as it seems generally useful, but I may be missing a reason that's not possible.

Additional context

Also of note, I tried being a little sneaky and attempted to smuggle the subquery expression through one of the other Expr variants:

  1. (SELECT * FROM test) AS foo - Parse error at AS
  2. (SELECT * FROM test) OR true - Silently elided
  3. 1 IN (SELECT * FROM test) - Silently elided
  4. NOT (SELECT * FROM test) - Silently elided
  5. EXISTS (SELECT * FROM test) - Holy moly, this one actually works!

Might be interesting to run down why the IN variant doesn't work given that EXISTS does as both of those allow for subqueries while the others are fairly non-sensical.

Activity

  1. changed the title [-]Subquery expressions in UDTF's parsed but elided[/-] [+]Subquery expressions in UDTF arguments are parsed but elided[/+] on Aug 28, 2025
  2. davisp commented on Aug 28, 2025

    @davisp
    MemberAuthor

    The issue appears to be a combination of failed parsing along with a flat_map filtering out the error.

    We're converting the SQLExpr::Subquery to Expr::Subquery here, while in the context of a flat_map here.

    The return of parse_scalar_subquery is:

    Err(
        Diagnostic(
            Diagnostic {
                kind: Error,
                message: "Too many columns! The subquery should only return one column",
                span: None,
                notes: [],
                helps: [
                    DiagnosticHelp {
                        message: "Select only one column in the subquery",
                        span: None,
                    },
                ],
            },
            Plan(
                "Too many columns! The subquery should only return one column: test.ts, test.value",
            ),
        ),
    )
    

    So I'm guessing the bug here is that flat_map is swallowing the error rather than propagating it. I validated that changing the subquery to (SELECT 1 FROM test) properly includes the subquery.

    Although, does anyone know if there's a requirement that a function argument has to be a scalar value? Given that the function is getting the subquery and not the result of the subquery, it certainly seems like it should be up to the table function to decide if it wants a single column query or if an arbitrary number is ok, especially in the context of something like gap_fill. Looking back at the history, it appears that we could theoretically change that as part of this bug fix since the bug has existed since table functions were added two years ago. Given that there aren't any bugs filed against this, I'm gonna assume that virtually no one uses subquery expressions as table function arguments it or has only ever used perfectly formed scalar queries.

    Either way, that certainly feels like a follow up change that can be made separately if we decide to allow it.

  3. lhadhazy commented on Sep 11, 2026

    @lhadhazy

    This does not reproduce on main any more, and I think it can be closed.

    Checked on 6263df2ca (DataFusion CLI v55.0.0), using generate_series so that no custom table function is needed:

    -- baseline
    > SELECT * FROM generate_series(1, 3);
    +-------+
    | value |
    +-------+
    | 1     |
    | 2     |
    | 3     |
    +-------+
    
    -- the shape reported here: a subquery as one argument among several
    > SELECT * FROM generate_series((SELECT 1), 3);
    Error during planning: Arguments must be literals
    
    -- a subquery as the only argument
    > SELECT * FROM generate_series((SELECT 3));
    Error during planning: Arguments must be literals

    The error rather than the absence of one is the point, so here is the control that separates the two cases — a call with no arguments at all reports something different:

    > SELECT * FROM generate_series();
    Error during planning: generate_series function requires 1 to 3 arguments

    Since the one-argument subquery call reports Arguments must be literals and not the argument-count error, the subquery is reaching generate_series rather than being dropped on the way. That is the second of the two outcomes in the original report — an error instead of a silently missing argument.

    The change appears to be commit b09205afc, "fix: propagate errors for unsupported table function arguments instead of silently dropping them" (#21135, merged 2026-03-28), which replaced the .flat_map(...).collect::<Vec<_>>() in datafusion/sql/src/relation/mod.rs with .map(...).collect::<Result<Vec<_>>>()?. flat_map over a Result yields no elements for an Err, which is the mechanism described in the comment above. That PR closed #21125, which reported the same defect through a different symptom, and did not reference this issue, which is probably why this one stayed open.

    One gap is left behind, if it is of interest. The sqllogictest cases that PR added to table_functions.slt cover named arguments and wildcards, both of which take the else branch of that match. A subquery argument takes the other branch, through sql_expr_to_logical_expr and parse_scalar_subquery, and has no test. I am happy to open a small PR adding one so the behaviour this issue describes cannot regress unnoticed — happy to skip it if you would rather not grow the file.

  4. alamb commented on Oct 28, 2026

    @alamb
    Contributor

    Thanks, I agree this issue is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions