Disallow unquoted literals in LIKE clause in DESCRIBE statement

[Yury-Fridlyand]

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

Description

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

To test this fix for opensearch-project#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 opensearch-project#650 is a keywordsCanBeId which was in the user query which cased V2 parser to fail and fall back to V1.

Issues Resolved

opensearch-project#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.

[MitchellGale]

LGTM :D

[forestmvey]

FYI you spelled unquoted wrong in the branch names.

[forestmvey]

Looks good. FYI you spelled unquoted wrong in the branch names.

[Yury-Fridlyand]

FYI you spelled unquoted wrong in the branch names.

D'oh, again. I renamed integ branch, the dev one will be deleted after merge.

[GumpacG]

Why remove the describe test case?

[Yury-Fridlyand]

Because I made it incompatible

[Yury-Fridlyand]

1. PPL does not require update. DESCRIBE command has the same syntax as source=, changing the syntax is unacceptable.
2. Syntax fix in legacy engine is more complicated, because parsing of the query performed not by ANTLR.
   

   opensearch-project-sql/legacy/src/main/java/org/opensearch/sql/legacy/domain/IndexStatement.java

   Lines 26 to 50 in 2b918a6

   	private void parseQuery() {
   	String[] statement = query.split(" ");
   	int tokenLength = statement.length;
   	try {
   	for (int i = 1; i < tokenLength; i++) {
   	switch (statement[i].toUpperCase()) {
   	case "TABLES":
   	if (i + 1 < tokenLength && statement[i + 1].equalsIgnoreCase("LIKE")) {
   	if (i + 2 < tokenLength) {
   	indexPattern = replaceWildcard(statement[i + 2]);
   	i += 2;
   	}
   	}
   	break;
   	case "COLUMNS":
   	if (i + 1 < tokenLength && statement[i + 1].equalsIgnoreCase("LIKE")) {
   	if (i + 2 < tokenLength) {
   	columnPattern = replaceWildcard(statement[i + 2]);
   	i += 2;
   	}
   	}
   	break;
   	}
   	}

[MaxKsyunz]

I'd make it clear in the title that only DESCRIBE statement is affected.