-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Support DuckDB style struct syntax #11214
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
955c1d0
to
5a046da
Compare
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
datafusion/sql/src/expr/mod.rs
Outdated
} | ||
} | ||
|
||
internal_err!("Expected a simplified result, but none was found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to return a not_impl_err
error?
User may only use SqlToRel
without a SessionState
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jayzhan211 -- this is so cool!
DataFusion CLI v39.0.0
> select {'foo': 1};
+------------------------------------+
| named_struct(Utf8("foo"),Int64(1)) |
+------------------------------------+
| {foo: 1} |
+------------------------------------+
1 row(s) fetched.
Elapsed 0.015 seconds.
> select {'foo': {'bar': 2}};
+--------------------------------------------------------------+
| named_struct(Utf8("foo"),named_struct(Utf8("bar"),Int64(2))) |
+--------------------------------------------------------------+
| {foo: {bar: 2}} |
+--------------------------------------------------------------+
1 row(s) fetched.
Elapsed 0.002 seconds.
I took the liberty of merging up from main to resolve a conflict, and since I had the PR open I also implemented @jonahgao 's suggestion and updated some comments. Hope you don't mind
THank you again
|
||
user_defined_sql_planners.push(extract_planner); | ||
} | ||
let user_defined_sql_planners: Vec<Arc<dyn UserDefinedSQLPlanner>> = vec![ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also took the opportunity while I had this PR checked out to resolve a conflict to consolidate the code in the constructor
cc @dharanad and @xinlifoobar
* struct literal Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add nested Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fmt Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rm useless comment Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * switch to NYI error, derive debug/clone * improve documentation strings * Avoid stack overflow by putting code in a new function --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* struct literal Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * add nested Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * fmt Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * rm useless comment Signed-off-by: jayzhan211 <jayzhan211@gmail.com> * switch to NYI error, derive debug/clone * improve documentation strings * Avoid stack overflow by putting code in a new function --------- Signed-off-by: jayzhan211 <jayzhan211@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Which issue does this PR close?
Closes #9820 .
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?