Update to sqlparser 0.17.0
#2500
Conversation
alamb
changed the title
| } | ||
| } | ||
|
|
||
| SQLExpr::ArrayIndex { obj, indexs } => { |
alamb
force-pushed
the
alamb/update_sqlparser2
branch
from
d7b0e4f to
23edcd2
Compare
alamb
changed the title
sqlparser 0.17.0
datafusion/core/src/sql/planner.rs
Outdated
| ScalarValue::Int64(Some(s.parse().unwrap())) | ||
| } | ||
| SQLExpr::Value(Value::SingleQuotedString(s)) => ScalarValue::Utf8(Some(s)), | ||
| SQLExpr::Identifier(Ident { value, .. }) => ScalarValue::Utf8(Some(value)), |
There was a problem hiding this comment.
Looks likes, there are mistakes in tests, I found that current tests are not valid:
In select.rs, you need to do some changes:
// " - is used for identifiers, but in this test, should be some_struct['bar'], because it's a name of the field, not an identifier.
let sql = "SELECT some_struct[\"bar\"] as l0 FROM structs LIMIT 3";
There was a problem hiding this comment.
Shall we do this as part of a follow on PR? Or would you like to do it as part of this one?
There was a problem hiding this comment.
You can do it in your PR. Probably, it's a good idea to drop this line and change tests to correct variant: some_struct['bar']. Right now, GetIndexedExpr doesn't support dynamic expression as key, and converting Identifier to ScalarValue::Utf8 is not correct, because it can be a variable for example.
|
Awesome! I am planning to submit a PR with dynamic key for I will do it tomorrow. |
|
|
||
| // Original column is micros, convert to millis and check timestamp | ||
| let sql = "SELECT some_struct[\"bar\"] as l0 FROM structs LIMIT 3"; | ||
| let sql = "SELECT some_struct['bar'] as l0 FROM structs LIMIT 3"; |
There was a problem hiding this comment.
See #2500 (comment) for an explanation of this change
3 participants
I am preparing a sqlparser release and this PR tests the new code with DataFusion as a double check. See apache/datafusion-sqlparser-rs#488Upgrades to newly released sqlparser 0.17
I did need to make some changes to sql parser to handle changes to
ArrayAccesswhich was partially based on changes from @ovr 's PR #2196