From 34a5fe50c2b7017ea7431084ede5f4f011f802cb Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 9 Oct 2024 12:05:58 -0700 Subject: [PATCH] Disable join types in validators (#3056) (#3060) * Disable join types in validators * Removed methods from SQLQueryValidationVisitor due to grammar file change --------- (cherry picked from commit ac8678c60c3e98c7e3791b2abb2b0e71c643256c) Signed-off-by: Tomoyuki Morita Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../S3GlueGrammarElementValidator.java | 10 +++++++ .../validator/SQLQueryValidationVisitor.java | 28 ------------------ .../SecurityLakeGrammarElementValidator.java | 10 +++++++ .../validator/SQLQueryValidatorTest.java | 29 ++++++++----------- 4 files changed, 32 insertions(+), 45 deletions(-) diff --git a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/S3GlueGrammarElementValidator.java b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/S3GlueGrammarElementValidator.java index e7a0ce1b36..9ed1fd9e9e 100644 --- a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/S3GlueGrammarElementValidator.java +++ b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/S3GlueGrammarElementValidator.java @@ -9,20 +9,25 @@ import static org.opensearch.sql.spark.validator.GrammarElement.CLUSTER_BY; import static org.opensearch.sql.spark.validator.GrammarElement.CREATE_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.CREATE_VIEW; +import static org.opensearch.sql.spark.validator.GrammarElement.CROSS_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.DESCRIBE_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.DISTRIBUTE_BY; import static org.opensearch.sql.spark.validator.GrammarElement.DROP_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.DROP_VIEW; import static org.opensearch.sql.spark.validator.GrammarElement.FILE; +import static org.opensearch.sql.spark.validator.GrammarElement.FULL_OUTER_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.HINTS; import static org.opensearch.sql.spark.validator.GrammarElement.INLINE_TABLE; import static org.opensearch.sql.spark.validator.GrammarElement.INSERT; +import static org.opensearch.sql.spark.validator.GrammarElement.LEFT_ANTI_JOIN; +import static org.opensearch.sql.spark.validator.GrammarElement.LEFT_SEMI_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.LOAD; import static org.opensearch.sql.spark.validator.GrammarElement.MANAGE_RESOURCE; import static org.opensearch.sql.spark.validator.GrammarElement.MISC_FUNCTIONS; import static org.opensearch.sql.spark.validator.GrammarElement.REFRESH_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.REFRESH_RESOURCE; import static org.opensearch.sql.spark.validator.GrammarElement.RESET; +import static org.opensearch.sql.spark.validator.GrammarElement.RIGHT_OUTER_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.SET; import static org.opensearch.sql.spark.validator.GrammarElement.SHOW_FUNCTIONS; import static org.opensearch.sql.spark.validator.GrammarElement.SHOW_VIEWS; @@ -50,6 +55,11 @@ public class S3GlueGrammarElementValidator extends DenyListGrammarElementValidat HINTS, INLINE_TABLE, FILE, + CROSS_JOIN, + LEFT_SEMI_JOIN, + RIGHT_OUTER_JOIN, + FULL_OUTER_JOIN, + LEFT_ANTI_JOIN, TABLESAMPLE, TABLE_VALUED_FUNCTION, TRANSFORM, diff --git a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SQLQueryValidationVisitor.java b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SQLQueryValidationVisitor.java index 9ec0fb0109..d50503418e 100644 --- a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SQLQueryValidationVisitor.java +++ b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SQLQueryValidationVisitor.java @@ -9,15 +9,12 @@ import org.opensearch.sql.spark.antlr.parser.SqlBaseParser; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AddTableColumnsContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AddTablePartitionContext; -import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AlterClusterByContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AlterTableAlterColumnContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AlterViewQueryContext; -import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AlterViewSchemaBindingContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AnalyzeContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.AnalyzeTablesContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.CacheTableContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.ClearCacheContext; -import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.ClusterBySpecContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.CreateNamespaceContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.CreateTableContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.CreateTableLikeContext; @@ -81,7 +78,6 @@ import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.TransformClauseContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.TruncateTableContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.UncacheTableContext; -import org.opensearch.sql.spark.antlr.parser.SqlBaseParser.UnsetNamespacePropertiesContext; import org.opensearch.sql.spark.antlr.parser.SqlBaseParserBaseVisitor; /** This visitor validate grammar using GrammarElementValidator */ @@ -101,12 +97,6 @@ public Void visitSetNamespaceProperties(SetNamespacePropertiesContext ctx) { return super.visitSetNamespaceProperties(ctx); } - @Override - public Void visitUnsetNamespaceProperties(UnsetNamespacePropertiesContext ctx) { - validateAllowed(GrammarElement.ALTER_NAMESPACE); - return super.visitUnsetNamespaceProperties(ctx); - } - @Override public Void visitAddTableColumns(AddTableColumnsContext ctx) { validateAllowed(GrammarElement.ALTER_NAMESPACE); @@ -173,12 +163,6 @@ public Void visitRecoverPartitions(RecoverPartitionsContext ctx) { return super.visitRecoverPartitions(ctx); } - @Override - public Void visitAlterClusterBy(AlterClusterByContext ctx) { - validateAllowed(GrammarElement.ALTER_NAMESPACE); - return super.visitAlterClusterBy(ctx); - } - @Override public Void visitSetNamespaceLocation(SetNamespaceLocationContext ctx) { validateAllowed(GrammarElement.ALTER_NAMESPACE); @@ -191,12 +175,6 @@ public Void visitAlterViewQuery(AlterViewQueryContext ctx) { return super.visitAlterViewQuery(ctx); } - @Override - public Void visitAlterViewSchemaBinding(AlterViewSchemaBindingContext ctx) { - validateAllowed(GrammarElement.ALTER_VIEW); - return super.visitAlterViewSchemaBinding(ctx); - } - @Override public Void visitRenameTable(RenameTableContext ctx) { if (ctx.VIEW() != null) { @@ -337,12 +315,6 @@ public Void visitCtes(CtesContext ctx) { return super.visitCtes(ctx); } - @Override - public Void visitClusterBySpec(ClusterBySpecContext ctx) { - validateAllowed(GrammarElement.CLUSTER_BY); - return super.visitClusterBySpec(ctx); - } - @Override public Void visitQueryOrganization(QueryOrganizationContext ctx) { if (ctx.CLUSTER() != null) { diff --git a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SecurityLakeGrammarElementValidator.java b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SecurityLakeGrammarElementValidator.java index ca8f2b5bdd..7dd2b0ee89 100644 --- a/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SecurityLakeGrammarElementValidator.java +++ b/async-query-core/src/main/java/org/opensearch/sql/spark/validator/SecurityLakeGrammarElementValidator.java @@ -14,6 +14,7 @@ import static org.opensearch.sql.spark.validator.GrammarElement.CREATE_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.CREATE_NAMESPACE; import static org.opensearch.sql.spark.validator.GrammarElement.CREATE_VIEW; +import static org.opensearch.sql.spark.validator.GrammarElement.CROSS_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.CSV_FUNCTIONS; import static org.opensearch.sql.spark.validator.GrammarElement.DESCRIBE_FUNCTION; import static org.opensearch.sql.spark.validator.GrammarElement.DESCRIBE_NAMESPACE; @@ -24,9 +25,12 @@ import static org.opensearch.sql.spark.validator.GrammarElement.DROP_NAMESPACE; import static org.opensearch.sql.spark.validator.GrammarElement.DROP_VIEW; import static org.opensearch.sql.spark.validator.GrammarElement.FILE; +import static org.opensearch.sql.spark.validator.GrammarElement.FULL_OUTER_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.HINTS; import static org.opensearch.sql.spark.validator.GrammarElement.INLINE_TABLE; import static org.opensearch.sql.spark.validator.GrammarElement.INSERT; +import static org.opensearch.sql.spark.validator.GrammarElement.LEFT_ANTI_JOIN; +import static org.opensearch.sql.spark.validator.GrammarElement.LEFT_SEMI_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.LOAD; import static org.opensearch.sql.spark.validator.GrammarElement.MANAGE_RESOURCE; import static org.opensearch.sql.spark.validator.GrammarElement.MISC_FUNCTIONS; @@ -35,6 +39,7 @@ import static org.opensearch.sql.spark.validator.GrammarElement.REFRESH_TABLE; import static org.opensearch.sql.spark.validator.GrammarElement.REPAIR_TABLE; import static org.opensearch.sql.spark.validator.GrammarElement.RESET; +import static org.opensearch.sql.spark.validator.GrammarElement.RIGHT_OUTER_JOIN; import static org.opensearch.sql.spark.validator.GrammarElement.SET; import static org.opensearch.sql.spark.validator.GrammarElement.SHOW_COLUMNS; import static org.opensearch.sql.spark.validator.GrammarElement.SHOW_CREATE_TABLE; @@ -76,6 +81,11 @@ public class SecurityLakeGrammarElementValidator extends DenyListGrammarElementV HINTS, INLINE_TABLE, FILE, + CROSS_JOIN, + LEFT_SEMI_JOIN, + RIGHT_OUTER_JOIN, + FULL_OUTER_JOIN, + LEFT_ANTI_JOIN, TABLESAMPLE, TABLE_VALUED_FUNCTION, TRANSFORM, diff --git a/async-query-core/src/test/java/org/opensearch/sql/spark/validator/SQLQueryValidatorTest.java b/async-query-core/src/test/java/org/opensearch/sql/spark/validator/SQLQueryValidatorTest.java index 6726b56994..695a083809 100644 --- a/async-query-core/src/test/java/org/opensearch/sql/spark/validator/SQLQueryValidatorTest.java +++ b/async-query-core/src/test/java/org/opensearch/sql/spark/validator/SQLQueryValidatorTest.java @@ -34,11 +34,9 @@ private enum TestElement { // DDL Statements ALTER_DATABASE( "ALTER DATABASE inventory SET DBPROPERTIES ('Edit-date' = '01/01/2001');", - "ALTER DATABASE dbx.tab1 UNSET PROPERTIES ('winner');", "ALTER DATABASE dbx.tab1 SET LOCATION '/path/to/part/ways';"), ALTER_TABLE( "ALTER TABLE default.StudentInfo PARTITION (age='10') RENAME TO PARTITION (age='15');", - "ALTER TABLE dbx.tab1 UNSET TBLPROPERTIES ('winner');", "ALTER TABLE StudentInfo ADD columns (LastName string, DOB timestamp);", "ALTER TABLE StudentInfo ADD IF NOT EXISTS PARTITION (age=18);", "ALTER TABLE StudentInfo RENAME COLUMN name TO FirstName;", @@ -50,12 +48,10 @@ private enum TestElement { "ALTER TABLE StudentInfo DROP IF EXISTS PARTITION (age=18);", "ALTER TABLE dbx.tab1 PARTITION (a='1', b='2') SET LOCATION '/path/to/part/ways';", "ALTER TABLE dbx.tab1 RECOVER PARTITIONS;", - "ALTER TABLE dbx.tab1 CLUSTER BY NONE;", "ALTER TABLE dbx.tab1 SET LOCATION '/path/to/part/ways';"), ALTER_VIEW( "ALTER VIEW tempdb1.v1 RENAME TO tempdb1.v2;", - "ALTER VIEW tempdb1.v2 AS SELECT * FROM tempdb1.v1;", - "ALTER VIEW tempdb1.v2 WITH SCHEMA BINDING"), + "ALTER VIEW tempdb1.v2 AS SELECT * FROM tempdb1.v1;"), CREATE_DATABASE("CREATE DATABASE IF NOT EXISTS customer_db;\n"), CREATE_FUNCTION("CREATE FUNCTION simple_udf AS 'SimpleUdf' USING JAR '/tmp/SimpleUdf.jar';"), CREATE_TABLE( @@ -94,8 +90,7 @@ private enum TestElement { EXPLAIN("EXPLAIN SELECT * FROM my_table;"), COMMON_TABLE_EXPRESSION( "WITH cte AS (SELECT * FROM my_table WHERE age > 30) SELECT * FROM cte;"), - CLUSTER_BY_CLAUSE( - "SELECT * FROM my_table CLUSTER BY age;", "ALTER TABLE testTable CLUSTER BY (age);"), + CLUSTER_BY_CLAUSE("SELECT * FROM my_table CLUSTER BY age;"), DISTRIBUTE_BY_CLAUSE("SELECT * FROM my_table DISTRIBUTE BY name;"), GROUP_BY_CLAUSE("SELECT name, count(*) FROM my_table GROUP BY name;"), HAVING_CLAUSE("SELECT name, count(*) FROM my_table GROUP BY name HAVING count(*) > 1;"), @@ -370,12 +365,12 @@ void testS3glueQueries() { v.ng(TestElement.INLINE_TABLE); v.ng(TestElement.FILE); v.ok(TestElement.INNER_JOIN); - v.ok(TestElement.CROSS_JOIN); + v.ng(TestElement.CROSS_JOIN); v.ok(TestElement.LEFT_OUTER_JOIN); - v.ok(TestElement.LEFT_SEMI_JOIN); - v.ok(TestElement.RIGHT_OUTER_JOIN); - v.ok(TestElement.FULL_OUTER_JOIN); - v.ok(TestElement.LEFT_ANTI_JOIN); + v.ng(TestElement.LEFT_SEMI_JOIN); + v.ng(TestElement.RIGHT_OUTER_JOIN); + v.ng(TestElement.FULL_OUTER_JOIN); + v.ng(TestElement.LEFT_ANTI_JOIN); v.ok(TestElement.LIKE_PREDICATE); v.ok(TestElement.LIMIT_CLAUSE); v.ok(TestElement.OFFSET_CLAUSE); @@ -487,12 +482,12 @@ void testSecurityLakeQueries() { v.ng(TestElement.INLINE_TABLE); v.ng(TestElement.FILE); v.ok(TestElement.INNER_JOIN); - v.ok(TestElement.CROSS_JOIN); + v.ng(TestElement.CROSS_JOIN); v.ok(TestElement.LEFT_OUTER_JOIN); - v.ok(TestElement.LEFT_SEMI_JOIN); - v.ok(TestElement.RIGHT_OUTER_JOIN); - v.ok(TestElement.FULL_OUTER_JOIN); - v.ok(TestElement.LEFT_ANTI_JOIN); + v.ng(TestElement.LEFT_SEMI_JOIN); + v.ng(TestElement.RIGHT_OUTER_JOIN); + v.ng(TestElement.FULL_OUTER_JOIN); + v.ng(TestElement.LEFT_ANTI_JOIN); v.ok(TestElement.LIKE_PREDICATE); v.ok(TestElement.LIMIT_CLAUSE); v.ok(TestElement.OFFSET_CLAUSE);