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

Presto SQL Parser Now Supports Trailing Commas #23927

Closed
wants to merge 8 commits into from

Conversation

chbhat10
Copy link

@chbhat10 chbhat10 commented Oct 31, 2024

Description

This pull request adds support for "trailing commas" in the SQL parser for Presto. This change allows users to include a trailing comma in their SELECT statements, aligning with the style supported by other SQL engines.

Motivation and Context

The change is motivated by two main reasons:

  1. As more SQL engines support trailing commas, it reduces the friction when switching between different SQL environments.
  2. It simplifies editing multiline queries, as users no longer need to worry about removing or adding commas when modifying the list of selected columns.
    This change addresses the issue "Support Google-style trailing comma in SELECT". Allowing trailing commas is a minor deviation from ANSI SQL, but it aligns with the style adopted by many programming languages like Python, Rust, and TypeScript, which enhances usability and reduces syntax errors.

Impact

This change introduces a new feature in the SQL parser, allowing trailing commas in SELECT statements. It does not affect existing functionality and is backward compatible.

Test Plan

The change has been tested by adding unit tests that cover various scenarios involving trailing commas in SELECT statements. These tests ensure that the parser correctly handles the new syntax without introducing regressions.

Contributor checklist

  • Please make sure your submission complies with our [development]
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • [?] If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • [?] CI passed. [Tested locally with maven, waiting for CI]

Copy link

linux-foundation-easycla bot commented Oct 31, 2024

CLA Not Signed

Copy link

Codenotify: Notifying subscribers in CODENOTIFY files for diff fcac3b4...d5c54a4.

Notify File(s)
@aditi-pandit presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@elharo presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@kaikalur presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
@rschlussel presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4

@chbhat10
Copy link
Author

Let me re-create this as I have to do the EasyCLA with the correct account(s)

@chbhat10 chbhat10 closed this Oct 31, 2024
@chbhat10
Copy link
Author

chbhat10 commented Oct 31, 2024

Recreated as 23928

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant