Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support queries end with semi colon #609

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Support queries end with semi colon #609

merged 4 commits into from
Jul 29, 2020

Conversation

dai-chen
Copy link
Member

@dai-chen dai-chen commented Jul 22, 2020 •
edited
Loading

Issue #, if available: #603

Description of changes: Changed ANTLR grammar in both old and new engine to support optional semicolon. Removed semicolon before sending to old Druid parser (which is handwritten without grammar file to change).

Testing: Added new UT and IT. Based on our new test workflow, each legacy IT runs twice with/without new engine enabled.

Documentation: Added a sentence to declare the support for queries end with semicolon: https://github.com/dai-chen/sql/blob/support-semi-colon/docs/user/dql/basics.rst#syntax

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dai-chen dai-chen added enhancement New feature or request SQL labels Jul 22, 2020
@dai-chen dai-chen self-assigned this Jul 22, 2020
@dai-chen dai-chen marked this pull request as ready for review July 23, 2020 16:23
penghuo
penghuo approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

Thanks for the change.

chloe-zh
chloe-zh approved these changes Jul 28, 2020
View reviewed changes
Copy link
Member

@chloe-zh chloe-zh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes! BTW, we also need to support semi colon in workbench to adapt this changes since the workbench is recognizing the semi colon as the separators among queries.

@dai-chen dai-chen merged commit 0211d7e into opendistro-for-elasticsearch:develop Jul 29, 2020
@dai-chen dai-chen deleted the support-semi-colon branch July 29, 2020 16:06
@dai-chen
Copy link
Member Author

LGTM, thanks for the changes! BTW, we also need to support semi colon in workbench to adapt this changes since the workbench is recognizing the semi colon as the separators among queries.

Semicolon at the end of a query is optional. Is there anything possibly broken in SQL Workbench in this case?

dai-chen added a commit that referenced this pull request Aug 4, 2020
@dai-chen
Merge #584, #600, #609 for OD 1.8 (#658)
a8ae9e0 
* Issue 584 Fix object/nested field select issue

* Issue 600 Fix CAST bool to integer issue

* Issue 609 Support queries end with semi colon
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chloe-zh chloe-zh chloe-zh approved these changes

@penghuo penghuo penghuo approved these changes

Assignees

@dai-chen dai-chen

Labels
enhancement New feature or request SQL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dai-chen @penghuo @chloe-zh