Skip to content

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: apache/datafusion-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
Updates the requirements on [sqlparser](https://github.com/sqlparser-rs/sqlparser-rs) to permit the latest version.
- [Release notes](https://github.com/sqlparser-rs/sqlparser-rs/releases)
- [Changelog](https://github.com/sqlparser-rs/sqlparser-rs/blob/main/CHANGELOG.md)
- [Commits](apache/datafusion-sqlparser-rs@v0.30.0...v0.31.0)

---
updated-dependencies:
- dependency-name: sqlparser
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@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

@alamb alamb deleted the sqlparser branch October 18, 2024 20:46
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