From 29969daa4784cce39615d04e0d93f76f53f32f26 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Tue, 4 Jan 2022 16:44:56 +0530 Subject: [PATCH 01/49] Initial commit, build working --- sql/pom.xml | 141 +++++- sql/src/main/codegen/config.fmpp | 430 ++++++++++++++++++ sql/src/main/codegen/includes/test.ftl | 11 + .../parser/DruidSqlParserImplFactory.java | 35 ++ .../sql/calcite/planner/PlannerFactory.java | 2 + 5 files changed, 618 insertions(+), 1 deletion(-) create mode 100644 sql/src/main/codegen/config.fmpp create mode 100644 sql/src/main/codegen/includes/test.ftl create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplFactory.java diff --git a/sql/pom.xml b/sql/pom.xml index 5b402c762102..71121246bc10 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -18,7 +18,8 @@ ~ under the License. --> - + 4.0.0 druid-sql @@ -255,6 +256,144 @@ + + + org.apache.maven.plugins + maven-dependency-plugin + 2.1 + + + + + unpack-parser-template + initialize + + unpack + + + + + org.apache.calcite + calcite-core + ${calcite.version} + jar + true + ${project.build.directory}/ + **/Parser.jj + + + org.apache.calcite + calcite-core + ${calcite.version} + jar + true + ${project.build.directory}/ + **/config.fmpp + + + + + + + + + com.googlecode.fmpp-maven-plugin + fmpp-maven-plugin + 1.0 + + + org.freemarker + freemarker + 2.3.25-incubating + + + + + + generate-fmpp-sources + generate-sources + + generate + + + ${project.build.directory}/codegen/config.fmpp + ${project.build.directory}/generated-sources/annotations + ${project.build.directory}/codegen/templates + + + + + + + + org.codehaus.mojo + javacc-maven-plugin + 2.4 + + + + generate-sources + javacc + + javacc + + + ${project.build.directory}/generated-sources/annotations + + **/Parser.jj + + 2 + false + ${project.build.directory}/generated-sources/annotations + + + + + + + maven-resources-plugin + 3.1.0 + + + copy-fmpp-resources + initialize + + copy-resources + + + ${project.build.directory}/codegen + + + src/main/codegen + false + + + + + + + + + + org.codehaus.mojo + build-helper-maven-plugin + 3.1.0 + + + add-source + generate-sources + + add-source + + + + src/generated-sources/annotations + + + + + diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp new file mode 100644 index 000000000000..8a54570ec54f --- /dev/null +++ b/sql/src/main/codegen/config.fmpp @@ -0,0 +1,430 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This file is an FMPP (http://fmpp.sourceforge.net/) configuration file to +# allow clients to extend Calcite's SQL parser to support application specific +# SQL statements, literals or data types. +# +# Calcite's parser grammar file (Parser.jj) is written in javacc +# (http://javacc.java.net/) with Freemarker (http://freemarker.org/) variables +# to allow clients to: +# 1. have custom parser implementation class and package name. +# 2. insert new parser method implementations written in javacc to parse +# custom: +# a) SQL statements. +# b) literals. +# c) data types. +# 3. add new keywords to support custom SQL constructs added as part of (2). +# 4. add import statements needed by inserted custom parser implementations. +# +# Parser template file (Parser.jj) along with this file are packaged as +# part of the calcite-core-.jar under "codegen" directory. + +data: { + parser: { + # Generated parser implementation package and class name. + package: "org.apache.druid.sql.calcite.parser", + class: "DruidSqlParserImpl", + + # List of additional classes and packages to import. + # Example. "org.apache.calcite.sql.*", "java.util.List". + imports: [ + ] + + # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved + # keyword add it to 'nonReservedKeywords' section. + keywords: [ + "TEST" + ] + + # List of keywords from "keywords" section that are not reserved. + nonReservedKeywords: [ + "A" + "ABSENT" + "ABSOLUTE" + "ACTION" + "ADA" + "ADD" + "ADMIN" + "AFTER" + "ALWAYS" + "APPLY" + "ASC" + "ASSERTION" + "ASSIGNMENT" + "ATTRIBUTE" + "ATTRIBUTES" + "BEFORE" + "BERNOULLI" + "BREADTH" + "C" + "CASCADE" + "CATALOG" + "CATALOG_NAME" + "CENTURY" + "CHAIN" + "CHARACTER_SET_CATALOG" + "CHARACTER_SET_NAME" + "CHARACTER_SET_SCHEMA" + "CHARACTERISTICS" + "CHARACTERS" + "CLASS_ORIGIN" + "COBOL" + "COLLATION" + "COLLATION_CATALOG" + "COLLATION_NAME" + "COLLATION_SCHEMA" + "COLUMN_NAME" + "COMMAND_FUNCTION" + "COMMAND_FUNCTION_CODE" + "COMMITTED" + "CONDITION_NUMBER" + "CONDITIONAL" + "CONNECTION" + "CONNECTION_NAME" + "CONSTRAINT_CATALOG" + "CONSTRAINT_NAME" + "CONSTRAINT_SCHEMA" + "CONSTRAINTS" + "CONSTRUCTOR" + "CONTINUE" + "CURSOR_NAME" + "DATA" + "DATABASE" + "DATETIME_INTERVAL_CODE" + "DATETIME_INTERVAL_PRECISION" + "DECADE" + "DEFAULTS" + "DEFERRABLE" + "DEFERRED" + "DEFINED" + "DEFINER" + "DEGREE" + "DEPTH" + "DERIVED" + "DESC" + "DESCRIPTION" + "DESCRIPTOR" + "DIAGNOSTICS" + "DISPATCH" + "DOMAIN" + "DOW" + "DOY" + "DYNAMIC_FUNCTION" + "DYNAMIC_FUNCTION_CODE" + "ENCODING" + "EPOCH" + "ERROR" + "EXCEPTION" + "EXCLUDE" + "EXCLUDING" + "FINAL" + "FIRST" + "FOLLOWING" + "FORMAT" + "FORTRAN" + "FOUND" + "FRAC_SECOND" + "G" + "GENERAL" + "GENERATED" + "GEOMETRY" + "GO" + "GOTO" + "GRANTED" + "HIERARCHY" + "IGNORE" + "IMMEDIATE" + "IMMEDIATELY" + "IMPLEMENTATION" + "INCLUDING" + "INCREMENT" + "INITIALLY" + "INPUT" + "INSTANCE" + "INSTANTIABLE" + "INVOKER" + "ISODOW" + "ISOYEAR" + "ISOLATION" + "JAVA" + "JSON" + "K" + "KEY" + "KEY_MEMBER" + "KEY_TYPE" + "LABEL" + "LAST" + "LENGTH" + "LEVEL" + "LIBRARY" + "LOCATOR" + "M" + "MAP" + "MATCHED" + "MAXVALUE" + "MICROSECOND" + "MESSAGE_LENGTH" + "MESSAGE_OCTET_LENGTH" + "MESSAGE_TEXT" + "MILLISECOND" + "MILLENNIUM" + "MINVALUE" + "MORE_" + "MUMPS" + "NAME" + "NAMES" + "NANOSECOND" + "NESTING" + "NORMALIZED" + "NULLABLE" + "NULLS" + "NUMBER" + "OBJECT" + "OCTETS" + "OPTION" + "OPTIONS" + "ORDERING" + "ORDINALITY" + "OTHERS" + "OUTPUT" + "OVERRIDING" + "PAD" + "PARAMETER_MODE" + "PARAMETER_NAME" + "PARAMETER_ORDINAL_POSITION" + "PARAMETER_SPECIFIC_CATALOG" + "PARAMETER_SPECIFIC_NAME" + "PARAMETER_SPECIFIC_SCHEMA" + "PARTIAL" + "PASCAL" + "PASSING" + "PASSTHROUGH" + "PAST" + "PATH" + "PLACING" + "PLAN" + "PLI" + "PRECEDING" + "PRESERVE" + "PRIOR" + "PRIVILEGES" + "PUBLIC" + "QUARTER" + "READ" + "RELATIVE" + "REPEATABLE" + "REPLACE" + "RESPECT" + "RESTART" + "RESTRICT" + "RETURNED_CARDINALITY" + "RETURNED_LENGTH" + "RETURNED_OCTET_LENGTH" + "RETURNED_SQLSTATE" + "RETURNING" + "ROLE" + "ROUTINE" + "ROUTINE_CATALOG" + "ROUTINE_NAME" + "ROUTINE_SCHEMA" + "ROW_COUNT" + "SCALAR" + "SCALE" + "SCHEMA" + "SCHEMA_NAME" + "SCOPE_CATALOGS" + "SCOPE_NAME" + "SCOPE_SCHEMA" + "SECTION" + "SECURITY" + "SELF" + "SEQUENCE" + "SERIALIZABLE" + "SERVER" + "SERVER_NAME" + "SESSION" + "SETS" + "SIMPLE" + "SIZE" + "SOURCE" + "SPACE" + "SPECIFIC_NAME" + "SQL_BIGINT" + "SQL_BINARY" + "SQL_BIT" + "SQL_BLOB" + "SQL_BOOLEAN" + "SQL_CHAR" + "SQL_CLOB" + "SQL_DATE" + "SQL_DECIMAL" + "SQL_DOUBLE" + "SQL_FLOAT" + "SQL_INTEGER" + "SQL_INTERVAL_DAY" + "SQL_INTERVAL_DAY_TO_HOUR" + "SQL_INTERVAL_DAY_TO_MINUTE" + "SQL_INTERVAL_DAY_TO_SECOND" + "SQL_INTERVAL_HOUR" + "SQL_INTERVAL_HOUR_TO_MINUTE" + "SQL_INTERVAL_HOUR_TO_SECOND" + "SQL_INTERVAL_MINUTE" + "SQL_INTERVAL_MINUTE_TO_SECOND" + "SQL_INTERVAL_MONTH" + "SQL_INTERVAL_SECOND" + "SQL_INTERVAL_YEAR" + "SQL_INTERVAL_YEAR_TO_MONTH" + "SQL_LONGVARBINARY" + "SQL_LONGVARNCHAR" + "SQL_LONGVARCHAR" + "SQL_NCHAR" + "SQL_NCLOB" + "SQL_NUMERIC" + "SQL_NVARCHAR" + "SQL_REAL" + "SQL_SMALLINT" + "SQL_TIME" + "SQL_TIMESTAMP" + "SQL_TINYINT" + "SQL_TSI_DAY" + "SQL_TSI_FRAC_SECOND" + "SQL_TSI_HOUR" + "SQL_TSI_MICROSECOND" + "SQL_TSI_MINUTE" + "SQL_TSI_MONTH" + "SQL_TSI_QUARTER" + "SQL_TSI_SECOND" + "SQL_TSI_WEEK" + "SQL_TSI_YEAR" + "SQL_VARBINARY" + "SQL_VARCHAR" + "STATE" + "STATEMENT" + "STRUCTURE" + "STYLE" + "SUBCLASS_ORIGIN" + "SUBSTITUTE" + "TABLE_NAME" + "TEMPORARY" + "TIES" + "TIMESTAMPADD" + "TIMESTAMPDIFF" + "TOP_LEVEL_COUNT" + "TRANSACTION" + "TRANSACTIONS_ACTIVE" + "TRANSACTIONS_COMMITTED" + "TRANSACTIONS_ROLLED_BACK" + "TRANSFORM" + "TRANSFORMS" + "TRIGGER_CATALOG" + "TRIGGER_NAME" + "TRIGGER_SCHEMA" + "TYPE" + "UNBOUNDED" + "UNCOMMITTED" + "UNCONDITIONAL" + "UNDER" + "UNNAMED" + "USAGE" + "USER_DEFINED_TYPE_CATALOG" + "USER_DEFINED_TYPE_CODE" + "USER_DEFINED_TYPE_NAME" + "USER_DEFINED_TYPE_SCHEMA" + "UTF8" + "UTF16" + "UTF32" + "VERSION" + "VIEW" + "WEEK" + "WRAPPER" + "WORK" + "WRITE" + "XML" + "ZONE" + ] + + # List of additional join types. Each is a method with no arguments. + # Example: LeftSemiJoin() + joinTypes: [ + ] + + # List of methods for parsing custom SQL statements. + # Return type of method implementation should be 'SqlNode'. + # Example: SqlShowDatabases(), SqlShowTables(). + statementParserMethods: [ + "Test()" + ] + + # List of methods for parsing custom literals. + # Return type of method implementation should be "SqlNode". + # Example: ParseJsonLiteral(). + literalParserMethods: [ + ] + + # List of methods for parsing custom data types. + # Return type of method implementation should be "SqlTypeNameSpec". + # Example: SqlParseTimeStampZ(). + dataTypeParserMethods: [ + ] + + # List of methods for parsing builtin function calls. + # Return type of method implementation should be "SqlNode". + # Example: DateFunctionCall(). + builtinFunctionCallMethods: [ + ] + + # List of methods for parsing extensions to "ALTER " calls. + # Each must accept arguments "(SqlParserPos pos, String scope)". + # Example: "SqlUploadJarNode" + alterStatementParserMethods: [ + ] + + # List of methods for parsing extensions to "CREATE [OR REPLACE]" calls. + # Each must accept arguments "(SqlParserPos pos, boolean replace)". + createStatementParserMethods: [ + ] + + # List of methods for parsing extensions to "DROP" calls. + # Each must accept arguments "(SqlParserPos pos)". + dropStatementParserMethods: [ + ] + + # Binary operators tokens + binaryOperatorsTokens: [ + ] + + # Binary operators initialization + extraBinaryExpressions: [ + ] + + # List of files in @includes directory that have parser method + # implementations for parsing custom SQL statements, literals or types + # given as part of "statementParserMethods", "literalParserMethods" or + # "dataTypeParserMethods". + implementationFiles: [ + "test.ftl" + ] + + includePosixOperators: false + includeCompoundIdentifier: true + includeBraces: true + includeAdditionalDeclarations: false + } +} + +freemarkerLinks: { + includes: includes/ +} \ No newline at end of file diff --git a/sql/src/main/codegen/includes/test.ftl b/sql/src/main/codegen/includes/test.ftl new file mode 100644 index 000000000000..715f4bd1ce59 --- /dev/null +++ b/sql/src/main/codegen/includes/test.ftl @@ -0,0 +1,11 @@ +/* + * Production for + * TEST + */ + +SqlNode Test(): +{} +{ + + { return null; } +} \ No newline at end of file diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplFactory.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplFactory.java new file mode 100644 index 000000000000..546ee8532d67 --- /dev/null +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserImplFactory.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.parser; + +import org.apache.calcite.sql.parser.SqlAbstractParserImpl; +import org.apache.calcite.sql.parser.SqlParserImplFactory; + +import java.io.Reader; + +public class DruidSqlParserImplFactory implements SqlParserImplFactory +{ + + @Override + public SqlAbstractParserImpl getParser(Reader stream) + { + return new DruidSqlParserImpl(stream); + } +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java index b1aa57654071..0631995f74f9 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java @@ -41,6 +41,7 @@ import org.apache.druid.server.security.Access; import org.apache.druid.server.security.AuthorizerMapper; import org.apache.druid.server.security.NoopEscalator; +import org.apache.druid.sql.calcite.parser.DruidSqlParserImplFactory; import org.apache.druid.sql.calcite.run.QueryMakerFactory; import org.apache.druid.sql.calcite.schema.DruidSchemaCatalog; import org.apache.druid.sql.calcite.schema.DruidSchemaName; @@ -57,6 +58,7 @@ public class PlannerFactory .setQuotedCasing(Casing.UNCHANGED) .setQuoting(Quoting.DOUBLE_QUOTE) .setConformance(DruidConformance.instance()) + .setParserFactory(new DruidSqlParserImplFactory()) // Custom sql parser factory .build(); private final DruidSchemaCatalog rootSchema; From f70aad95900e5b247be184fd7d42619fbde83270 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 5 Jan 2022 00:27:41 +0530 Subject: [PATCH 02/49] Add for SqlSelect and OrderBy --- sql/pom.xml | 3 - sql/src/main/codegen/config.fmpp | 6 +- sql/src/main/codegen/includes/select.ftl | 69 +++++++++++++++ .../sql/calcite/parser/DruidSqlOrderBy.java | 77 +++++++++++++++++ .../sql/calcite/parser/DruidSqlSelect.java | 83 +++++++++++++++++++ .../druid/sql/calcite/CalciteQueryTest.java | 32 +++++-- 6 files changed, 257 insertions(+), 13 deletions(-) create mode 100644 sql/src/main/codegen/includes/select.ftl create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java diff --git a/sql/pom.xml b/sql/pom.xml index 71121246bc10..16974c7d5e6e 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -263,7 +263,6 @@ 2.1 - unpack-parser-template @@ -310,7 +309,6 @@ - generate-fmpp-sources generate-sources @@ -332,7 +330,6 @@ 2.4 - generate-sources javacc diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 8a54570ec54f..045749935492 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -41,12 +41,14 @@ data: { # List of additional classes and packages to import. # Example. "org.apache.calcite.sql.*", "java.util.List". imports: [ + "org.apache.calcite.sql.SqlSelect" ] # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved # keyword add it to 'nonReservedKeywords' section. keywords: [ "TEST" + "CONTEXT" ] # List of keywords from "keywords" section that are not reserved. @@ -366,6 +368,7 @@ data: { # Example: SqlShowDatabases(), SqlShowTables(). statementParserMethods: [ "Test()" + "DruidSqlSelect()" ] # List of methods for parsing custom literals. @@ -415,7 +418,8 @@ data: { # given as part of "statementParserMethods", "literalParserMethods" or # "dataTypeParserMethods". implementationFiles: [ - "test.ftl" + "test.ftl", + "select.ftl" ] includePosixOperators: false diff --git a/sql/src/main/codegen/includes/select.ftl b/sql/src/main/codegen/includes/select.ftl new file mode 100644 index 000000000000..0b4d205bb14f --- /dev/null +++ b/sql/src/main/codegen/includes/select.ftl @@ -0,0 +1,69 @@ +SqlNode DruidSqlSelect() : +{ + org.apache.calcite.sql.SqlNode stmt; + org.apache.calcite.sql.SqlNode selectNode; + org.apache.calcite.sql.SqlNodeList contextParameters = null; +} +{ + selectNode = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY) + [ + + contextParameters = ParenthesizedKeyValueOptionCommaList() + ] + { + if(contextParameters != null) { + if(selectNode instanceof org.apache.calcite.sql.SqlSelect) { + return new org.apache.druid.sql.calcite.parser.DruidSqlSelect((org.apache.calcite.sql.SqlSelect) selectNode, contextParameters); + } else if(selectNode instanceof org.apache.calcite.sql.SqlOrderBy) { + return new org.apache.druid.sql.calcite.parser.DruidSqlOrderBy((org.apache.calcite.sql.SqlOrderBy) selectNode, contextParameters); + } + } + return selectNode; + } +} + + +/** +* The following has been directly copied from a later version of the Calcite's Parser.jj. This should be removed while +* upgrading the Calcite's version +*/ + +SqlNodeList ParenthesizedKeyValueOptionCommaList() : +{ + final Span s; + final List list = new ArrayList(); +} +{ + { s = span(); } + + KeyValueOption(list) + ( + + KeyValueOption(list) + )* + { + return new SqlNodeList(list, s.end(this)); + } +} + +/** +* Parses an option with format key=val whose key is a simple identifier or string literal +* and value is a string literal. +*/ +void KeyValueOption(List list) : +{ + final SqlNode key; + final SqlNode value; +} +{ + ( + key = SimpleIdentifier() + | + key = StringLiteral() + ) + + value = StringLiteral() { + list.add(key); + list.add(value); + } +} \ No newline at end of file diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java new file mode 100644 index 000000000000..584fe9146fc9 --- /dev/null +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.parser; + +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlOrderBy; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlSelectOperator; +import org.apache.calcite.sql.SqlWriter; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Extends the Select call to have custom context parameters specific to Druid + * These custom parameters are to be manually extracted. + * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) + */ +public class DruidSqlOrderBy extends SqlOrderBy +{ + // Unsure if this should be kept as is, but this allows reusing super.unparse + public static final SqlOperator OPERATOR = SqlOrderBy.OPERATOR; + private final SqlNodeList context; + + public DruidSqlOrderBy( + SqlOrderBy orderByNode, + @Nullable SqlNodeList context + ) + { + super( + orderByNode.getParserPosition(), + orderByNode.getOperandList().get(0), + (SqlNodeList) orderByNode.getOperandList().get(1), + orderByNode.getOperandList().get(2), + orderByNode.getOperandList().get(3) + ); + this.context = context; + } + + @Override + public SqlOperator getOperator() + { + return OPERATOR; + } + + public SqlNodeList getContext() + { + return context; + } + + + @Override + public void unparse(SqlWriter writer, int leftPrec, int rightPrec) + { + super.unparse(writer, leftPrec, rightPrec); + writer.keyword("DESCRIBE"); + writer.keyword("SPACE"); + writer.keyword("POWER"); + } + +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java new file mode 100644 index 000000000000..51efca6415ff --- /dev/null +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.parser; + +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlSelectOperator; +import org.apache.calcite.sql.SqlWriter; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Extends the Select call to have custom context parameters specific to Druid + * These custom parameters are to be manually extracted. + * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) + */ +public class DruidSqlSelect extends SqlSelect +{ + // Unsure if this should be kept as is, but this allows reusing super.unparse + public static final SqlOperator OPERATOR = SqlSelectOperator.INSTANCE; + + private final SqlNodeList context; + + public DruidSqlSelect( + SqlSelect selectNode, + @Nullable SqlNodeList context + ) + { + super( + selectNode.getParserPosition(), + (SqlNodeList) selectNode.getOperandList().get(0), // No better getter to extract this + selectNode.getSelectList(), + selectNode.getFrom(), + selectNode.getWhere(), + selectNode.getGroup(), + selectNode.getHaving(), + selectNode.getWindowList(), + selectNode.getOrderList(), + selectNode.getOffset(), + selectNode.getFetch() + ); + this.context = context; + } + + @Override + public SqlOperator getOperator() + { + return OPERATOR; + } + + public SqlNodeList getContext() + { + return context; + } + + + @Override + public void unparse(SqlWriter writer, int leftPrec, int rightPrec) + { + super.unparse(writer, leftPrec, rightPrec); + writer.keyword("DESCRIBE"); + writer.keyword("SPACE"); + writer.keyword("POWER"); + } + +} diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 01a994b8ae93..80a4246ecbbd 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -290,6 +290,23 @@ public void testInformationSchemaColumnsOnTable() throws Exception ); } + @Test + public void testtest() throws Exception + { + testQuery( + "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", + ImmutableList.of(), + ImmutableList.of() + ); + +// testQuery( +// "VALUES ( )", +// ImmutableList.of(), +// ImmutableList.of() +// ); + + } + @Test public void testInformationSchemaColumnsOnForbiddenTable() throws Exception { @@ -393,7 +410,6 @@ public void testAggregatorsOnInformationSchemaColumns() throws Exception } - @Test public void testTopNLimitWrapping() throws Exception { @@ -2268,7 +2284,7 @@ public void testGroupByWithSelectAndOrderByProjections() throws Exception + " SUBSTRING(dim1, 2)\n" + "FROM druid.foo\n" + "GROUP BY dim1\n" - + "ORDER BY CHARACTER_LENGTH(dim1) DESC, dim1", + + "ORDER BY CHARACTER_LENGTH(dim1) DESC, dim1 SET CONTEXT ('x'='11', 'y'='234')", ImmutableList.of( GroupByQuery.builder() .setDataSource(CalciteTests.DATASOURCE1) @@ -2607,7 +2623,8 @@ public void testUnionAllTablesColumnTypeMismatchStringLong() + "WHERE dim2 = 'a' OR dim2 = 'en'\n" + "GROUP BY 1, 2", "Possible error: SQL requires union between inputs that are not simple table scans and involve a " + - "filter or aliasing. Or column types of tables being unioned are not of same type."); + "filter or aliasing. Or column types of tables being unioned are not of same type." + ); } @Test @@ -2622,7 +2639,7 @@ public void testUnionAllTablesWhenMappingIsRequired() + "WHERE c = 'a' OR c = 'def'\n" + "GROUP BY 1", "Possible error: SQL requires union between two tables " + - "and column names queried for each table are different Left: [dim1], Right: [dim2]." + "and column names queried for each table are different Left: [dim1], Right: [dim2]." ); } @@ -2649,7 +2666,7 @@ public void testUnionAllTablesWhenCastAndMappingIsRequired() + "WHERE c = 'a' OR c = 'def'\n" + "GROUP BY 1", "Possible error: SQL requires union between inputs that are not simple table scans and involve " + - "a filter or aliasing. Or column types of tables being unioned are not of same type." + "a filter or aliasing. Or column types of tables being unioned are not of same type." ); } @@ -5437,7 +5454,7 @@ public void testCountStarWithTimeFilterUsingStringLiteralsInvalid_isUnplannable( // This error message isn't ideal but it is at least better than silently ignoring the problem. assertQueryIsUnplannable( "SELECT COUNT(*) FROM druid.foo\n" - + "WHERE __time >= 'z2000-01-01 00:00:00' AND __time < '2001-01-01 00:00:00'\n", + + "WHERE __time >= 'z2000-01-01 00:00:00' AND __time < '2001-01-01 00:00:00'\n", "Possible error: Illegal TIMESTAMP constant: CAST('z2000-01-01 00:00:00'):TIMESTAMP(3) NOT NULL" ); } @@ -7730,7 +7747,6 @@ public void testFilterOnCurrentTimestampWithIntervalArithmetic() throws Exceptio } - @Test public void testFilterOnCurrentTimestampLosAngeles() throws Exception { @@ -12361,7 +12377,6 @@ public void testLookupWithNull() throws Exception } - @Test public void testRoundFuc() throws Exception { @@ -12801,7 +12816,6 @@ public void testBitwiseAggregatorsGroupBy() throws Exception } - @Test public void testStringAgg() throws Exception { From 7b2c884064dda378963b2bae8c945aeb566cfc0e Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 6 Jan 2022 17:58:47 +0530 Subject: [PATCH 03/49] Integrate the changes such that set context works --- .../sql/calcite/parser/DruidSqlOrderBy.java | 33 +++++++-- .../sql/calcite/parser/DruidSqlSelect.java | 42 ++++++++--- .../sql/calcite/planner/DruidPlanner.java | 69 ++++++++++++++----- .../druid/sql/calcite/rel/DruidRel.java | 15 ++-- .../druid/sql/calcite/rel/DruidUnionRel.java | 13 +++- .../sql/calcite/run/NativeQueryMaker.java | 14 +++- .../druid/sql/calcite/run/QueryMaker.java | 8 +++ .../druid/sql/calcite/CalciteQueryTest.java | 16 ++--- 8 files changed, 163 insertions(+), 47 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java index 584fe9146fc9..0a2525ae30e7 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java @@ -19,14 +19,19 @@ package org.apache.druid.sql.calcite.parser; +import com.google.common.base.Preconditions; +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.SqlOrderBy; import org.apache.calcite.sql.SqlSelect; -import org.apache.calcite.sql.SqlSelectOperator; import org.apache.calcite.sql.SqlWriter; import org.checkerframework.checker.nullness.qual.Nullable; +import java.util.LinkedHashMap; +import java.util.Map; + /** * Extends the Select call to have custom context parameters specific to Druid * These custom parameters are to be manually extracted. @@ -36,11 +41,11 @@ public class DruidSqlOrderBy extends SqlOrderBy { // Unsure if this should be kept as is, but this allows reusing super.unparse public static final SqlOperator OPERATOR = SqlOrderBy.OPERATOR; - private final SqlNodeList context; + private final Map context; public DruidSqlOrderBy( SqlOrderBy orderByNode, - @Nullable SqlNodeList context + @Nullable SqlNodeList contextMapAsList ) { super( @@ -50,7 +55,25 @@ public DruidSqlOrderBy( orderByNode.getOperandList().get(2), orderByNode.getOperandList().get(3) ); - this.context = context; + this.context = new LinkedHashMap<>(); + if (contextMapAsList != null) { + int listSize = contextMapAsList.size(); + Preconditions.checkArgument(listSize % 2 == 0); + for (int i = 0; i < listSize / 2; ++i) { + SqlNode keyNode = contextMapAsList.get(2 * i); + SqlNode valueNode = contextMapAsList.get(2 * i + 1); + if (!(keyNode instanceof SqlLiteral)) { + // Log some warning about conversion + } + if (!(valueNode instanceof SqlLiteral)) { + // Log some warning about conversion + + } + String key = keyNode.toString(); + String value = valueNode.toString(); + context.put(key, value); + } + } } @Override @@ -59,7 +82,7 @@ public SqlOperator getOperator() return OPERATOR; } - public SqlNodeList getContext() + public Map getContext() { return context; } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java index 51efca6415ff..25d92e5db30f 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java @@ -19,12 +19,18 @@ package org.apache.druid.sql.calcite.parser; +import com.google.common.base.Preconditions; +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.SqlSelectOperator; import org.apache.calcite.sql.SqlWriter; -import org.checkerframework.checker.nullness.qual.Nullable; + +import javax.annotation.Nullable; +import java.util.LinkedHashMap; +import java.util.Map; /** * Extends the Select call to have custom context parameters specific to Druid @@ -36,11 +42,12 @@ public class DruidSqlSelect extends SqlSelect // Unsure if this should be kept as is, but this allows reusing super.unparse public static final SqlOperator OPERATOR = SqlSelectOperator.INSTANCE; - private final SqlNodeList context; + @Nullable + private final Map context; public DruidSqlSelect( SqlSelect selectNode, - @Nullable SqlNodeList context + @Nullable SqlNodeList contextMapAsList ) { super( @@ -56,7 +63,25 @@ public DruidSqlSelect( selectNode.getOffset(), selectNode.getFetch() ); - this.context = context; + context = new LinkedHashMap<>(); + if (contextMapAsList != null) { + int listSize = contextMapAsList.size(); + Preconditions.checkArgument(listSize % 2 == 0); + for (int i = 0; i < listSize / 2; ++i) { + SqlNode keyNode = contextMapAsList.get(2 * i); + SqlNode valueNode = contextMapAsList.get(2 * i + 1); + if (!(keyNode instanceof SqlLiteral)) { + // Log some warning about conversion + } + if (!(valueNode instanceof SqlLiteral)) { + // Log some warning about conversion + + } + String key = keyNode.toString(); + String value = valueNode.toString(); + context.put(key, value); + } + } } @Override @@ -65,7 +90,7 @@ public SqlOperator getOperator() return OPERATOR; } - public SqlNodeList getContext() + public Map getContext() { return context; } @@ -75,9 +100,10 @@ public SqlNodeList getContext() public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { super.unparse(writer, leftPrec, rightPrec); - writer.keyword("DESCRIBE"); - writer.keyword("SPACE"); - writer.keyword("POWER"); + if (context != null) { + writer.keyword("SET"); + writer.keyword("CONTEXT"); + } } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 621653591edb..797854355346 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -79,6 +79,8 @@ import org.apache.druid.server.security.Resource; import org.apache.druid.server.security.ResourceAction; import org.apache.druid.server.security.ResourceType; +import org.apache.druid.sql.calcite.parser.DruidSqlOrderBy; +import org.apache.druid.sql.calcite.parser.DruidSqlSelect; import org.apache.druid.sql.calcite.rel.DruidConvention; import org.apache.druid.sql.calcite.rel.DruidQuery; import org.apache.druid.sql.calcite.rel.DruidRel; @@ -94,6 +96,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.stream.Collectors; @@ -157,7 +160,7 @@ public ValidationResult validate() throws SqlParseException, ValidationException /** * Prepare an SQL query for execution, including some initial parsing and validation and any dynamic parameter type * resolution, to support prepared statements via JDBC. - * + *

* In some future this could perhaps re-use some of the work done by {@link #validate()} * instead of repeating it, but that day is not today. */ @@ -185,10 +188,10 @@ public PrepareResult prepare() throws SqlParseException, ValidationException, Re /** * Plan an SQL query for execution, returning a {@link PlannerResult} which can be used to actually execute the query. - * + *

* Ideally, the query can be planned into a native Druid query, using {@link #planWithDruidConvention}, but will * fall-back to {@link #planWithBindableConvention} if this is not possible. - * + *

* In some future this could perhaps re-use some of the work done by {@link #validate()} * instead of repeating it, but that day is not today. */ @@ -205,7 +208,10 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo final RelRoot rootQueryRel = planner.rel(validatedQueryNode); try { - return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(), parsed.getInsertNode()); + Map sqlContext = parsed.getContextMap().entrySet().stream().collect( + Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue) + ); + return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(), parsed.getInsertNode(), sqlContext); } catch (Exception e) { Throwable cannotPlanException = Throwables.getCauseOfType(e, RelOptPlanner.CannotPlanException.class); @@ -250,12 +256,12 @@ public void close() * {@link #planner} since we do not re-use artifacts or keep track of state between * {@link #validate}, {@link #prepare}, and {@link #plan} and instead repeat parsing and validation * for each step. - * + *

* Currently, that state tracking is done in {@link org.apache.druid.sql.SqlLifecycle}, which will create a new * planner for each of the corresponding steps so this isn't strictly necessary at this time, this method is here as * much to make this situation explicit and provide context for a future refactor as anything else (and some tests * do re-use the planner between validate, prepare, and plan, which will run into this issue). - * + *

* This could be improved by tying {@link org.apache.druid.sql.SqlLifecycle} and {@link DruidPlanner} states more * closely with the state of {@link #planner}, instead of repeating parsing and validation between each of these * steps. @@ -272,7 +278,8 @@ private void resetPlanner() private PlannerResult planWithDruidConvention( final RelRoot root, @Nullable final SqlExplain explain, - @Nullable final SqlInsert insert + @Nullable final SqlInsert insert, + @Nullable final Map sqlContext ) throws ValidationException, RelConversionException { final RelRoot possiblyLimitedRoot = possiblyWrapRootWithOuterLimitFromContext(root); @@ -309,7 +316,7 @@ private PlannerResult planWithDruidConvention( "Authorization sanity check failed" ); - return druidRel.runQuery(); + return druidRel.runQuery(sqlContext); }; return new PlannerResult(resultsSupplier, queryMaker.getResultType()); @@ -319,7 +326,7 @@ private PlannerResult planWithDruidConvention( /** * Construct a {@link PlannerResult} for a fall-back 'bindable' rel, for things that are not directly translatable * to native Druid queries such as system tables and just a general purpose (but definitely not optimized) fall-back. - * + *

* See {@link #planWithDruidConvention} which will handle things which are directly translatable * to native Druid queries. */ @@ -474,11 +481,12 @@ private String explainSqlPlanAsNativeQueries(DruidRel rel) throws JsonProcess * node * For eg, a DruidRel structure of kind: * DruidUnionRel - * DruidUnionRel - * DruidRel (A) - * DruidRel (B) - * DruidRel(C) + * DruidUnionRel + * DruidRel (A) + * DruidRel (B) + * DruidRel(C) * will return [DruidRel(A), DruidRel(B), DruidRel(C)] + * * @param outermostDruidRel The outermost rel which is to be flattened * @return a list of DruidRel's which donot have a DruidUnionRel nested in between them */ @@ -493,7 +501,8 @@ private List> flattenOutermostRel(DruidRel outermostDruidRel) * Recursive function (DFS) which traverses the nodes and collects the corresponding {@link DruidRel} into a list if * they are not of the type {@link DruidUnionRel} or else calls the method with the child nodes. The DFS order of the * nodes are retained, since that is the order in which they will actually be called in {@link DruidUnionRel#runQuery()} - * @param druidRel The current relNode + * + * @param druidRel The current relNode * @param flattendListAccumulator Accumulator list which needs to be appended by this method */ private void flattenOutermostRel(DruidRel druidRel, List> flattendListAccumulator) @@ -513,7 +522,7 @@ private void flattenOutermostRel(DruidRel druidRel, List> flatten * This method wraps the root with a {@link LogicalSort} that applies a limit (no ordering change). If the outer rel * is already a {@link Sort}, we can merge our outerLimit into it, similar to what is going on in * {@link org.apache.druid.sql.calcite.rule.SortCollapseRule}. - * + *

* The {@link PlannerContext#CTX_SQL_OUTER_LIMIT} flag that controls this wrapping is meant for internal use only by * the web console, allowing it to apply a limit to queries without rewriting the original SQL. * @@ -732,11 +741,20 @@ private static class ParsedNodes private SqlNode query; - private ParsedNodes(@Nullable SqlExplain explain, @Nullable SqlInsert insert, SqlNode query) + @Nullable + private Map contextMap; + + private ParsedNodes( + @Nullable SqlExplain explain, + @Nullable SqlInsert insert, + SqlNode query, + @Nullable Map contextMap + ) { this.explain = explain; this.insert = insert; this.query = query; + this.contextMap = contextMap; } static ParsedNodes create(final SqlNode node) throws ValidationException @@ -744,6 +762,7 @@ static ParsedNodes create(final SqlNode node) throws ValidationException SqlExplain explain = null; SqlInsert insert = null; SqlNode query = node; + Map contextMap = null; if (query.getKind() == SqlKind.EXPLAIN) { explain = (SqlExplain) query; @@ -755,11 +774,21 @@ static ParsedNodes create(final SqlNode node) throws ValidationException query = insert.getSource(); } + if (query instanceof DruidSqlSelect) { + DruidSqlSelect druidSqlSelect = (DruidSqlSelect) query; + contextMap = druidSqlSelect.getContext(); + } + + if (query instanceof DruidSqlOrderBy) { + DruidSqlOrderBy druidSqlOrderBy = (DruidSqlOrderBy) query; + contextMap = druidSqlOrderBy.getContext(); + } + if (!query.isA(SqlKind.QUERY)) { throw new ValidationException(StringUtils.format("Cannot execute [%s].", query.getKind())); } - return new ParsedNodes(explain, insert, query); + return new ParsedNodes(explain, insert, query, contextMap); } @Nullable @@ -778,5 +807,11 @@ public SqlNode getQueryNode() { return query; } + + @Nullable + public Map getContextMap() + { + return contextMap; + } } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java index 6f601ec5aa52..07eecf4b2c2c 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java @@ -26,6 +26,7 @@ import org.apache.druid.sql.calcite.planner.PlannerContext; import javax.annotation.Nullable; +import java.util.Map; import java.util.Set; public abstract class DruidRel extends AbstractRelNode @@ -45,13 +46,18 @@ protected DruidRel(RelOptCluster cluster, RelTraitSet traitSet, PlannerContext p @Nullable public abstract PartialDruidQuery getPartialDruidQuery(); - public Sequence runQuery() + public Sequence runQuery(@Nullable Map sqlContext) { // runQuery doesn't need to finalize aggregations, because the fact that runQuery is happening suggests this // is the outermost query, and it will actually get run as a native query. Druid's native query layer will // finalize aggregations for the outermost query even if we don't explicitly ask it to. - return getPlannerContext().getQueryMaker().runQuery(toDruidQuery(false)); + return getPlannerContext().getQueryMaker().runQuery(toDruidQuery(false), sqlContext); + } + + public Sequence runQuery() + { + return runQuery(null); } public abstract T withPartialQuery(PartialDruidQuery newQueryBuilder); @@ -70,20 +76,19 @@ public boolean isValidDruidQuery() /** * Convert this DruidRel to a DruidQuery. This must be an inexpensive operation, since it is done often throughout * query planning. - * + *

* This method must not return null. * * @param finalizeAggregations true if this query should include explicit finalization for all of its * aggregators, where required. Useful for subqueries where Druid's native query layer * does not do this automatically. - * * @throws CannotBuildQueryException */ public abstract DruidQuery toDruidQuery(boolean finalizeAggregations); /** * Convert this DruidRel to a DruidQuery for purposes of explaining. This must be an inexpensive operation. - * + *

* This method must not return null. * * @throws CannotBuildQueryException diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java index 25e6e9f52326..76b3302816d9 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java @@ -39,6 +39,7 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -47,7 +48,7 @@ * but rather, it represents the concatenation of a series of native queries in the SQL layer. Therefore, * {@link #getPartialDruidQuery()} returns null, and this rel cannot be built on top of. It must be the outer rel in a * query plan. - * + *

* See {@link DruidUnionDataSourceRel} for a version that does a regular Druid query using a {@link UnionDataSource}. * In the future we expect that {@link UnionDataSource} will gain the ability to union query datasources together, and * then this rel could be replaced by {@link DruidUnionDataSourceRel}. @@ -101,20 +102,26 @@ public PartialDruidQuery getPartialDruidQuery() @Override @SuppressWarnings("unchecked") - public Sequence runQuery() + public Sequence runQuery(@Nullable Map sqlContext) { // Lazy: run each query in sequence, not all at once. if (limit == 0) { return Sequences.empty(); } else { final Sequence baseSequence = Sequences.concat( - FluentIterable.from(rels).transform(rel -> ((DruidRel) rel).runQuery()) + FluentIterable.from(rels).transform(rel -> ((DruidRel) rel).runQuery(sqlContext)) ); return limit > 0 ? baseSequence.limit(limit) : baseSequence; } } + @Override + public Sequence runQuery() + { + return runQuery(null); + } + @Override public DruidUnionRel withPartialQuery(final PartialDruidQuery newQueryBuilder) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java index d160230b7d71..13808db46842 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java @@ -57,11 +57,13 @@ import org.joda.time.DateTime; import org.joda.time.Interval; +import javax.annotation.Nullable; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; @@ -112,7 +114,17 @@ public boolean feature(QueryFeature feature) @Override public Sequence runQuery(final DruidQuery druidQuery) { - final Query query = druidQuery.getQuery(); + return runQuery(druidQuery, null); + } + + @Override + public Sequence runQuery(final DruidQuery druidQuery, @Nullable final Map sqlContext) + { + Query query = druidQuery.getQuery(); + + if (sqlContext != null) { + query = query.withOverriddenContext(sqlContext); + } if (plannerContext.getPlannerConfig().isRequireTimeCondition() && !(druidQuery.getDataSource() instanceof InlineDataSource)) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java index c76504b2bddc..7b79181f728c 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java @@ -20,9 +20,12 @@ package org.apache.druid.sql.calcite.run; import org.apache.calcite.rel.type.RelDataType; +import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.sql.calcite.rel.DruidQuery; +import java.util.Map; + /** * Interface for executing Druid queries. Each one is created by a {@link QueryMakerFactory} and is tied to a * specific SQL query. Extends {@link QueryFeatureInspector}, so calling code can tell what this executor supports. @@ -39,4 +42,9 @@ public interface QueryMaker extends QueryFeatureInspector * created for. The returned arrays match the row type given by {@link #getResultType()}. */ Sequence runQuery(DruidQuery druidQuery); + + default Sequence runQuery(DruidQuery druidQuery, Map sqlContext) + { + return runQuery(druidQuery); + } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 80a4246ecbbd..2e7e48afd3e8 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -293,18 +293,18 @@ public void testInformationSchemaColumnsOnTable() throws Exception @Test public void testtest() throws Exception { + // testQuery( + // "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", + // ImmutableList.of(), + // ImmutableList.of() + // ); + testQuery( - "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", +// "SELECT * FROM druid.foo", + "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", ImmutableList.of(), ImmutableList.of() ); - -// testQuery( -// "VALUES ( )", -// ImmutableList.of(), -// ImmutableList.of() -// ); - } @Test From c9d9bc901f4b32db12a59b166e14f796a34f14f3 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 6 Jan 2022 19:20:03 +0530 Subject: [PATCH 04/49] Fix nullptr in DruidPlanner --- .../apache/druid/sql/calcite/planner/DruidPlanner.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 797854355346..6102121203df 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -208,9 +208,12 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo final RelRoot rootQueryRel = planner.rel(validatedQueryNode); try { - Map sqlContext = parsed.getContextMap().entrySet().stream().collect( - Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue) - ); + Map sqlContext = null; + if (parsed.getContextMap() != null) { + sqlContext = parsed.getContextMap().entrySet().stream().collect( + Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue) + ); + } return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(), parsed.getInsertNode(), sqlContext); } catch (Exception e) { From ab57bc0b28bd4b2868fe68f35899273020786b68 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 10:22:07 +0530 Subject: [PATCH 05/49] Add files for SqlInsert --- sql/src/main/codegen/config.fmpp | 9 +- sql/src/main/codegen/includes/insert.ftl | 27 ++++++ .../sql/calcite/parser/DruidSqlInsert.java | 86 +++++++++++++++++++ 3 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 sql/src/main/codegen/includes/insert.ftl create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 045749935492..6276247c9deb 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -47,8 +47,7 @@ data: { # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved # keyword add it to 'nonReservedKeywords' section. keywords: [ - "TEST" - "CONTEXT" + "CLUSTER" ] # List of keywords from "keywords" section that are not reserved. @@ -367,8 +366,7 @@ data: { # Return type of method implementation should be 'SqlNode'. # Example: SqlShowDatabases(), SqlShowTables(). statementParserMethods: [ - "Test()" - "DruidSqlSelect()" + "DruidSqlInsert()" ] # List of methods for parsing custom literals. @@ -418,8 +416,7 @@ data: { # given as part of "statementParserMethods", "literalParserMethods" or # "dataTypeParserMethods". implementationFiles: [ - "test.ftl", - "select.ftl" + "insert.ftl" ] includePosixOperators: false diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl new file mode 100644 index 000000000000..7228ab68e91d --- /dev/null +++ b/sql/src/main/codegen/includes/insert.ftl @@ -0,0 +1,27 @@ +SqlNode DruidSqlInsert() : +{ + org.apache.calcite.sql.SqlNode insertNode; + org.apache.calcite.sql.SqlNode partitionBy = null; + org.apache.calcite.sql.SqlNode clusterBy = null; +} +{ + insertNode = SqlInsert() + [ + + partitionBy = StringLiteral() + ] + [ + + clusterBy = StringLiteral() + ] + { + if(partitionBy == null && clusterBy == null) { + return insertNode; + } + if(!(insertNode instanceof org.apache.calcite.sql.SqlInsert)) { + return insertNode; + } + org.apache.calcite.sql.SqlInsert sqlInsert = (org.apache.calcite.sql.SqlInsert) insertNode; + return new org.apache.druid.sql.calcite.parser.DruidSqlInsert(sqlInsert, partitionBy, clusterBy); +} +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java new file mode 100644 index 000000000000..479fdf14f6d8 --- /dev/null +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.parser; + +import org.apache.calcite.sql.SqlInsert; +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlSelect; +import org.apache.calcite.sql.SqlWriter; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Extends the Select call to have custom context parameters specific to Druid + * These custom parameters are to be manually extracted. + * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) + */ +public class DruidSqlInsert extends SqlInsert +{ + // Unsure if this should be kept as is, but this allows reusing super.unparse + public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; + + private SqlNode partitionBy; + private SqlNode clusterBy; + + + public DruidSqlInsert( + @Nonnull SqlInsert insertNode, + @Nullable SqlNode partitionBy, + @Nullable SqlNode clusterBy + ) + { + super( + insertNode.getParserPosition(), + (SqlNodeList) insertNode.getOperandList().get(0), // No better getter to extract this + insertNode.getTargetTable(), + insertNode.getSource(), + insertNode.getTargetColumnList() + ); + this.partitionBy = partitionBy; + this.clusterBy = clusterBy; + } + + @Override + public SqlOperator getOperator() + { + return OPERATOR; + } + + @Override + public void unparse(SqlWriter writer, int leftPrec, int rightPrec) + { + super.unparse(writer, leftPrec, rightPrec); + if(partitionBy != null) { + writer.keyword("PARTITION"); + writer.keyword("BY"); + writer.keyword(SqlLiteral.stringValue(partitionBy)); + } + if(clusterBy != null) { + writer.keyword("CLUSTER"); + writer.keyword("BY"); + writer.keyword(SqlLiteral.stringValue(clusterBy)); + } + } + +} From 7a3e479eec6acd802fbf4f6df28422f26e130fcb Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 10:31:11 +0530 Subject: [PATCH 06/49] Revert setcontext changes, only insert changes being exposed --- sql/src/main/codegen/includes/select.ftl | 69 ----------- sql/src/main/codegen/includes/test.ftl | 11 -- .../sql/calcite/parser/DruidSqlOrderBy.java | 100 ---------------- .../sql/calcite/parser/DruidSqlSelect.java | 109 ------------------ .../sql/calcite/planner/DruidPlanner.java | 35 +----- .../druid/sql/calcite/rel/DruidRel.java | 9 +- .../druid/sql/calcite/rel/DruidUnionRel.java | 10 +- .../sql/calcite/run/NativeQueryMaker.java | 10 -- .../druid/sql/calcite/run/QueryMaker.java | 4 - 9 files changed, 7 insertions(+), 350 deletions(-) delete mode 100644 sql/src/main/codegen/includes/select.ftl delete mode 100644 sql/src/main/codegen/includes/test.ftl delete mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java delete mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java diff --git a/sql/src/main/codegen/includes/select.ftl b/sql/src/main/codegen/includes/select.ftl deleted file mode 100644 index 0b4d205bb14f..000000000000 --- a/sql/src/main/codegen/includes/select.ftl +++ /dev/null @@ -1,69 +0,0 @@ -SqlNode DruidSqlSelect() : -{ - org.apache.calcite.sql.SqlNode stmt; - org.apache.calcite.sql.SqlNode selectNode; - org.apache.calcite.sql.SqlNodeList contextParameters = null; -} -{ - selectNode = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY) - [ - - contextParameters = ParenthesizedKeyValueOptionCommaList() - ] - { - if(contextParameters != null) { - if(selectNode instanceof org.apache.calcite.sql.SqlSelect) { - return new org.apache.druid.sql.calcite.parser.DruidSqlSelect((org.apache.calcite.sql.SqlSelect) selectNode, contextParameters); - } else if(selectNode instanceof org.apache.calcite.sql.SqlOrderBy) { - return new org.apache.druid.sql.calcite.parser.DruidSqlOrderBy((org.apache.calcite.sql.SqlOrderBy) selectNode, contextParameters); - } - } - return selectNode; - } -} - - -/** -* The following has been directly copied from a later version of the Calcite's Parser.jj. This should be removed while -* upgrading the Calcite's version -*/ - -SqlNodeList ParenthesizedKeyValueOptionCommaList() : -{ - final Span s; - final List list = new ArrayList(); -} -{ - { s = span(); } - - KeyValueOption(list) - ( - - KeyValueOption(list) - )* - { - return new SqlNodeList(list, s.end(this)); - } -} - -/** -* Parses an option with format key=val whose key is a simple identifier or string literal -* and value is a string literal. -*/ -void KeyValueOption(List list) : -{ - final SqlNode key; - final SqlNode value; -} -{ - ( - key = SimpleIdentifier() - | - key = StringLiteral() - ) - - value = StringLiteral() { - list.add(key); - list.add(value); - } -} \ No newline at end of file diff --git a/sql/src/main/codegen/includes/test.ftl b/sql/src/main/codegen/includes/test.ftl deleted file mode 100644 index 715f4bd1ce59..000000000000 --- a/sql/src/main/codegen/includes/test.ftl +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Production for - * TEST - */ - -SqlNode Test(): -{} -{ - - { return null; } -} \ No newline at end of file diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java deleted file mode 100644 index 0a2525ae30e7..000000000000 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlOrderBy.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.druid.sql.calcite.parser; - -import com.google.common.base.Preconditions; -import org.apache.calcite.sql.SqlLiteral; -import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.sql.SqlNodeList; -import org.apache.calcite.sql.SqlOperator; -import org.apache.calcite.sql.SqlOrderBy; -import org.apache.calcite.sql.SqlSelect; -import org.apache.calcite.sql.SqlWriter; -import org.checkerframework.checker.nullness.qual.Nullable; - -import java.util.LinkedHashMap; -import java.util.Map; - -/** - * Extends the Select call to have custom context parameters specific to Druid - * These custom parameters are to be manually extracted. - * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) - */ -public class DruidSqlOrderBy extends SqlOrderBy -{ - // Unsure if this should be kept as is, but this allows reusing super.unparse - public static final SqlOperator OPERATOR = SqlOrderBy.OPERATOR; - private final Map context; - - public DruidSqlOrderBy( - SqlOrderBy orderByNode, - @Nullable SqlNodeList contextMapAsList - ) - { - super( - orderByNode.getParserPosition(), - orderByNode.getOperandList().get(0), - (SqlNodeList) orderByNode.getOperandList().get(1), - orderByNode.getOperandList().get(2), - orderByNode.getOperandList().get(3) - ); - this.context = new LinkedHashMap<>(); - if (contextMapAsList != null) { - int listSize = contextMapAsList.size(); - Preconditions.checkArgument(listSize % 2 == 0); - for (int i = 0; i < listSize / 2; ++i) { - SqlNode keyNode = contextMapAsList.get(2 * i); - SqlNode valueNode = contextMapAsList.get(2 * i + 1); - if (!(keyNode instanceof SqlLiteral)) { - // Log some warning about conversion - } - if (!(valueNode instanceof SqlLiteral)) { - // Log some warning about conversion - - } - String key = keyNode.toString(); - String value = valueNode.toString(); - context.put(key, value); - } - } - } - - @Override - public SqlOperator getOperator() - { - return OPERATOR; - } - - public Map getContext() - { - return context; - } - - - @Override - public void unparse(SqlWriter writer, int leftPrec, int rightPrec) - { - super.unparse(writer, leftPrec, rightPrec); - writer.keyword("DESCRIBE"); - writer.keyword("SPACE"); - writer.keyword("POWER"); - } - -} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java deleted file mode 100644 index 25d92e5db30f..000000000000 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlSelect.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.druid.sql.calcite.parser; - -import com.google.common.base.Preconditions; -import org.apache.calcite.sql.SqlLiteral; -import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.sql.SqlNodeList; -import org.apache.calcite.sql.SqlOperator; -import org.apache.calcite.sql.SqlSelect; -import org.apache.calcite.sql.SqlSelectOperator; -import org.apache.calcite.sql.SqlWriter; - -import javax.annotation.Nullable; -import java.util.LinkedHashMap; -import java.util.Map; - -/** - * Extends the Select call to have custom context parameters specific to Druid - * These custom parameters are to be manually extracted. - * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) - */ -public class DruidSqlSelect extends SqlSelect -{ - // Unsure if this should be kept as is, but this allows reusing super.unparse - public static final SqlOperator OPERATOR = SqlSelectOperator.INSTANCE; - - @Nullable - private final Map context; - - public DruidSqlSelect( - SqlSelect selectNode, - @Nullable SqlNodeList contextMapAsList - ) - { - super( - selectNode.getParserPosition(), - (SqlNodeList) selectNode.getOperandList().get(0), // No better getter to extract this - selectNode.getSelectList(), - selectNode.getFrom(), - selectNode.getWhere(), - selectNode.getGroup(), - selectNode.getHaving(), - selectNode.getWindowList(), - selectNode.getOrderList(), - selectNode.getOffset(), - selectNode.getFetch() - ); - context = new LinkedHashMap<>(); - if (contextMapAsList != null) { - int listSize = contextMapAsList.size(); - Preconditions.checkArgument(listSize % 2 == 0); - for (int i = 0; i < listSize / 2; ++i) { - SqlNode keyNode = contextMapAsList.get(2 * i); - SqlNode valueNode = contextMapAsList.get(2 * i + 1); - if (!(keyNode instanceof SqlLiteral)) { - // Log some warning about conversion - } - if (!(valueNode instanceof SqlLiteral)) { - // Log some warning about conversion - - } - String key = keyNode.toString(); - String value = valueNode.toString(); - context.put(key, value); - } - } - } - - @Override - public SqlOperator getOperator() - { - return OPERATOR; - } - - public Map getContext() - { - return context; - } - - - @Override - public void unparse(SqlWriter writer, int leftPrec, int rightPrec) - { - super.unparse(writer, leftPrec, rightPrec); - if (context != null) { - writer.keyword("SET"); - writer.keyword("CONTEXT"); - } - } - -} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 6102121203df..a8a8be590d5c 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -79,8 +79,6 @@ import org.apache.druid.server.security.Resource; import org.apache.druid.server.security.ResourceAction; import org.apache.druid.server.security.ResourceType; -import org.apache.druid.sql.calcite.parser.DruidSqlOrderBy; -import org.apache.druid.sql.calcite.parser.DruidSqlSelect; import org.apache.druid.sql.calcite.rel.DruidConvention; import org.apache.druid.sql.calcite.rel.DruidQuery; import org.apache.druid.sql.calcite.rel.DruidRel; @@ -208,13 +206,7 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo final RelRoot rootQueryRel = planner.rel(validatedQueryNode); try { - Map sqlContext = null; - if (parsed.getContextMap() != null) { - sqlContext = parsed.getContextMap().entrySet().stream().collect( - Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue) - ); - } - return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(), parsed.getInsertNode(), sqlContext); + return planWithDruidConvention(rootQueryRel, parsed.getExplainNode(), parsed.getInsertNode()); } catch (Exception e) { Throwable cannotPlanException = Throwables.getCauseOfType(e, RelOptPlanner.CannotPlanException.class); @@ -281,8 +273,7 @@ private void resetPlanner() private PlannerResult planWithDruidConvention( final RelRoot root, @Nullable final SqlExplain explain, - @Nullable final SqlInsert insert, - @Nullable final Map sqlContext + @Nullable final SqlInsert insert ) throws ValidationException, RelConversionException { final RelRoot possiblyLimitedRoot = possiblyWrapRootWithOuterLimitFromContext(root); @@ -319,7 +310,7 @@ private PlannerResult planWithDruidConvention( "Authorization sanity check failed" ); - return druidRel.runQuery(sqlContext); + return druidRel.runQuery(); }; return new PlannerResult(resultsSupplier, queryMaker.getResultType()); @@ -744,9 +735,6 @@ private static class ParsedNodes private SqlNode query; - @Nullable - private Map contextMap; - private ParsedNodes( @Nullable SqlExplain explain, @Nullable SqlInsert insert, @@ -757,7 +745,6 @@ private ParsedNodes( this.explain = explain; this.insert = insert; this.query = query; - this.contextMap = contextMap; } static ParsedNodes create(final SqlNode node) throws ValidationException @@ -777,16 +764,6 @@ static ParsedNodes create(final SqlNode node) throws ValidationException query = insert.getSource(); } - if (query instanceof DruidSqlSelect) { - DruidSqlSelect druidSqlSelect = (DruidSqlSelect) query; - contextMap = druidSqlSelect.getContext(); - } - - if (query instanceof DruidSqlOrderBy) { - DruidSqlOrderBy druidSqlOrderBy = (DruidSqlOrderBy) query; - contextMap = druidSqlOrderBy.getContext(); - } - if (!query.isA(SqlKind.QUERY)) { throw new ValidationException(StringUtils.format("Cannot execute [%s].", query.getKind())); } @@ -810,11 +787,5 @@ public SqlNode getQueryNode() { return query; } - - @Nullable - public Map getContextMap() - { - return contextMap; - } } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java index 07eecf4b2c2c..9d4b2f5bcf7d 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java @@ -46,18 +46,13 @@ protected DruidRel(RelOptCluster cluster, RelTraitSet traitSet, PlannerContext p @Nullable public abstract PartialDruidQuery getPartialDruidQuery(); - public Sequence runQuery(@Nullable Map sqlContext) + public Sequence runQuery() { // runQuery doesn't need to finalize aggregations, because the fact that runQuery is happening suggests this // is the outermost query, and it will actually get run as a native query. Druid's native query layer will // finalize aggregations for the outermost query even if we don't explicitly ask it to. - return getPlannerContext().getQueryMaker().runQuery(toDruidQuery(false), sqlContext); - } - - public Sequence runQuery() - { - return runQuery(null); + return getPlannerContext().getQueryMaker().runQuery(toDruidQuery(false)); } public abstract T withPartialQuery(PartialDruidQuery newQueryBuilder); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java index 76b3302816d9..74f2032bea43 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java @@ -102,26 +102,20 @@ public PartialDruidQuery getPartialDruidQuery() @Override @SuppressWarnings("unchecked") - public Sequence runQuery(@Nullable Map sqlContext) + public Sequence runQuery() { // Lazy: run each query in sequence, not all at once. if (limit == 0) { return Sequences.empty(); } else { final Sequence baseSequence = Sequences.concat( - FluentIterable.from(rels).transform(rel -> ((DruidRel) rel).runQuery(sqlContext)) + FluentIterable.from(rels).transform(rel -> ((DruidRel) rel).runQuery()) ); return limit > 0 ? baseSequence.limit(limit) : baseSequence; } } - @Override - public Sequence runQuery() - { - return runQuery(null); - } - @Override public DruidUnionRel withPartialQuery(final PartialDruidQuery newQueryBuilder) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java index 13808db46842..00bd6d3299d8 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java @@ -113,19 +113,9 @@ public boolean feature(QueryFeature feature) @Override public Sequence runQuery(final DruidQuery druidQuery) - { - return runQuery(druidQuery, null); - } - - @Override - public Sequence runQuery(final DruidQuery druidQuery, @Nullable final Map sqlContext) { Query query = druidQuery.getQuery(); - if (sqlContext != null) { - query = query.withOverriddenContext(sqlContext); - } - if (plannerContext.getPlannerConfig().isRequireTimeCondition() && !(druidQuery.getDataSource() instanceof InlineDataSource)) { if (Intervals.ONLY_ETERNITY.equals(findBaseDataSourceIntervals(query))) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java index 7b79181f728c..525f5535a6e6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java @@ -43,8 +43,4 @@ public interface QueryMaker extends QueryFeatureInspector */ Sequence runQuery(DruidQuery druidQuery); - default Sequence runQuery(DruidQuery druidQuery, Map sqlContext) - { - return runQuery(druidQuery); - } } From d79166902a01e1e9082c4c832445e8c3731b16e8 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 14:23:20 +0530 Subject: [PATCH 07/49] Cleanup code, ClusterBy will create a new SqlNode of type OrderBy --- sql/src/main/codegen/config.fmpp | 4 ++- sql/src/main/codegen/includes/insert.ftl | 33 +++++++++++++++---- .../sql/calcite/parser/DruidSqlInsert.java | 13 +++++--- .../sql/calcite/planner/DruidPlanner.java | 11 +++++++ 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 6276247c9deb..d9d0ae2771c9 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -41,7 +41,9 @@ data: { # List of additional classes and packages to import. # Example. "org.apache.calcite.sql.*", "java.util.List". imports: [ - "org.apache.calcite.sql.SqlSelect" + "org.apache.calcite.sql.SqlNode" + "org.apache.calcite.sql.SqlInsert" + "org.apache.druid.sql.calcite.parser.DruidSqlInsert" ] # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index 7228ab68e91d..9af86cced4b2 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -1,8 +1,8 @@ SqlNode DruidSqlInsert() : { - org.apache.calcite.sql.SqlNode insertNode; - org.apache.calcite.sql.SqlNode partitionBy = null; - org.apache.calcite.sql.SqlNode clusterBy = null; + SqlNode insertNode; + SqlNode partitionBy = null; + SqlNodeList clusterBy = null; } { insertNode = SqlInsert() @@ -12,16 +12,35 @@ SqlNode DruidSqlInsert() : ] [ - clusterBy = StringLiteral() + clusterBy = ClusterItems() ] { if(partitionBy == null && clusterBy == null) { return insertNode; } - if(!(insertNode instanceof org.apache.calcite.sql.SqlInsert)) { + if(!(insertNode instanceof SqlInsert)) { return insertNode; } - org.apache.calcite.sql.SqlInsert sqlInsert = (org.apache.calcite.sql.SqlInsert) insertNode; - return new org.apache.druid.sql.calcite.parser.DruidSqlInsert(sqlInsert, partitionBy, clusterBy); + SqlInsert sqlInsert = (SqlInsert) insertNode; + return new DruidSqlInsert(sqlInsert, partitionBy, clusterBy); } } + +SqlNodeList ClusterItems() : +{ + List list; + final Span s; + SqlNode e; +} +{ + e = OrderItem() { + s = span(); + list = startList(e); + } + ( + LOOKAHEAD(2) e = OrderItem() { list.add(e); } + )* + { + return new SqlNodeList(list, s.addAll(list).pos()); + } +} \ No newline at end of file diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 479fdf14f6d8..857c47b420dd 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -41,13 +41,13 @@ public class DruidSqlInsert extends SqlInsert public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; private SqlNode partitionBy; - private SqlNode clusterBy; + private SqlNodeList clusterBy; public DruidSqlInsert( @Nonnull SqlInsert insertNode, @Nullable SqlNode partitionBy, - @Nullable SqlNode clusterBy + @Nullable SqlNodeList clusterBy ) { super( @@ -61,6 +61,11 @@ public DruidSqlInsert( this.clusterBy = clusterBy; } + public SqlNodeList getClusterBy() + { + return clusterBy; + } + @Override public SqlOperator getOperator() { @@ -71,12 +76,12 @@ public SqlOperator getOperator() public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { super.unparse(writer, leftPrec, rightPrec); - if(partitionBy != null) { + if (partitionBy != null) { writer.keyword("PARTITION"); writer.keyword("BY"); writer.keyword(SqlLiteral.stringValue(partitionBy)); } - if(clusterBy != null) { + if (clusterBy != null) { writer.keyword("CLUSTER"); writer.keyword("BY"); writer.keyword(SqlLiteral.stringValue(clusterBy)); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index a8a8be590d5c..7832a0ff5a74 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -56,6 +56,7 @@ import org.apache.calcite.sql.SqlInsert; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlOrderBy; import org.apache.calcite.sql.parser.SqlParseException; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.validate.SqlValidator; @@ -74,11 +75,13 @@ import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.query.Query; import org.apache.druid.query.QueryContexts; +import org.apache.druid.query.scan.ScanQuery; import org.apache.druid.segment.DimensionHandlerUtils; import org.apache.druid.server.security.Action; import org.apache.druid.server.security.Resource; import org.apache.druid.server.security.ResourceAction; import org.apache.druid.server.security.ResourceType; +import org.apache.druid.sql.calcite.parser.DruidSqlInsert; import org.apache.druid.sql.calcite.rel.DruidConvention; import org.apache.druid.sql.calcite.rel.DruidQuery; import org.apache.druid.sql.calcite.rel.DruidRel; @@ -764,6 +767,14 @@ static ParsedNodes create(final SqlNode node) throws ValidationException query = insert.getSource(); } + if (insert instanceof DruidSqlInsert) { + DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; + if (druidSqlInsert.getClusterBy() != null) { + // OFFSET and FETCH would be NULL when specifying CLUSTER BY in the query + query = new SqlOrderBy(query.getParserPosition(), query, druidSqlInsert.getClusterBy(), null, null); + } + } + if (!query.isA(SqlKind.QUERY)) { throw new ValidationException(StringUtils.format("Cannot execute [%s].", query.getKind())); } From 5b741a6ebf6162eab28ed4d0ccd02150a73bc103 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 14:41:35 +0530 Subject: [PATCH 08/49] Cleanup code, revert unnesecary changes --- sql/pom.xml | 3 +- sql/src/main/codegen/includes/insert.ftl | 2 +- .../sql/calcite/parser/DruidSqlInsert.java | 21 +-- .../sql/calcite/planner/DruidPlanner.java | 23 ++- .../druid/sql/calcite/CalciteQueryTest.java | 145 ++++++++++++++---- 5 files changed, 143 insertions(+), 51 deletions(-) diff --git a/sql/pom.xml b/sql/pom.xml index 16974c7d5e6e..9df1c5406ad4 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -18,8 +18,7 @@ ~ under the License. --> - + 4.0.0 druid-sql diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index 9af86cced4b2..d51fed1144ff 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -43,4 +43,4 @@ SqlNodeList ClusterItems() : { return new SqlNodeList(list, s.addAll(list).pos()); } -} \ No newline at end of file +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 857c47b420dd..3557bed20269 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -31,18 +31,16 @@ import javax.annotation.Nullable; /** - * Extends the Select call to have custom context parameters specific to Druid - * These custom parameters are to be manually extracted. - * This class extends {@link SqlSelect} so that the node can be used further for conversions (Sql to Rel) + * Extends the Insert call to hold custom paramaters specific to druid i.e. PARTITION BY and CLUSTER BY + * This class extends the {@link SqlInsert} so that the node can be used for further conversion */ public class DruidSqlInsert extends SqlInsert { // Unsure if this should be kept as is, but this allows reusing super.unparse public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; - private SqlNode partitionBy; - private SqlNodeList clusterBy; - + final private SqlNode partitionBy; + final private SqlNodeList clusterBy; public DruidSqlInsert( @Nonnull SqlInsert insertNode, @@ -61,11 +59,13 @@ public DruidSqlInsert( this.clusterBy = clusterBy; } + @Nullable public SqlNodeList getClusterBy() { return clusterBy; } + @Nonnull @Override public SqlOperator getOperator() { @@ -82,9 +82,12 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) writer.keyword(SqlLiteral.stringValue(partitionBy)); } if (clusterBy != null) { - writer.keyword("CLUSTER"); - writer.keyword("BY"); - writer.keyword(SqlLiteral.stringValue(clusterBy)); + writer.sep("CLUSTER BY"); + SqlWriter.Frame frame = writer.startList("", ""); + for (SqlNode clusterByOpts : clusterBy.getList()) { + clusterByOpts.unparse(writer, leftPrec, rightPrec); + } + writer.endList(frame); } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 7832a0ff5a74..51dace6a28a4 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -75,7 +75,6 @@ import org.apache.druid.java.util.emitter.EmittingLogger; import org.apache.druid.query.Query; import org.apache.druid.query.QueryContexts; -import org.apache.druid.query.scan.ScanQuery; import org.apache.druid.segment.DimensionHandlerUtils; import org.apache.druid.server.security.Action; import org.apache.druid.server.security.Resource; @@ -161,7 +160,7 @@ public ValidationResult validate() throws SqlParseException, ValidationException /** * Prepare an SQL query for execution, including some initial parsing and validation and any dynamic parameter type * resolution, to support prepared statements via JDBC. - *

+ * * In some future this could perhaps re-use some of the work done by {@link #validate()} * instead of repeating it, but that day is not today. */ @@ -189,10 +188,10 @@ public PrepareResult prepare() throws SqlParseException, ValidationException, Re /** * Plan an SQL query for execution, returning a {@link PlannerResult} which can be used to actually execute the query. - *

+ * * Ideally, the query can be planned into a native Druid query, using {@link #planWithDruidConvention}, but will * fall-back to {@link #planWithBindableConvention} if this is not possible. - *

+ * * In some future this could perhaps re-use some of the work done by {@link #validate()} * instead of repeating it, but that day is not today. */ @@ -254,12 +253,12 @@ public void close() * {@link #planner} since we do not re-use artifacts or keep track of state between * {@link #validate}, {@link #prepare}, and {@link #plan} and instead repeat parsing and validation * for each step. - *

+ * * Currently, that state tracking is done in {@link org.apache.druid.sql.SqlLifecycle}, which will create a new * planner for each of the corresponding steps so this isn't strictly necessary at this time, this method is here as * much to make this situation explicit and provide context for a future refactor as anything else (and some tests * do re-use the planner between validate, prepare, and plan, which will run into this issue). - *

+ * * This could be improved by tying {@link org.apache.druid.sql.SqlLifecycle} and {@link DruidPlanner} states more * closely with the state of {@link #planner}, instead of repeating parsing and validation between each of these * steps. @@ -323,7 +322,7 @@ private PlannerResult planWithDruidConvention( /** * Construct a {@link PlannerResult} for a fall-back 'bindable' rel, for things that are not directly translatable * to native Druid queries such as system tables and just a general purpose (but definitely not optimized) fall-back. - *

+ * * See {@link #planWithDruidConvention} which will handle things which are directly translatable * to native Druid queries. */ @@ -478,10 +477,10 @@ private String explainSqlPlanAsNativeQueries(DruidRel rel) throws JsonProcess * node * For eg, a DruidRel structure of kind: * DruidUnionRel - * DruidUnionRel - * DruidRel (A) - * DruidRel (B) - * DruidRel(C) + * DruidUnionRel + * DruidRel (A) + * DruidRel (B) + * DruidRel(C) * will return [DruidRel(A), DruidRel(B), DruidRel(C)] * * @param outermostDruidRel The outermost rel which is to be flattened @@ -519,7 +518,7 @@ private void flattenOutermostRel(DruidRel druidRel, List> flatten * This method wraps the root with a {@link LogicalSort} that applies a limit (no ordering change). If the outer rel * is already a {@link Sort}, we can merge our outerLimit into it, similar to what is going on in * {@link org.apache.druid.sql.calcite.rule.SortCollapseRule}. - *

+ * * The {@link PlannerContext#CTX_SQL_OUTER_LIMIT} flag that controls this wrapping is meant for internal use only by * the web console, allowing it to apply a limit to queries without rewriting the original SQL. * diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 2e7e48afd3e8..688649a36731 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -290,23 +290,6 @@ public void testInformationSchemaColumnsOnTable() throws Exception ); } - @Test - public void testtest() throws Exception - { - // testQuery( - // "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", - // ImmutableList.of(), - // ImmutableList.of() - // ); - - testQuery( -// "SELECT * FROM druid.foo", - "SELECT * FROM druid.foo SET CONTEXT (artist='blondie', song='maria')", - ImmutableList.of(), - ImmutableList.of() - ); - } - @Test public void testInformationSchemaColumnsOnForbiddenTable() throws Exception { @@ -410,6 +393,7 @@ public void testAggregatorsOnInformationSchemaColumns() throws Exception } + @Test public void testTopNLimitWrapping() throws Exception { @@ -2284,7 +2268,7 @@ public void testGroupByWithSelectAndOrderByProjections() throws Exception + " SUBSTRING(dim1, 2)\n" + "FROM druid.foo\n" + "GROUP BY dim1\n" - + "ORDER BY CHARACTER_LENGTH(dim1) DESC, dim1 SET CONTEXT ('x'='11', 'y'='234')", + + "ORDER BY CHARACTER_LENGTH(dim1) DESC, dim1", ImmutableList.of( GroupByQuery.builder() .setDataSource(CalciteTests.DATASOURCE1) @@ -2623,8 +2607,7 @@ public void testUnionAllTablesColumnTypeMismatchStringLong() + "WHERE dim2 = 'a' OR dim2 = 'en'\n" + "GROUP BY 1, 2", "Possible error: SQL requires union between inputs that are not simple table scans and involve a " + - "filter or aliasing. Or column types of tables being unioned are not of same type." - ); + "filter or aliasing. Or column types of tables being unioned are not of same type."); } @Test @@ -2639,7 +2622,7 @@ public void testUnionAllTablesWhenMappingIsRequired() + "WHERE c = 'a' OR c = 'def'\n" + "GROUP BY 1", "Possible error: SQL requires union between two tables " + - "and column names queried for each table are different Left: [dim1], Right: [dim2]." + "and column names queried for each table are different Left: [dim1], Right: [dim2]." ); } @@ -2666,7 +2649,7 @@ public void testUnionAllTablesWhenCastAndMappingIsRequired() + "WHERE c = 'a' OR c = 'def'\n" + "GROUP BY 1", "Possible error: SQL requires union between inputs that are not simple table scans and involve " + - "a filter or aliasing. Or column types of tables being unioned are not of same type." + "a filter or aliasing. Or column types of tables being unioned are not of same type." ); } @@ -3928,12 +3911,10 @@ public void testGroupByWithFilterMatchingNothingWithGroupByLiteral() throws Exce new CountAggregatorFactory("a0"), new LongMaxAggregatorFactory("a1", "cnt") )) - .context(QUERY_CONTEXT_DEFAULT) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) .build() ), - ImmutableList.of( - new Object[]{0L, NullHandling.sqlCompatible() ? null : Long.MIN_VALUE} - ) + ImmutableList.of() ); } @@ -5454,7 +5435,7 @@ public void testCountStarWithTimeFilterUsingStringLiteralsInvalid_isUnplannable( // This error message isn't ideal but it is at least better than silently ignoring the problem. assertQueryIsUnplannable( "SELECT COUNT(*) FROM druid.foo\n" - + "WHERE __time >= 'z2000-01-01 00:00:00' AND __time < '2001-01-01 00:00:00'\n", + + "WHERE __time >= 'z2000-01-01 00:00:00' AND __time < '2001-01-01 00:00:00'\n", "Possible error: Illegal TIMESTAMP constant: CAST('z2000-01-01 00:00:00'):TIMESTAMP(3) NOT NULL" ); } @@ -7747,6 +7728,7 @@ public void testFilterOnCurrentTimestampWithIntervalArithmetic() throws Exceptio } + @Test public void testFilterOnCurrentTimestampLosAngeles() throws Exception { @@ -12377,6 +12359,7 @@ public void testLookupWithNull() throws Exception } + @Test public void testRoundFuc() throws Exception { @@ -12816,6 +12799,7 @@ public void testBitwiseAggregatorsGroupBy() throws Exception } + @Test public void testStringAgg() throws Exception { @@ -13389,4 +13373,111 @@ public void testCommonVirtualExpressionWithDifferentValueType() throws Exception ImmutableList.of() ); } + + // When optimization in Grouping#applyProject is applied, and it reduces a Group By query to a timeseries, we + // want it to return empty bucket if no row matches + @Test + public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithSingleConstantDimension() throws Exception + { + skipVectorize(); + testQuery( + "SELECT 'A' from foo WHERE m1 = 50 AND dim1 = 'wat' GROUP BY 'foobar'", + ImmutableList.of( + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters( + and( + selector("m1", "50", null), + selector("dim1", "wat", null) + ) + ) + .granularity(Granularities.ALL) + .postAggregators( + new ExpressionPostAggregator("p0", "'A'", null, ExprMacroTable.nil()) + ) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) + .build() + ), + ImmutableList.of() + ); + + + // dim1 is not getting reduced to 'wat' in this case in Calcite (ProjectMergeRule is not getting applied), + // therefore the query is not optimized to a timeseries query + testQuery( + "SELECT 'A' from foo WHERE dim1 = 'wat' GROUP BY dim1", + ImmutableList.of( + GroupByQuery.builder() + .setDataSource( + "foo" + ) + .setInterval(querySegmentSpec(Intervals.ETERNITY)) + .setGranularity(Granularities.ALL) + .addDimension(new DefaultDimensionSpec("dim1", "d0", ColumnType.STRING)) + .setDimFilter(selector("dim1", "wat", null)) + .setPostAggregatorSpecs( + ImmutableList.of( + new ExpressionPostAggregator("p0", "'A'", null, ExprMacroTable.nil()) + ) + ) + + .build() + ), + ImmutableList.of() + ); + } + + @Test + public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithMutlipleConstantDimensions() throws Exception + { + skipVectorize(); + testQuery( + "SELECT 'A', dim1 from foo WHERE m1 = 50 AND dim1 = 'wat' GROUP BY dim1", + ImmutableList.of( + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters( + and( + selector("m1", "50", null), + selector("dim1", "wat", null) + ) + ) + .granularity(Granularities.ALL) + .postAggregators( + new ExpressionPostAggregator("p0", "'A'", null, ExprMacroTable.nil()), + new ExpressionPostAggregator("p1", "'wat'", null, ExprMacroTable.nil()) + ) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) + .build() + ), + ImmutableList.of() + ); + + // Sanity test, that even when dimensions are reduced, but should produce a valid result (i.e. when filters are + // correct, then they should + testQuery( + "SELECT 'A', dim1 from foo WHERE m1 = 2.0 AND dim1 = '10.1' GROUP BY dim1", + ImmutableList.of( + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters( + and( + selector("m1", "2.0", null), + selector("dim1", "10.1", null) + ) + ) + .granularity(Granularities.ALL) + .postAggregators( + new ExpressionPostAggregator("p0", "'A'", null, ExprMacroTable.nil()), + new ExpressionPostAggregator("p1", "'10.1'", null, ExprMacroTable.nil()) + ) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) + .build() + ), + ImmutableList.of(new Object[]{"A", "10.1"}) + ); + } } From 1845ddfdd0308ceae896cc9576c5da9df9a0c183 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 14:46:48 +0530 Subject: [PATCH 09/49] Remove contextMap from ParsedNodes --- .../org/apache/druid/sql/calcite/planner/DruidPlanner.java | 6 ++---- .../java/org/apache/druid/sql/calcite/rel/DruidRel.java | 6 +++--- .../org/apache/druid/sql/calcite/rel/DruidUnionRel.java | 3 +-- .../org/apache/druid/sql/calcite/run/NativeQueryMaker.java | 4 +--- .../java/org/apache/druid/sql/calcite/run/QueryMaker.java | 4 ---- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index abdaf9f02421..f4f844408b35 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -741,8 +741,7 @@ private static class ParsedNodes private ParsedNodes( @Nullable SqlExplain explain, @Nullable SqlInsert insert, - SqlNode query, - @Nullable Map contextMap + SqlNode query ) { this.explain = explain; @@ -755,7 +754,6 @@ static ParsedNodes create(final SqlNode node) throws ValidationException SqlExplain explain = null; SqlInsert insert = null; SqlNode query = node; - Map contextMap = null; if (query.getKind() == SqlKind.EXPLAIN) { explain = (SqlExplain) query; @@ -779,7 +777,7 @@ static ParsedNodes create(final SqlNode node) throws ValidationException throw new ValidationException(StringUtils.format("Cannot execute [%s].", query.getKind())); } - return new ParsedNodes(explain, insert, query, contextMap); + return new ParsedNodes(explain, insert, query); } @Nullable diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java index 9d4b2f5bcf7d..6f601ec5aa52 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java @@ -26,7 +26,6 @@ import org.apache.druid.sql.calcite.planner.PlannerContext; import javax.annotation.Nullable; -import java.util.Map; import java.util.Set; public abstract class DruidRel extends AbstractRelNode @@ -71,19 +70,20 @@ public boolean isValidDruidQuery() /** * Convert this DruidRel to a DruidQuery. This must be an inexpensive operation, since it is done often throughout * query planning. - *

+ * * This method must not return null. * * @param finalizeAggregations true if this query should include explicit finalization for all of its * aggregators, where required. Useful for subqueries where Druid's native query layer * does not do this automatically. + * * @throws CannotBuildQueryException */ public abstract DruidQuery toDruidQuery(boolean finalizeAggregations); /** * Convert this DruidRel to a DruidQuery for purposes of explaining. This must be an inexpensive operation. - *

+ * * This method must not return null. * * @throws CannotBuildQueryException diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java index 74f2032bea43..25e6e9f52326 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidUnionRel.java @@ -39,7 +39,6 @@ import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -48,7 +47,7 @@ * but rather, it represents the concatenation of a series of native queries in the SQL layer. Therefore, * {@link #getPartialDruidQuery()} returns null, and this rel cannot be built on top of. It must be the outer rel in a * query plan. - *

+ * * See {@link DruidUnionDataSourceRel} for a version that does a regular Druid query using a {@link UnionDataSource}. * In the future we expect that {@link UnionDataSource} will gain the ability to union query datasources together, and * then this rel could be replaced by {@link DruidUnionDataSourceRel}. diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java index 00bd6d3299d8..d160230b7d71 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java @@ -57,13 +57,11 @@ import org.joda.time.DateTime; import org.joda.time.Interval; -import javax.annotation.Nullable; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; @@ -114,7 +112,7 @@ public boolean feature(QueryFeature feature) @Override public Sequence runQuery(final DruidQuery druidQuery) { - Query query = druidQuery.getQuery(); + final Query query = druidQuery.getQuery(); if (plannerContext.getPlannerConfig().isRequireTimeCondition() && !(druidQuery.getDataSource() instanceof InlineDataSource)) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java index 525f5535a6e6..c76504b2bddc 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/QueryMaker.java @@ -20,12 +20,9 @@ package org.apache.druid.sql.calcite.run; import org.apache.calcite.rel.type.RelDataType; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.sql.calcite.rel.DruidQuery; -import java.util.Map; - /** * Interface for executing Druid queries. Each one is created by a {@link QueryMakerFactory} and is tied to a * specific SQL query. Extends {@link QueryFeatureInspector}, so calling code can tell what this executor supports. @@ -42,5 +39,4 @@ public interface QueryMaker extends QueryFeatureInspector * created for. The returned arrays match the row type given by {@link #getResultType()}. */ Sequence runQuery(DruidQuery druidQuery); - } From 2817a859d70a31d18a184590d5b4d83972b1a27c Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 15:01:58 +0530 Subject: [PATCH 10/49] Add getter for partitionBy --- .../druid/sql/calcite/parser/DruidSqlInsert.java | 9 +++++++++ .../druid/sql/calcite/planner/DruidPlanner.java | 15 +++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 3557bed20269..41eeb8f20ab1 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -65,6 +65,15 @@ public SqlNodeList getClusterBy() return clusterBy; } + @Nullable + public String getPartitionBy() + { + if (partitionBy == null) { + return null; + } + return SqlLiteral.unchain(partitionBy).toValue(); + } + @Nonnull @Override public SqlOperator getOperator() diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index f4f844408b35..c489038b6fb1 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -763,13 +763,16 @@ static ParsedNodes create(final SqlNode node) throws ValidationException if (query.getKind() == SqlKind.INSERT) { insert = (SqlInsert) query; query = insert.getSource(); - } - if (insert instanceof DruidSqlInsert) { - DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; - if (druidSqlInsert.getClusterBy() != null) { - // OFFSET and FETCH would be NULL when specifying CLUSTER BY in the query - query = new SqlOrderBy(query.getParserPosition(), query, druidSqlInsert.getClusterBy(), null, null); + if (insert instanceof DruidSqlInsert) { + DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; + if (druidSqlInsert.getClusterBy() != null) { + // OFFSET and FETCH would be NULL when specifying CLUSTER BY in the query + if (query instanceof SqlOrderBy) { + throw new ValidationException("Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query"); + } + query = new SqlOrderBy(query.getParserPosition(), query, druidSqlInsert.getClusterBy(), null, null); + } } } From 7d63e44d0e738fe4a02a838d1135d4d6e8d93909 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 18:59:29 +0530 Subject: [PATCH 11/49] Checkstyle fix --- .../apache/druid/sql/calcite/parser/DruidSqlInsert.java | 7 +++---- .../org/apache/druid/sql/calcite/planner/DruidPlanner.java | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 41eeb8f20ab1..ff446cf3572d 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -24,7 +24,6 @@ import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.SqlOperator; -import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.SqlWriter; import javax.annotation.Nonnull; @@ -39,8 +38,8 @@ public class DruidSqlInsert extends SqlInsert // Unsure if this should be kept as is, but this allows reusing super.unparse public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; - final private SqlNode partitionBy; - final private SqlNodeList clusterBy; + private final SqlNode partitionBy; + private final SqlNodeList clusterBy; public DruidSqlInsert( @Nonnull SqlInsert insertNode, @@ -88,7 +87,7 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) if (partitionBy != null) { writer.keyword("PARTITION"); writer.keyword("BY"); - writer.keyword(SqlLiteral.stringValue(partitionBy)); + writer.keyword(getPartitionBy()); } if (clusterBy != null) { writer.sep("CLUSTER BY"); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index c489038b6fb1..c46b9b63cb55 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -96,7 +96,6 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.stream.Collectors; From a22eb0e29af3ed1fc77a67e38784143ef148eca8 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 17 Jan 2022 22:02:01 +0530 Subject: [PATCH 12/49] Add generated files to ignorelist of forbiddenapis --- pom.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pom.xml b/pom.xml index 803bf188eed2..e2122970c5d0 100644 --- a/pom.xml +++ b/pom.xml @@ -1314,6 +1314,8 @@ **/SomeAvroDatum.class + **/DruidSqlParserImpl.class + **/SimpleCharStream.class **.SuppressForbidden From 1119dd0b9a3abbf82bfa1998a30ff2eaca03ca69 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Tue, 18 Jan 2022 14:12:54 +0530 Subject: [PATCH 13/49] indentation fix, add license to insert.ftl --- sql/src/main/codegen/includes/insert.ftl | 37 ++++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index d51fed1144ff..5607d2ad6ec5 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -1,3 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + SqlNode DruidSqlInsert() : { SqlNode insertNode; @@ -15,15 +34,15 @@ SqlNode DruidSqlInsert() : clusterBy = ClusterItems() ] { - if(partitionBy == null && clusterBy == null) { - return insertNode; - } - if(!(insertNode instanceof SqlInsert)) { - return insertNode; - } - SqlInsert sqlInsert = (SqlInsert) insertNode; - return new DruidSqlInsert(sqlInsert, partitionBy, clusterBy); -} + if(partitionBy == null && clusterBy == null) { + return insertNode; + } + if(!(insertNode instanceof SqlInsert)) { + return insertNode; + } + SqlInsert sqlInsert = (SqlInsert) insertNode; + return new DruidSqlInsert(sqlInsert, partitionBy, clusterBy); + } } SqlNodeList ClusterItems() : From 50ce3fb45d757e73b1f33388a143ba90b411a697 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Tue, 18 Jan 2022 17:32:49 +0530 Subject: [PATCH 14/49] Add testcases which plan using modified syntax --- .../sql/calcite/CalciteInsertDmlTest.java | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index 8709a899a753..b9b794cd452b 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -60,6 +60,7 @@ import org.hamcrest.MatcherAssert; import org.junit.After; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.internal.matchers.ThrowableMessageMatcher; @@ -271,6 +272,115 @@ public void testInsertFromExternal() .verify(); } + @Test + public void testInsertWithPartitionBy() + { + // Test correctness of the query when only PARTITION BY clause is present + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("floor_m1", ColumnType.FLOAT) + .add("dim1", ColumnType.STRING) + .build(); + + // Test correctness of the query when only PARTITION BY clause is present + testInsertQuery() + .sql( + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITION BY 'day'") + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1", "v0") + .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .build() + ) + .verify(); + } + + @Test + public void testInsertWithClusterBy() + { + // Test correctness of the query when only CLUSTER BY clause is present + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("floor_m1", ColumnType.FLOAT) + .add("dim1", ColumnType.STRING) + .build(); + testInsertQuery() + .sql( + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo CLUSTER BY 2, dim1") + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1", "v0") + .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .orderBy( + ImmutableList.of( + new ScanQuery.OrderBy("v0", ScanQuery.Order.ASCENDING), + new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) + ) + ) + .build() + ) + .verify(); + } + + @Test + public void testInsertWithPartitionByAndClusterBy() + { + // Test correctness of the query when both PARTITION BY and CLUSTER BY clause is present + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("floor_m1", ColumnType.FLOAT) + .add("dim1", ColumnType.STRING) + .build(); + testInsertQuery() + .sql( + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITION BY 'day' CLUSTER BY 2, dim1") + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1", "v0") + .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .orderBy( + ImmutableList.of( + new ScanQuery.OrderBy("v0", ScanQuery.Order.ASCENDING), + new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) + ) + ) + .build() + ) + .verify(); + } + + // Currently EXPLAIN PLAN FOR doesn't work with the modified syntax + @Ignore + @Test + public void testExplainInsertWithPartitionByAndClusterBy() + { + Assert.assertThrows( + SqlPlanningException.class, + () -> + testQuery( + StringUtils.format( + "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITION BY 'day' CLUSTER BY 1", + externSql(externalDataSource) + ), + ImmutableList.of(), + ImmutableList.of() + ) + ); + didTest = true; + } + @Test public void testExplainInsertFromExternal() throws Exception { From 472291cc49497997ec4303321113fdd9746e6ad1 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Tue, 18 Jan 2022 19:17:33 +0530 Subject: [PATCH 15/49] Handle LIMIT/OFFSET clauses, add more testcases --- .../sql/calcite/planner/DruidPlanner.java | 39 ++++++++- .../sql/calcite/CalciteInsertDmlTest.java | 79 ++++++++++++++++++- 2 files changed, 114 insertions(+), 4 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index c46b9b63cb55..c1bf39194e85 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -56,6 +56,7 @@ import org.apache.calcite.sql.SqlInsert; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.SqlOrderBy; import org.apache.calcite.sql.parser.SqlParseException; import org.apache.calcite.sql.type.SqlTypeName; @@ -67,6 +68,7 @@ import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; import org.apache.calcite.util.Pair; +import org.apache.druid.common.guava.GuavaUtils; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.guava.BaseSequence; import org.apache.druid.java.util.common.guava.Sequence; @@ -763,14 +765,45 @@ static ParsedNodes create(final SqlNode node) throws ValidationException insert = (SqlInsert) query; query = insert.getSource(); + // Processing to be done when the original query has either of the PARTITION BY or CLUSTER BY clause if (insert instanceof DruidSqlInsert) { DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; + + if (druidSqlInsert.getPartitionBy() != null) { + // TODO: Pass the PARTITION BY clause in the query context + } + if (druidSqlInsert.getClusterBy() != null) { - // OFFSET and FETCH would be NULL when specifying CLUSTER BY in the query + // If we have a CLUSTER BY clause, extract the information in that CLUSTER BY and create a new SqlOrderBy + // node + SqlNode offset = null; + SqlNode fetch = null; + SqlNodeList orderByList = null; + if (query instanceof SqlOrderBy) { - throw new ValidationException("Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query"); + SqlOrderBy sqlOrderBy = (SqlOrderBy) query; + // Extract the query present inside the SqlOrderBy (which is free of ORDER BY, OFFSET and FETCH clauses) + query = sqlOrderBy.query; + + offset = sqlOrderBy.offset; + fetch = sqlOrderBy.fetch; + orderByList = sqlOrderBy.orderList; + // If the orderList is non-empty (i.e. there existed an ORDER BY clause in the query) and CLUSTER BY clause + // is also non-empty, throw an error + if (!(orderByList == null || orderByList.equals(SqlNodeList.EMPTY)) + && druidSqlInsert.getClusterBy() != null) { + throw new ValidationException( + "Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query"); + } } - query = new SqlOrderBy(query.getParserPosition(), query, druidSqlInsert.getClusterBy(), null, null); + // Creates a new SqlOrderBy query, which may have our CLUSTER BY overwritten + query = new SqlOrderBy( + query.getParserPosition(), + query, + GuavaUtils.firstNonNull(orderByList, druidSqlInsert.getClusterBy()), + offset, + fetch + ); } } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index b9b794cd452b..3f3a649efd7f 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -282,7 +282,6 @@ public void testInsertWithPartitionBy() .add("dim1", ColumnType.STRING) .build(); - // Test correctness of the query when only PARTITION BY clause is present testInsertQuery() .sql( "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITION BY 'day'") @@ -361,6 +360,84 @@ public void testInsertWithPartitionByAndClusterBy() .verify(); } + @Test + public void testInsertWithPartitionByAndLimitOffset() + { + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("floor_m1", ColumnType.FLOAT) + .add("dim1", ColumnType.STRING) + .build(); + + testInsertQuery() + .sql( + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo LIMIT 10 OFFSET 20 PARTITION BY 'day'") + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1", "v0") + .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .limit(10) + .offset(20) + .build() + ) + .verify(); + } + + @Test + public void testInsertWithPartitionByAndOrderBy() + { + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("floor_m1", ColumnType.FLOAT) + .add("dim1", ColumnType.STRING) + .build(); + testInsertQuery() + .sql( + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo ORDER BY 2, dim1 PARTITION BY 'day'") + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1", "v0") + .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .orderBy( + ImmutableList.of( + new ScanQuery.OrderBy("v0", ScanQuery.Order.ASCENDING), + new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) + ) + ) + .build() + ) + .verify(); + } + + @Test + public void testInsertWithClusterByAndOrderBy() + { + + // Throws a ValidationException, which gets converted to a SqlPlanningException before throwing to end user + Assert.assertThrows( + SqlPlanningException.class, + () -> + testQuery( + StringUtils.format( + "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTER BY 3", + externSql(externalDataSource) + ), + ImmutableList.of(), + ImmutableList.of() + ) + ); + didTest = true; + } + + // Currently EXPLAIN PLAN FOR doesn't work with the modified syntax @Ignore @Test From 6c2fcf2f566961b9c001173d9772968ed560f5d7 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 19 Jan 2022 13:10:04 +0530 Subject: [PATCH 16/49] Exclude generated classes from spotbug/forbiddenapis --- codestyle/spotbugs-exclude.xml | 11 +++++++++++ pom.xml | 2 ++ 2 files changed, 13 insertions(+) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index ae93e382cea8..b35b138d3dfe 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -28,6 +28,17 @@ Reference: https://github.com/apache/druid/pull/7894/files --> + + + + + + + + + + + diff --git a/pom.xml b/pom.xml index e2122970c5d0..f196fe8f63df 100644 --- a/pom.xml +++ b/pom.xml @@ -1315,6 +1315,7 @@ **/SomeAvroDatum.class **/DruidSqlParserImpl.class + **/DruidSqlParserImplTokenManager.class **/SimpleCharStream.class @@ -1890,6 +1891,7 @@ **/dependency-reduced-pom.xml .editorconfig **/hadoop.indexer.libs.version + **/codegen/** From 0b7dec692353fdd8d415df308abbd7ad95335a66 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 19 Jan 2022 17:13:50 +0530 Subject: [PATCH 17/49] Remove version from mvn-dep plugin, add sl4j dep --- sql/pom.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sql/pom.xml b/sql/pom.xml index 9df1c5406ad4..b8a393c23a58 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -180,6 +180,11 @@ validation-api provided + + org.slf4j + slf4j-api + 1.7.25 + @@ -259,7 +264,6 @@ org.apache.maven.plugins maven-dependency-plugin - 2.1 1.21.0 3.0.0 2.0.0 diff --git a/sql/pom.xml b/sql/pom.xml index b8a393c23a58..9a6dd6a2ffdb 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -261,13 +261,13 @@ + + org.apache.maven.plugins maven-dependency-plugin - unpack-parser-template initialize @@ -284,21 +284,39 @@ ${project.build.directory}/ **/Parser.jj - - org.apache.calcite - calcite-core - ${calcite.version} - jar - true - ${project.build.directory}/ - **/config.fmpp - + + + maven-resources-plugin + 3.2.0 + + + copy-fmpp-resources + initialize + + copy-resources + + + ${project.build.directory}/codegen + + + src/main/codegen + false + + + + + + + + com.googlecode.fmpp-maven-plugin fmpp-maven-plugin @@ -326,7 +344,7 @@ - + org.codehaus.mojo javacc-maven-plugin @@ -351,30 +369,7 @@ - - maven-resources-plugin - 3.1.0 - - - copy-fmpp-resources - initialize - - copy-resources - - - ${project.build.directory}/codegen - - - src/main/codegen - false - - - - - - - - + org.codehaus.mojo build-helper-maven-plugin @@ -396,5 +391,6 @@ + diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index d9d0ae2771c9..fb68eb337097 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -32,6 +32,16 @@ # Parser template file (Parser.jj) along with this file are packaged as # part of the calcite-core-.jar under "codegen" directory. +# This file is directly copied from calite-core-1.21.0.jar/codegen/config.fmpp, and then modified slightly. +# While not a necessary requirement, it would be ideal if it is kept in line with calcite-core's version. In the newer +# Calcite versions, there is a default_config.fmpp which will free us from maintaining this file. +# +# Following clauses are modified in the file: +# 1. data.parser.package & data.parser.class +# 2. data.parser.imports +# 3. data.parser.nonReservedKeywords (Added "CLUSTER" at the end of the list) +# 4. data.parser.statementParserMethods +# 5. data.parser.implementationFiles data: { parser: { # Generated parser implementation package and class name. @@ -49,7 +59,6 @@ data: { # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved # keyword add it to 'nonReservedKeywords' section. keywords: [ - "CLUSTER" ] # List of keywords from "keywords" section that are not reserved. @@ -357,6 +366,7 @@ data: { "WRITE" "XML" "ZONE" + "CLUSTER" ] # List of additional join types. Each is a method with no arguments. diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index ff446cf3572d..892221d064dd 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -31,7 +31,8 @@ /** * Extends the Insert call to hold custom paramaters specific to druid i.e. PARTITION BY and CLUSTER BY - * This class extends the {@link SqlInsert} so that the node can be used for further conversion + * This class extends the {@link SqlInsert} so that this SqlNode can be used in + * {@link org.apache.calcite.sql2rel.SqlToRelConverter} for getting converted into RelNode, and further processing */ public class DruidSqlInsert extends SqlInsert { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 19e9e7154157..b71cd7c3eca0 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -71,6 +71,7 @@ import org.apache.druid.common.guava.GuavaUtils; import org.apache.druid.common.utils.IdUtils; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.BaseSequence; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.java.util.common.guava.Sequences; @@ -792,6 +793,17 @@ static ParsedNodes create(final SqlNode node) throws ValidationException ingestionGranularity = druidSqlInsert.getPartitionBy(); + // Check if ingestionGranularity is valid + if (ingestionGranularity != null) { + try { + Granularity.fromString(ingestionGranularity); + } + catch (IllegalArgumentException e) { + throw new ValidationException(StringUtils.format( + "Granularity passed in PARTITION BY clause is invalid", ingestionGranularity)); + } + } + if (druidSqlInsert.getClusterBy() != null) { // If we have a CLUSTER BY clause, extract the information in that CLUSTER BY and create a new SqlOrderBy // node @@ -801,9 +813,10 @@ static ParsedNodes create(final SqlNode node) throws ValidationException if (query instanceof SqlOrderBy) { SqlOrderBy sqlOrderBy = (SqlOrderBy) query; - // Extract the query present inside the SqlOrderBy (which is free of ORDER BY, OFFSET and FETCH clauses) + // This represents the underlying query free of OFFSET, FETCH and ORDER BY clauses + // For a sqlOrderBy.query like "SELECT dim1, sum(dim2) FROM foo OFFSET 10 FETCH 30 ORDER BY dim1 GROUP BY dim1 + // this would represent the "SELECT dim1, sum(dim2) from foo GROUP BY dim1 query = sqlOrderBy.query; - offset = sqlOrderBy.offset; fetch = sqlOrderBy.fetch; orderByList = sqlOrderBy.orderList; diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index bfcf3a7f24e5..f9810bee3ef8 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -447,22 +447,46 @@ public void testInsertWithPartitionByAndOrderBy() } @Test - public void testInsertWithClusterByAndOrderBy() + public void testInsertWithClusterByAndOrderBy() throws Exception { + try { + testQuery( + StringUtils.format( + "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTER BY 3", + externSql(externalDataSource) + ), + ImmutableList.of(), + ImmutableList.of() + ); + Assert.fail("Exception should be thrown"); + } + catch (SqlPlanningException e) { + Assert.assertEquals( + "Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query", + e.getMessage() + ); + } + didTest = true; + } + @Test + public void testInsertWithPartitionByContainingInvalidGranularity() throws Exception + { // Throws a ValidationException, which gets converted to a SqlPlanningException before throwing to end user - Assert.assertThrows( - SqlPlanningException.class, - () -> - testQuery( - StringUtils.format( - "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTER BY 3", - externSql(externalDataSource) - ), - ImmutableList.of(), - ImmutableList.of() - ) - ); + try { + testQuery( + "INSERT INTO dst SELECT * FROM foo PARTITION BY 'invalid_granularity'", + ImmutableList.of(), + ImmutableList.of() + ); + Assert.fail("Exception should be thrown"); + } + catch (SqlPlanningException e) { + Assert.assertEquals( + "Granularity passed in PARTITION BY clause is invalid", + e.getMessage() + ); + } didTest = true; } From 4f4cb77e0cef627d1acefef3350665e10447bb38 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 27 Jan 2022 03:10:39 +0530 Subject: [PATCH 20/49] Improve testcase for cluster by --- .../druid/sql/calcite/CalciteInsertDmlTest.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index f9810bee3ef8..27f32f18e640 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -321,22 +321,30 @@ public void testInsertWithClusterBy() .add("__time", ColumnType.LONG) .add("floor_m1", ColumnType.FLOAT) .add("dim1", ColumnType.STRING) + .add("EXPR$3", ColumnType.DOUBLE) .build(); testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo CLUSTER BY 2, dim1") + "INSERT INTO druid.dst " + + "SELECT __time, FLOOR(m1) as floor_m1, dim1, CEIL(m2) FROM foo " + + "CLUSTER BY 2, dim1 DESC, CEIL(m2)" + ) .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( newScanQueryBuilder() .dataSource("foo") .intervals(querySegmentSpec(Filtration.eternity())) - .columns("__time", "dim1", "v0") - .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) + .columns("__time", "dim1", "v0", "v1") + .virtualColumns( + expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT), + expressionVirtualColumn("v1", "ceil(\"m2\")", ColumnType.DOUBLE) + ) .orderBy( ImmutableList.of( new ScanQuery.OrderBy("v0", ScanQuery.Order.ASCENDING), - new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) + new ScanQuery.OrderBy("dim1", ScanQuery.Order.DESCENDING), + new ScanQuery.OrderBy("v1", ScanQuery.Order.ASCENDING) ) ) .build() From 83714000ca977e85842386d25148eda9b6f60c6f Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 27 Jan 2022 03:24:42 +0530 Subject: [PATCH 21/49] Fix build, add CLUSTER to kwds as well --- sql/src/main/codegen/config.fmpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index fb68eb337097..70205472ee43 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -39,9 +39,10 @@ # Following clauses are modified in the file: # 1. data.parser.package & data.parser.class # 2. data.parser.imports -# 3. data.parser.nonReservedKeywords (Added "CLUSTER" at the end of the list) -# 4. data.parser.statementParserMethods -# 5. data.parser.implementationFiles +# 3. data.parser.nonReservedKeywords (Added "CLUSTER") +# 4. data.parser.keywords (Added "CLUSTER") +# 5. data.parser.statementParserMethods +# 6. data.parser.implementationFiles data: { parser: { # Generated parser implementation package and class name. @@ -59,6 +60,7 @@ data: { # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved # keyword add it to 'nonReservedKeywords' section. keywords: [ + "CLUSTER" ] # List of keywords from "keywords" section that are not reserved. @@ -93,6 +95,7 @@ data: { "CHARACTERISTICS" "CHARACTERS" "CLASS_ORIGIN" + "CLUSTER" "COBOL" "COLLATION" "COLLATION_CATALOG" @@ -366,7 +369,6 @@ data: { "WRITE" "XML" "ZONE" - "CLUSTER" ] # List of additional join types. Each is a method with no arguments. From 94d9175124e13b79b952c7b3cd9127ea99ac6c76 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 27 Jan 2022 14:55:41 +0530 Subject: [PATCH 22/49] fix format string --- .../org/apache/druid/sql/calcite/planner/DruidPlanner.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index b71cd7c3eca0..c184426ea3a6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -799,8 +799,7 @@ static ParsedNodes create(final SqlNode node) throws ValidationException Granularity.fromString(ingestionGranularity); } catch (IllegalArgumentException e) { - throw new ValidationException(StringUtils.format( - "Granularity passed in PARTITION BY clause is invalid", ingestionGranularity)); + throw new ValidationException("Granularity passed in PARTITION BY clause is invalid"); } } From 585cd614a5782d11c94bdf5680ae171f2303553e Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 27 Jan 2022 15:14:04 +0530 Subject: [PATCH 23/49] Make CLUSTER reserved keyword --- sql/src/main/codegen/config.fmpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 70205472ee43..465d98d42986 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -39,10 +39,9 @@ # Following clauses are modified in the file: # 1. data.parser.package & data.parser.class # 2. data.parser.imports -# 3. data.parser.nonReservedKeywords (Added "CLUSTER") -# 4. data.parser.keywords (Added "CLUSTER") -# 5. data.parser.statementParserMethods -# 6. data.parser.implementationFiles +# 3. data.parser.keywords (Added "CLUSTER") +# 4. data.parser.statementParserMethods +# 5. data.parser.implementationFiles data: { parser: { # Generated parser implementation package and class name. @@ -95,7 +94,6 @@ data: { "CHARACTERISTICS" "CHARACTERS" "CLASS_ORIGIN" - "CLUSTER" "COBOL" "COLLATION" "COLLATION_CATALOG" From 1e54241253dc134890837badccc3a35bb7bb7ac7 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Sat, 29 Jan 2022 00:09:12 +0530 Subject: [PATCH 24/49] POM changes - move plugin version to parent, scope=provided for slf4j --- pom.xml | 21 ++++++++++++++++--- sql/pom.xml | 15 ++----------- .../sql/calcite/parser/DruidSqlInsert.java | 2 +- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index cc4ccf74b7d7..bf98f4ea74bb 100644 --- a/pom.xml +++ b/pom.xml @@ -82,8 +82,8 @@ 2.2.4 1.17.0 1.9.2 - + 1.21.0 3.0.0 2.0.0 @@ -1506,7 +1506,12 @@ - + + com.googlecode.fmpp-maven-plugin + fmpp-maven-plugin + 1.0 + + org.apache.maven.plugins maven-surefire-plugin 2.22.2 @@ -1606,6 +1611,16 @@ exec-maven-plugin 1.6.0 + + org.codehaus.mojo + javacc-maven-plugin + 2.4 + + + org.codehaus.mojo + build-helper-maven-plugin + 3.1.0 + org.apache.maven.plugins maven-javadoc-plugin diff --git a/sql/pom.xml b/sql/pom.xml index 9a6dd6a2ffdb..7855dff16bc9 100644 --- a/sql/pom.xml +++ b/sql/pom.xml @@ -183,7 +183,7 @@ org.slf4j slf4j-api - 1.7.25 + provided @@ -294,7 +294,6 @@ ${project.build.directory}/codegen --> maven-resources-plugin - 3.2.0 copy-fmpp-resources @@ -320,14 +319,6 @@ com.googlecode.fmpp-maven-plugin fmpp-maven-plugin - 1.0 - - - org.freemarker - freemarker - 2.3.25-incubating - - generate-fmpp-sources @@ -348,7 +339,6 @@ org.codehaus.mojo javacc-maven-plugin - 2.4 generate-sources @@ -373,7 +363,6 @@ org.codehaus.mojo build-helper-maven-plugin - 3.1.0 add-source @@ -391,6 +380,6 @@ - + diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 892221d064dd..455c4355f5ba 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -36,7 +36,7 @@ */ public class DruidSqlInsert extends SqlInsert { - // Unsure if this should be kept as is, but this allows reusing super.unparse + // This allows reusing super.unparse public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; private final SqlNode partitionBy; From 4330703594ed0754e5bd72abbae78ca56a3ae169 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Sat, 29 Jan 2022 02:14:48 +0530 Subject: [PATCH 25/49] Rename cluster to clustered, partition to partitioned --- sql/src/main/codegen/config.fmpp | 7 ++-- sql/src/main/codegen/includes/insert.ftl | 16 +++---- .../sql/calcite/parser/DruidSqlInsert.java | 37 ++++++++-------- .../sql/calcite/planner/DruidPlanner.java | 20 ++++----- .../sql/calcite/CalciteInsertDmlTest.java | 42 +++++++++---------- 5 files changed, 61 insertions(+), 61 deletions(-) diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 465d98d42986..93f3c13943bd 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -39,7 +39,7 @@ # Following clauses are modified in the file: # 1. data.parser.package & data.parser.class # 2. data.parser.imports -# 3. data.parser.keywords (Added "CLUSTER") +# 3. data.parser.keywords (Added "CLUSTERED", "PARTITIONED") # 4. data.parser.statementParserMethods # 5. data.parser.implementationFiles data: { @@ -59,7 +59,8 @@ data: { # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved # keyword add it to 'nonReservedKeywords' section. keywords: [ - "CLUSTER" + "CLUSTERED" + "PARTITIONED" ] # List of keywords from "keywords" section that are not reserved. @@ -440,4 +441,4 @@ data: { freemarkerLinks: { includes: includes/ -} \ No newline at end of file +} diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index 5607d2ad6ec5..72d32fe1f86e 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -20,28 +20,28 @@ SqlNode DruidSqlInsert() : { SqlNode insertNode; - SqlNode partitionBy = null; - SqlNodeList clusterBy = null; + SqlNode partitionedBy = null; + SqlNodeList clusteredBy = null; } { insertNode = SqlInsert() [ - - partitionBy = StringLiteral() + + partitionedBy = StringLiteral() ] [ - - clusterBy = ClusterItems() + + clusteredBy = ClusterItems() ] { - if(partitionBy == null && clusterBy == null) { + if(partitionedBy == null && clusteredBy == null) { return insertNode; } if(!(insertNode instanceof SqlInsert)) { return insertNode; } SqlInsert sqlInsert = (SqlInsert) insertNode; - return new DruidSqlInsert(sqlInsert, partitionBy, clusterBy); + return new DruidSqlInsert(sqlInsert, partitionedBy, clusteredBy); } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 455c4355f5ba..80d0657c1f2f 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -30,7 +30,7 @@ import javax.annotation.Nullable; /** - * Extends the Insert call to hold custom paramaters specific to druid i.e. PARTITION BY and CLUSTER BY + * Extends the Insert call to hold custom paramaters specific to druid i.e. PARTITIONED BY and CLUSTERED BY * This class extends the {@link SqlInsert} so that this SqlNode can be used in * {@link org.apache.calcite.sql2rel.SqlToRelConverter} for getting converted into RelNode, and further processing */ @@ -39,13 +39,13 @@ public class DruidSqlInsert extends SqlInsert // This allows reusing super.unparse public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; - private final SqlNode partitionBy; - private final SqlNodeList clusterBy; + private final SqlNode partitionedBy; + private final SqlNodeList clusteredBy; public DruidSqlInsert( @Nonnull SqlInsert insertNode, - @Nullable SqlNode partitionBy, - @Nullable SqlNodeList clusterBy + @Nullable SqlNode partitionedBy, + @Nullable SqlNodeList clusteredBy ) { super( @@ -55,23 +55,23 @@ public DruidSqlInsert( insertNode.getSource(), insertNode.getTargetColumnList() ); - this.partitionBy = partitionBy; - this.clusterBy = clusterBy; + this.partitionedBy = partitionedBy; + this.clusteredBy = clusteredBy; } @Nullable - public SqlNodeList getClusterBy() + public SqlNodeList getClusteredBy() { - return clusterBy; + return clusteredBy; } @Nullable - public String getPartitionBy() + public String getPartitionedBy() { - if (partitionBy == null) { + if (partitionedBy == null) { return null; } - return SqlLiteral.unchain(partitionBy).toValue(); + return SqlLiteral.unchain(partitionedBy).toValue(); } @Nonnull @@ -85,15 +85,14 @@ public SqlOperator getOperator() public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { super.unparse(writer, leftPrec, rightPrec); - if (partitionBy != null) { - writer.keyword("PARTITION"); - writer.keyword("BY"); - writer.keyword(getPartitionBy()); + if (partitionedBy != null) { + writer.keyword("PARTITIONED BY"); + writer.keyword(getPartitionedBy()); } - if (clusterBy != null) { - writer.sep("CLUSTER BY"); + if (clusteredBy != null) { + writer.sep("CLUSTERED BY"); SqlWriter.Frame frame = writer.startList("", ""); - for (SqlNode clusterByOpts : clusterBy.getList()) { + for (SqlNode clusterByOpts : getClusteredBy().getList()) { clusterByOpts.unparse(writer, leftPrec, rightPrec); } writer.endList(frame); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index c184426ea3a6..0c0b431c54c3 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -787,11 +787,11 @@ static ParsedNodes create(final SqlNode node) throws ValidationException insert = (SqlInsert) query; query = insert.getSource(); - // Processing to be done when the original query has either of the PARTITION BY or CLUSTER BY clause + // Processing to be done when the original query has either of the PARTITIONED BY or CLUSTERED BY clause if (insert instanceof DruidSqlInsert) { DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; - ingestionGranularity = druidSqlInsert.getPartitionBy(); + ingestionGranularity = druidSqlInsert.getPartitionedBy(); // Check if ingestionGranularity is valid if (ingestionGranularity != null) { @@ -799,12 +799,12 @@ static ParsedNodes create(final SqlNode node) throws ValidationException Granularity.fromString(ingestionGranularity); } catch (IllegalArgumentException e) { - throw new ValidationException("Granularity passed in PARTITION BY clause is invalid"); + throw new ValidationException("Granularity passed in PARTITIONED BY clause is invalid"); } } - if (druidSqlInsert.getClusterBy() != null) { - // If we have a CLUSTER BY clause, extract the information in that CLUSTER BY and create a new SqlOrderBy + if (druidSqlInsert.getClusteredBy() != null) { + // If we have a CLUSTERED BY clause, extract the information in that CLUSTERED BY and create a new SqlOrderBy // node SqlNode offset = null; SqlNode fetch = null; @@ -819,19 +819,19 @@ static ParsedNodes create(final SqlNode node) throws ValidationException offset = sqlOrderBy.offset; fetch = sqlOrderBy.fetch; orderByList = sqlOrderBy.orderList; - // If the orderList is non-empty (i.e. there existed an ORDER BY clause in the query) and CLUSTER BY clause + // If the orderList is non-empty (i.e. there existed an ORDER BY clause in the query) and CLUSTERED BY clause // is also non-empty, throw an error if (!(orderByList == null || orderByList.equals(SqlNodeList.EMPTY)) - && druidSqlInsert.getClusterBy() != null) { + && druidSqlInsert.getClusteredBy() != null) { throw new ValidationException( - "Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query"); + "Cannot have both ORDER BY and CLUSTERED BY clauses in the same INSERT query"); } } - // Creates a new SqlOrderBy query, which may have our CLUSTER BY overwritten + // Creates a new SqlOrderBy query, which may have our CLUSTERED BY overwritten query = new SqlOrderBy( query.getParserPosition(), query, - GuavaUtils.firstNonNull(orderByList, druidSqlInsert.getClusterBy()), + GuavaUtils.firstNonNull(orderByList, druidSqlInsert.getClusteredBy()), offset, fetch ); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index 27f32f18e640..ab2a08f4198a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -284,9 +284,9 @@ public void testInsertFromExternal() } @Test - public void testInsertWithPartitionBy() + public void testInsertWithPartitionedBy() { - // Test correctness of the query when only PARTITION BY clause is present + // Test correctness of the query when only PARTITIONED BY clause is present RowSignature targetRowSignature = RowSignature.builder() .add("__time", ColumnType.LONG) .add("floor_m1", ColumnType.FLOAT) @@ -298,7 +298,7 @@ public void testInsertWithPartitionBy() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITION BY 'day'") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY 'day'") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -314,9 +314,9 @@ public void testInsertWithPartitionBy() } @Test - public void testInsertWithClusterBy() + public void testInsertWithClusteredBy() { - // Test correctness of the query when only CLUSTER BY clause is present + // Test correctness of the query when only CLUSTERED BY clause is present RowSignature targetRowSignature = RowSignature.builder() .add("__time", ColumnType.LONG) .add("floor_m1", ColumnType.FLOAT) @@ -327,7 +327,7 @@ public void testInsertWithClusterBy() .sql( "INSERT INTO druid.dst " + "SELECT __time, FLOOR(m1) as floor_m1, dim1, CEIL(m2) FROM foo " - + "CLUSTER BY 2, dim1 DESC, CEIL(m2)" + + "CLUSTERED BY 2, dim1 DESC, CEIL(m2)" ) .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) @@ -353,9 +353,9 @@ public void testInsertWithClusterBy() } @Test - public void testInsertWithPartitionByAndClusterBy() + public void testInsertWithPartitionedByAndClusteredBy() { - // Test correctness of the query when both PARTITION BY and CLUSTER BY clause is present + // Test correctness of the query when both PARTITIONED BY and CLUSTERED BY clause is present RowSignature targetRowSignature = RowSignature.builder() .add("__time", ColumnType.LONG) .add("floor_m1", ColumnType.FLOAT) @@ -367,7 +367,7 @@ public void testInsertWithPartitionByAndClusterBy() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITION BY 'day' CLUSTER BY 2, dim1") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY 'day' CLUSTERED BY 2, dim1") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -389,7 +389,7 @@ public void testInsertWithPartitionByAndClusterBy() } @Test - public void testInsertWithPartitionByAndLimitOffset() + public void testInsertWithPartitionedByAndLimitOffset() { RowSignature targetRowSignature = RowSignature.builder() .add("__time", ColumnType.LONG) @@ -402,7 +402,7 @@ public void testInsertWithPartitionByAndLimitOffset() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo LIMIT 10 OFFSET 20 PARTITION BY 'day'") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo LIMIT 10 OFFSET 20 PARTITIONED BY 'day'") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -420,7 +420,7 @@ public void testInsertWithPartitionByAndLimitOffset() } @Test - public void testInsertWithPartitionByAndOrderBy() + public void testInsertWithPartitionedByAndOrderBy() { RowSignature targetRowSignature = RowSignature.builder() .add("__time", ColumnType.LONG) @@ -433,7 +433,7 @@ public void testInsertWithPartitionByAndOrderBy() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo ORDER BY 2, dim1 PARTITION BY 'day'") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo ORDER BY 2, dim1 PARTITIONED BY 'day'") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -455,12 +455,12 @@ public void testInsertWithPartitionByAndOrderBy() } @Test - public void testInsertWithClusterByAndOrderBy() throws Exception + public void testInsertWithClusteredByAndOrderBy() throws Exception { try { testQuery( StringUtils.format( - "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTER BY 3", + "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTERED BY 3", externSql(externalDataSource) ), ImmutableList.of(), @@ -470,7 +470,7 @@ public void testInsertWithClusterByAndOrderBy() throws Exception } catch (SqlPlanningException e) { Assert.assertEquals( - "Cannot have both ORDER BY and CLUSTER BY clauses in the same INSERT query", + "Cannot have both ORDER BY and CLUSTERED BY clauses in the same INSERT query", e.getMessage() ); } @@ -478,12 +478,12 @@ public void testInsertWithClusterByAndOrderBy() throws Exception } @Test - public void testInsertWithPartitionByContainingInvalidGranularity() throws Exception + public void testInsertWithPartitionedByContainingInvalidGranularity() throws Exception { // Throws a ValidationException, which gets converted to a SqlPlanningException before throwing to end user try { testQuery( - "INSERT INTO dst SELECT * FROM foo PARTITION BY 'invalid_granularity'", + "INSERT INTO dst SELECT * FROM foo PARTITIONED BY 'invalid_granularity'", ImmutableList.of(), ImmutableList.of() ); @@ -491,7 +491,7 @@ public void testInsertWithPartitionByContainingInvalidGranularity() throws Excep } catch (SqlPlanningException e) { Assert.assertEquals( - "Granularity passed in PARTITION BY clause is invalid", + "Granularity passed in PARTITIONED BY clause is invalid", e.getMessage() ); } @@ -502,14 +502,14 @@ public void testInsertWithPartitionByContainingInvalidGranularity() throws Excep // Currently EXPLAIN PLAN FOR doesn't work with the modified syntax @Ignore @Test - public void testExplainInsertWithPartitionByAndClusterBy() + public void testExplainInsertWithPartitionedByAndClusteredBy() { Assert.assertThrows( SqlPlanningException.class, () -> testQuery( StringUtils.format( - "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITION BY 'day' CLUSTER BY 1", + "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITIONED BY 'day' CLUSTERED BY 1", externSql(externalDataSource) ), ImmutableList.of(), From 0ae557052635f7fd468e100282efed929fe70280 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Sat, 29 Jan 2022 02:35:17 +0530 Subject: [PATCH 26/49] Disallow ORDER BY in SELECT inside INSERT queries --- .../sql/calcite/planner/DruidPlanner.java | 20 +++--- .../sql/calcite/CalciteInsertDmlTest.java | 62 ++++++++----------- 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 0c0b431c54c3..ee9e92364167 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -787,6 +787,15 @@ static ParsedNodes create(final SqlNode node) throws ValidationException insert = (SqlInsert) query; query = insert.getSource(); + // Check if ORDER BY clause is not provided to the underlying query + if (query instanceof SqlOrderBy) { + SqlOrderBy sqlOrderBy = (SqlOrderBy) query; + SqlNodeList orderByList = sqlOrderBy.orderList; + if (!(orderByList == null || orderByList.equals(SqlNodeList.EMPTY))) { + throw new ValidationException("Cannot have ORDER BY on an INSERT query, use CLUSTERED BY instead."); + } + } + // Processing to be done when the original query has either of the PARTITIONED BY or CLUSTERED BY clause if (insert instanceof DruidSqlInsert) { DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; @@ -808,7 +817,6 @@ static ParsedNodes create(final SqlNode node) throws ValidationException // node SqlNode offset = null; SqlNode fetch = null; - SqlNodeList orderByList = null; if (query instanceof SqlOrderBy) { SqlOrderBy sqlOrderBy = (SqlOrderBy) query; @@ -818,20 +826,12 @@ static ParsedNodes create(final SqlNode node) throws ValidationException query = sqlOrderBy.query; offset = sqlOrderBy.offset; fetch = sqlOrderBy.fetch; - orderByList = sqlOrderBy.orderList; - // If the orderList is non-empty (i.e. there existed an ORDER BY clause in the query) and CLUSTERED BY clause - // is also non-empty, throw an error - if (!(orderByList == null || orderByList.equals(SqlNodeList.EMPTY)) - && druidSqlInsert.getClusteredBy() != null) { - throw new ValidationException( - "Cannot have both ORDER BY and CLUSTERED BY clauses in the same INSERT query"); - } } // Creates a new SqlOrderBy query, which may have our CLUSTERED BY overwritten query = new SqlOrderBy( query.getParserPosition(), query, - GuavaUtils.firstNonNull(orderByList, druidSqlInsert.getClusteredBy()), + druidSqlInsert.getClusteredBy(), offset, fetch ); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index ab2a08f4198a..ad5ffe947c8b 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -419,41 +419,6 @@ public void testInsertWithPartitionedByAndLimitOffset() .verify(); } - @Test - public void testInsertWithPartitionedByAndOrderBy() - { - RowSignature targetRowSignature = RowSignature.builder() - .add("__time", ColumnType.LONG) - .add("floor_m1", ColumnType.FLOAT) - .add("dim1", ColumnType.STRING) - .build(); - - Map queryContext = new HashMap<>(DEFAULT_CONTEXT); - queryContext.put(QueryContexts.INGESTION_GRANULARITY, "day"); - - testInsertQuery() - .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo ORDER BY 2, dim1 PARTITIONED BY 'day'") - .expectTarget("dst", targetRowSignature) - .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) - .expectQuery( - newScanQueryBuilder() - .dataSource("foo") - .intervals(querySegmentSpec(Filtration.eternity())) - .columns("__time", "dim1", "v0") - .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) - .orderBy( - ImmutableList.of( - new ScanQuery.OrderBy("v0", ScanQuery.Order.ASCENDING), - new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) - ) - ) - .context(queryContext) - .build() - ) - .verify(); - } - @Test public void testInsertWithClusteredByAndOrderBy() throws Exception { @@ -470,7 +435,7 @@ public void testInsertWithClusteredByAndOrderBy() throws Exception } catch (SqlPlanningException e) { Assert.assertEquals( - "Cannot have both ORDER BY and CLUSTERED BY clauses in the same INSERT query", + "Cannot have ORDER BY on an INSERT query, use CLUSTERED BY instead.", e.getMessage() ); } @@ -498,6 +463,29 @@ public void testInsertWithPartitionedByContainingInvalidGranularity() throws Exc didTest = true; } + @Test + public void testInsertWithOrderBy() throws Exception + { + try { + testQuery( + StringUtils.format( + "INSERT INTO dst SELECT * FROM %s ORDER BY 2", + externSql(externalDataSource) + ), + ImmutableList.of(), + ImmutableList.of() + ); + Assert.fail("Exception should be thrown"); + } + catch (SqlPlanningException e) { + Assert.assertEquals( + "Cannot have ORDER BY on an INSERT query, use CLUSTERED BY instead.", + e.getMessage() + ); + } finally { + didTest = true; + } + } // Currently EXPLAIN PLAN FOR doesn't work with the modified syntax @Ignore @@ -593,7 +581,7 @@ public void testInsertFromExternalProjectSort() // INSERT with a particular column ordering. testInsertQuery() - .sql("INSERT INTO dst SELECT x || y AS xy, z FROM %s ORDER BY 1, 2", externSql(externalDataSource)) + .sql("INSERT INTO dst SELECT x || y AS xy, z FROM %s CLUSTERED BY 1, 2", externSql(externalDataSource)) .authentication(CalciteTests.SUPER_USER_AUTH_RESULT) .expectTarget("dst", RowSignature.builder().add("xy", ColumnType.STRING).add("z", ColumnType.LONG).build()) .expectResources(dataSourceWrite("dst"), ExternalOperatorConversion.EXTERNAL_RESOURCE_ACTION) From 37728f2b70d4f4b4a864e780ddcaac52dd220418 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Sat, 29 Jan 2022 03:41:20 +0530 Subject: [PATCH 27/49] Indent ftl properly, try cleanup around Insert and DruidSqlInsert --- sql/src/main/codegen/includes/insert.ftl | 35 +++++++++---------- .../sql/calcite/planner/DruidPlanner.java | 1 + 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index 72d32fe1f86e..ab047fef56db 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -24,25 +24,24 @@ SqlNode DruidSqlInsert() : SqlNodeList clusteredBy = null; } { - insertNode = SqlInsert() - [ - - partitionedBy = StringLiteral() - ] - [ - - clusteredBy = ClusterItems() - ] - { - if(partitionedBy == null && clusteredBy == null) { - return insertNode; - } - if(!(insertNode instanceof SqlInsert)) { - return insertNode; - } - SqlInsert sqlInsert = (SqlInsert) insertNode; - return new DruidSqlInsert(sqlInsert, partitionedBy, clusteredBy); + insertNode = SqlInsert() + [ + + partitionedBy = StringLiteral() + ] + [ + + clusteredBy = ClusterItems() + ] + { + if (!(insertNode instanceof SqlInsert)) { + // This shouldn't be encountered, but done as a defensive practice. SqlInsert() always returns a node of type + // SqlInsert + return insertNode; } + SqlInsert sqlInsert = (SqlInsert) insertNode; + return new DruidSqlInsert(sqlInsert, partitionedBy, clusteredBy); + } } SqlNodeList ClusterItems() : diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index ee9e92364167..604eb5b5b1a8 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -797,6 +797,7 @@ static ParsedNodes create(final SqlNode node) throws ValidationException } // Processing to be done when the original query has either of the PARTITIONED BY or CLUSTERED BY clause + // The following condition should always be true however added defensively if (insert instanceof DruidSqlInsert) { DruidSqlInsert druidSqlInsert = (DruidSqlInsert) insert; From 4e63d276a15e1b8cc9b38751b1c03eecb0e02c92 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Sat, 29 Jan 2022 04:14:52 +0530 Subject: [PATCH 28/49] checkstyle fix --- .../org/apache/druid/sql/calcite/planner/DruidPlanner.java | 1 - .../org/apache/druid/sql/calcite/CalciteInsertDmlTest.java | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 604eb5b5b1a8..fecd70e0c3c8 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -68,7 +68,6 @@ import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; import org.apache.calcite.util.Pair; -import org.apache.druid.common.guava.GuavaUtils; import org.apache.druid.common.utils.IdUtils; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularity; diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index ad5ffe947c8b..cf5335d2d072 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -482,7 +482,8 @@ public void testInsertWithOrderBy() throws Exception "Cannot have ORDER BY on an INSERT query, use CLUSTERED BY instead.", e.getMessage() ); - } finally { + } + finally { didTest = true; } } From 1b62e6cc442dd7644aa61ffba530cac3b22b97a9 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 2 Feb 2022 15:08:21 +0530 Subject: [PATCH 29/49] Update the partition by clause --- sql/src/main/codegen/config.fmpp | 3 + sql/src/main/codegen/includes/insert.ftl | 46 ++++++- .../builtin/TimeFloorOperatorConversion.java | 3 +- .../sql/calcite/parser/DruidSqlInsert.java | 15 +-- .../sql/calcite/parser/DruidSqlUtils.java | 117 ++++++++++++++++++ .../sql/calcite/planner/DruidPlanner.java | 38 +++--- 6 files changed, 189 insertions(+), 33 deletions(-) create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 93f3c13943bd..e3d3c07f51ab 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -53,7 +53,10 @@ data: { imports: [ "org.apache.calcite.sql.SqlNode" "org.apache.calcite.sql.SqlInsert" + "org.apache.druid.java.util.common.granularity.Granularity" + "org.apache.druid.java.util.common.granularity.Granularities" "org.apache.druid.sql.calcite.parser.DruidSqlInsert" + "org.apache.druid.sql.calcite.parser.DruidSqlUtils" ] # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index ab047fef56db..cee48f5e7341 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -19,15 +19,15 @@ SqlNode DruidSqlInsert() : { - SqlNode insertNode; - SqlNode partitionedBy = null; - SqlNodeList clusteredBy = null; + SqlNode insertNode; + Granularity partitionedBy = null; + SqlNodeList clusteredBy = null; } { insertNode = SqlInsert() [ - partitionedBy = StringLiteral() + partitionedBy = PartitionGranularity() ] [ @@ -62,3 +62,41 @@ SqlNodeList ClusterItems() : return new SqlNodeList(list, s.addAll(list).pos()); } } + +Granularity PartitionGranularity() : +{ + SqlNode e = null; +} +{ + ( + + { + return Granularities.HOUR; + } + | + + { + return Granularities.DAY; + } + | + + { + return Granularities.MONTH; + } + | + + { + return Granularities.YEAR; + } + | +

+ * Validation on the sqlNode is contingent to following conditions: + * 1. sqlNode is an instance of SqlCall + * 2. Operator is either one of TIME_FLOOR or FLOOR + * 3. Number of operands in the call are 2 + * 4. First operand is a SimpleIdentifier representing __time + * 5. If operator is TIME_FLOOR, the second argument is a literal, and can be converted to the Granularity class + * 6. If operator is FLOOR, the second argument is a TimeUnit, and can be mapped using {@link org.apache.druid.sql.calcite.expression.TimeUnits} + *

+ * Since it is to be used primarily while parsing the SqlNode, it is wrapped in {@code convertSqlNodeToGranularityThrowingParseExceptions} + * + * @param sqlNode SqlNode representing a call to a function + * @return + * @throws ParseException SqlNode cannot be converted a granularity + */ + public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws ParseException + { + if (!(sqlNode instanceof SqlCall)) { + throw new ParseException("abc"); + } + SqlCall sqlCall = (SqlCall) sqlNode; + + List operandList = sqlCall.getOperandList(); + Preconditions.checkArgument(operandList.size() == 2, "Invalid number of arguments"); + + SqlOperator operator = sqlCall.getOperator(); + + + SqlNode timeOperandSqlNode = operandList.get(0); + Preconditions.checkArgument(timeOperandSqlNode.getKind().equals(SqlKind.IDENTIFIER), "abc"); + SqlIdentifier timeOperandSqlIdentifier = (SqlIdentifier) timeOperandSqlNode; + Preconditions.checkArgument(timeOperandSqlIdentifier.getSimple().equals(ColumnHolder.TIME_COLUMN_NAME), "abc"); + + if (operator.getName().equals(TimeFloorOperatorConversion.SQL_FUNCTION_NAME)) { + SqlNode granularitySqlNode = operandList.get(1); + Preconditions.checkArgument(granularitySqlNode.getKind().equals(SqlKind.LITERAL), "abc"); + String granularityString = SqlLiteral.unchain(granularitySqlNode).toValue(); + Period period = new Period(granularityString); + return new PeriodGranularity(period, null, null); + + } else if (operator.getName().equals("FLOOR")) { + SqlNode granularitySqlNode = operandList.get(1); + // In future versions of Calcite, this can be checked via + // granularitySqlNode.getKind().equals(SqlKind.INTERVAL_QUALIFIER) + Preconditions.checkArgument(granularitySqlNode instanceof SqlIntervalQualifier, "abc"); + SqlIntervalQualifier granularityIntervalQualifier = (SqlIntervalQualifier) granularitySqlNode; + + Period period = TimeUnits.toPeriod(granularityIntervalQualifier.timeUnitRange); + Preconditions.checkNotNull(period, "abc"); + return new PeriodGranularity(period, null, null); + } + + throw new ParseException("Unable to convert"); + } +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index fecd70e0c3c8..9eb122f3e7b6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -203,9 +203,19 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo final ParsedNodes parsed = ParsedNodes.create(planner.parse(plannerContext.getSql())); - if (parsed.getIngestionGranularity() != null) { - plannerContext.getQueryContext().put(QueryContexts.INGESTION_GRANULARITY, parsed.getIngestionGranularity()); + try { + if (parsed.getIngestionGranularity() != null) { + plannerContext.getQueryContext() + .put( + QueryContexts.INGESTION_GRANULARITY, + plannerContext.getJsonMapper().writeValueAsString(parsed.getIngestionGranularity()) + ); + } } + catch (JsonProcessingException e) { + + } + // the planner's type factory is not available until after parsing this.rexBuilder = new RexBuilder(planner.getTypeFactory()); @@ -747,21 +757,21 @@ public T next() private static class ParsedNodes { @Nullable - private SqlExplain explain; + private final SqlExplain explain; @Nullable - private SqlInsert insert; + private final SqlInsert insert; - private SqlNode query; + private final SqlNode query; @Nullable - private String ingestionGranularity; + private final Granularity ingestionGranularity; private ParsedNodes( @Nullable SqlExplain explain, @Nullable SqlInsert insert, SqlNode query, - @Nullable String ingestionGranularity + @Nullable Granularity ingestionGranularity ) { this.explain = explain; @@ -775,7 +785,7 @@ static ParsedNodes create(final SqlNode node) throws ValidationException SqlExplain explain = null; SqlInsert insert = null; SqlNode query = node; - String ingestionGranularity = null; + Granularity ingestionGranularity = null; if (query.getKind() == SqlKind.EXPLAIN) { explain = (SqlExplain) query; @@ -802,16 +812,6 @@ static ParsedNodes create(final SqlNode node) throws ValidationException ingestionGranularity = druidSqlInsert.getPartitionedBy(); - // Check if ingestionGranularity is valid - if (ingestionGranularity != null) { - try { - Granularity.fromString(ingestionGranularity); - } - catch (IllegalArgumentException e) { - throw new ValidationException("Granularity passed in PARTITIONED BY clause is invalid"); - } - } - if (druidSqlInsert.getClusteredBy() != null) { // If we have a CLUSTERED BY clause, extract the information in that CLUSTERED BY and create a new SqlOrderBy // node @@ -864,7 +864,7 @@ public SqlNode getQueryNode() } @Nullable - public String getIngestionGranularity() + public Granularity getIngestionGranularity() { return ingestionGranularity; } From 960870af97bfc05c1278ec9fda0a4ee91ccc3b29 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 2 Feb 2022 22:28:00 +0530 Subject: [PATCH 30/49] Update the error messages in parsing class --- .../sql/calcite/parser/DruidSqlUtils.java | 47 ++++++++++++++----- .../sql/calcite/planner/DruidPlanner.java | 12 ++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java index 8137a7cf07cc..9dfbf42df6bf 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java @@ -27,6 +27,7 @@ import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlOperator; +import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.granularity.PeriodGranularity; import org.apache.druid.segment.column.ColumnHolder; @@ -78,40 +79,62 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws ParseException { if (!(sqlNode instanceof SqlCall)) { - throw new ParseException("abc"); + throw new ParseException(StringUtils.format("Unable to parse the granularity from %s", sqlNode.toString())); } SqlCall sqlCall = (SqlCall) sqlNode; List operandList = sqlCall.getOperandList(); - Preconditions.checkArgument(operandList.size() == 2, "Invalid number of arguments"); - - SqlOperator operator = sqlCall.getOperator(); + Preconditions.checkArgument( + operandList.size() == 2, + "Invalid number of arguments passed to the floor function in PARTIITONED BY" + ); + String operatorName = sqlCall.getOperator().getName(); + // Check if the first argument passed in the floor function is __time SqlNode timeOperandSqlNode = operandList.get(0); - Preconditions.checkArgument(timeOperandSqlNode.getKind().equals(SqlKind.IDENTIFIER), "abc"); + Preconditions.checkArgument( + timeOperandSqlNode.getKind().equals(SqlKind.IDENTIFIER), + StringUtils.format("First argument to %s in PARTITIONED BY can only be __time", operatorName) + ) SqlIdentifier timeOperandSqlIdentifier = (SqlIdentifier) timeOperandSqlNode; - Preconditions.checkArgument(timeOperandSqlIdentifier.getSimple().equals(ColumnHolder.TIME_COLUMN_NAME), "abc"); + Preconditions.checkArgument( + timeOperandSqlIdentifier.getSimple().equals(ColumnHolder.TIME_COLUMN_NAME), + StringUtils.format("First argument to %s in PARTITIONED BY can only be __time", operatorName) + ); - if (operator.getName().equals(TimeFloorOperatorConversion.SQL_FUNCTION_NAME)) { + // If the floor function is of form TIME_FLOOR(__time, 'PT1H') + if (operatorName.equals(TimeFloorOperatorConversion.SQL_FUNCTION_NAME)) { SqlNode granularitySqlNode = operandList.get(1); - Preconditions.checkArgument(granularitySqlNode.getKind().equals(SqlKind.LITERAL), "abc"); + Preconditions.checkArgument( + granularitySqlNode.getKind().equals(SqlKind.LITERAL), + "Second argument to the TIME_FLOOR function in PARTITIONED BY is invalid" + ); String granularityString = SqlLiteral.unchain(granularitySqlNode).toValue(); Period period = new Period(granularityString); return new PeriodGranularity(period, null, null); - } else if (operator.getName().equals("FLOOR")) { + } else if (operatorName.equals("FLOOR")) { // If the floor function is of form FLOOR(__time TO DAY) SqlNode granularitySqlNode = operandList.get(1); // In future versions of Calcite, this can be checked via // granularitySqlNode.getKind().equals(SqlKind.INTERVAL_QUALIFIER) - Preconditions.checkArgument(granularitySqlNode instanceof SqlIntervalQualifier, "abc"); + Preconditions.checkArgument( + granularitySqlNode instanceof SqlIntervalQualifier, + "Second argument to the FLOOR function in PARTITIONED BY is invalid" + ); SqlIntervalQualifier granularityIntervalQualifier = (SqlIntervalQualifier) granularitySqlNode; Period period = TimeUnits.toPeriod(granularityIntervalQualifier.timeUnitRange); - Preconditions.checkNotNull(period, "abc"); + Preconditions.checkNotNull( + period, + StringUtils.format( + "Unable to convert %s to valid granularity for ingestion", + granularityIntervalQualifier.timeUnitRange.toString() + ) + ); return new PeriodGranularity(period, null, null); } - throw new ParseException("Unable to convert"); + throw new ParseException("Unable to parse the PARTITIONED BY clause"); } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 9eb122f3e7b6..5cff33c346dc 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -205,18 +205,16 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo try { if (parsed.getIngestionGranularity() != null) { - plannerContext.getQueryContext() - .put( - QueryContexts.INGESTION_GRANULARITY, - plannerContext.getJsonMapper().writeValueAsString(parsed.getIngestionGranularity()) - ); + plannerContext.getQueryContext().put( + QueryContexts.INGESTION_GRANULARITY, + plannerContext.getJsonMapper().writeValueAsString(parsed.getIngestionGranularity()) + ); } } catch (JsonProcessingException e) { - + throw new ValidationException("Unable to serialize partition granularity."); } - // the planner's type factory is not available until after parsing this.rexBuilder = new RexBuilder(planner.getTypeFactory()); final SqlNode parameterizedQueryNode = rewriteDynamicParameters(parsed.getQueryNode()); From 8717d0b136776a7347bc4214cdd5ed0ba08a81f7 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Wed, 2 Feb 2022 22:30:14 +0530 Subject: [PATCH 31/49] semicolon fix, javadoc --- .../org/apache/druid/sql/calcite/parser/DruidSqlUtils.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java index 9dfbf42df6bf..cf8041d310a4 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java @@ -26,7 +26,6 @@ import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; -import org.apache.calcite.sql.SqlOperator; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.granularity.PeriodGranularity; @@ -68,12 +67,12 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql * 3. Number of operands in the call are 2 * 4. First operand is a SimpleIdentifier representing __time * 5. If operator is TIME_FLOOR, the second argument is a literal, and can be converted to the Granularity class - * 6. If operator is FLOOR, the second argument is a TimeUnit, and can be mapped using {@link org.apache.druid.sql.calcite.expression.TimeUnits} + * 6. If operator is FLOOR, the second argument is a TimeUnit, and can be mapped using {@link TimeUnits} *

* Since it is to be used primarily while parsing the SqlNode, it is wrapped in {@code convertSqlNodeToGranularityThrowingParseExceptions} * * @param sqlNode SqlNode representing a call to a function - * @return + * @return Granularity as intended by the function call * @throws ParseException SqlNode cannot be converted a granularity */ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws ParseException @@ -96,7 +95,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa Preconditions.checkArgument( timeOperandSqlNode.getKind().equals(SqlKind.IDENTIFIER), StringUtils.format("First argument to %s in PARTITIONED BY can only be __time", operatorName) - ) + ); SqlIdentifier timeOperandSqlIdentifier = (SqlIdentifier) timeOperandSqlNode; Preconditions.checkArgument( timeOperandSqlIdentifier.getSimple().equals(ColumnHolder.TIME_COLUMN_NAME), From 72ae0bd12032115e56fdc0d2eaf411d8d3711a32 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 3 Feb 2022 02:26:53 +0530 Subject: [PATCH 32/49] Add test cases for the util class --- sql/src/main/codegen/config.fmpp | 2 +- sql/src/main/codegen/includes/insert.ftl | 2 +- .../builtin/TimeFloorOperatorConversion.java | 2 +- ...SqlUtils.java => DruidSqlParserUtils.java} | 3 +- .../parser/DruidSqlParserUtilsTest.java | 130 ++++++++++++++++++ 5 files changed, 134 insertions(+), 5 deletions(-) rename sql/src/main/java/org/apache/druid/sql/calcite/parser/{DruidSqlUtils.java => DruidSqlParserUtils.java} (99%) create mode 100644 sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index e3d3c07f51ab..d751b62b9d67 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -56,7 +56,7 @@ data: { "org.apache.druid.java.util.common.granularity.Granularity" "org.apache.druid.java.util.common.granularity.Granularities" "org.apache.druid.sql.calcite.parser.DruidSqlInsert" - "org.apache.druid.sql.calcite.parser.DruidSqlUtils" + "org.apache.druid.sql.calcite.parser.DruidSqlParserUtils" ] # List of new keywords. Example: "DATABASES", "TABLES". If the keyword is not a reserved diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index cee48f5e7341..da8344cd84db 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -96,7 +96,7 @@ Granularity PartitionGranularity() : | e = Expression(ExprContext.ACCEPT_SUB_QUERY) { - return DruidSqlUtils.convertSqlNodeToGranularityThrowingParseExceptions(e); + return DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(e); } ) } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java index 488c7fedc312..1279b6c0602b 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java @@ -53,7 +53,7 @@ public class TimeFloorOperatorConversion implements SqlOperatorConversion { public static final String SQL_FUNCTION_NAME = "TIME_FLOOR"; - private static final SqlFunction SQL_FUNCTION = OperatorConversions + public static final SqlFunction SQL_FUNCTION = OperatorConversions .operatorBuilder(SQL_FUNCTION_NAME) .operandTypes(SqlTypeFamily.TIMESTAMP, SqlTypeFamily.CHARACTER, SqlTypeFamily.TIMESTAMP, SqlTypeFamily.CHARACTER) .requiredOperands(2) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java similarity index 99% rename from sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java rename to sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java index cf8041d310a4..c05ed4776af3 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java @@ -36,8 +36,7 @@ import java.util.List; -// TODO: More appropriate name -public class DruidSqlUtils +public class DruidSqlParserUtils { /** * Delegates to {@code convertSqlNodeToGranularity} and converts the exceptions to {@link ParseException} diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java new file mode 100644 index 000000000000..19cbb1709f2e --- /dev/null +++ b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java @@ -0,0 +1,130 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.parser; + +import com.google.common.collect.ImmutableList; +import org.apache.calcite.avatica.util.TimeUnit; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlIntervalQualifier; +import org.apache.calcite.sql.SqlLiteral; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.sql.calcite.expression.TimeUnits; +import org.apache.druid.sql.calcite.expression.builtin.TimeFloorOperatorConversion; +import org.junit.Assert; +import org.junit.Test; +import org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.junit.Assert.*; + +@RunWith(Enclosed.class) +public class DruidSqlParserUtilsTest +{ + + /** + * Sanity checking that the formats of TIME_FLOOR(__time, Period) + */ + @RunWith(Parameterized.class) + public static class TimeFloorToGranularityConversionTest + { + @Parameterized.Parameters(name = "{1}") + public static Iterable constructorFeeder() + { + return ImmutableList.of( + new Object[]{"PT1H", Granularities.HOUR} + ); + } + + String periodString; + Granularity expectedGranularity; + + public TimeFloorToGranularityConversionTest(String periodString, Granularity expectedGranularity) + { + this.periodString = periodString; + this.expectedGranularity = expectedGranularity; + } + + @Test + public void testGranularityFromTimeFloor() throws ParseException + { + final SqlNodeList args = new SqlNodeList(SqlParserPos.ZERO); + args.add(new SqlIdentifier("__time", SqlParserPos.ZERO)); + args.add(SqlLiteral.createCharString(this.periodString, SqlParserPos.ZERO)); + final SqlNode timeFloorCall = TimeFloorOperatorConversion.SQL_FUNCTION.createCall(args); + Granularity actualGranularity = DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions( + timeFloorCall); + assertEquals(expectedGranularity, actualGranularity); + } + } + + /** + * Sanity checking that FLOOR(__time TO TimeUnit()) works as intended with the supported granularities + */ + @RunWith(Parameterized.class) + public static class FloorToGranularityConversionTest + { + @Parameterized.Parameters(name = "{1}") + public static Iterable constructorFeeder() + { + return ImmutableList.of( + new Object[]{TimeUnit.SECOND, Granularities.SECOND}, + new Object[]{TimeUnit.MINUTE, Granularities.MINUTE}, + new Object[]{TimeUnit.HOUR, Granularities.HOUR}, + new Object[]{TimeUnit.DAY, Granularities.DAY}, + new Object[]{TimeUnit.WEEK, Granularities.WEEK}, + new Object[]{TimeUnit.MONTH, Granularities.MONTH}, + new Object[]{TimeUnit.QUARTER, Granularities.QUARTER}, + new Object[]{TimeUnit.YEAR, Granularities.YEAR} + ); + } + + TimeUnit timeUnit; + Granularity expectedGranularity; + + public FloorToGranularityConversionTest(TimeUnit timeUnit, Granularity expectedGranularity) + { + this.timeUnit = timeUnit; + this.expectedGranularity = expectedGranularity; + } + + @Test + public void testGetGranularityFromFloor() throws ParseException + { + // parserPos doesn't matter + final SqlNodeList args = new SqlNodeList(SqlParserPos.ZERO); + args.add(new SqlIdentifier("__time", SqlParserPos.ZERO)); + args.add(new SqlIntervalQualifier(this.timeUnit, null, SqlParserPos.ZERO)); + final SqlNode floorCall = SqlStdOperatorTable.FLOOR.createCall(args); + Granularity actualGranularity = DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(floorCall); + assertEquals(expectedGranularity, actualGranularity); + } + } + + public static class FloorToGranularityConversionTestErrors + { + } +} From 5cbcb3596d8aeac9e56bb955b757d9cd2a1aa940 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 3 Feb 2022 02:31:22 +0530 Subject: [PATCH 33/49] checkstyle fix --- .../druid/sql/calcite/parser/DruidSqlParserUtils.java | 4 ++-- .../druid/sql/calcite/parser/DruidSqlParserUtilsTest.java | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java index c05ed4776af3..b951d1c6a9f7 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java @@ -102,7 +102,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa ); // If the floor function is of form TIME_FLOOR(__time, 'PT1H') - if (operatorName.equals(TimeFloorOperatorConversion.SQL_FUNCTION_NAME)) { + if (operatorName.equalsIgnoreCase(TimeFloorOperatorConversion.SQL_FUNCTION_NAME)) { SqlNode granularitySqlNode = operandList.get(1); Preconditions.checkArgument( granularitySqlNode.getKind().equals(SqlKind.LITERAL), @@ -112,7 +112,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa Period period = new Period(granularityString); return new PeriodGranularity(period, null, null); - } else if (operatorName.equals("FLOOR")) { // If the floor function is of form FLOOR(__time TO DAY) + } else if ("FLOOR".equalsIgnoreCase(operatorName)) { // If the floor function is of form FLOOR(__time TO DAY) SqlNode granularitySqlNode = operandList.get(1); // In future versions of Calcite, this can be checked via // granularitySqlNode.getKind().equals(SqlKind.INTERVAL_QUALIFIER) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java index 19cbb1709f2e..372a7a591c51 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java @@ -28,10 +28,8 @@ import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; -import org.apache.calcite.sql.type.SqlTypeName; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; -import org.apache.druid.sql.calcite.expression.TimeUnits; import org.apache.druid.sql.calcite.expression.builtin.TimeFloorOperatorConversion; import org.junit.Assert; import org.junit.Test; @@ -39,8 +37,6 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import static org.junit.Assert.*; - @RunWith(Enclosed.class) public class DruidSqlParserUtilsTest { @@ -77,7 +73,7 @@ public void testGranularityFromTimeFloor() throws ParseException final SqlNode timeFloorCall = TimeFloorOperatorConversion.SQL_FUNCTION.createCall(args); Granularity actualGranularity = DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions( timeFloorCall); - assertEquals(expectedGranularity, actualGranularity); + Assert.assertEquals(expectedGranularity, actualGranularity); } } @@ -120,7 +116,7 @@ public void testGetGranularityFromFloor() throws ParseException args.add(new SqlIntervalQualifier(this.timeUnit, null, SqlParserPos.ZERO)); final SqlNode floorCall = SqlStdOperatorTable.FLOOR.createCall(args); Granularity actualGranularity = DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(floorCall); - assertEquals(expectedGranularity, actualGranularity); + Assert.assertEquals(expectedGranularity, actualGranularity); } } From 38aa94156cb7961b9059713e7e197ee16aca3280 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 3 Feb 2022 10:26:18 +0530 Subject: [PATCH 34/49] Partitioned by now a required clause --- sql/src/main/codegen/includes/insert.ftl | 6 ++---- .../druid/sql/calcite/parser/DruidSqlInsert.java | 13 +++++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index da8344cd84db..49bcc1d68793 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -25,10 +25,8 @@ SqlNode DruidSqlInsert() : } { insertNode = SqlInsert() - [ - - partitionedBy = PartitionGranularity() - ] + + partitionedBy = PartitionGranularity() [ clusteredBy = ClusterItems() diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index ddf9e7720db5..40a94dff9c28 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -19,6 +19,7 @@ package org.apache.druid.sql.calcite.parser; +import com.google.common.base.Preconditions; import org.apache.calcite.sql.SqlInsert; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; @@ -40,11 +41,13 @@ public class DruidSqlInsert extends SqlInsert public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; private final Granularity partitionedBy; + + @Nullable private final SqlNodeList clusteredBy; public DruidSqlInsert( @Nonnull SqlInsert insertNode, - @Nullable Granularity partitionedBy, + @Nonnull Granularity partitionedBy, @Nullable SqlNodeList clusteredBy ) { @@ -55,6 +58,7 @@ public DruidSqlInsert( insertNode.getSource(), insertNode.getTargetColumnList() ); + Preconditions.checkNotNull(partitionedBy); // Shouldn't hit due to how the parser is written this.partitionedBy = partitionedBy; this.clusteredBy = clusteredBy; } @@ -65,7 +69,6 @@ public SqlNodeList getClusteredBy() return clusteredBy; } - @Nullable public Granularity getPartitionedBy() { return partitionedBy; @@ -82,10 +85,8 @@ public SqlOperator getOperator() public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { super.unparse(writer, leftPrec, rightPrec); - if (partitionedBy != null) { - writer.keyword("PARTITIONED BY"); - writer.keyword(getPartitionedBy().toString()); // TODO: Can this be made cleaner - } + writer.keyword("PARTITIONED BY"); + writer.keyword(getPartitionedBy().toString()); // TODO: Can this be made cleaner if (clusteredBy != null) { writer.sep("CLUSTERED BY"); SqlWriter.Frame frame = writer.startList("", ""); From 7df06106be5e1de4088d9332b2813edfbe8fb908 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 3 Feb 2022 15:06:38 +0530 Subject: [PATCH 35/49] Update testcases, minor review comments --- .../sql/calcite/parser/DruidSqlInsert.java | 4 +- .../calcite/parser/DruidSqlParserUtils.java | 33 ++++- .../sql/calcite/CalciteInsertDmlTest.java | 139 ++++++++++++++---- 3 files changed, 141 insertions(+), 35 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 40a94dff9c28..98a41af87132 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -31,7 +31,7 @@ import javax.annotation.Nullable; /** - * Extends the Insert call to hold custom paramaters specific to druid i.e. PARTITIONED BY and CLUSTERED BY + * Extends the 'insert' call to hold custom parameters specific to Druid i.e. PARTITIONED BY and CLUSTERED BY * This class extends the {@link SqlInsert} so that this SqlNode can be used in * {@link org.apache.calcite.sql2rel.SqlToRelConverter} for getting converted into RelNode, and further processing */ @@ -87,7 +87,7 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) super.unparse(writer, leftPrec, rightPrec); writer.keyword("PARTITIONED BY"); writer.keyword(getPartitionedBy().toString()); // TODO: Can this be made cleaner - if (clusteredBy != null) { + if (getClusteredBy() != null) { writer.sep("CLUSTERED BY"); SqlWriter.Frame frame = writer.startList("", ""); for (SqlNode clusterByOpts : getClusteredBy().getList()) { diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java index b951d1c6a9f7..0d1c1fee0b9f 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java @@ -76,15 +76,22 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql */ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws ParseException { + + final String genericParseFailedMessageFormatString = "Unable to parse the granularity from %s. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or %s function."; + if (!(sqlNode instanceof SqlCall)) { - throw new ParseException(StringUtils.format("Unable to parse the granularity from %s", sqlNode.toString())); + throw new ParseException(StringUtils.format( + genericParseFailedMessageFormatString, + sqlNode.toString(), + TimeFloorOperatorConversion.SQL_FUNCTION_NAME + )); } SqlCall sqlCall = (SqlCall) sqlNode; List operandList = sqlCall.getOperandList(); Preconditions.checkArgument( operandList.size() == 2, - "Invalid number of arguments passed to the floor function in PARTIITONED BY" + "Invalid number of arguments passed to the floor function in PARTIITONED BY clause" ); String operatorName = sqlCall.getOperator().getName(); @@ -93,12 +100,12 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa SqlNode timeOperandSqlNode = operandList.get(0); Preconditions.checkArgument( timeOperandSqlNode.getKind().equals(SqlKind.IDENTIFIER), - StringUtils.format("First argument to %s in PARTITIONED BY can only be __time", operatorName) + StringUtils.format("First argument to %s in PARTITIONED BY clause can only be __time", operatorName) ); SqlIdentifier timeOperandSqlIdentifier = (SqlIdentifier) timeOperandSqlNode; Preconditions.checkArgument( timeOperandSqlIdentifier.getSimple().equals(ColumnHolder.TIME_COLUMN_NAME), - StringUtils.format("First argument to %s in PARTITIONED BY can only be __time", operatorName) + StringUtils.format("First argument to %s in PARTITIONED BY clause can only be __time", operatorName) ); // If the floor function is of form TIME_FLOOR(__time, 'PT1H') @@ -106,10 +113,16 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa SqlNode granularitySqlNode = operandList.get(1); Preconditions.checkArgument( granularitySqlNode.getKind().equals(SqlKind.LITERAL), - "Second argument to the TIME_FLOOR function in PARTITIONED BY is invalid" + "Second argument to the TIME_FLOOR function in PARTITIONED BY clause is invalid" ); String granularityString = SqlLiteral.unchain(granularitySqlNode).toValue(); - Period period = new Period(granularityString); + Period period; + try { + period = new Period(granularityString); + } + catch (IllegalArgumentException e) { + throw new ParseException(StringUtils.format("Unable to create period from string '%s'", granularityString)); + } return new PeriodGranularity(period, null, null); } else if ("FLOOR".equalsIgnoreCase(operatorName)) { // If the floor function is of form FLOOR(__time TO DAY) @@ -118,7 +131,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa // granularitySqlNode.getKind().equals(SqlKind.INTERVAL_QUALIFIER) Preconditions.checkArgument( granularitySqlNode instanceof SqlIntervalQualifier, - "Second argument to the FLOOR function in PARTITIONED BY is invalid" + "Second argument to the FLOOR function in PARTITIONED BY clause cannot be converted to a valid granularity." ); SqlIntervalQualifier granularityIntervalQualifier = (SqlIntervalQualifier) granularitySqlNode; @@ -133,6 +146,10 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa return new PeriodGranularity(period, null, null); } - throw new ParseException("Unable to parse the PARTITIONED BY clause"); + throw new ParseException(StringUtils.format( + genericParseFailedMessageFormatString, + sqlNode.toString(), + TimeFloorOperatorConversion.SQL_FUNCTION_NAME + )); } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index cf5335d2d072..eaa4117f9503 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -29,6 +29,7 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.jackson.JacksonUtils; import org.apache.druid.query.Query; import org.apache.druid.query.QueryContexts; @@ -100,6 +101,11 @@ public class CalciteInsertDmlTest extends BaseCalciteQueryTest .build() ); + private static final Map PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT = ImmutableMap.of( + QueryContexts.INGESTION_GRANULARITY, + "{\"type\":\"all\"}" + ); + private boolean didTest = false; @After @@ -118,7 +124,7 @@ public void tearDown() throws Exception public void testInsertFromTable() { testInsertQuery() - .sql("INSERT INTO dst SELECT * FROM foo") + .sql("INSERT INTO dst SELECT * FROM foo PARTITIONED BY ALL TIME") .expectTarget("dst", FOO_TABLE_SIGNATURE) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -126,6 +132,7 @@ public void testInsertFromTable() .dataSource("foo") .intervals(querySegmentSpec(Filtration.eternity())) .columns("__time", "cnt", "dim1", "dim2", "dim3", "m1", "m2", "unique_dim1") + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -135,7 +142,7 @@ public void testInsertFromTable() public void testInsertFromView() { testInsertQuery() - .sql("INSERT INTO dst SELECT * FROM view.aview") + .sql("INSERT INTO dst SELECT * FROM view.aview PARTITIONED BY ALL TIME") .expectTarget("dst", RowSignature.builder().add("dim1_firstchar", ColumnType.STRING).build()) .expectResources(viewRead("aview"), dataSourceWrite("dst")) .expectQuery( @@ -145,6 +152,7 @@ public void testInsertFromView() .virtualColumns(expressionVirtualColumn("v0", "substring(\"dim1\", 0, 1)", ColumnType.STRING)) .filters(selector("dim2", "a", null)) .columns("v0") + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -154,7 +162,7 @@ public void testInsertFromView() public void testInsertIntoExistingTable() { testInsertQuery() - .sql("INSERT INTO foo SELECT * FROM foo") + .sql("INSERT INTO foo SELECT * FROM foo PARTITIONED BY ALL TIME") .expectTarget("foo", FOO_TABLE_SIGNATURE) .expectResources(dataSourceRead("foo"), dataSourceWrite("foo")) .expectQuery( @@ -162,6 +170,7 @@ public void testInsertIntoExistingTable() .dataSource("foo") .intervals(querySegmentSpec(Filtration.eternity())) .columns("__time", "cnt", "dim1", "dim2", "dim3", "m1", "m2", "unique_dim1") + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -171,7 +180,7 @@ public void testInsertIntoExistingTable() public void testInsertIntoQualifiedTable() { testInsertQuery() - .sql("INSERT INTO druid.dst SELECT * FROM foo") + .sql("INSERT INTO druid.dst SELECT * FROM foo PARTITIONED BY ALL TIME") .expectTarget("dst", FOO_TABLE_SIGNATURE) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -179,6 +188,7 @@ public void testInsertIntoQualifiedTable() .dataSource("foo") .intervals(querySegmentSpec(Filtration.eternity())) .columns("__time", "cnt", "dim1", "dim2", "dim3", "m1", "m2", "unique_dim1") + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -188,7 +198,7 @@ public void testInsertIntoQualifiedTable() public void testInsertIntoInvalidDataSourceName() { testInsertQuery() - .sql("INSERT INTO \"in/valid\" SELECT dim1, dim2 FROM foo") + .sql("INSERT INTO \"in/valid\" SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .expectValidationError(SqlPlanningException.class, "INSERT dataSource cannot contain the '/' character.") .verify(); } @@ -197,7 +207,7 @@ public void testInsertIntoInvalidDataSourceName() public void testInsertUsingColumnList() { testInsertQuery() - .sql("INSERT INTO dst (foo, bar) SELECT dim1, dim2 FROM foo") + .sql("INSERT INTO dst (foo, bar) SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .expectValidationError(SqlPlanningException.class, "INSERT with target column list is not supported.") .verify(); } @@ -206,7 +216,7 @@ public void testInsertUsingColumnList() public void testUpsert() { testInsertQuery() - .sql("UPSERT INTO dst SELECT * FROM foo") + .sql("UPSERT INTO dst SELECT * FROM foo PARTITIONED BY ALL TIME") .expectValidationError(SqlPlanningException.class, "UPSERT is not supported.") .verify(); } @@ -215,7 +225,7 @@ public void testUpsert() public void testInsertIntoSystemTable() { testInsertQuery() - .sql("INSERT INTO INFORMATION_SCHEMA.COLUMNS SELECT * FROM foo") + .sql("INSERT INTO INFORMATION_SCHEMA.COLUMNS SELECT * FROM foo PARTITIONED BY ALL TIME") .expectValidationError( SqlPlanningException.class, "Cannot INSERT into [INFORMATION_SCHEMA.COLUMNS] because it is not a Druid datasource." @@ -227,7 +237,7 @@ public void testInsertIntoSystemTable() public void testInsertIntoView() { testInsertQuery() - .sql("INSERT INTO view.aview SELECT * FROM foo") + .sql("INSERT INTO view.aview SELECT * FROM foo PARTITIONED BY ALL TIME") .expectValidationError( SqlPlanningException.class, "Cannot INSERT into [view.aview] because it is not a Druid datasource." @@ -239,7 +249,7 @@ public void testInsertIntoView() public void testInsertFromUnauthorizedDataSource() { testInsertQuery() - .sql("INSERT INTO dst SELECT * FROM \"%s\"", CalciteTests.FORBIDDEN_DATASOURCE) + .sql("INSERT INTO dst SELECT * FROM \"%s\" PARTITIONED BY ALL TIME", CalciteTests.FORBIDDEN_DATASOURCE) .expectValidationError(ForbiddenException.class) .verify(); } @@ -248,7 +258,7 @@ public void testInsertFromUnauthorizedDataSource() public void testInsertIntoUnauthorizedDataSource() { testInsertQuery() - .sql("INSERT INTO \"%s\" SELECT * FROM foo", CalciteTests.FORBIDDEN_DATASOURCE) + .sql("INSERT INTO \"%s\" SELECT * FROM foo PARTITIONED BY ALL TIME", CalciteTests.FORBIDDEN_DATASOURCE) .expectValidationError(ForbiddenException.class) .verify(); } @@ -257,7 +267,7 @@ public void testInsertIntoUnauthorizedDataSource() public void testInsertIntoNonexistentSchema() { testInsertQuery() - .sql("INSERT INTO nonexistent.dst SELECT * FROM foo") + .sql("INSERT INTO nonexistent.dst SELECT * FROM foo PARTITIONED BY ALL TIME") .expectValidationError( SqlPlanningException.class, "Cannot INSERT into [nonexistent.dst] because it is not a Druid datasource." @@ -269,7 +279,7 @@ public void testInsertIntoNonexistentSchema() public void testInsertFromExternal() { testInsertQuery() - .sql("INSERT INTO dst SELECT * FROM %s", externSql(externalDataSource)) + .sql("INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME", externSql(externalDataSource)) .authentication(CalciteTests.SUPER_USER_AUTH_RESULT) .expectTarget("dst", externalDataSource.getSignature()) .expectResources(dataSourceWrite("dst"), ExternalOperatorConversion.EXTERNAL_RESOURCE_ACTION) @@ -278,6 +288,7 @@ public void testInsertFromExternal() .dataSource(externalDataSource) .intervals(querySegmentSpec(Filtration.eternity())) .columns("x", "y", "z") + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -298,7 +309,7 @@ public void testInsertWithPartitionedBy() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY 'day'") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY TIME_FLOOR(__time, 'PT1H')") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -313,6 +324,58 @@ public void testInsertWithPartitionedBy() .verify(); } + @Test + public void testPartitionedBySupportedClauses() + { + RowSignature targetRowSignature = RowSignature.builder() + .add("__time", ColumnType.LONG) + .add("dim1", ColumnType.STRING) + .build(); + + Map partitionedByArgumentToGranularityMap = + ImmutableMap.builder() + .put("HOUR", Granularities.HOUR) + .put("DAY", Granularities.DAY) + .put("MONTH", Granularities.MONTH) + .put("YEAR", Granularities.YEAR) + .put("ALL TIME", Granularities.ALL) + .put("FLOOR(__time TO QUARTER)", Granularities.QUARTER) + .put("TIME_FLOOR(__time, 'PT1H')", Granularities.HOUR) + .build(); + + partitionedByArgumentToGranularityMap.forEach((partitionedByArgument, expectedGranularity) -> { + Map queryContext = null; + try { + queryContext = ImmutableMap.of( + QueryContexts.INGESTION_GRANULARITY, queryJsonMapper.writeValueAsString(expectedGranularity) + ); + } + catch (JsonProcessingException e) { + // Won't reach here + Assert.fail(e.getMessage()); + } + + testInsertQuery() + .sql(StringUtils.format( + "INSERT INTO druid.dst SELECT __time, dim1 FROM foo PARTITIONED BY %s", + partitionedByArgument + )) + .expectTarget("dst", targetRowSignature) + .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) + .expectQuery( + newScanQueryBuilder() + .dataSource("foo") + .intervals(querySegmentSpec(Filtration.eternity())) + .columns("__time", "dim1") + .context(queryContext) + .build() + ) + .verify(); + didTest = false; + }); + didTest = true; + } + @Test public void testInsertWithClusteredBy() { @@ -367,7 +430,7 @@ public void testInsertWithPartitionedByAndClusteredBy() testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY 'day' CLUSTERED BY 2, dim1") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY DAY CLUSTERED BY 2, dim1") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -382,7 +445,7 @@ public void testInsertWithPartitionedByAndClusteredBy() new ScanQuery.OrderBy("dim1", ScanQuery.Order.ASCENDING) ) ) - .context(queryContext) + .context(queryContextWithGranularity(Granularities.DAY)) .build() ) .verify(); @@ -425,7 +488,7 @@ public void testInsertWithClusteredByAndOrderBy() throws Exception try { testQuery( StringUtils.format( - "INSERT INTO dst SELECT * FROM %s ORDER BY 2 CLUSTERED BY 3", + "INSERT INTO dst SELECT * FROM %s ORDER BY 2 PARTITIONED BY ALL TIME", externSql(externalDataSource) ), ImmutableList.of(), @@ -469,7 +532,7 @@ public void testInsertWithOrderBy() throws Exception try { testQuery( StringUtils.format( - "INSERT INTO dst SELECT * FROM %s ORDER BY 2", + "INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME ORDER BY 2", externSql(externalDataSource) ), ImmutableList.of(), @@ -498,7 +561,7 @@ public void testExplainInsertWithPartitionedByAndClusteredBy() () -> testQuery( StringUtils.format( - "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITIONED BY 'day' CLUSTERED BY 1", + "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITIONED BY DAY CLUSTERED BY 1", externSql(externalDataSource) ), ImmutableList.of(), @@ -508,6 +571,7 @@ public void testExplainInsertWithPartitionedByAndClusteredBy() didTest = true; } + @Ignore @Test public void testExplainInsertFromExternal() throws Exception { @@ -534,7 +598,10 @@ public void testExplainInsertFromExternal() throws Exception // Use testQuery for EXPLAIN (not testInsertQuery). testQuery( new PlannerConfig(), - StringUtils.format("EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s", externSql(externalDataSource)), + StringUtils.format( + "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME", + externSql(externalDataSource) + ), CalciteTests.SUPER_USER_AUTH_RESULT, ImmutableList.of(), ImmutableList.of( @@ -549,6 +616,7 @@ public void testExplainInsertFromExternal() throws Exception didTest = true; } + @Ignore @Test public void testExplainInsertFromExternalUnauthorized() { @@ -557,7 +625,10 @@ public void testExplainInsertFromExternalUnauthorized() ForbiddenException.class, () -> testQuery( - StringUtils.format("EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s", externSql(externalDataSource)), + StringUtils.format( + "EXPLAIN PLAN FOR INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME", + externSql(externalDataSource) + ), ImmutableList.of(), ImmutableList.of() ) @@ -571,7 +642,7 @@ public void testExplainInsertFromExternalUnauthorized() public void testInsertFromExternalUnauthorized() { testInsertQuery() - .sql("INSERT INTO dst SELECT * FROM %s", externSql(externalDataSource)) + .sql("INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME", externSql(externalDataSource)) .expectValidationError(ForbiddenException.class) .verify(); } @@ -582,7 +653,10 @@ public void testInsertFromExternalProjectSort() // INSERT with a particular column ordering. testInsertQuery() - .sql("INSERT INTO dst SELECT x || y AS xy, z FROM %s CLUSTERED BY 1, 2", externSql(externalDataSource)) + .sql( + "INSERT INTO dst SELECT x || y AS xy, z FROM %s PARTITIONED BY ALL TIME CLUSTERED BY 1, 2", + externSql(externalDataSource) + ) .authentication(CalciteTests.SUPER_USER_AUTH_RESULT) .expectTarget("dst", RowSignature.builder().add("xy", ColumnType.STRING).add("z", ColumnType.LONG).build()) .expectResources(dataSourceWrite("dst"), ExternalOperatorConversion.EXTERNAL_RESOURCE_ACTION) @@ -598,6 +672,7 @@ public void testInsertFromExternalProjectSort() new ScanQuery.OrderBy("z", ScanQuery.Order.ASCENDING) ) ) + .context(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -610,7 +685,7 @@ public void testInsertFromExternalAggregate() testInsertQuery() .sql( - "INSERT INTO dst SELECT x, SUM(z) AS sum_z, COUNT(*) AS cnt FROM %s GROUP BY 1", + "INSERT INTO dst SELECT x, SUM(z) AS sum_z, COUNT(*) AS cnt FROM %s GROUP BY 1 PARTITIONED BY ALL TIME", externSql(externalDataSource) ) .authentication(CalciteTests.SUPER_USER_AUTH_RESULT) @@ -633,6 +708,7 @@ public void testInsertFromExternalAggregate() new LongSumAggregatorFactory("a0", "z"), new CountAggregatorFactory("a1") ) + .setContext(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -645,7 +721,7 @@ public void testInsertFromExternalAggregateAll() testInsertQuery() .sql( - "INSERT INTO dst SELECT COUNT(*) AS cnt FROM %s", + "INSERT INTO dst SELECT COUNT(*) AS cnt FROM %s PARTITIONED BY ALL TIME", externSql(externalDataSource) ) .authentication(CalciteTests.SUPER_USER_AUTH_RESULT) @@ -662,6 +738,7 @@ public void testInsertFromExternalAggregateAll() .setInterval(querySegmentSpec(Filtration.eternity())) .setGranularity(Granularities.ALL) .setAggregatorSpecs(new CountAggregatorFactory("a0")) + .setContext(PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT) .build() ) .verify(); @@ -682,6 +759,18 @@ private String externSql(final ExternalDataSource externalDataSource) } } + private Map queryContextWithGranularity(Granularity granularity) + { + String granularityString = null; + try { + granularityString = queryJsonMapper.writeValueAsString(granularity); + } + catch (JsonProcessingException e) { + Assert.fail(e.getMessage()); + } + return ImmutableMap.of(QueryContexts.INGESTION_GRANULARITY, granularityString); + } + private InsertDmlTester testInsertQuery() { return new InsertDmlTester(); From d8d57f449a629edb8403d021d83d9302e64ae439 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Thu, 3 Feb 2022 19:59:56 +0530 Subject: [PATCH 36/49] fix test cases --- .../sql/calcite/parser/DruidSqlInsert.java | 1 - .../sql/calcite/CalciteInsertDmlTest.java | 21 +++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 98a41af87132..4f735e5392bd 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -96,5 +96,4 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) writer.endList(frame); } } - } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index eaa4117f9503..6d74af6a981d 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -304,9 +304,6 @@ public void testInsertWithPartitionedBy() .add("dim1", ColumnType.STRING) .build(); - Map queryContext = new HashMap<>(DEFAULT_CONTEXT); - queryContext.put(QueryContexts.INGESTION_GRANULARITY, "day"); - testInsertQuery() .sql( "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY TIME_FLOOR(__time, 'PT1H')") @@ -318,7 +315,7 @@ public void testInsertWithPartitionedBy() .intervals(querySegmentSpec(Filtration.eternity())) .columns("__time", "dim1", "v0") .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) - .context(queryContext) + .context(queryContextWithGranularity(Granularities.HOUR)) .build() ) .verify(); @@ -390,7 +387,7 @@ public void testInsertWithClusteredBy() .sql( "INSERT INTO druid.dst " + "SELECT __time, FLOOR(m1) as floor_m1, dim1, CEIL(m2) FROM foo " - + "CLUSTERED BY 2, dim1 DESC, CEIL(m2)" + + "PARTITIONED BY ALL TIME CLUSTERED BY 2, dim1 DESC, CEIL(m2)" ) .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) @@ -410,6 +407,7 @@ public void testInsertWithClusteredBy() new ScanQuery.OrderBy("v1", ScanQuery.Order.ASCENDING) ) ) + .context(queryContextWithGranularity(Granularities.ALL)) .build() ) .verify(); @@ -460,12 +458,9 @@ public void testInsertWithPartitionedByAndLimitOffset() .add("dim1", ColumnType.STRING) .build(); - Map queryContext = new HashMap<>(DEFAULT_CONTEXT); - queryContext.put(QueryContexts.INGESTION_GRANULARITY, "day"); - testInsertQuery() .sql( - "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo LIMIT 10 OFFSET 20 PARTITIONED BY 'day'") + "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo LIMIT 10 OFFSET 20 PARTITIONED BY DAY") .expectTarget("dst", targetRowSignature) .expectResources(dataSourceRead("foo"), dataSourceWrite("dst")) .expectQuery( @@ -476,7 +471,7 @@ public void testInsertWithPartitionedByAndLimitOffset() .virtualColumns(expressionVirtualColumn("v0", "floor(\"m1\")", ColumnType.FLOAT)) .limit(10) .offset(20) - .context(queryContext) + .context(queryContextWithGranularity(Granularities.DAY)) .build() ) .verify(); @@ -519,7 +514,7 @@ public void testInsertWithPartitionedByContainingInvalidGranularity() throws Exc } catch (SqlPlanningException e) { Assert.assertEquals( - "Granularity passed in PARTITIONED BY clause is invalid", + "Unable to parse the granularity from 'invalid_granularity'. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or TIME_FLOOR function.", e.getMessage() ); } @@ -532,7 +527,7 @@ public void testInsertWithOrderBy() throws Exception try { testQuery( StringUtils.format( - "INSERT INTO dst SELECT * FROM %s PARTITIONED BY ALL TIME ORDER BY 2", + "INSERT INTO dst SELECT * FROM %s ORDER BY 2 PARTITIONED BY ALL TIME", externSql(externalDataSource) ), ImmutableList.of(), @@ -763,7 +758,7 @@ private Map queryContextWithGranularity(Granularity granularity) { String granularityString = null; try { - granularityString = queryJsonMapper.writeValueAsString(granularity); + granularityString = queryJsonMapper.writeValueAsString(granularity); } catch (JsonProcessingException e) { Assert.fail(e.getMessage()); From d36bc2ba2163c488b223566557fa364a078679b1 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 01:40:05 +0530 Subject: [PATCH 37/49] add tests, cleanup error message --- .../calcite/parser/DruidSqlParserUtils.java | 24 +++-- .../parser/DruidSqlParserUtilsTest.java | 92 ++++++++++++++++++- 2 files changed, 108 insertions(+), 8 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java index 0d1c1fee0b9f..f0fd19568937 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java @@ -54,7 +54,6 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql return g; } - /** * This method is used to extract the granularity from a SqlNode representing following function calls: * 1. FLOOR(__time TO TimeUnit) @@ -77,7 +76,7 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws ParseException { - final String genericParseFailedMessageFormatString = "Unable to parse the granularity from %s. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or %s function."; + final String genericParseFailedMessageFormatString = "Unable to parse the granularity from %s. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or %s function"; if (!(sqlNode instanceof SqlCall)) { throw new ParseException(StringUtils.format( @@ -88,13 +87,23 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa } SqlCall sqlCall = (SqlCall) sqlNode; + String operatorName = sqlCall.getOperator().getName(); + + Preconditions.checkArgument( + "FLOOR".equalsIgnoreCase(operatorName) + || TimeFloorOperatorConversion.SQL_FUNCTION_NAME.equalsIgnoreCase(operatorName), + StringUtils.format( + "PARTITIONED BY clause can only parse FLOOR and %s functions.", + TimeFloorOperatorConversion.SQL_FUNCTION_NAME + ) + ); + List operandList = sqlCall.getOperandList(); Preconditions.checkArgument( operandList.size() == 2, - "Invalid number of arguments passed to the floor function in PARTIITONED BY clause" + StringUtils.format("Invalid number of arguments passed to %s in PARTIITONED BY clause", operatorName) ); - String operatorName = sqlCall.getOperator().getName(); // Check if the first argument passed in the floor function is __time SqlNode timeOperandSqlNode = operandList.get(0); @@ -113,7 +122,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa SqlNode granularitySqlNode = operandList.get(1); Preconditions.checkArgument( granularitySqlNode.getKind().equals(SqlKind.LITERAL), - "Second argument to the TIME_FLOOR function in PARTITIONED BY clause is invalid" + "Second argument to TIME_FLOOR in PARTITIONED BY clause should be string representing a period" ); String granularityString = SqlLiteral.unchain(granularitySqlNode).toValue(); Period period; @@ -121,7 +130,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa period = new Period(granularityString); } catch (IllegalArgumentException e) { - throw new ParseException(StringUtils.format("Unable to create period from string '%s'", granularityString)); + throw new ParseException(StringUtils.format("Unable to create period from %s", granularitySqlNode.toString())); } return new PeriodGranularity(period, null, null); @@ -131,7 +140,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa // granularitySqlNode.getKind().equals(SqlKind.INTERVAL_QUALIFIER) Preconditions.checkArgument( granularitySqlNode instanceof SqlIntervalQualifier, - "Second argument to the FLOOR function in PARTITIONED BY clause cannot be converted to a valid granularity." + "Second argument to the FLOOR function in PARTITIONED BY clause cannot be converted to a valid granularity" ); SqlIntervalQualifier granularityIntervalQualifier = (SqlIntervalQualifier) granularitySqlNode; @@ -146,6 +155,7 @@ public static Granularity convertSqlNodeToGranularity(SqlNode sqlNode) throws Pa return new PeriodGranularity(period, null, null); } + // Shouldn't reach here throw new ParseException(StringUtils.format( genericParseFailedMessageFormatString, sqlNode.toString(), diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java index 372a7a591c51..13133fe15dd6 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java @@ -28,6 +28,8 @@ import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.sql.calcite.expression.builtin.TimeFloorOperatorConversion; @@ -42,7 +44,7 @@ public class DruidSqlParserUtilsTest { /** - * Sanity checking that the formats of TIME_FLOOR(__time, Period) + * Sanity checking that the formats of TIME_FLOOR(__time, Period) work as expected */ @RunWith(Parameterized.class) public static class TimeFloorToGranularityConversionTest @@ -122,5 +124,93 @@ public void testGetGranularityFromFloor() throws ParseException public static class FloorToGranularityConversionTestErrors { + /** + * incorrect node + * incorrect function name + * Incorrect number of arguments + * incorrect first argument + * incorrect second argument + */ + + /** + * Tests clause like "PARTITIONED BY 'day'" + */ + @Test + public void testConvertSqlNodeToGranularityWithIncorrectNode() + { + SqlNode sqlNode = SqlLiteral.createCharString("day", SqlParserPos.ZERO); + ParseException e = Assert.assertThrows( + ParseException.class, + () -> { + DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(sqlNode); + } + ); + Assert.assertEquals( + "Unable to parse the granularity from 'day'. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or TIME_FLOOR function", + e.getMessage() + ); + } + + /** + * Tests clause like "PARTITIONED BY CEIL(__time TO DAY)" + */ + @Test + public void testConvertSqlNodeToGranularityWithIncorrectFunctionCall() + { + final SqlNodeList args = new SqlNodeList(SqlParserPos.ZERO); + args.add(new SqlIdentifier("__time", SqlParserPos.ZERO)); + args.add(new SqlIntervalQualifier(TimeUnit.DAY, null, SqlParserPos.ZERO)); + final SqlNode sqlNode = SqlStdOperatorTable.CEIL.createCall(args); + ParseException e = Assert.assertThrows( + ParseException.class, + () -> { + DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(sqlNode); + } + ); + Assert.assertEquals( + StringUtils.format( + "PARTITIONED BY clause can only parse FLOOR and %s functions.", + TimeFloorOperatorConversion.SQL_FUNCTION_NAME + ), + e.getMessage() + ); + } + + /** + * Tests clause like "PARTITIONED BY FLOOR(__time)" + */ + @Test + public void testConvertSqlNodeToGranularityWithIncorrectNumberOfArguments() + { + final SqlNodeList args = new SqlNodeList(SqlParserPos.ZERO); + args.add(new SqlIdentifier("__time", SqlParserPos.ZERO)); + final SqlNode sqlNode = SqlStdOperatorTable.FLOOR.createCall(args); + ParseException e = Assert.assertThrows( + ParseException.class, + () -> { + DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(sqlNode); + } + ); + Assert.assertEquals("Invalid number of arguments passed to FLOOR in PARTIITONED BY clause", e.getMessage()); + } + + /** + * Tests clause like "PARTITIONED BY (timestamps TO DAY)" + */ + @Test + public void testConvertSqlNodeToGranularityWithWrongIdentifier() + { + final SqlNodeList args = new SqlNodeList(SqlParserPos.ZERO); + args.add(new SqlIdentifier("timestamps", SqlParserPos.ZERO)); + args.add(new SqlIntervalQualifier(TimeUnit.DAY, null, SqlParserPos.ZERO)); + final SqlNode sqlNode = SqlStdOperatorTable.FLOOR.createCall(args); + ParseException e = Assert.assertThrows( + ParseException.class, + () -> { + DruidSqlParserUtils.convertSqlNodeToGranularityThrowingParseExceptions(sqlNode); + } + ); + Assert.assertEquals("First argument to FLOOR in PARTITIONED BY clause can only be __time", e.getMessage()); + } } } From eaaad27fd40949932cfae537f2bdb72054dc98e6 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 01:49:10 +0530 Subject: [PATCH 38/49] Change the value of insert segment granularity in querycontext --- .../java/org/apache/druid/query/QueryContexts.java | 1 - .../druid/sql/calcite/parser/DruidSqlInsert.java | 2 ++ .../druid/sql/calcite/planner/DruidPlanner.java | 2 +- .../druid/sql/calcite/CalciteInsertDmlTest.java | 12 ++++++------ .../sql/calcite/parser/DruidSqlParserUtilsTest.java | 2 -- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/QueryContexts.java b/processing/src/main/java/org/apache/druid/query/QueryContexts.java index 7df44542f6e3..22cb68b79609 100644 --- a/processing/src/main/java/org/apache/druid/query/QueryContexts.java +++ b/processing/src/main/java/org/apache/druid/query/QueryContexts.java @@ -68,7 +68,6 @@ public class QueryContexts public static final String ENABLE_DEBUG = "debug"; public static final String BY_SEGMENT_KEY = "bySegment"; public static final String BROKER_SERVICE_NAME = "brokerService"; - public static final String INGESTION_GRANULARITY = "ingestionGranularity"; public static final boolean DEFAULT_BY_SEGMENT = false; public static final boolean DEFAULT_POPULATE_CACHE = true; diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 4f735e5392bd..48940f8f3087 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -37,6 +37,8 @@ */ public class DruidSqlInsert extends SqlInsert { + public static final String SQL_INSERT_SEGMENT_GRANULARITY = "sqlInsertSegmentGranularity"; + // This allows reusing super.unparse public static final SqlOperator OPERATOR = SqlInsert.OPERATOR; diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 5cff33c346dc..06571bfdfff1 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -206,7 +206,7 @@ public PlannerResult plan() throws SqlParseException, ValidationException, RelCo try { if (parsed.getIngestionGranularity() != null) { plannerContext.getQueryContext().put( - QueryContexts.INGESTION_GRANULARITY, + DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, plannerContext.getJsonMapper().writeValueAsString(parsed.getIngestionGranularity()) ); } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index 6d74af6a981d..7fb912bf10a3 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -32,7 +32,6 @@ import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.jackson.JacksonUtils; import org.apache.druid.query.Query; -import org.apache.druid.query.QueryContexts; import org.apache.druid.query.aggregation.CountAggregatorFactory; import org.apache.druid.query.aggregation.LongSumAggregatorFactory; import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; @@ -53,6 +52,7 @@ import org.apache.druid.sql.calcite.external.ExternalDataSource; import org.apache.druid.sql.calcite.external.ExternalOperatorConversion; import org.apache.druid.sql.calcite.filtration.Filtration; +import org.apache.druid.sql.calcite.parser.DruidSqlInsert; import org.apache.druid.sql.calcite.planner.Calcites; import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.planner.PlannerContext; @@ -102,7 +102,7 @@ public class CalciteInsertDmlTest extends BaseCalciteQueryTest ); private static final Map PARTITIONED_BY_ALL_TIME_QUERY_CONTEXT = ImmutableMap.of( - QueryContexts.INGESTION_GRANULARITY, + DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, "{\"type\":\"all\"}" ); @@ -344,7 +344,7 @@ public void testPartitionedBySupportedClauses() Map queryContext = null; try { queryContext = ImmutableMap.of( - QueryContexts.INGESTION_GRANULARITY, queryJsonMapper.writeValueAsString(expectedGranularity) + DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, queryJsonMapper.writeValueAsString(expectedGranularity) ); } catch (JsonProcessingException e) { @@ -424,7 +424,7 @@ public void testInsertWithPartitionedByAndClusteredBy() .build(); Map queryContext = new HashMap<>(DEFAULT_CONTEXT); - queryContext.put(QueryContexts.INGESTION_GRANULARITY, "day"); + queryContext.put(DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, "day"); testInsertQuery() .sql( @@ -514,7 +514,7 @@ public void testInsertWithPartitionedByContainingInvalidGranularity() throws Exc } catch (SqlPlanningException e) { Assert.assertEquals( - "Unable to parse the granularity from 'invalid_granularity'. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or TIME_FLOOR function.", + "Unable to parse the granularity from 'invalid_granularity'. Expected HOUR, DAY, MONTH, YEAR, ALL TIME, FLOOR function or TIME_FLOOR function", e.getMessage() ); } @@ -763,7 +763,7 @@ private Map queryContextWithGranularity(Granularity granularity) catch (JsonProcessingException e) { Assert.fail(e.getMessage()); } - return ImmutableMap.of(QueryContexts.INGESTION_GRANULARITY, granularityString); + return ImmutableMap.of(DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, granularityString); } private InsertDmlTester testInsertQuery() diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java index 13133fe15dd6..57067afd9be0 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtilsTest.java @@ -28,7 +28,6 @@ import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.java.util.common.granularity.Granularity; @@ -42,7 +41,6 @@ @RunWith(Enclosed.class) public class DruidSqlParserUtilsTest { - /** * Sanity checking that the formats of TIME_FLOOR(__time, Period) work as expected */ From b90ab169b9163d9797b3280dc7c0dac8ac46a20f Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 02:03:21 +0530 Subject: [PATCH 39/49] minor fixes --- .../org/apache/druid/sql/calcite/parser/DruidSqlInsert.java | 2 +- .../apache/druid/sql/calcite/parser/DruidSqlParserUtils.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java index 48940f8f3087..46a6edf37a63 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlInsert.java @@ -88,7 +88,7 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) { super.unparse(writer, leftPrec, rightPrec); writer.keyword("PARTITIONED BY"); - writer.keyword(getPartitionedBy().toString()); // TODO: Can this be made cleaner + writer.keyword(getPartitionedBy().toString()); // TODO: Make it cleaner by directly unparsing the SqlNode if (getClusteredBy() != null) { writer.sep("CLUSTERED BY"); SqlWriter.Frame frame = writer.startList("", ""); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java index f0fd19568937..3b794d6a184b 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java @@ -58,7 +58,7 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql * This method is used to extract the granularity from a SqlNode representing following function calls: * 1. FLOOR(__time TO TimeUnit) * 2. TIME_FLOOR(__time, 'PT1H') - *

+ * * Validation on the sqlNode is contingent to following conditions: * 1. sqlNode is an instance of SqlCall * 2. Operator is either one of TIME_FLOOR or FLOOR @@ -66,7 +66,7 @@ public static Granularity convertSqlNodeToGranularityThrowingParseExceptions(Sql * 4. First operand is a SimpleIdentifier representing __time * 5. If operator is TIME_FLOOR, the second argument is a literal, and can be converted to the Granularity class * 6. If operator is FLOOR, the second argument is a TimeUnit, and can be mapped using {@link TimeUnits} - *

+ * * Since it is to be used primarily while parsing the SqlNode, it is wrapped in {@code convertSqlNodeToGranularityThrowingParseExceptions} * * @param sqlNode SqlNode representing a call to a function From e335214152813597cbafc47fa5ab8054cfd1aab8 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 09:40:12 +0530 Subject: [PATCH 40/49] intellij inspections --- .../org/apache/druid/sql/calcite/CalciteInsertDmlTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java index 7fb912bf10a3..8c500fc519d8 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java @@ -68,7 +68,6 @@ import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -423,9 +422,6 @@ public void testInsertWithPartitionedByAndClusteredBy() .add("dim1", ColumnType.STRING) .build(); - Map queryContext = new HashMap<>(DEFAULT_CONTEXT); - queryContext.put(DruidSqlInsert.SQL_INSERT_SEGMENT_GRANULARITY, "day"); - testInsertQuery() .sql( "INSERT INTO druid.dst SELECT __time, FLOOR(m1) as floor_m1, dim1 FROM foo PARTITIONED BY DAY CLUSTERED BY 2, dim1") From 2cef674eeea45507c85d8752b84477bed2459b84 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 16:41:17 +0530 Subject: [PATCH 41/49] Exclude druidsqlinsert from codecov --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index bf98f4ea74bb..2647f69f642e 100644 --- a/pom.xml +++ b/pom.xml @@ -1206,6 +1206,7 @@ org/apache/druid/query/TruncatedResponseContextException.class + org/apache/druid/sql/calcite/parser/DruidSqlInsert.class org/apache/druid/common/aws/AWSCredentials* From 8d581366f459a898c9f464a763d874e1e68397b9 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Fri, 4 Feb 2022 16:48:26 +0530 Subject: [PATCH 42/49] Exclude druidsqlinsert from codecov2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2647f69f642e..d900fc380b20 100644 --- a/pom.xml +++ b/pom.xml @@ -1206,7 +1206,7 @@ org/apache/druid/query/TruncatedResponseContextException.class - org/apache/druid/sql/calcite/parser/DruidSqlInsert.class + org/apache/druid/sql/calcite/parser/DruidSqlInsert.class org/apache/druid/common/aws/AWSCredentials* From 973d3fa3518d77d38d70cc9078c7b25ced71b7c4 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 7 Feb 2022 11:31:35 +0530 Subject: [PATCH 43/49] Missed testcase --- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 4c63b2142b4d..636e2ee96b67 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13604,7 +13604,7 @@ public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithMutlipleCons public void testSurfaceErrorsWhenInsertingThroughIncorrectSelectStatment() { assertQueryIsUnplannable( - "INSERT INTO druid.dst SELECT dim2, dim1, m1 FROM foo2 UNION SELECT dim1, dim2, m1 FROM foo", + "INSERT INTO druid.dst SELECT dim2, dim1, m1 FROM foo2 UNION SELECT dim1, dim2, m1 FROM foo PARTITIONED BY ALL TIME", "Possible error: SQL requires 'UNION' but only 'UNION ALL' is supported." ); } From dc2b45e3281e0f3a122923d3ca8e17ef8a3b0335 Mon Sep 17 00:00:00 2001 From: Laksh Singla Date: Mon, 7 Feb 2022 23:04:04 +0530 Subject: [PATCH 44/49] DruidSqlInsert.unparse working --- sql/src/main/codegen/includes/insert.ftl | 30 +++++++++++++------ .../sql/calcite/parser/DruidSqlInsert.java | 8 +++-- .../calcite/parser/DruidSqlParserUtils.java | 4 +-- .../sql/calcite/CalciteInsertDmlTest.java | 4 +-- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/sql/src/main/codegen/includes/insert.ftl b/sql/src/main/codegen/includes/insert.ftl index 49bcc1d68793..1a0fc1db9d22 100644 --- a/sql/src/main/codegen/includes/insert.ftl +++ b/sql/src/main/codegen/includes/insert.ftl @@ -17,10 +17,11 @@ * under the License. */ +// Using fully qualified name for Pair class, since Calcite also has a same class name being used in the Parser.jj SqlNode DruidSqlInsert() : { SqlNode insertNode; - Granularity partitionedBy = null; + org.apache.druid.java.util.common.Pair partitionedBy = null; SqlNodeList clusteredBy = null; } { @@ -38,7 +39,7 @@ SqlNode DruidSqlInsert() : return insertNode; } SqlInsert sqlInsert = (SqlInsert) insertNode; - return new DruidSqlInsert(sqlInsert, partitionedBy, clusteredBy); + return new DruidSqlInsert(sqlInsert, partitionedBy.lhs, partitionedBy.rhs, clusteredBy); } } @@ -61,40 +62,51 @@ SqlNodeList ClusterItems() : } } -Granularity PartitionGranularity() : +org.apache.druid.java.util.common.Pair PartitionGranularity() : { SqlNode e = null; + org.apache.druid.java.util.common.granularity.Granularity granularity = null; + String unparseString = null; } { ( { - return Granularities.HOUR; + granularity = Granularities.HOUR; + unparseString = "HOUR"; } | { - return Granularities.DAY; + granularity = Granularities.DAY; + unparseString = "DAY"; } | { - return Granularities.MONTH; + granularity = Granularities.MONTH; + unparseString = "MONTH"; } | { - return Granularities.YEAR; + granularity = Granularities.YEAR; + unparseString = "YEAR"; } |