Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(deps): update sqlparser requirement from 0.30 to 0.32 w/ API update #5457

Merged
merged 4 commits into from
Mar 8, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Mar 2, 2023

Which issue does this PR close?

Closes #5451

Rationale for this change

New version of sqlparser was released and we need a few changes to DataFusion to acknowledge we don't support newly added syntax

What changes are included in this PR?

Changes from @dependabot in #5451 and updates to make ti compile

Are these changes tested?

Existing tests cover it

Are there any user-facing changes?

No

@github-actions github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner labels Mar 2, 2023
@alamb alamb marked this pull request as draft March 3, 2023 01:36
@alamb
Copy link
Contributor Author

alamb commented Mar 3, 2023

@ursabot benchmark help

@alamb
Copy link
Contributor Author

alamb commented Mar 3, 2023

🤔 looks like some real problem in sql-parser upgrade -- I will investigate

@alamb alamb self-assigned this Mar 6, 2023
@alamb
Copy link
Contributor Author

alamb commented Mar 6, 2023

Specifically

https://github.com/apache/arrow-datafusion/blob/21e33a3d1047aa97be60b2efb4516efdd1d2b6bb/datafusion/sql/tests/integration_test.rs#L491-L500

Fails with:



---- table_with_column_alias stdout ----
thread 'table_with_column_alias' panicked at 'called `Result::unwrap()` on an `Err` value: SQL(ParserError("Expected identifier, found: ,"))', datafusion/sql/tests/integration_test.rs:2412:34
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/panicking.rs:64:14
   2: core::result::unwrap_failed
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/result.rs:1791:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/result.rs:1113:23
   4: integration_test::quick_test
             at ./tests/integration_test.rs:2412:16
   5: integration_test::table_with_column_alias
             at ./tests/integration_test.rs:499:5
   6: integration_test::table_with_column_alias::{{closure}}
             at ./tests/integration_test.rs:491:30
   7: core::ops::function::FnOnce::call_once
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs:507:5
   8: core::ops::function::FnOnce::call_once
             at /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs:507:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@alamb
Copy link
Contributor Author

alamb commented Mar 6, 2023

Upstream issue: sqlparser-rs/sqlparser-rs#826

@alamb alamb changed the title build(deps): update sqlparser requirement from 0.30 to 0.31 w/ fixes build(deps): update sqlparser requirement from 0.30 to 0.32 w/ fixes Mar 6, 2023
dependabot bot and others added 4 commits March 6, 2023 13:21
@alamb alamb changed the title build(deps): update sqlparser requirement from 0.30 to 0.32 w/ fixes build(deps): update sqlparser requirement from 0.30 to 0.32 w/ API update Mar 6, 2023
@alamb alamb marked this pull request as ready for review March 6, 2023 18:34
@alamb alamb requested a review from Dandandan March 7, 2023 12:15
@alamb
Copy link
Contributor Author

alamb commented Mar 8, 2023

@andygrove if you have time to review this upgrade PR I would like to get it in prior to the release on Friday

@alamb
Copy link
Contributor Author

alamb commented Mar 8, 2023

Thank you @appletreeisyellow and @tustvold !

@alamb alamb merged commit 84530a2 into apache:main Mar 8, 2023
@ursabot
Copy link

ursabot commented Mar 8, 2023

Benchmark runs are scheduled for baseline = 8a1b133 and contender = 84530a2. 84530a2 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants