Repository navigation
Subquery expressions in UDTF arguments are parsed but elided #17349
Description
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:
(SELECT * FROM test) AS foo- Parse error atAS(SELECT * FROM test) OR true- Silently elided1 IN (SELECT * FROM test)- Silently elidedNOT (SELECT * FROM test)- Silently elidedEXISTS (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
[-]Subquery expressions in UDTF's parsed but elided[/-][+]Subquery expressions in UDTF arguments are parsed but elided[/+]davisp commentedon Aug 28, 2025
The issue appears to be a combination of failed parsing along with a flat_map filtering out the error.
We're converting the
SQLExpr::SubquerytoExpr::Subqueryhere, while in the context of aflat_maphere.The return of
parse_scalar_subqueryis:So I'm guessing the bug here is that
flat_mapis 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.
lhadhazy commentedon Sep 11, 2026
This does not reproduce on
mainany more, and I think it can be closed.Checked on
6263df2ca(DataFusion CLI v55.0.0), usinggenerate_seriesso that no custom table function is needed: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:
Since the one-argument subquery call reports
Arguments must be literalsand not the argument-count error, the subquery is reachinggenerate_seriesrather 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<_>>()indatafusion/sql/src/relation/mod.rswith.map(...).collect::<Result<Vec<_>>>()?.flat_mapover aResultyields no elements for anErr, 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.sltcover named arguments and wildcards, both of which take theelsebranch of thatmatch. A subquery argument takes the other branch, throughsql_expr_to_logical_exprandparse_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.alamb commentedon Oct 28, 2026
Thanks, I agree this issue is fixed