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

Disallow unquoted literals in LIKE clause in DESCRIBE statement #1181

Conversation

Yury-Fridlyand
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand commented Dec 15, 2022

Description

Update SQL parser to disallow unquoted strings as identifiers wildcards clause.

To test this fix for #650 disable legacy engine first:

diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.>
index ab146404f..29c501117 100644
--- a/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java
+++ b/legacy/src/main/java/org/opensearch/sql/legacy/plugin/RestSqlAction.java
@@ -161,10 +161,11 @@ public class RestSqlAction extends BaseRestHandler {
                             QueryContext.getRequestId());
                         result.accept(channel);
                     } else {
-                        LOG.debug("[{}] Request {} is not supported and falling back to old SQL engine",
+                        LOG.info("[{}] Request {} is not supported and falling back to nowhere",
                             QueryContext.getRequestId(), newSqlRequest);
-                        QueryAction queryAction = explainRequest(client, sqlRequest, format);
-                        executeSqlRequest(request, queryAction, client, channel);
+                        throw new SQLFeatureDisabledException("buuuu");
+                        //QueryAction queryAction = explainRequest(client, sqlRequest, format);
+                        //executeSqlRequest(request, queryAction, client, channel);
                     }
                 } catch (Exception e) {
                     logAndPublishMetrics(e);
(

Before
With patch, without fix

opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE %time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE calcs;
Output longer than terminal width
Do you want to display data vertically for better visual effect? [y/N]

After

opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE flights COLUMNS LIKE %time%;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}
opensearchsql> DESCRIBE TABLES LIKE calcs;
{'reason': 'Invalid SQL query', 'details': 'buuuu', 'type': 'SQLFeatureDisabledException'}

Another queries for test:

DESCRIBE TABLES LIKE acc%;
SHOW TABLES LIKE %;

and

DESCRIBE TABLES LIKE 'acc%';
SHOW TABLES LIKE "%";

Notes

As discussed, we should prohibit usage of raw identifies in SQL queries where wildcards are allowed. A user should specify string literal instead (a quoted string with ' or "). Backtick quote ` is not accepted.
This behavior was copied from legacy engine and it is an erroneous approach.
The root cause of #650 is a keywordsCanBeId which was in the user query which cased V2 parser to fail and fall back to V1.

Breaking change for 3.x release. Don't backport.

See team review and discussion in Bit-Quill#154.

Issues Resolved

Fixes #650

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@Yury-Fridlyand Yury-Fridlyand added SQL breaking legacy Issues related to legacy query engine to be deprecated labels Dec 15, 2022
@Yury-Fridlyand Yury-Fridlyand marked this pull request as ready for review December 15, 2022 20:15
@Yury-Fridlyand Yury-Fridlyand requested a review from a team as a code owner December 15, 2022 20:15
@codecov-commenter

This comment was marked as spam.

@penghuo
Copy link
Collaborator

penghuo commented Jan 3, 2023

The syntax section should be changed.

@Yury-Fridlyand Yury-Fridlyand force-pushed the integ-unquoted-percent-in-like-clause branch from 6169755 to ac4875f Compare January 4, 2023 01:21
@Yury-Fridlyand
Copy link
Collaborator Author

The syntax section should be changed.

Rebased and updated in ac4875f.

Copy link
Collaborator

@dai-chen dai-chen 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 fix!

@dai-chen dai-chen merged commit 1b58f7d into opensearch-project:main Jan 4, 2023
@Yury-Fridlyand Yury-Fridlyand deleted the integ-unquoted-percent-in-like-clause branch January 4, 2023 18:26
YANG-DB pushed a commit to YANG-DB/sql that referenced this pull request Jan 6, 2023
…pensearch-project#1181)

* Allow quoted literals only in `DESCRIBE` and `SHOW` clauses. Tests.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase. - Typo fix.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Update syntax section.

Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Creation and updating 2.x branch (opensearch-project#6)

* Create the 2.x branch
* Add  workflow_dispatch: to github CLI

Signed-off-by: YANGDB <[email protected]>
YANG-DB added a commit to YANG-DB/sql that referenced this pull request Jan 6, 2023
…pensearch-project#1181) (#3)

* Allow quoted literals only in `DESCRIBE` and `SHOW` clauses. Tests.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase. - Typo fix.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Update syntax section.

Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
YANG-DB added a commit that referenced this pull request Jan 13, 2023
* remove sub-folders which upgraded as new repositories
 - sql-cli
 - sql-odbc
 - sql-jdbc
 - dashboards-query-workbench
 - add additional workflow run triggers
 - move the BI tests to jdbc/odbc
 - move the sql-cli outside and clone for the doctest build

Signed-off-by: YANGDB <[email protected]>

* Merge remote-tracking branch 'upstream/main'

Signed-off-by: YANGDB <[email protected]>

* add sql-jdbc dependendcy to gradle

Signed-off-by: YANGDB <[email protected]>

* Disallow unquoted literals in `LIKE` clause in `DESCRIBE` statement (#1181)

* Allow quoted literals only in `DESCRIBE` and `SHOW` clauses. Tests.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase. - Typo fix.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Update syntax section.

Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Creation and updating 2.x branch (#6)

* Create the 2.x branch
* Add  workflow_dispatch: to github CLI

Signed-off-by: YANGDB <[email protected]>

* remove sub-folders which upgraded as new repositories
 - sql-cli
 - sql-odbc
 - sql-jdbc
 - dashboards-query-workbench

Signed-off-by: YANG-DB <[email protected]>

* add additional workflow run triggers

Signed-off-by: YANGDB <[email protected]>

* Merge remote-tracking branch 'upstream/main'
 - move the BI tests to jdbc/odbc
 - move the sql-cli outside and clone for the doctest build

Signed-off-by: YANGDB <[email protected]>

* Disallow unquoted literals in `LIKE` clause in `DESCRIBE` statement (#1181) (#3)

* Allow quoted literals only in `DESCRIBE` and `SHOW` clauses. Tests.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix doctest after rebase. - Typo fix.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Update syntax section.

Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>

Signed-off-by: YANG-DB <[email protected]>
Signed-off-by: YANGDB <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: lior perry <[email protected]>
Co-authored-by: yang-db <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking legacy Issues related to legacy query engine to be deprecated SQL v3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] SQL describe query with column filter changes response format
4 participants