Skip to content

plan_to_sql produces incorrect SQL for optimised Aggregate plans #12773

Closed

Description

@samuelcolvin

Update, see #12773 (comment) for slightly different bug using HEAD.


Describe the bug

I'm seeing

thread 'main' panicked at /Users/samuel/.cargo/registry/src/index.crates.io-6f17d22bba15001f/datafusion-common-42.0.0/src/dfschema.rs:341:31:
index out of bounds: the len is 2 but the index is 2

When trying to create SQL from a logical plan, it looks like the problem is that the TableScan within the Aggregate has

                projection: Some(
                    [
                        0,
                        2,
                    ],
                ),

Which Unparse is ignoring when it tries iterates over schema/columns

To Reproduce

use std::sync::Arc;

use datafusion::arrow::datatypes::{DataType, Field, Schema, TimeUnit};
use datafusion::common::Result;
use datafusion::datasource::empty::EmptyTable;
use datafusion::prelude::{SessionContext};
use datafusion::sql::unparser::plan_to_sql;

#[tokio::main]
async fn main() -> Result<()> {
    let schema = Arc::new(Schema::new(vec![
        Field::new("timestamp", DataType::Timestamp(TimeUnit::Microsecond, None), false),
        Field::new("service", DataType::Utf8, true),
        Field::new("value", DataType::Int64, true),
    ]));

    let ctx = SessionContext::new();
    ctx.register_table("records", Arc::new(EmptyTable::new(schema)))?;

    let sql = "SELECT date_trunc('hour', timestamp), avg(value) from records group by 1 order by 1 desc";
    let df = ctx.sql(sql).await?;
    let logical_plan = ctx.state().optimize(df.logical_plan())?;

    let p = plan_to_sql(&logical_plan)?;
    println!("sql: {}", p);
    Ok(())
}

Without the ctx.state().optimize(...) this runs fine.

Expected behavior

Ideally this should result in valid SQL, if not then an Err with a useful description would be better.

Additional context

No response

Activity

  1. ahirner commented on Oct 6, 2024

    @ahirner
    Contributor

    Were you able to reproduce it on HEAD? The function passed on 6f8c74c with:

    sql: SELECT records."timestamp", records."value" FROM records ORDER BY "date_trunc(Utf8(""hour""),records.timestamp)" DESC NULLS FIRST
    

    PS: interested in this issue, since I got out-of-bounds panics in optimizing plans with wildcard projections. In my case a call to Analyzer::execute_and_check was missing though.

  2. samuelcolvin commented on Oct 6, 2024

    @samuelcolvin
    ContributorAuthor

    Okay, some progress using main @ 6f8c74c, it doesn't panic, but (almost worse) the SQL is wrong.

    With just df.logical_plan(), I get (looks correct):

    SELECT date_trunc('hour', records."timestamp"), avg(records."value") FROM records GROUP BY date_trunc('hour', records."timestamp") ORDER BY "date_trunc(Utf8(""hour""),records.timestamp)" DESC NULLS FIRST

    With ctx.state().optimize(df.logical_plan())? I get

    SELECT records."timestamp", records."value" FROM records ORDER BY "date_trunc(Utf8(""hour""),records.timestamp)" DESC NULLS FIRST```
  3. changed the title [-]`plan_to_sql` panics for some valid logical expressions [/-] [+]`plan_to_sql` produces incorrect SQL for optimised `Aggregate` plans[/+] on Oct 6, 2024
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