Registering a record batch with the name json_table causes ParserError

[dadepo]

Describe the bug

Registering a record batch under the name json_table leads to a ParserError.

To Reproduce

You can find this repo https://github.com/dadepo/df-repro with code that can be used to reproduce the problem.

Running with the record batch registered with json_table will fail while the one registered with json_values_table runs successfully.

Expected behavior

I believe it should be possible to use json_table as an alias for a record batch.

Additional context

The issue is probably connected with this PR to add mysql's JSON_TABLE to sqlparser apache/datafusion-sqlparser-rs#1019

[Jefffrey]

I guess this is an unfortunate side effect of having JSON_TABLE be a keyword for a MySQL function.

You can remedy this by escaping it in the SQL via backticks to indicate it shouldn't be a keyword.

From your reproduction:

pub fn set_up_data_test() -> Result<SessionContext> {

    // define a schema.
    let schema = Arc::new(Schema::new(vec![
        Field::new("index", DataType::UInt8, false),
        Field::new("json_data", DataType::Utf8, true),
    ]));

    // define data.
    let batch = RecordBatch::try_new(
        schema,
        vec![
            Arc::new(UInt8Array::from_iter_values([1, 2, 3, 4, 5, 6, 7, 8])),
            Arc::new(StringArray::from(vec![
                Some(r#" { "this" : "is", "a": [ "test" ] } "#),
                Some(r#"{"a":[2,3.5,true,false,null,"x"]}"#),
                Some(r#"[ "one", "two" ]"#),
                Some(r#"123"#),
                Some(r#"12.3"#),
                Some(r#"true"#),
                Some(r#"false"#),
                None,
            ])),
        ],
    )?;

    // declare a new context
    let ctx = SessionContext::new();
    // declare a table in memory.
    ctx.register_batch("json_table", batch)?;

    // define a schema.
    let schema = Arc::new(Schema::new(vec![
        Field::new("index", DataType::UInt8, false),
        Field::new("json_data", DataType::Utf8, true),
    ]));

    let batch = RecordBatch::try_new(
        schema,
        vec![
            Arc::new(UInt8Array::from_iter_values([1, 2, 3, 4, 5, 6, 7, 8])),
            Arc::new(StringArray::from(vec![
                Some(r#" { "this" : "is", "a": [ "test" ] } "#),
                Some(r#"{"a":[2,3.5,true,false,null,"x"]}"#),
                Some(r#"[ "one", "two" ]"#),
                Some(r#"123"#),
                Some(r#"12.3"#),
                Some(r#"true"#),
                Some(r#"false"#),
                None,
            ])),
        ],
    )?;

    // declare a new context
    let ctx = SessionContext::new(); // <-------------- HERE: ctx redefined loses above registration of json_table
    // declare a table in memory.
    ctx.register_batch("json_value_table", batch)?;

    Ok(ctx)
}

#[tokio::main]
async fn main() -> Result<()> {

    let ctx = set_up_data_test()?;

    // Fails with Error: SQL(ParserError("Expected (, found: EOF"), None)
    let df = ctx
        .sql(
            r#"SELECT * FROM `json_table`"#, // <------- HERE: escape with backticks
        )
        .await?;

    df.clone().show().await?;


    Ok(())
}

• Technically double quotes could be used as well: r#"SELECT * FROM "json_table""#

I also noticed something strange in your reproduction code, where you redefine the ctx in set_up_data_test() meaning you lose the registration of the json_table table anyway.

• Taking as of dadepo/df-repro@05a0a2e
• Unless this was an intention redefinition of ctx for testing purposes

We could probably update the docs, like here for example https://arrow.apache.org/datafusion/user-guide/sql/select.html

To indicate quoting isn't only for respecting capitalization, but also for using keywords as table/column names

[dadepo]

I guess this is an unfortunate side effect of having JSON_TABLE be a keyword for a MySQL function.

Indeed.

You can remedy this by escaping it in the SQL via backticks to indicate it shouldn't be a keyword

But since Datafusion is agnostic of any specific database, I think this issue should be circumvent within Datafusion itself.

I actually wasted a huge amount of time, trying to figure out why my queries were failing with ParseError, and it was only by chance I spotted it was related to the name json_table. So indeed escaping with backticks is a workaround, it would be nicer if this can be handled within Datafusion itself - if possible.

[Jefffrey]

That makes sense. Looks like @viirya has submitted the PR to fix this in sqlparser-rs, so will need that to land, then wait for next sqlparser-rs release to fix this bug

[lovasoa]

I think the JSON_TABLE function is part of the SQL standard. So SELECT * FROM json_table is not valid SQL, and datafusion is correct in refusing it.

I think this issue should be closed. It's the old behavior of accepting the query that was incorrect.

[alamb]

Maybe we can improve the error message in the sql parser so it is clearer to people that they have unknowingly used a keyword and should use a different table name (or double quotes) 🤔

[viirya]

We can improve the error message, I think. I don't think the table function is well known among users as it is not supported by most engines. The current refusing of the query looks very confusing to end users.

[lovasoa]

It should be possible to support both json_table as an unquoted table name and as a table valued function call. I think it would be the most helpful behavior.