Skip to content

chore(deps): Update sqlparser to 0.54.0 - #14255

Merged
goldmedal merged 9 commits into
apache:mainfrom
alamb:alamb/upgrade_sqlparser
Feb 5, 2025
Merged

chore(deps): Update sqlparser to 0.54.0#14255
goldmedal merged 9 commits into
apache:mainfrom
alamb:alamb/upgrade_sqlparser

Conversation

@alamb

@alambalamb commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Keep up with latest SQL parser dependencies

What changes are included in this PR?

  1. Update to sqlparser to latest released
  2. Update DataFusion to use the latest APIs

Are these changes tested?

Yes by CI

Are there any user-facing changes?

@github-actionsgithub-actionsBot added sql SQL Planner logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) common Related to common crate labels Jan 23, 2025
alamb
alamb commented Jan 23, 2025
View reviewed changes
Comment threaddatafusion/common/src/column.rs Outdated
alamb
alamb commented Jan 23, 2025
View reviewed changes
Comment threaddatafusion/sqllogictest/test_files/interval.slt
----
5.000000000 secs

60 mons

@alambalambJan 23, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this certainly seems much more correct to me -- I also removed the now out of date comment.

Comment threaddatafusion/sql/src/expr/mod.rs
}

fn sql_subscript_to_expr(
fn sql_compound_field_access_to_expr(

@alambalambJan 23, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code to plan CompoundFieldAccess is directly copy/pasted from @goldmedal 's PR

@alambalamb mentioned this pull request Jan 23, 2025
Closed
@alambalamb changed the title chore(deps): Update sqlparser to 0.54.0 @alambchore(deps): Update sqlparser to 0.54.0Jan 24, 2025
@alamb
alamb marked this pull request as draft January 24, 2025 08:23
@alambalamb mentioned this pull request Jan 26, 2025
Closed
@alamb
alamb marked this pull request as ready for review January 27, 2025 17:55
alamb
alamb commented Jan 27, 2025
View reviewed changes
Comment threaddatafusion/common/src/column.rs
///
/// For example, `foo.bar` would be represented as a two element vector
/// `["foo", "bar"]`
fn from_idents(mut idents: Vec<String>) -> Option<Self> {

@alambalambJan 27, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the signature to be easier to work with (pass the vec by ownership rather than &mut)

Comment threaddatafusion/common/src/column.rs Outdated
goldmedal
goldmedal approved these changes Feb 5, 2025
View reviewed changes

@goldmedalgoldmedal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alamb. Overall look good to me 👍

Comment threaddatafusion/sql/src/planner.rs Outdated
Comment threaddatafusion/sqllogictest/test_files/joins.slt
Comment on lines +2626 to +2628
# left_join_using_qualified (snowflake syntax)
query error DataFusion error: This feature is not implemented: Invalid identifier in USING clause\. Expected single identifier, got t2\.c2
SELECT t1.c1, t2.c2 FROM test_partition_table t1 JOIN test_partition_table t2 USING (t2.c2) ORDER BY t2.c2;

@goldmedalgoldmedalFeb 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@goldmedal

goldmedal commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

@alamb There are some conflicts here.

@alamb

alamb commented Feb 5, 2025

Copy link
Copy Markdown
ContributorAuthor

@alamb There are some conflicts here.

Thank you for the review @goldmedal -- I have merged up and resolved the conflicts

@goldmedal
goldmedal merged commit 304488d into apache:mainFeb 5, 2025
@goldmedal

goldmedal commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

🚀

@alambalamb mentioned this pull request Mar 7, 2025
Closed
@alexwilcoxson-relalexwilcoxson-rel mentioned this pull request Apr 2, 2025
Closed
@alamb

alamb commented May 21, 2025

Copy link
Copy Markdown
ContributorAuthor

This appears to have caused a regression:

@alambalamb mentioned this pull request Jun 13, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonahgaojonahgaojonahgao left review comments
@goldmedalgoldmedalgoldmedal approved these changes

Assignees

No one assigned

Labels

commonRelated to common cratelogical-exprLogical plan and expressionssqlSQL PlannersqllogictestSQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alamb@goldmedal@jonahgao