Skip to content

Commit

Permalink
Disable join types in validators (#3056)
Browse files Browse the repository at this point in the history
* Disable join types in validators

Signed-off-by: Tomoyuki Morita <[email protected]>

* Removed methods from SQLQueryValidationVisitor due to grammar file change

Signed-off-by: Tomoyuki Morita <[email protected]>

---------

Signed-off-by: Tomoyuki Morita <[email protected]>
  • Loading branch information
ykmr1224 authored Oct 8, 2024
1 parent 564ab60 commit ac8678c
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand All @@ -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(
Expand Down Expand Up @@ -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;"),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ac8678c

Please sign in to comment.