You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2019/12/24 17:44:36 UTC
[GitHub] [drill] ihuzenko opened a new pull request #1940: DRILL-7406:
Update Calcite to 1.21.0
ihuzenko opened a new pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940
1. DRILL-7386 - added tests to TestHiveStructs.
2. DRILL-4527 - the DrillAvgVarianceConvertlet can't be removed without test failures.
3. DRILL-6215 - switched to prepared statement in JdbcRecordReader.
4. DRILL-6905 - added test into TestExampleQueries.
5. DRILL-7415 - Fixed jdbc show tables when 2 tables with same name are present in different schemas.
6. DRILL-7340 - Fixed jdbc filter pushdown when few jdbc datasources enabled.
7. Split SqlConverter into multiple source files.
8. Minor refactorings for jdbc and other places.
Jira: [DRILL-7406](https://issues.apache.org/jira/browse/DRILL-7406) .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323519
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
Review comment:
Please do not add todo-s in production code unless you have created Jira with the description of the needed fix in order to add Jira number here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362041807
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java
##########
@@ -37,17 +32,19 @@
import org.apache.calcite.tools.RelConversionException;
import org.apache.calcite.tools.ValidationException;
import org.apache.calcite.util.NlsString;
-import org.apache.calcite.util.Pair;
import org.apache.calcite.util.Util;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
-import org.apache.drill.exec.planner.sql.SqlConverter;
import org.apache.drill.exec.planner.sql.parser.DrillParserUtil;
import org.apache.drill.exec.planner.sql.parser.SqlShowTables;
import org.apache.drill.exec.store.AbstractSchema;
import org.apache.drill.exec.store.ischema.InfoSchemaTableType;
import org.apache.drill.exec.work.foreman.ForemanSetupException;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.IS_SCHEMA_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_SCHEMA;
Review comment:
This is part of ```InfoSchemaConstants``` interface for constants. I guess that there are some implicit conventions and changing this thee names will require renaming of all others. @arina-ielchiieva could you please confirm that such renaming makes sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361660974
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
+ return (cx, call) -> {
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ final List<SqlNode> operands = call.getOperandList();
+ final List<RexNode> exprs = new LinkedList<>();
+
+ String timeUnit = ((SqlIntervalQualifier) operands.get(0)).timeUnitRange.toString();
+
+ RelDataTypeFactory typeFactory = cx.getTypeFactory();
+
+ //RelDataType nullableReturnType =
+
+ for (SqlNode node : operands) {
+ exprs.add(cx.convertExpression(node));
+ }
+
+ final RelDataType returnType;
+ if (call.getOperator() == SqlStdOperatorTable.EXTRACT) {
+ // Legacy code:
+ // The return type is wrong!
+ // Legacy code choose SqlTypeName.BIGINT simply to avoid conflicting against Calcite's inference mechanism
+ // (, which chose BIGINT in validation phase already)
+ // Determine NULL-able using 2nd argument's Null-able.
Review comment:
NULL-able -> nullability
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361645269
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcRules;
+import org.apache.calcite.adapter.jdbc.JdbcToEnumerableConverterRule;
+import org.apache.calcite.linq4j.tree.ConstantUntypedNull;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.planner.RuleInstance;
+import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Convention with set of rules to register for jdbc plugin
+ */
+class DrillJdbcConvention extends JdbcConvention {
+
+ /**
+ * Unwanted Calcite's JdbcRules are filtered out by the predicate
+ */
+ private static final Predicate<RelOptRule> RULES_TO_EXCLUDE = rule -> {
Review comment:
Please rename it to something like `IS_EXCLUDED_RULE`, since it is a predicate now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] asfgit closed pull request #1940: DRILL-7406: Update
Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361674822
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java
##########
@@ -0,0 +1,297 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.jdbc.DynamicSchema;
+import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
+import org.apache.calcite.plan.ConventionTraitDef;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCostFactory;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.volcano.VolcanoPlanner;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelRoot;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.metadata.JaninoRelMetadataProvider;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.runtime.Hook;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.parser.SqlParseException;
+import org.apache.calcite.sql.parser.SqlParser;
+import org.apache.calcite.sql.util.ChainedSqlOperatorTable;
+import org.apache.calcite.sql2rel.SqlToRelConverter;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry;
+import org.apache.drill.exec.ops.QueryContext;
+import org.apache.drill.exec.ops.UdfUtilities;
+import org.apache.drill.exec.planner.cost.DrillCostBase;
+import org.apache.drill.exec.planner.logical.DrillConstExecutor;
+import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.exec.planner.physical.DrillDistributionTraitDef;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.planner.sql.DrillConvertletTable;
+import org.apache.drill.exec.planner.sql.DrillParserConfig;
+import org.apache.drill.exec.planner.sql.SchemaUtilites;
+import org.apache.drill.exec.planner.sql.parser.impl.DrillSqlParseException;
+import org.apache.drill.exec.planner.types.DrillRelDataTypeSystem;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.util.Utilities;
+
+/**
+ * Class responsible for managing:
+ * <ul>
+ * <li>parsing - {@link #parse(String)}<li/>
+ * <li>validation - {@link #validate(SqlNode)}<li/>
+ * <li>conversion to rel - {@link #toRel(SqlNode)} (String)}<li/>
+ * <ul/>
+ */
+public class SqlConverter {
+ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SqlConverter.class);
+
+ private final JavaTypeFactory typeFactory;
+ private final SqlParser.Config parserConfig;
+ private final DrillCalciteCatalogReader catalog;
+ private final PlannerSettings settings;
+ private final SchemaPlus rootSchema;
+ private final SchemaPlus defaultSchema;
+ private final SqlOperatorTable opTab;
+ private final RelOptCostFactory costFactory;
+ private final DrillValidator validator;
+ private final boolean isInnerQuery;
+ private final boolean isExpandedView;
+ private final UdfUtilities util;
+ private final FunctionImplementationRegistry functions;
+ private final String temporarySchema;
+ private final UserSession session;
+ private final DrillConfig drillConfig;
+ // Allow the default config to be modified using immutable configs
+ private SqlToRelConverter.Config sqlToRelConverterConfig;
+ private RelOptCluster cluster;
+ private VolcanoPlanner planner;
+ private boolean useRootSchema = false;
+
+ static {
+ /*
+ * Sets value to false to avoid simplifying project expressions
+ * during creating new projects since it may cause changing data mode
+ * which causes to assertion errors during type validation
+ */
+ Hook.REL_BUILDER_SIMPLIFY.add(Hook.propertyJ(false));
+ }
+
+ public SqlConverter(QueryContext context) {
+ this.settings = context.getPlannerSettings();
+ this.util = context;
+ this.functions = context.getFunctionRegistry();
+ this.parserConfig = new DrillParserConfig(settings);
+ this.sqlToRelConverterConfig = SqlToRelConverter.configBuilder()
+ .withInSubQueryThreshold((int) settings.getInSubqueryThreshold())
+ .withConvertTableAccess(false)
+ .withExpand(false)
+ .withRelBuilderFactory(DrillRelFactories.LOGICAL_BUILDER)
+ .build();
+ this.isInnerQuery = false;
+ this.isExpandedView = false;
+ this.typeFactory = new JavaTypeFactoryImpl(DrillRelDataTypeSystem.DRILL_REL_DATATYPE_SYSTEM);
+ this.defaultSchema = context.getNewDefaultSchema();
+ this.rootSchema = SchemaUtilites.rootSchema(defaultSchema);
+ this.temporarySchema = context.getConfig().getString(ExecConstants.DEFAULT_TEMPORARY_WORKSPACE);
+ this.session = context.getSession();
+ this.drillConfig = context.getConfig();
+ this.catalog = new DrillCalciteCatalogReader(
+ rootSchema,
+ parserConfig.caseSensitive(),
+ DynamicSchema.from(defaultSchema).path(null),
+ typeFactory,
+ drillConfig,
+ session,
+ temporarySchema,
+ this::isUseRootSchema,
+ this::getDefaultSchema);
+ this.opTab = new ChainedSqlOperatorTable(Arrays.asList(context.getDrillOperatorTable(), catalog));
+ this.costFactory = (settings.useDefaultCosting()) ? null : new DrillCostBase.DrillCostFactory();
+ this.validator = new DrillValidator(opTab, catalog, typeFactory, parserConfig.conformance(), session);
+ validator.setIdentifierExpansion(true);
+ cluster = null;
+ }
+
+ SqlConverter(SqlConverter parent, SchemaPlus defaultSchema, SchemaPlus rootSchema,
+ DrillCalciteCatalogReader catalog) {
+ this.parserConfig = parent.parserConfig;
+ this.sqlToRelConverterConfig = parent.sqlToRelConverterConfig;
+ this.defaultSchema = defaultSchema;
+ this.functions = parent.functions;
+ this.util = parent.util;
+ this.isInnerQuery = true;
+ this.isExpandedView = true;
+ this.typeFactory = parent.typeFactory;
+ this.costFactory = parent.costFactory;
+ this.settings = parent.settings;
+ this.rootSchema = rootSchema;
+ this.catalog = catalog;
+ this.opTab = parent.opTab;
+ this.planner = parent.planner;
+ this.validator = new DrillValidator(opTab, catalog, typeFactory, parserConfig.conformance(), parent.session);
+ this.temporarySchema = parent.temporarySchema;
+ this.session = parent.session;
+ this.drillConfig = parent.drillConfig;
+ validator.setIdentifierExpansion(true);
+ this.cluster = parent.cluster;
+ }
+
+ public SqlNode parse(String sql) {
+ try {
+ SqlParser parser = SqlParser.create(sql, parserConfig);
+ return parser.parseStmt();
+ } catch (SqlParseException parseError) {
+ DrillSqlParseException dex = new DrillSqlParseException(sql, parseError);
+ UserException.Builder builder = UserException
+ .parseError(dex)
+ .addContext(dex.getSqlWithErrorPointer());
+ if (isInnerQuery) {
+ builder.message("Failure parsing a view your query is dependent upon.");
+ }
+ throw builder.build(logger);
+ }
+ }
+
+ public SqlNode validate(final SqlNode parsedNode) {
+ try {
+ return validator.validate(parsedNode);
+ } catch (RuntimeException e) {
+ UserException.Builder builder = UserException
+ .validationError(e);
+ if (isInnerQuery) {
+ builder.message("Failure validating a view your query is dependent upon.");
+ }
+ throw builder.build(logger);
+ }
+ }
+
+ public RelRoot toRel(final SqlNode validatedNode) {
+ initCluster(initPlanner());
+ DrillViewExpander viewExpander = new DrillViewExpander(this);
+ final SqlToRelConverter sqlToRelConverter = new SqlToRelConverter(
+ viewExpander, validator, catalog, cluster,
+ DrillConvertletTable.INSTANCE, sqlToRelConverterConfig);
+
+ boolean topLevelQuery = !isInnerQuery || isExpandedView;
+ RelRoot rel = sqlToRelConverter.convertQuery(validatedNode, false, topLevelQuery);
+
+ // If extra expressions used in ORDER BY were added to the project list,
+ // add another project to remove them.
+ if (topLevelQuery && rel.rel.getRowType().getFieldCount() - rel.fields.size() > 0) {
+ RexBuilder builder = rel.rel.getCluster().getRexBuilder();
+
+ RelNode relNode = rel.rel;
+ List<RexNode> expressions = rel.fields.stream()
+ .map(f -> builder.makeInputRef(relNode, f.left))
+ .collect(Collectors.toList());
+
+ RelNode project = LogicalProject.create(rel.rel, expressions, rel.validatedRowType);
+ rel = RelRoot.of(project, rel.validatedRowType, rel.kind);
+ }
+ return rel.withRel(sqlToRelConverter.flattenTypes(rel.rel, true));
+ }
+
+ public RelDataType getOutputType(SqlNode validatedNode) {
+ return validator.getValidatedNodeType(validatedNode);
+ }
+
+ public JavaTypeFactory getTypeFactory() {
+ return typeFactory;
+ }
+
+ public DrillConfig getDrillConfig() {
+ return drillConfig;
+ }
+
+ public UserSession getSession() {
+ return session;
+ }
+
+ public RelOptCostFactory getCostFactory() {
+ return costFactory;
+ }
+
+ public SchemaPlus getRootSchema() {
+ return rootSchema;
+ }
+
+ public SchemaPlus getDefaultSchema() {
+ return defaultSchema;
+ }
+
+ public boolean isCaseSensitive() {
+ return parserConfig.caseSensitive();
+ }
+
+ /** Disallow temporary tables presence in sql statement (ex: in view definitions) */
+ public void disallowTemporaryTables() {
+ catalog.disallowTemporaryTables();
+ }
+
+ String getTemporarySchema() {
+ return temporarySchema;
+ }
+
+ boolean isUseRootSchema() {
Review comment:
Please rename it to `useRootSchema`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361650077
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private static final Logger logger = LoggerFactory.getLogger(JdbcCatalogSchema.class);
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
Review comment:
Not a requirement, just advice: may be used apache-commons method `StringUtils.lowerCase`:
```suggestion
String parentKey = StringUtils.lowerCase(catalogName);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361660596
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
+ return (cx, call) -> {
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ final List<SqlNode> operands = call.getOperandList();
+ final List<RexNode> exprs = new LinkedList<>();
+
+ String timeUnit = ((SqlIntervalQualifier) operands.get(0)).timeUnitRange.toString();
+
+ RelDataTypeFactory typeFactory = cx.getTypeFactory();
+
+ //RelDataType nullableReturnType =
Review comment:
Please remove this line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361676747
##########
File path: logical/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java
##########
@@ -18,52 +18,49 @@
package org.apache.drill.common.expression;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.drill.common.exceptions.DrillRuntimeException;
import org.apache.drill.common.types.TypeProtos.MajorType;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableMap;
import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
public class FunctionCallFactory {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FunctionCallFactory.class);
Review comment:
Please remove it since it is not used anywhere.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323447
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
Review comment:
Why are using logger from different class?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323411
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcRules;
+import org.apache.calcite.adapter.jdbc.JdbcToEnumerableConverterRule;
+import org.apache.calcite.linq4j.tree.ConstantUntypedNull;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.planner.RuleInstance;
+import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Convention with set of rules to register for jdbc plugin
+ */
+class DrillJdbcConvention extends JdbcConvention {
+
+ /**
+ * Unwanted Calcite's JdbcRules are filtered out by the predicate
+ */
+ private static final Predicate<RelOptRule> CALCITE_RULES_TO_INCLUDE = rule -> {
+ final Class<?> rc = rule.getClass();
Review comment:
I tend not to add final keyword relying on the effective final notion but it's up to you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323267
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/CapitalizingJdbcSchema.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcSchema;
+import org.apache.calcite.schema.Function;
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+
+class CapitalizingJdbcSchema extends AbstractSchema {
+
+ final Map<String, CapitalizingJdbcSchema> schemaMap;
Review comment:
private?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361661949
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
+ return (cx, call) -> {
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ final List<SqlNode> operands = call.getOperandList();
+ final List<RexNode> exprs = new LinkedList<>();
+
+ String timeUnit = ((SqlIntervalQualifier) operands.get(0)).timeUnitRange.toString();
+
+ RelDataTypeFactory typeFactory = cx.getTypeFactory();
+
+ //RelDataType nullableReturnType =
+
+ for (SqlNode node : operands) {
+ exprs.add(cx.convertExpression(node));
+ }
+
+ final RelDataType returnType;
+ if (call.getOperator() == SqlStdOperatorTable.EXTRACT) {
+ // Legacy code:
+ // The return type is wrong!
+ // Legacy code choose SqlTypeName.BIGINT simply to avoid conflicting against Calcite's inference mechanism
+ // (, which chose BIGINT in validation phase already)
+ // Determine NULL-able using 2nd argument's Null-able.
+ returnType = typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.BIGINT), exprs.get(1).getType().isNullable());
+ } else {
+ // Determine NULL-able using 2nd argument's Null-able.
+ returnType = typeFactory.createTypeWithNullability(
+ typeFactory.createSqlType(
+ TypeInferenceUtils.getSqlTypeNameForTimeUnit(timeUnit)),
+ exprs.get(1).getType().isNullable());
+ }
+
+ return rexBuilder.makeCall(returnType, call.getOperator(), exprs);
+ };
+ }
+
+ /**
+ * SQRT needs it's own convertlet because calcite overrides it to POWER(x, 0.5)
+ * which is not suitable for Infinity value case
+ */
+ private static SqlRexConvertlet sqrt() {
+ return (cx, call) -> {
+ RexNode operand = cx.convertExpression(call.operand(0));
+ return cx.getRexBuilder().makeCall(SqlStdOperatorTable.SQRT, operand);
+ };
+ }
+
+ /**
+ * Rewrites COALESCE function into CASE WHEN IS NOT NULL operand1 THEN operand1...
+ * all Calcite interval representations correctly.
+ * Custom convertlet to avoid rewriting TIMESTAMP_DIFF by Calcite,
+ * since Drill does not support Reinterpret function and does not handle
+ */
+ private static SqlRexConvertlet coalesce() {
+ return (cx, call) -> {
+ int operandsCount = call.operandCount();
+ if (operandsCount == 1) {
+ return cx.convertExpression(call.operand(0));
+ } else {
+ List<RexNode> caseOperands = new ArrayList<>();
+ for (int i = 0; i < operandsCount - 1; i++) {
+ RexNode caseOperand = cx.convertExpression(call.operand(i));
+ caseOperands.add(cx.getRexBuilder().makeCall(
+ SqlStdOperatorTable.IS_NOT_NULL, caseOperand));
+ caseOperands.add(caseOperand);
+ }
+ caseOperands.add(cx.convertExpression(call.operand(operandsCount - 1)));
+ return cx.getRexBuilder().makeCall(SqlStdOperatorTable.CASE, caseOperands);
+ }
+ };
+ }
+
+ private static SqlRexConvertlet timestampDiff() {
+ return (cx, call) -> {
+ SqlLiteral unitLiteral = call.operand(0);
+ SqlIntervalQualifier qualifier =
+ new SqlIntervalQualifier(unitLiteral.symbolValue(TimeUnit.class), null, SqlParserPos.ZERO);
+
+ List<RexNode> operands = Arrays.asList(
+ cx.convertExpression(qualifier),
+ cx.convertExpression(call.operand(1)),
+ cx.convertExpression(call.operand(2)));
+
+ RelDataTypeFactory typeFactory = cx.getTypeFactory();
+
+ RelDataType returnType = typeFactory.createTypeWithNullability(
+ typeFactory.createSqlType(SqlTypeName.BIGINT),
+ cx.getValidator().getValidatedNodeType(call.operand(1)).isNullable()
+ || cx.getValidator().getValidatedNodeType(call.operand(2)).isNullable());
+
+ return cx.getRexBuilder().makeCall(returnType,
+ SqlStdOperatorTable.TIMESTAMP_DIFF, operands);
+ };
+ }
+
+ private static SqlRexConvertlet row() {
+ return (cx, call) -> {
+ List<RexNode> args = call.getOperandList().stream()
+ .map(cx::convertExpression)
+ .collect(Collectors.toList());
+ return cx.getRexBuilder().makeCall(SqlStdOperatorTable.ROW, args);
+ };
+ }
+
+ private static SqlRexConvertlet avgVariance(Function<SqlNode, SqlNode> expandFunc) {
+ return (cx, call) -> cx.convertExpression(expandFunc.apply(call.operand(0)));
+ }
+
+ private static SqlNode expandAvg(final SqlNode arg) {
+ final SqlParserPos pos = SqlParserPos.ZERO;
Review comment:
final
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361324076
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,638 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.StoragePluginRegistryImpl;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = String.format(
+ "jdbc:h2:%s", dirTestWatcher.getTmpDir().getCanonicalPath());
+
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root")) {
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ Assert.assertNotNull("Script for test tables generation 'h2-test-data.sql' " +
+ "cannot be found in test resources", scriptFile);
+ try (FileReader fileReader = new FileReader(scriptFile.getFile())) {
+ RunScript.execute(connection, fileReader);
+ }
+ }
+
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig(
+ "org.h2.Driver",
+ connString,
+ "root",
+ "root",
+ true);
+ jdbcStorageConfig.setEnabled(true);
+
+ String pluginName = "h2";
+ DrillbitContext context = cluster.drillbit().getContext();
+ JdbcStoragePlugin jdbcStoragePlugin = new JdbcStoragePlugin(jdbcStorageConfig,
+ context, pluginName);
+ StoragePluginRegistryImpl pluginRegistry = (StoragePluginRegistryImpl) context.getStorage();
+ pluginRegistry.addPluginToPersistentStoreIfAbsent(pluginName, jdbcStorageConfig, jdbcStoragePlugin);
+ }
+
+ private static void initMySQL() throws Exception {
+ String mysqlPluginName = "mysql";
+ String mysqlDBName = "drill_mysql_test";
+ int mysqlPort = QueryTestUtil.getFreePortNumber(2215, 300);
+
+ MysqldConfig config = MysqldConfig.aMysqldConfig(Version.v5_6_21)
+ .withPort(mysqlPort)
+ .withUser("mysqlUser", "mysqlPass")
+ .withTimeZone(DateTimeZone.UTC.toTimeZone())
+ .build();
+
+ SchemaConfig.Builder schemaConfig = SchemaConfig.aSchemaConfig(mysqlDBName)
+ .withScripts(ScriptResolver.classPathScript("mysql-test-data.sql"));
+
+ String osName = System.getProperty("os.name").toLowerCase();
+ if (osName.startsWith("linux")) {
+ schemaConfig.withScripts(ScriptResolver.classPathScript("mysql-test-data-linux.sql"));
+ }
+
+ mysqld = EmbeddedMysql.anEmbeddedMysql(config)
+ .addSchema(schemaConfig.build())
+ .start();
+
+ StoragePluginRegistryImpl pluginRegistry = (StoragePluginRegistryImpl) cluster.drillbit().getContext().getStorage();
Review comment:
Use newly created method from ClusterFixture, see comment above
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361667298
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+import java.util.function.BooleanSupplier;
+import java.util.function.Supplier;
+
+import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.jdbc.CalciteSchema;
+import org.apache.calcite.jdbc.DynamicSchema;
+import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Util;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.planner.logical.DrillTable;
+import org.apache.drill.exec.planner.sql.SchemaUtilites;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.store.dfs.FileSelection;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheBuilder;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheLoader;
+import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+/**
+ * Extension of {@link CalciteCatalogReader} to add ability to check for temporary tables first
+ * if schema is not indicated near table name during query parsing
+ * or indicated workspace is default temporary workspace.
+ */
+class DrillCalciteCatalogReader extends CalciteCatalogReader {
+
+ private final DrillConfig drillConfig;
+ private final UserSession session;
+ private final String temporarySchema;
+ private boolean allowTemporaryTables;
+ private final BooleanSupplier useRootSchema;
+ private final Supplier<SchemaPlus> defaultSchemaSupplier;
+
+ private final LoadingCache<DrillTableKey, MetadataProviderManager> tableCache;
+
+ DrillCalciteCatalogReader(SchemaPlus rootSchema,
+ boolean caseSensitive,
+ List<String> defaultSchema,
+ JavaTypeFactory typeFactory,
+ DrillConfig drillConfig,
+ UserSession session,
+ String temporarySchema,
+ BooleanSupplier useRootSchema,
+ Supplier<SchemaPlus> defaultSchemaSupplier) {
+ super(DynamicSchema.from(rootSchema), defaultSchema,
+ typeFactory, getConnectionConfig(caseSensitive));
+ this.drillConfig = drillConfig;
+ this.session = session;
+ this.allowTemporaryTables = true;
+ this.tableCache =
+ CacheBuilder.newBuilder()
+ .build(new CacheLoader<DrillTableKey, MetadataProviderManager>() {
+ @Override
+ public MetadataProviderManager load(DrillTableKey key) {
+ return key.getMetadataProviderManager();
+ }
+ });
+ this.temporarySchema = temporarySchema;
+ this.useRootSchema = useRootSchema;
+ this.defaultSchemaSupplier = defaultSchemaSupplier;
+ }
+
+ /**
+ * Disallow temporary tables presence in sql statement (ex: in view definitions)
+ */
+ void disallowTemporaryTables() {
+ this.allowTemporaryTables = false;
+ }
+
+ List<String> getTemporaryNames(List<String> names) {
+ if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) {
+ String tableName = FileSelection.removeLeadingSlash(names.get(names.size() - 1));
+ String temporaryTableName = session.resolveTemporaryTableName(tableName);
+ if (temporaryTableName != null) {
+ List<String> temporaryNames = new ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema));
+ temporaryNames.add(temporaryTableName);
+ return temporaryNames;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * If schema is not indicated (only one element in the list) or schema is default temporary workspace,
+ * we need to check among session temporary tables in default temporary workspace first.
+ * If temporary table is found and temporary tables usage is allowed, its table instance will be returned,
+ * otherwise search will be conducted in original workspace.
+ *
+ * @param names list of schema and table names, table name is always the last element
+ * @return table instance, null otherwise
+ * @throws UserException if temporary tables usage is disallowed
+ */
+ @Override
+ public Prepare.PreparingTable getTable(List<String> names) {
+ checkIfWeCanProceedForTemporaryTable(names);
+ Prepare.PreparingTable table = super.getTable(names);
+ DrillTable drillTable;
+ if (table != null && (drillTable = table.unwrap(DrillTable.class)) != null) {
+ drillTable.setOptions(session.getOptions());
+ drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names, drillTable)));
+ }
+ return table;
+ }
+
+ private void checkIfWeCanProceedForTemporaryTable(List<String> names) {
Review comment:
Could you please rename it to something simpler, like `checkTemporaryTable`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361673270
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillViewExpander.java
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.List;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelRoot;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.shaded.guava.com.google.common.base.Joiner;
+
+class DrillViewExpander implements RelOptTable.ViewExpander {
+
+ private final SqlConverter sqlConverter;
+
+ DrillViewExpander(SqlConverter sqlConverter) {
+ this.sqlConverter = sqlConverter;
+ }
+
+ @Override
+ public RelRoot expandView(RelDataType rowType, String queryString, List<String> schemaPath, List<String> viewPath) {
+ DrillCalciteCatalogReader catalogReader = newCatalogReader(sqlConverter.getRootSchema(), schemaPath);
+ SqlConverter parser = new SqlConverter(sqlConverter, sqlConverter.getDefaultSchema(),
+ sqlConverter.getRootSchema(), catalogReader);
+ return convertToRel(queryString, parser);
+ }
+
+ @Override
+ public RelRoot expandView(RelDataType rowType, String queryString, SchemaPlus rootSchema, List<String> schemaPath) {
+ final DrillCalciteCatalogReader catalogReader = newCatalogReader(rootSchema, schemaPath);
+ SchemaPlus schema = findSchema(queryString, rootSchema, schemaPath);
+ SqlConverter parser = new SqlConverter(sqlConverter, schema, rootSchema, catalogReader);
+ return convertToRel(queryString, parser);
+ }
+
+ private RelRoot convertToRel(String queryString, SqlConverter converter) {
+ converter.disallowTemporaryTables();
+ final SqlNode parsedNode = converter.parse(queryString);
+ final SqlNode validatedNode = converter.validate(parsedNode);
+ return converter.toRel(validatedNode);
+ }
+
+ private DrillCalciteCatalogReader newCatalogReader(SchemaPlus rootSchema, List<String> schemaPath) {
+ return new DrillCalciteCatalogReader(
+ rootSchema,
+ sqlConverter.isCaseSensitive(),
+ schemaPath,
+ sqlConverter.getTypeFactory(),
+ sqlConverter.getDrillConfig(),
+ sqlConverter.getSession(),
+ sqlConverter.getTemporarySchema(),
+ sqlConverter::isUseRootSchema,
+ sqlConverter::getDefaultSchema);
+ }
+
+ private SchemaPlus findSchema(String queryString, SchemaPlus rootSchema, List<String> schemaPath) {
+ SchemaPlus schema = rootSchema;
+ for (String s : schemaPath) {
+ SchemaPlus newSchema = schema.getSubSchema(s);
+ if (newSchema == null) {
+ throw UserException
+ .validationError()
+ .message("Failure while attempting to expand view. Requested schema %s not available in schema %s.",
+ s, schema.getName())
+ .addContext("View Context", Joiner.on(", ").join(schemaPath))
+ .addContext("View SQL", queryString)
+ .build(SqlConverter.logger);
Review comment:
logger
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362072204
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java
##########
@@ -37,17 +32,19 @@
import org.apache.calcite.tools.RelConversionException;
import org.apache.calcite.tools.ValidationException;
import org.apache.calcite.util.NlsString;
-import org.apache.calcite.util.Pair;
import org.apache.calcite.util.Util;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
-import org.apache.drill.exec.planner.sql.SqlConverter;
import org.apache.drill.exec.planner.sql.parser.DrillParserUtil;
import org.apache.drill.exec.planner.sql.parser.SqlShowTables;
import org.apache.drill.exec.store.AbstractSchema;
import org.apache.drill.exec.store.ischema.InfoSchemaTableType;
import org.apache.drill.exec.work.foreman.ForemanSetupException;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.IS_SCHEMA_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_SCHEMA;
Review comment:
I guess who initially added such constants named them this way, it means shared, referring to the column names which are the same for all information schema tables. You can rename all constants if you think this is needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361650565
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private static final Logger logger = LoggerFactory.getLogger(JdbcCatalogSchema.class);
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
Review comment:
Please add a comment that we store catalog names in `schemaMap` in lower case (see `AbstractSchema` constructor).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361677218
##########
File path: logical/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java
##########
@@ -18,52 +18,49 @@
package org.apache.drill.common.expression;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.drill.common.exceptions.DrillRuntimeException;
import org.apache.drill.common.types.TypeProtos.MajorType;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableMap;
import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
public class FunctionCallFactory {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FunctionCallFactory.class);
- private static Map<String, String> opToFuncTable = new HashMap<>();
-
- static {
- opToFuncTable.put("+", "add");
- opToFuncTable.put("-", "subtract");
- opToFuncTable.put("/", "divide");
- opToFuncTable.put("*", "multiply");
- opToFuncTable.put("%", "modulo");
- opToFuncTable.put("^", "xor");
- opToFuncTable.put("||", "concatOperator");
- opToFuncTable.put("or", "booleanOr");
- opToFuncTable.put("and", "booleanAnd");
- opToFuncTable.put(">", "greater_than");
- opToFuncTable.put("<", "less_than");
- opToFuncTable.put("==", "equal");
- opToFuncTable.put("=", "equal");
- opToFuncTable.put("!=", "not_equal");
- opToFuncTable.put("<>", "not_equal");
- opToFuncTable.put(">=", "greater_than_or_equal_to");
- opToFuncTable.put("<=", "less_than_or_equal_to");
- opToFuncTable.put("is null", "isnull");
- opToFuncTable.put("is not null", "isnotnull");
- opToFuncTable.put("is true", "istrue");
- opToFuncTable.put("is not true", "isnottrue");
- opToFuncTable.put("is false", "isfalse");
- opToFuncTable.put("is not false", "isnotfalse");
- opToFuncTable.put("similar to", "similar_to");
-
- opToFuncTable.put("!", "not");
- opToFuncTable.put("u-", "negative");
- }
+ private static final Map<String, String> OP_TO_FUNC_NAME = ImmutableMap.<String, String>builder()
+ .put("+", "add")
+ .put("-", "subtract")
+ .put("/", "divide")
+ .put("*", "multiply")
+ .put("%", "modulo")
+ .put("^", "xor")
+ .put("||", "concatOperator")
+ .put("or", "booleanOr")
+ .put("and", "booleanAnd")
+ .put(">", "greater_than")
+ .put("<", "less_than")
+ .put("==", "equal")
+ .put("=", "equal")
+ .put("!=", "not_equal")
+ .put("<>", "not_equal")
+ .put(">=", "greater_than_or_equal_to")
+ .put("<=", "less_than_or_equal_to")
+ .put("is null", "isnull")
+ .put("is not null", "isnotnull")
+ .put("is true", "istrue")
+ .put("is not true", "isnottrue")
+ .put("is false", "isfalse")
+ .put("is not false", "isnotfalse")
+ .put("similar to", "similar_to")
+ .put("!", "not")
+ .put("u-", "negative")
+ .build();
public static String replaceOpWithFuncName(String op) {
Review comment:
Could you please rename this method, since its implementation differs from its name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361667498
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+import java.util.function.BooleanSupplier;
+import java.util.function.Supplier;
+
+import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.jdbc.CalciteSchema;
+import org.apache.calcite.jdbc.DynamicSchema;
+import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Util;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.planner.logical.DrillTable;
+import org.apache.drill.exec.planner.sql.SchemaUtilites;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.store.dfs.FileSelection;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheBuilder;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheLoader;
+import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+/**
+ * Extension of {@link CalciteCatalogReader} to add ability to check for temporary tables first
+ * if schema is not indicated near table name during query parsing
+ * or indicated workspace is default temporary workspace.
+ */
+class DrillCalciteCatalogReader extends CalciteCatalogReader {
+
+ private final DrillConfig drillConfig;
+ private final UserSession session;
+ private final String temporarySchema;
+ private boolean allowTemporaryTables;
+ private final BooleanSupplier useRootSchema;
+ private final Supplier<SchemaPlus> defaultSchemaSupplier;
+
+ private final LoadingCache<DrillTableKey, MetadataProviderManager> tableCache;
+
+ DrillCalciteCatalogReader(SchemaPlus rootSchema,
+ boolean caseSensitive,
+ List<String> defaultSchema,
+ JavaTypeFactory typeFactory,
+ DrillConfig drillConfig,
+ UserSession session,
+ String temporarySchema,
+ BooleanSupplier useRootSchema,
+ Supplier<SchemaPlus> defaultSchemaSupplier) {
+ super(DynamicSchema.from(rootSchema), defaultSchema,
+ typeFactory, getConnectionConfig(caseSensitive));
+ this.drillConfig = drillConfig;
+ this.session = session;
+ this.allowTemporaryTables = true;
+ this.tableCache =
+ CacheBuilder.newBuilder()
+ .build(new CacheLoader<DrillTableKey, MetadataProviderManager>() {
+ @Override
+ public MetadataProviderManager load(DrillTableKey key) {
+ return key.getMetadataProviderManager();
+ }
+ });
+ this.temporarySchema = temporarySchema;
+ this.useRootSchema = useRootSchema;
+ this.defaultSchemaSupplier = defaultSchemaSupplier;
+ }
+
+ /**
+ * Disallow temporary tables presence in sql statement (ex: in view definitions)
+ */
+ void disallowTemporaryTables() {
+ this.allowTemporaryTables = false;
+ }
+
+ List<String> getTemporaryNames(List<String> names) {
+ if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) {
+ String tableName = FileSelection.removeLeadingSlash(names.get(names.size() - 1));
+ String temporaryTableName = session.resolveTemporaryTableName(tableName);
+ if (temporaryTableName != null) {
+ List<String> temporaryNames = new ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema));
+ temporaryNames.add(temporaryTableName);
+ return temporaryNames;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * If schema is not indicated (only one element in the list) or schema is default temporary workspace,
+ * we need to check among session temporary tables in default temporary workspace first.
+ * If temporary table is found and temporary tables usage is allowed, its table instance will be returned,
+ * otherwise search will be conducted in original workspace.
+ *
+ * @param names list of schema and table names, table name is always the last element
+ * @return table instance, null otherwise
+ * @throws UserException if temporary tables usage is disallowed
+ */
+ @Override
+ public Prepare.PreparingTable getTable(List<String> names) {
+ checkIfWeCanProceedForTemporaryTable(names);
+ Prepare.PreparingTable table = super.getTable(names);
+ DrillTable drillTable;
+ if (table != null && (drillTable = table.unwrap(DrillTable.class)) != null) {
+ drillTable.setOptions(session.getOptions());
+ drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names, drillTable)));
+ }
+ return table;
+ }
+
+ private void checkIfWeCanProceedForTemporaryTable(List<String> names) {
+ String originalTableName;
+ if (!allowTemporaryTables &&
+ (originalTableName = session.getOriginalTableNameFromTemporaryTable(names.get(names.size() - 1))) != null) {
Review comment:
Please extract assigning for the condition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323585
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return (CapitalizingJdbcSchema) connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
+ CapitalizingJdbcSchema parentSchema = schemaMap.get(parentKey);
+ if (parentSchema == null) {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(getSchemaPath(), schemaName, source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+
+ // if a catalog schema doesn't exist, we'll add this at the top level.
+ schemaMap.put(schema.getName(), schema);
+ } else {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(parentSchema.getSchemaPath(), schemaName,
+ source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+ parentSchema.schemaMap.put(schema.getName(), schema);
+ }
+ added = true;
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
Review comment:
Same here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361660344
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
Review comment:
```suggestion
* Custom convertlet to handle extract functions. Calcite rewrites
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361656838
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,601 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = "jdbc:h2:" + dirTestWatcher.getTmpDir().getCanonicalPath();
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ assertNotNull("Script for test tables generation 'h2-test-data.sql' cannot be found in test resources", scriptFile);
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root");
+ FileReader fileReader = new FileReader(scriptFile.getFile())) {
+ RunScript.execute(connection, fileReader);
+ }
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("org.h2.Driver", connString, "root", "root", true);
+ jdbcStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "h2"));
+ }
+
+ private static void initMySQL() throws Exception {
+ String mysqlDBName = "drill_mysql_test";
+ int mysqlPort = QueryTestUtil.getFreePortNumber(2215, 300);
+
+ MysqldConfig config = MysqldConfig.aMysqldConfig(Version.v5_6_21)
+ .withPort(mysqlPort)
+ .withUser("mysqlUser", "mysqlPass")
+ .withTimeZone(DateTimeZone.UTC.toTimeZone())
+ .build();
+
+ SchemaConfig.Builder schemaConfig = SchemaConfig.aSchemaConfig(mysqlDBName)
+ .withScripts(ScriptResolver.classPathScript("mysql-test-data.sql"));
+
+ String osName = System.getProperty("os.name").toLowerCase();
+ if (osName.startsWith("linux")) {
+ schemaConfig.withScripts(ScriptResolver.classPathScript("mysql-test-data-linux.sql"));
+ }
+
+ mysqld = EmbeddedMysql.anEmbeddedMysql(config).addSchema(schemaConfig.build()).start();
+
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", false);
+ jdbcStorageConfig.setEnabled(true);
+
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "mysql"));
+
+ if (osName.startsWith("linux")) {
+ // adds storage plugin with case insensitive table names
+ JdbcStorageConfig jdbcCaseSensitiveStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", true);
+ jdbcCaseSensitiveStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcCaseSensitiveStorageConfig, ctx, "mysqlCaseInsensitive"));
+ }
+ }
+
+ @Test
+ public void h2CrossSourceMultiFragmentJoin() throws Exception {
+ try {
+ client.alterSession(ExecConstants.SLICE_TARGET, 1);
+ run("select x.person_id, y.salary from h2.tmp.drill_h2_test.person x "
+ + "join %s y on x.person_id = y.person_id ", TABLE_NAME);
+ } finally {
+ client.resetSession(ExecConstants.SLICE_TARGET);
+ }
+ }
+
+ @Test
+ public void h2ValidateResult() throws Exception {
+ // Skip date, time, and timestamp types since h2 mangles these due to improper timezone support.
+ testBuilder()
+ .sqlQuery(
+ "select person_id, first_name, last_name, address, city, state, zip, json, bigint_field, smallint_field, " +
+ "numeric_field, boolean_field, double_field, float_field, real_field, time_field, timestamp_field, " +
+ "date_field, clob_field from h2.tmp.`drill_h2_test`.person")
+ .ordered()
+ .baselineColumns("person_id", "first_name", "last_name", "address", "city", "state", "zip", "json",
+ "bigint_field", "smallint_field", "numeric_field", "boolean_field", "double_field", "float_field",
+ "real_field", "time_field", "timestamp_field", "date_field", "clob_field")
+ .baselineValues(1, "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ "{ a : 5, b : 6 }", 123456L, 1, new BigDecimal("10.01"), false, 1.0, 1.1, 111.00,
+ DateUtility.parseLocalTime("13:00:01.0"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"),
+ DateUtility.parseLocalDate("2012-02-29"), "some clob data 1")
+ .baselineValues(2, "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ "{ foo : \"abc\" }", 95949L, 2, new BigDecimal("20.02"), true, 2.0, 2.1, 222.00,
+ DateUtility.parseLocalTime("23:59:59.0"), DateUtility.parseLocalDateTime("1999-09-09 23:59:59.0"),
+ DateUtility.parseLocalDate("1999-09-09"), "some more clob data")
+ .baselineValues(3, "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012, "{ z : [ 1, 2, 3 ] }",
+ 45456L, 3, new BigDecimal("30.04"), true, 3.0, 3.1, 333.00, DateUtility.parseLocalTime("11:34:21.0"),
+ DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2011-10-30"), "clobber")
+ .baselineValues(4, null, null, "2 15th St NW", "Washington", "DC", 20007, "{ z : { a : 1, b : 2, c : 3 } }",
+ -67L, 4, new BigDecimal("40.04"), false, 4.0, 4.1, 444.00, DateUtility.parseLocalTime("16:00:01.0"),
+ DateUtility.parseLocalDateTime("2015-06-01 16:00:01.0"), DateUtility.parseLocalDate("2015-06-01"), "xxx")
+ .baselineValues(5, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null)
+ .go();
+ }
+
+ @Test
+ public void h2PushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from h2.tmp.drill_h2_test.person) x "
+ + "join (select person_id from h2.tmp.drill_h2_test.person) y on x.person_id = y.person_id ";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ }
+
+ @Test
+ public void h2PushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.FIRST_NAME = s.FIRST_NAME\n" +
+ "WHERE e.LAST_NAME > 'hello'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2PushdownAggregation() throws Exception {
+ String query = "select count(*) from h2.tmp.drill_h2_test.person";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Aggregate operator",
+ plan, not(containsString("Aggregate")));
+ }
+
+ @Test
+ public void h2PushdownDoubleJoinAndFilter() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.person_ID = s.person_ID\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person ed\n" +
+ "ON e.person_ID = ed.person_ID\n" +
+ "WHERE s.first_name > 'abc' and ed.first_name > 'efg'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2ShowTablesDefaultSchema() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(1, queryBuilder().sql("show tables like 'PERSON'").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(1, queryBuilder().sql("show tables like 'person'").run().recordCount());
+ }
+
+ @Test
+ public void h2Describe() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(19, queryBuilder().sql("describe PERSON").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(19, queryBuilder().sql("describe person").run().recordCount());
+ }
+
+ @Test
+ public void h2EnsureDrillFunctionsAreNotPushedDown() throws Exception {
+ // This should verify that we're not trying to push CONVERT_FROM into the JDBC storage plugin. If were pushing
+ // this function down, the SQL query would fail.
+ run("select CONVERT_FROM(JSON, 'JSON') from h2.tmp.drill_h2_test.person where person_ID = 4");
+ }
+
+ @Test
+ public void h2PushdownFilter() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.person where person_ID = 1";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2CaseInsensitiveTableNames() throws Exception {
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PeRsOn").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PERSON").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.person").run().recordCount());
+ }
+
+ @Test
+ public void h2JdbcStoragePluginSerDe() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.PeRsOn";
+
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(5, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlValidateResult() throws Exception {
+
+ testBuilder()
+ .sqlQuery(
+ "select person_id, " +
+ "first_name, last_name, address, city, state, zip, " +
+ "bigint_field, smallint_field, numeric_field, " +
+ "boolean_field, double_field, float_field, real_field, " +
+ "date_field, datetime_field, year_field, time_field, " +
+ "json, text_field, tiny_text_field, medium_text_field, long_text_field, " +
+ "blob_field, bit_field, enum_field " +
+ "from mysql.`drill_mysql_test`.person")
+ .ordered()
+ .baselineColumns("person_id",
+ "first_name", "last_name", "address", "city", "state", "zip",
+ "bigint_field", "smallint_field", "numeric_field",
+ "boolean_field",
+ "double_field", "float_field", "real_field",
+ "date_field", "datetime_field", "year_field", "time_field",
+ "json", "text_field", "tiny_text_field", "medium_text_field", "long_text_field",
+ "blob_field", "bit_field", "enum_field")
+ .baselineValues(1,
+ "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ 123456789L, 1, new BigDecimal("10.01"),
+ false,
+ 1.0, 1.1, 1.2,
+ DateUtility.parseLocalDate("2012-02-29"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("13:00:01.0"),
+ "{ a : 5, b : 6 }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "xxx",
+ "a medium piece of text",
+ "a longer piece of text this is going on.....",
+ "this is a test".getBytes(),
+ true, "XXX")
+ .baselineValues(2,
+ "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ 45456767L, 3, new BigDecimal("30.04"),
+ true,
+ 3.0, 3.1, 3.2,
+ DateUtility.parseLocalDate("2011-10-30"), DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("11:34:21.0"),
+ "{ z : [ 1, 2, 3 ] }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "abc",
+ "a medium piece of text 2",
+ "somewhat more text",
+ "this is a test 2".getBytes(),
+ false, "YYY")
+ .baselineValues(3,
+ "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012,
+ 123090L, -3, new BigDecimal("55.12"),
+ false,
+ 5.0, 5.1, 5.55,
+ DateUtility.parseLocalDate("2015-06-01"), DateUtility.parseLocalDateTime("2015-09-22 15:46:10.0"), DateUtility.parseLocalDate("1901-01-01"), DateUtility.parseLocalTime("16:00:01.0"),
+ "{ [ a, b, c ] }",
+ "Neque porro quisquam est qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit",
+ "abc",
+ "a medium piece of text 3",
+ "somewhat more text",
+ "this is a test 3".getBytes(),
+ true, "ZZZ")
+ .baselineValues(5,
+ null, null, null, null, null, null,
+ null, null, null,
+ null,
+ null, null, null,
+ null, null, null, null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null, "XXX")
+ .go();
+ }
+
+ @Test
+ public void mySqlPushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from mysql.`drill_mysql_test`.person) x "
+ + "join (select person_id from mysql.`drill_mysql_test`.person) y on x.person_id = y.person_id";
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join")
+ .match();
+ }
+
+ @Test
+ public void mySqlPushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from " +
+ "mysql.`drill_mysql_test`.person e " +
+ "INNER JOIN " +
+ "mysql.`drill_mysql_test`.person s " +
+ "ON e.first_name = s.first_name " +
+ "WHERE e.last_name > 'hello'";
+
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join", "Filter")
+ .match();
+ }
+
+ @Test
+ public void mySqlPhysicalPlanSubmission() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person";
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(4, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlEmptyOutput() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person e limit 0";
+ run(query);
+ }
+
+ @Test
+ public void mySqlCaseSensitiveTableNames() throws Exception {
+ String osName = System.getProperty("os.name").toLowerCase();
+ Assume.assumeTrue(
+ "Skip tests for non-linux systems due to " +
+ "table names case-insensitivity problems on Windows and MacOS",
+ osName.startsWith("linux"));
+ run("use mysqlCaseInsensitive.`drill_mysql_test`");
+ // two table names match the filter ignoring the case
+ assertEquals(2, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ run("use mysql.`drill_mysql_test`");
+ // single table matches the filter considering table name the case
+ assertEquals(1, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ // checks that tables with names in different case are recognized correctly
+ assertEquals(1, queryBuilder().sql("describe caseSensitiveTable").run().recordCount());
+ assertEquals(2, queryBuilder().sql("describe CASESENSITIVETABLE").run().recordCount());
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAlias() throws Exception {
+ String query = "select count(*), 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2\n" +
+ "from mysql.`drill_mysql_test`.person";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$0", "EXPR$1", "EXPR$2")
+ .baselineValues(4L, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAliasesPermutations() throws Exception {
+ String query = "select EXPR$1, EXPR$0, EXPR$2\n" +
+ "from (select 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2, count(*) from mysql.`drill_mysql_test`.person)";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$1", "EXPR$0", "EXPR$2")
+ .baselineValues(1.618033988749895, 88L, 4L)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithAliases() throws Exception {
+ String query = "select person_id as ID, 1+1+2+3+5+8+13+21+34 as FIBONACCI_SUM, (1+sqrt(5))/2 as golden_ratio\n" +
+ "from mysql.`drill_mysql_test`.person limit 2";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("ID", "FIBONACCI_SUM", "golden_ratio")
+ .baselineValues(1, 88L, 1.618033988749895)
+ .baselineValues(2, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6893
+ public void mySqlJoinStar() throws Exception {
+ String query = "select * from (select person_id from mysql.`drill_mysql_test`.person) t1 join " +
+ "(select person_id from mysql.`drill_mysql_test`.person) t2 on t1.person_id = t2.person_id";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id", "person_id0")
+ .baselineValues(1, 1)
+ .baselineValues(2, 2)
+ .baselineValues(3, 3)
+ .baselineValues(5, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlSemiJoin() throws Exception {
+ String query =
+ "select person_id from mysql.`drill_mysql_test`.person t1\n" +
+ "where exists (" +
+ "select person_id from mysql.`drill_mysql_test`.person\n" +
+ "where t1.person_id = person_id)";
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id")
+ .baselineValuesForSingleColumn(1, 2, 3, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlInformationSchemaViews() throws Exception {
+ String query = "select * from information_schema.`views`";
+ run(query);
+ }
+
+ @Test // DRILL-7340
+ public void mySqlH2PredicatesPushdown() throws Exception {
+ String query = "SELECT * " +
+ "FROM mysql.`drill_mysql_test`.person m " +
+ "INNER JOIN h2.tmp.drill_h2_test.person h " +
+ "ON m.person_id = h.person_id " +
+ "WHERE m.first_name = 'first_name_1' AND h.last_name = 'last_name_1'";
+ String plan = queryBuilder().sql(query).explainText();
Review comment:
Please use `.planMatcher()`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] paul-rogers commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361894195
##########
File path: contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveStructs.java
##########
@@ -476,4 +476,25 @@ public void strWithUnionField() throws Exception {
.baselineValues(2, mapOf("n", 5, "u", "Text"))
.go();
}
+
+ @Test // DRILL-7386
+ public void countStructColumn() throws Exception {
+ testBuilder()
+ .sqlQuery("SELECT COUNT(str_n0) cnt FROM hive.struct_tbl")
+ .unOrdered()
+ .baselineColumns("cnt")
+ .baselineValues(3L)
+ .go();
+ }
+
+ @Test // DRILL-7386
+ public void typeOfFunctions() throws Exception {
+ testBuilder()
+ .sqlQuery("SELECT sqlTypeOf(%1$s) sto, typeOf(%1$s) to, modeOf(%1$s) mo, drillTypeOf(%1$s) dto " +
+ "FROM hive.struct_tbl LIMIT 1", "str_n0")
+ .unOrdered()
+ .baselineColumns("sto", "to", "mo", "dto")
+ .baselineValues("STRUCT", "MAP", "NOT NULL", "MAP")
Review comment:
Should we test this for more than a single case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323935
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,638 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.StoragePluginRegistryImpl;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = String.format(
+ "jdbc:h2:%s", dirTestWatcher.getTmpDir().getCanonicalPath());
+
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root")) {
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ Assert.assertNotNull("Script for test tables generation 'h2-test-data.sql' " +
+ "cannot be found in test resources", scriptFile);
+ try (FileReader fileReader = new FileReader(scriptFile.getFile())) {
+ RunScript.execute(connection, fileReader);
+ }
+ }
+
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig(
+ "org.h2.Driver",
+ connString,
+ "root",
+ "root",
+ true);
+ jdbcStorageConfig.setEnabled(true);
+
+ String pluginName = "h2";
+ DrillbitContext context = cluster.drillbit().getContext();
Review comment:
Please create method in `ClusterFixture`, for example `definePlugin`, similar to `defineWorkspace` instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361324240
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillTableKey.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.exec.planner.logical.DrillTable;
+import org.apache.drill.metastore.metadata.TableMetadataProvider;
+import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
+
+/**
+ * Key for storing / obtaining {@link TableMetadataProvider} instance from {@link LoadingCache}.
+ */
+final class DrillTableKey {
+ private final SchemaPath key;
+ final DrillTable drillTable;
Review comment:
private?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361644173
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/CapitalizingJdbcSchema.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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcSchema;
+import org.apache.calcite.schema.Function;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class CapitalizingJdbcSchema extends AbstractSchema {
+
+ private static final Logger logger = LoggerFactory.getLogger(CapitalizingJdbcSchema.class);
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final JdbcSchema inner;
+ private final boolean caseSensitive;
+
+ CapitalizingJdbcSchema(List<String> parentSchemaPath, String name, DataSource dataSource,
+ SqlDialect dialect, JdbcConvention convention, String catalog, String schema, boolean caseSensitive) {
+ super(parentSchemaPath, name);
+ this.schemaMap = new HashMap<>();
+ this.inner = new JdbcSchema(dataSource, dialect, convention, catalog, schema);
+ this.caseSensitive = caseSensitive;
+ }
+
+ @Override
+ public String getTypeName() {
+ return JdbcStorageConfig.NAME;
+ }
+
+ @Override
+ public Collection<Function> getFunctions(String name) {
+ return inner.getFunctions(name);
+ }
+
+ @Override
+ public Set<String> getFunctionNames() {
+ return inner.getFunctionNames();
+ }
+
+ @Override
+ public CapitalizingJdbcSchema getSubSchema(String name) {
+ return schemaMap.get(name);
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return schemaMap.keySet();
+ }
+
+ @Override
+ public Set<String> getTableNames() {
+ if (isCatalogSchema()) {
+ return Collections.emptySet();
+ }
+ return inner.getTableNames();
+ }
+
+ @Override
+ public Table getTable(String name) {
+ if (isCatalogSchema()) {
+ logger.warn("Failed attempt to find table '{}' in catalog schema '{}'", name, getName());
+ return null;
+ }
+ Table table = inner.getTable(name);
+ if (table == null
+ && !areTableNamesCaseSensitive()
+ && (table = inner.getTable(name.toUpperCase())) == null // Oracle and H2 changes unquoted identifiers to uppercase.
Review comment:
Or it may be rewritten to something like this:
```suggestion
Table table = inner.getTable(name);
if (table == null
&& !areTableNamesCaseSensitive()) {
// Oracle and H2 changes unquoted identifiers to uppercase.
table = inner.getTable(name.toUpperCase());
if (table == null) {
// Postgres changes unquoted identifiers to lowercase.
return inner.getTable(name.toLowerCase());
}
}
return table;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323570
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return (CapitalizingJdbcSchema) connSchema.getDefaultSchema();
Review comment:
Is casting without instance of check safe here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361660190
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,601 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = "jdbc:h2:" + dirTestWatcher.getTmpDir().getCanonicalPath();
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ assertNotNull("Script for test tables generation 'h2-test-data.sql' cannot be found in test resources", scriptFile);
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root");
+ FileReader fileReader = new FileReader(scriptFile.getFile())) {
+ RunScript.execute(connection, fileReader);
+ }
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("org.h2.Driver", connString, "root", "root", true);
+ jdbcStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "h2"));
+ }
+
+ private static void initMySQL() throws Exception {
+ String mysqlDBName = "drill_mysql_test";
+ int mysqlPort = QueryTestUtil.getFreePortNumber(2215, 300);
+
+ MysqldConfig config = MysqldConfig.aMysqldConfig(Version.v5_6_21)
+ .withPort(mysqlPort)
+ .withUser("mysqlUser", "mysqlPass")
+ .withTimeZone(DateTimeZone.UTC.toTimeZone())
+ .build();
+
+ SchemaConfig.Builder schemaConfig = SchemaConfig.aSchemaConfig(mysqlDBName)
+ .withScripts(ScriptResolver.classPathScript("mysql-test-data.sql"));
+
+ String osName = System.getProperty("os.name").toLowerCase();
+ if (osName.startsWith("linux")) {
+ schemaConfig.withScripts(ScriptResolver.classPathScript("mysql-test-data-linux.sql"));
+ }
+
+ mysqld = EmbeddedMysql.anEmbeddedMysql(config).addSchema(schemaConfig.build()).start();
+
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", false);
+ jdbcStorageConfig.setEnabled(true);
+
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "mysql"));
+
+ if (osName.startsWith("linux")) {
+ // adds storage plugin with case insensitive table names
+ JdbcStorageConfig jdbcCaseSensitiveStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", true);
+ jdbcCaseSensitiveStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcCaseSensitiveStorageConfig, ctx, "mysqlCaseInsensitive"));
+ }
+ }
+
+ @Test
+ public void h2CrossSourceMultiFragmentJoin() throws Exception {
+ try {
+ client.alterSession(ExecConstants.SLICE_TARGET, 1);
+ run("select x.person_id, y.salary from h2.tmp.drill_h2_test.person x "
+ + "join %s y on x.person_id = y.person_id ", TABLE_NAME);
+ } finally {
+ client.resetSession(ExecConstants.SLICE_TARGET);
+ }
+ }
+
+ @Test
+ public void h2ValidateResult() throws Exception {
+ // Skip date, time, and timestamp types since h2 mangles these due to improper timezone support.
+ testBuilder()
+ .sqlQuery(
+ "select person_id, first_name, last_name, address, city, state, zip, json, bigint_field, smallint_field, " +
+ "numeric_field, boolean_field, double_field, float_field, real_field, time_field, timestamp_field, " +
+ "date_field, clob_field from h2.tmp.`drill_h2_test`.person")
+ .ordered()
+ .baselineColumns("person_id", "first_name", "last_name", "address", "city", "state", "zip", "json",
+ "bigint_field", "smallint_field", "numeric_field", "boolean_field", "double_field", "float_field",
+ "real_field", "time_field", "timestamp_field", "date_field", "clob_field")
+ .baselineValues(1, "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ "{ a : 5, b : 6 }", 123456L, 1, new BigDecimal("10.01"), false, 1.0, 1.1, 111.00,
+ DateUtility.parseLocalTime("13:00:01.0"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"),
+ DateUtility.parseLocalDate("2012-02-29"), "some clob data 1")
+ .baselineValues(2, "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ "{ foo : \"abc\" }", 95949L, 2, new BigDecimal("20.02"), true, 2.0, 2.1, 222.00,
+ DateUtility.parseLocalTime("23:59:59.0"), DateUtility.parseLocalDateTime("1999-09-09 23:59:59.0"),
+ DateUtility.parseLocalDate("1999-09-09"), "some more clob data")
+ .baselineValues(3, "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012, "{ z : [ 1, 2, 3 ] }",
+ 45456L, 3, new BigDecimal("30.04"), true, 3.0, 3.1, 333.00, DateUtility.parseLocalTime("11:34:21.0"),
+ DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2011-10-30"), "clobber")
+ .baselineValues(4, null, null, "2 15th St NW", "Washington", "DC", 20007, "{ z : { a : 1, b : 2, c : 3 } }",
+ -67L, 4, new BigDecimal("40.04"), false, 4.0, 4.1, 444.00, DateUtility.parseLocalTime("16:00:01.0"),
+ DateUtility.parseLocalDateTime("2015-06-01 16:00:01.0"), DateUtility.parseLocalDate("2015-06-01"), "xxx")
+ .baselineValues(5, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null)
+ .go();
+ }
+
+ @Test
+ public void h2PushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from h2.tmp.drill_h2_test.person) x "
+ + "join (select person_id from h2.tmp.drill_h2_test.person) y on x.person_id = y.person_id ";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ }
+
+ @Test
+ public void h2PushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.FIRST_NAME = s.FIRST_NAME\n" +
+ "WHERE e.LAST_NAME > 'hello'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2PushdownAggregation() throws Exception {
+ String query = "select count(*) from h2.tmp.drill_h2_test.person";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Aggregate operator",
+ plan, not(containsString("Aggregate")));
+ }
+
+ @Test
+ public void h2PushdownDoubleJoinAndFilter() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.person_ID = s.person_ID\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person ed\n" +
+ "ON e.person_ID = ed.person_ID\n" +
+ "WHERE s.first_name > 'abc' and ed.first_name > 'efg'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2ShowTablesDefaultSchema() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(1, queryBuilder().sql("show tables like 'PERSON'").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(1, queryBuilder().sql("show tables like 'person'").run().recordCount());
+ }
+
+ @Test
+ public void h2Describe() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(19, queryBuilder().sql("describe PERSON").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(19, queryBuilder().sql("describe person").run().recordCount());
+ }
+
+ @Test
+ public void h2EnsureDrillFunctionsAreNotPushedDown() throws Exception {
+ // This should verify that we're not trying to push CONVERT_FROM into the JDBC storage plugin. If were pushing
+ // this function down, the SQL query would fail.
+ run("select CONVERT_FROM(JSON, 'JSON') from h2.tmp.drill_h2_test.person where person_ID = 4");
+ }
+
+ @Test
+ public void h2PushdownFilter() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.person where person_ID = 1";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2CaseInsensitiveTableNames() throws Exception {
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PeRsOn").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PERSON").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.person").run().recordCount());
+ }
+
+ @Test
+ public void h2JdbcStoragePluginSerDe() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.PeRsOn";
+
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(5, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlValidateResult() throws Exception {
+
+ testBuilder()
+ .sqlQuery(
+ "select person_id, " +
+ "first_name, last_name, address, city, state, zip, " +
+ "bigint_field, smallint_field, numeric_field, " +
+ "boolean_field, double_field, float_field, real_field, " +
+ "date_field, datetime_field, year_field, time_field, " +
+ "json, text_field, tiny_text_field, medium_text_field, long_text_field, " +
+ "blob_field, bit_field, enum_field " +
+ "from mysql.`drill_mysql_test`.person")
+ .ordered()
+ .baselineColumns("person_id",
+ "first_name", "last_name", "address", "city", "state", "zip",
+ "bigint_field", "smallint_field", "numeric_field",
+ "boolean_field",
+ "double_field", "float_field", "real_field",
+ "date_field", "datetime_field", "year_field", "time_field",
+ "json", "text_field", "tiny_text_field", "medium_text_field", "long_text_field",
+ "blob_field", "bit_field", "enum_field")
+ .baselineValues(1,
+ "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ 123456789L, 1, new BigDecimal("10.01"),
+ false,
+ 1.0, 1.1, 1.2,
+ DateUtility.parseLocalDate("2012-02-29"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("13:00:01.0"),
+ "{ a : 5, b : 6 }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "xxx",
+ "a medium piece of text",
+ "a longer piece of text this is going on.....",
+ "this is a test".getBytes(),
+ true, "XXX")
+ .baselineValues(2,
+ "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ 45456767L, 3, new BigDecimal("30.04"),
+ true,
+ 3.0, 3.1, 3.2,
+ DateUtility.parseLocalDate("2011-10-30"), DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("11:34:21.0"),
+ "{ z : [ 1, 2, 3 ] }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "abc",
+ "a medium piece of text 2",
+ "somewhat more text",
+ "this is a test 2".getBytes(),
+ false, "YYY")
+ .baselineValues(3,
+ "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012,
+ 123090L, -3, new BigDecimal("55.12"),
+ false,
+ 5.0, 5.1, 5.55,
+ DateUtility.parseLocalDate("2015-06-01"), DateUtility.parseLocalDateTime("2015-09-22 15:46:10.0"), DateUtility.parseLocalDate("1901-01-01"), DateUtility.parseLocalTime("16:00:01.0"),
+ "{ [ a, b, c ] }",
+ "Neque porro quisquam est qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit",
+ "abc",
+ "a medium piece of text 3",
+ "somewhat more text",
+ "this is a test 3".getBytes(),
+ true, "ZZZ")
+ .baselineValues(5,
+ null, null, null, null, null, null,
+ null, null, null,
+ null,
+ null, null, null,
+ null, null, null, null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null, "XXX")
+ .go();
+ }
+
+ @Test
+ public void mySqlPushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from mysql.`drill_mysql_test`.person) x "
+ + "join (select person_id from mysql.`drill_mysql_test`.person) y on x.person_id = y.person_id";
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join")
+ .match();
+ }
+
+ @Test
+ public void mySqlPushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from " +
+ "mysql.`drill_mysql_test`.person e " +
+ "INNER JOIN " +
+ "mysql.`drill_mysql_test`.person s " +
+ "ON e.first_name = s.first_name " +
+ "WHERE e.last_name > 'hello'";
+
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join", "Filter")
+ .match();
+ }
+
+ @Test
+ public void mySqlPhysicalPlanSubmission() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person";
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(4, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlEmptyOutput() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person e limit 0";
+ run(query);
+ }
+
+ @Test
+ public void mySqlCaseSensitiveTableNames() throws Exception {
+ String osName = System.getProperty("os.name").toLowerCase();
+ Assume.assumeTrue(
+ "Skip tests for non-linux systems due to " +
+ "table names case-insensitivity problems on Windows and MacOS",
+ osName.startsWith("linux"));
+ run("use mysqlCaseInsensitive.`drill_mysql_test`");
+ // two table names match the filter ignoring the case
+ assertEquals(2, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ run("use mysql.`drill_mysql_test`");
+ // single table matches the filter considering table name the case
+ assertEquals(1, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ // checks that tables with names in different case are recognized correctly
+ assertEquals(1, queryBuilder().sql("describe caseSensitiveTable").run().recordCount());
+ assertEquals(2, queryBuilder().sql("describe CASESENSITIVETABLE").run().recordCount());
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAlias() throws Exception {
+ String query = "select count(*), 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2\n" +
+ "from mysql.`drill_mysql_test`.person";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$0", "EXPR$1", "EXPR$2")
+ .baselineValues(4L, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAliasesPermutations() throws Exception {
+ String query = "select EXPR$1, EXPR$0, EXPR$2\n" +
+ "from (select 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2, count(*) from mysql.`drill_mysql_test`.person)";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$1", "EXPR$0", "EXPR$2")
+ .baselineValues(1.618033988749895, 88L, 4L)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithAliases() throws Exception {
+ String query = "select person_id as ID, 1+1+2+3+5+8+13+21+34 as FIBONACCI_SUM, (1+sqrt(5))/2 as golden_ratio\n" +
+ "from mysql.`drill_mysql_test`.person limit 2";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("ID", "FIBONACCI_SUM", "golden_ratio")
+ .baselineValues(1, 88L, 1.618033988749895)
+ .baselineValues(2, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6893
+ public void mySqlJoinStar() throws Exception {
+ String query = "select * from (select person_id from mysql.`drill_mysql_test`.person) t1 join " +
+ "(select person_id from mysql.`drill_mysql_test`.person) t2 on t1.person_id = t2.person_id";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id", "person_id0")
+ .baselineValues(1, 1)
+ .baselineValues(2, 2)
+ .baselineValues(3, 3)
+ .baselineValues(5, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlSemiJoin() throws Exception {
+ String query =
+ "select person_id from mysql.`drill_mysql_test`.person t1\n" +
+ "where exists (" +
+ "select person_id from mysql.`drill_mysql_test`.person\n" +
+ "where t1.person_id = person_id)";
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id")
+ .baselineValuesForSingleColumn(1, 2, 3, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlInformationSchemaViews() throws Exception {
+ String query = "select * from information_schema.`views`";
+ run(query);
+ }
+
+ @Test // DRILL-7340
+ public void mySqlH2PredicatesPushdown() throws Exception {
+ String query = "SELECT * " +
Review comment:
Looks like this issue may be reproduced even with the same database but with different storage plugins that point to that database, so there is no need to merge these two test classes. If it is possible, please split them back and rewrite this test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] paul-rogers commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361893444
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillValidator.java
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.validate.SelectScope;
+import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
+import org.apache.calcite.sql.validate.SqlValidatorImpl;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Static;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.store.ColumnExplorer;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+
+class DrillValidator extends SqlValidatorImpl {
+
+ private final UserSession session;
+
+ DrillValidator(SqlOperatorTable opTab, SqlValidatorCatalogReader catalogReader,
Review comment:
Better is to pass `OptionManager` which is a read-only interface to options.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361655859
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,601 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = "jdbc:h2:" + dirTestWatcher.getTmpDir().getCanonicalPath();
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ assertNotNull("Script for test tables generation 'h2-test-data.sql' cannot be found in test resources", scriptFile);
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root");
+ FileReader fileReader = new FileReader(scriptFile.getFile())) {
+ RunScript.execute(connection, fileReader);
+ }
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("org.h2.Driver", connString, "root", "root", true);
+ jdbcStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "h2"));
+ }
+
+ private static void initMySQL() throws Exception {
+ String mysqlDBName = "drill_mysql_test";
+ int mysqlPort = QueryTestUtil.getFreePortNumber(2215, 300);
+
+ MysqldConfig config = MysqldConfig.aMysqldConfig(Version.v5_6_21)
+ .withPort(mysqlPort)
+ .withUser("mysqlUser", "mysqlPass")
+ .withTimeZone(DateTimeZone.UTC.toTimeZone())
+ .build();
+
+ SchemaConfig.Builder schemaConfig = SchemaConfig.aSchemaConfig(mysqlDBName)
+ .withScripts(ScriptResolver.classPathScript("mysql-test-data.sql"));
+
+ String osName = System.getProperty("os.name").toLowerCase();
+ if (osName.startsWith("linux")) {
+ schemaConfig.withScripts(ScriptResolver.classPathScript("mysql-test-data-linux.sql"));
+ }
+
+ mysqld = EmbeddedMysql.anEmbeddedMysql(config).addSchema(schemaConfig.build()).start();
+
+ JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", false);
+ jdbcStorageConfig.setEnabled(true);
+
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcStorageConfig, ctx, "mysql"));
+
+ if (osName.startsWith("linux")) {
+ // adds storage plugin with case insensitive table names
+ JdbcStorageConfig jdbcCaseSensitiveStorageConfig = new JdbcStorageConfig("com.mysql.cj.jdbc.Driver",
+ String.format("jdbc:mysql://localhost:%s/%s?useJDBCCompliantTimezoneShift=true", mysqlPort, mysqlDBName),
+ "mysqlUser", "mysqlPass", true);
+ jdbcCaseSensitiveStorageConfig.setEnabled(true);
+ cluster.defineStoragePlugin(ctx -> new JdbcStoragePlugin(jdbcCaseSensitiveStorageConfig, ctx, "mysqlCaseInsensitive"));
+ }
+ }
+
+ @Test
+ public void h2CrossSourceMultiFragmentJoin() throws Exception {
+ try {
+ client.alterSession(ExecConstants.SLICE_TARGET, 1);
+ run("select x.person_id, y.salary from h2.tmp.drill_h2_test.person x "
+ + "join %s y on x.person_id = y.person_id ", TABLE_NAME);
+ } finally {
+ client.resetSession(ExecConstants.SLICE_TARGET);
+ }
+ }
+
+ @Test
+ public void h2ValidateResult() throws Exception {
+ // Skip date, time, and timestamp types since h2 mangles these due to improper timezone support.
+ testBuilder()
+ .sqlQuery(
+ "select person_id, first_name, last_name, address, city, state, zip, json, bigint_field, smallint_field, " +
+ "numeric_field, boolean_field, double_field, float_field, real_field, time_field, timestamp_field, " +
+ "date_field, clob_field from h2.tmp.`drill_h2_test`.person")
+ .ordered()
+ .baselineColumns("person_id", "first_name", "last_name", "address", "city", "state", "zip", "json",
+ "bigint_field", "smallint_field", "numeric_field", "boolean_field", "double_field", "float_field",
+ "real_field", "time_field", "timestamp_field", "date_field", "clob_field")
+ .baselineValues(1, "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ "{ a : 5, b : 6 }", 123456L, 1, new BigDecimal("10.01"), false, 1.0, 1.1, 111.00,
+ DateUtility.parseLocalTime("13:00:01.0"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"),
+ DateUtility.parseLocalDate("2012-02-29"), "some clob data 1")
+ .baselineValues(2, "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ "{ foo : \"abc\" }", 95949L, 2, new BigDecimal("20.02"), true, 2.0, 2.1, 222.00,
+ DateUtility.parseLocalTime("23:59:59.0"), DateUtility.parseLocalDateTime("1999-09-09 23:59:59.0"),
+ DateUtility.parseLocalDate("1999-09-09"), "some more clob data")
+ .baselineValues(3, "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012, "{ z : [ 1, 2, 3 ] }",
+ 45456L, 3, new BigDecimal("30.04"), true, 3.0, 3.1, 333.00, DateUtility.parseLocalTime("11:34:21.0"),
+ DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2011-10-30"), "clobber")
+ .baselineValues(4, null, null, "2 15th St NW", "Washington", "DC", 20007, "{ z : { a : 1, b : 2, c : 3 } }",
+ -67L, 4, new BigDecimal("40.04"), false, 4.0, 4.1, 444.00, DateUtility.parseLocalTime("16:00:01.0"),
+ DateUtility.parseLocalDateTime("2015-06-01 16:00:01.0"), DateUtility.parseLocalDate("2015-06-01"), "xxx")
+ .baselineValues(5, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null)
+ .go();
+ }
+
+ @Test
+ public void h2PushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from h2.tmp.drill_h2_test.person) x "
+ + "join (select person_id from h2.tmp.drill_h2_test.person) y on x.person_id = y.person_id ";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ }
+
+ @Test
+ public void h2PushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.FIRST_NAME = s.FIRST_NAME\n" +
+ "WHERE e.LAST_NAME > 'hello'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2PushdownAggregation() throws Exception {
+ String query = "select count(*) from h2.tmp.drill_h2_test.person";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Aggregate operator",
+ plan, not(containsString("Aggregate")));
+ }
+
+ @Test
+ public void h2PushdownDoubleJoinAndFilter() throws Exception {
+ String query = "select * from \n" +
+ "h2.tmp.drill_h2_test.person e\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person s\n" +
+ "ON e.person_ID = s.person_ID\n" +
+ "INNER JOIN \n" +
+ "h2.tmp.drill_h2_test.person ed\n" +
+ "ON e.person_ID = ed.person_ID\n" +
+ "WHERE s.first_name > 'abc' and ed.first_name > 'efg'";
+
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Join operator",
+ plan, not(containsString("Join")));
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2ShowTablesDefaultSchema() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(1, queryBuilder().sql("show tables like 'PERSON'").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(1, queryBuilder().sql("show tables like 'person'").run().recordCount());
+ }
+
+ @Test
+ public void h2Describe() throws Exception {
+ run("use h2.tmp.drill_h2_test");
+ assertEquals(19, queryBuilder().sql("describe PERSON").run().recordCount());
+
+ // check table names case insensitivity
+ assertEquals(19, queryBuilder().sql("describe person").run().recordCount());
+ }
+
+ @Test
+ public void h2EnsureDrillFunctionsAreNotPushedDown() throws Exception {
+ // This should verify that we're not trying to push CONVERT_FROM into the JDBC storage plugin. If were pushing
+ // this function down, the SQL query would fail.
+ run("select CONVERT_FROM(JSON, 'JSON') from h2.tmp.drill_h2_test.person where person_ID = 4");
+ }
+
+ @Test
+ public void h2PushdownFilter() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.person where person_ID = 1";
+ String plan = queryBuilder().sql(query).explainText();
+
+ assertThat("Query plan shouldn't contain Filter operator",
+ plan, not(containsString("Filter")));
+ }
+
+ @Test
+ public void h2CaseInsensitiveTableNames() throws Exception {
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PeRsOn").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.PERSON").run().recordCount());
+ assertEquals(5, queryBuilder().sql("select * from h2.tmp.drill_h2_test.person").run().recordCount());
+ }
+
+ @Test
+ public void h2JdbcStoragePluginSerDe() throws Exception {
+ String query = "select * from h2.tmp.drill_h2_test.PeRsOn";
+
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(5, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlValidateResult() throws Exception {
+
+ testBuilder()
+ .sqlQuery(
+ "select person_id, " +
+ "first_name, last_name, address, city, state, zip, " +
+ "bigint_field, smallint_field, numeric_field, " +
+ "boolean_field, double_field, float_field, real_field, " +
+ "date_field, datetime_field, year_field, time_field, " +
+ "json, text_field, tiny_text_field, medium_text_field, long_text_field, " +
+ "blob_field, bit_field, enum_field " +
+ "from mysql.`drill_mysql_test`.person")
+ .ordered()
+ .baselineColumns("person_id",
+ "first_name", "last_name", "address", "city", "state", "zip",
+ "bigint_field", "smallint_field", "numeric_field",
+ "boolean_field",
+ "double_field", "float_field", "real_field",
+ "date_field", "datetime_field", "year_field", "time_field",
+ "json", "text_field", "tiny_text_field", "medium_text_field", "long_text_field",
+ "blob_field", "bit_field", "enum_field")
+ .baselineValues(1,
+ "first_name_1", "last_name_1", "1401 John F Kennedy Blvd", "Philadelphia", "PA", 19107,
+ 123456789L, 1, new BigDecimal("10.01"),
+ false,
+ 1.0, 1.1, 1.2,
+ DateUtility.parseLocalDate("2012-02-29"), DateUtility.parseLocalDateTime("2012-02-29 13:00:01.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("13:00:01.0"),
+ "{ a : 5, b : 6 }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "xxx",
+ "a medium piece of text",
+ "a longer piece of text this is going on.....",
+ "this is a test".getBytes(),
+ true, "XXX")
+ .baselineValues(2,
+ "first_name_2", "last_name_2", "One Ferry Building", "San Francisco", "CA", 94111,
+ 45456767L, 3, new BigDecimal("30.04"),
+ true,
+ 3.0, 3.1, 3.2,
+ DateUtility.parseLocalDate("2011-10-30"), DateUtility.parseLocalDateTime("2011-10-30 11:34:21.0"), DateUtility.parseLocalDate("2015-01-01"), DateUtility.parseLocalTime("11:34:21.0"),
+ "{ z : [ 1, 2, 3 ] }",
+ "It is a long established fact that a reader will be distracted by the readable content of a page when looking at its layout",
+ "abc",
+ "a medium piece of text 2",
+ "somewhat more text",
+ "this is a test 2".getBytes(),
+ false, "YYY")
+ .baselineValues(3,
+ "first_name_3", "last_name_3", "176 Bowery", "New York", "NY", 10012,
+ 123090L, -3, new BigDecimal("55.12"),
+ false,
+ 5.0, 5.1, 5.55,
+ DateUtility.parseLocalDate("2015-06-01"), DateUtility.parseLocalDateTime("2015-09-22 15:46:10.0"), DateUtility.parseLocalDate("1901-01-01"), DateUtility.parseLocalTime("16:00:01.0"),
+ "{ [ a, b, c ] }",
+ "Neque porro quisquam est qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit",
+ "abc",
+ "a medium piece of text 3",
+ "somewhat more text",
+ "this is a test 3".getBytes(),
+ true, "ZZZ")
+ .baselineValues(5,
+ null, null, null, null, null, null,
+ null, null, null,
+ null,
+ null, null, null,
+ null, null, null, null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null, "XXX")
+ .go();
+ }
+
+ @Test
+ public void mySqlPushdownJoin() throws Exception {
+ String query = "select x.person_id from (select person_id from mysql.`drill_mysql_test`.person) x "
+ + "join (select person_id from mysql.`drill_mysql_test`.person) y on x.person_id = y.person_id";
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join")
+ .match();
+ }
+
+ @Test
+ public void mySqlPushdownJoinAndFilterPushDown() throws Exception {
+ String query = "select * from " +
+ "mysql.`drill_mysql_test`.person e " +
+ "INNER JOIN " +
+ "mysql.`drill_mysql_test`.person s " +
+ "ON e.first_name = s.first_name " +
+ "WHERE e.last_name > 'hello'";
+
+ queryBuilder()
+ .sql(query)
+ .planMatcher()
+ .exclude("Join", "Filter")
+ .match();
+ }
+
+ @Test
+ public void mySqlPhysicalPlanSubmission() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person";
+ String plan = queryBuilder().sql(query).explainJson();
+ assertEquals(4, queryBuilder().physical(plan).run().recordCount());
+ }
+
+ @Test
+ public void mySqlEmptyOutput() throws Exception {
+ String query = "select * from mysql.`drill_mysql_test`.person e limit 0";
+ run(query);
+ }
+
+ @Test
+ public void mySqlCaseSensitiveTableNames() throws Exception {
+ String osName = System.getProperty("os.name").toLowerCase();
+ Assume.assumeTrue(
+ "Skip tests for non-linux systems due to " +
+ "table names case-insensitivity problems on Windows and MacOS",
+ osName.startsWith("linux"));
+ run("use mysqlCaseInsensitive.`drill_mysql_test`");
+ // two table names match the filter ignoring the case
+ assertEquals(2, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ run("use mysql.`drill_mysql_test`");
+ // single table matches the filter considering table name the case
+ assertEquals(1, queryBuilder().sql("show tables like 'caseSensitiveTable'").run().recordCount());
+
+ // checks that tables with names in different case are recognized correctly
+ assertEquals(1, queryBuilder().sql("describe caseSensitiveTable").run().recordCount());
+ assertEquals(2, queryBuilder().sql("describe CASESENSITIVETABLE").run().recordCount());
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAlias() throws Exception {
+ String query = "select count(*), 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2\n" +
+ "from mysql.`drill_mysql_test`.person";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$0", "EXPR$1", "EXPR$2")
+ .baselineValues(4L, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithoutAliasesPermutations() throws Exception {
+ String query = "select EXPR$1, EXPR$0, EXPR$2\n" +
+ "from (select 1+1+2+3+5+8+13+21+34, (1+sqrt(5))/2, count(*) from mysql.`drill_mysql_test`.person)";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("EXPR$1", "EXPR$0", "EXPR$2")
+ .baselineValues(1.618033988749895, 88L, 4L)
+ .go();
+ }
+
+ @Test // DRILL-6734
+ public void mySqlExpressionsWithAliases() throws Exception {
+ String query = "select person_id as ID, 1+1+2+3+5+8+13+21+34 as FIBONACCI_SUM, (1+sqrt(5))/2 as golden_ratio\n" +
+ "from mysql.`drill_mysql_test`.person limit 2";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("ID", "FIBONACCI_SUM", "golden_ratio")
+ .baselineValues(1, 88L, 1.618033988749895)
+ .baselineValues(2, 88L, 1.618033988749895)
+ .go();
+ }
+
+ @Test // DRILL-6893
+ public void mySqlJoinStar() throws Exception {
+ String query = "select * from (select person_id from mysql.`drill_mysql_test`.person) t1 join " +
+ "(select person_id from mysql.`drill_mysql_test`.person) t2 on t1.person_id = t2.person_id";
+
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id", "person_id0")
+ .baselineValues(1, 1)
+ .baselineValues(2, 2)
+ .baselineValues(3, 3)
+ .baselineValues(5, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlSemiJoin() throws Exception {
+ String query =
+ "select person_id from mysql.`drill_mysql_test`.person t1\n" +
+ "where exists (" +
+ "select person_id from mysql.`drill_mysql_test`.person\n" +
+ "where t1.person_id = person_id)";
+ testBuilder()
+ .sqlQuery(query)
+ .unOrdered()
+ .baselineColumns("person_id")
+ .baselineValuesForSingleColumn(1, 2, 3, 5)
+ .go();
+ }
+
+ @Test
+ public void mySqlInformationSchemaViews() throws Exception {
+ String query = "select * from information_schema.`views`";
+ run(query);
+ }
+
+ @Test // DRILL-7340
+ public void mySqlH2PredicatesPushdown() throws Exception {
+ String query = "SELECT * " +
+ "FROM mysql.`drill_mysql_test`.person m " +
+ "INNER JOIN h2.tmp.drill_h2_test.person h " +
+ "ON m.person_id = h.person_id " +
+ "WHERE m.first_name = 'first_name_1' AND h.last_name = 'last_name_1'";
+ String plan = queryBuilder().sql(query).explainText();
+ assertThat("Query plan shouldn't contain Filter operator", plan, not(containsString("Filter")));
+ assertThat("H2 filter should be inside Jdbc select", plan,
+ containsString("Jdbc(sql=[SELECT * FROM \"TMP\".\"DRILL_H2_TEST\".\"PERSON\" WHERE \"LAST_NAME\" = 'last_name_1' ])"));
+ assertThat("MySQL filter should be inside Jdbc select", plan,
+ containsString("Jdbc(sql=[SELECT * FROM `drill_mysql_test`.`person` WHERE `first_name` = 'first_name_1' ])"));
+ }
+
+ @Test
+ public void h2ShowTables() throws Exception {
+ run("USE h2");
+ String sql = "SHOW TABLES";
+ testBuilder()
+ .sqlQuery(sql)
+ .unOrdered()
+ .baselineColumns("TABLE_SCHEMA", "TABLE_NAME")
+ .baselineValues("h2.tmp.drill_h2_test_1", "PERSON")
+ .go();
+ }
+
+ @Test
+ public void debugTest() throws Exception {
Review comment:
Please remove this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362056288
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
Review comment:
I agree that static missed here but I would like to leave naming as is. The name is short and the return type is clear.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323795
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcWithMySQLandH2PluginsIT.java
##########
@@ -0,0 +1,638 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import com.wix.mysql.EmbeddedMysql;
+import com.wix.mysql.ScriptResolver;
+import com.wix.mysql.config.MysqldConfig;
+import com.wix.mysql.config.SchemaConfig;
+import com.wix.mysql.distribution.Version;
+import org.apache.drill.categories.JdbcStorageTest;
+import org.apache.drill.common.config.DrillProperties;
+import org.apache.drill.exec.ExecConstants;
+import org.apache.drill.exec.expr.fn.impl.DateUtility;
+
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.StoragePluginRegistryImpl;
+import org.apache.drill.exec.util.StoragePluginTestUtils;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.QueryTestUtil;
+import org.h2.tools.RunScript;
+import org.joda.time.DateTimeZone;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.FileReader;
+import java.math.BigDecimal;
+import java.net.URL;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+/**
+ * JDBC storage plugin tests against H2 and MySQL data sources.
+ */
+@Category(JdbcStorageTest.class)
+public class TestJdbcWithMySQLandH2PluginsIT extends ClusterTest {
+
+ private static final String TABLE_PATH = "jdbcmulti/";
+ private static final String TABLE_NAME = String.format("%s.`%s`", StoragePluginTestUtils.DFS_PLUGIN_NAME, TABLE_PATH);
+
+ private static EmbeddedMysql mysqld;
+
+ @BeforeClass
+ public static void init() throws Exception {
+ startCluster(ClusterFixture.builder(dirTestWatcher));
+ dirTestWatcher.copyResourceToRoot(Paths.get(TABLE_PATH));
+ initH2();
+ initMySQL();
+ }
+
+ @AfterClass
+ public static void stopMysql() {
+ if (mysqld != null) {
+ mysqld.stop();
+ }
+ }
+
+ @After
+ public void resetSchema() {
+ client.resetSession(DrillProperties.SCHEMA);
+ }
+
+ private static void initH2() throws Exception {
+ Class.forName("org.h2.Driver");
+ String connString = String.format(
+ "jdbc:h2:%s", dirTestWatcher.getTmpDir().getCanonicalPath());
+
+ try (Connection connection = DriverManager.getConnection(connString, "root", "root")) {
+ URL scriptFile = TestJdbcWithMySQLandH2PluginsIT.class.getClassLoader().getResource("h2-test-data.sql");
+ Assert.assertNotNull("Script for test tables generation 'h2-test-data.sql' " +
Review comment:
Mostly in Drill static import is used in tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361473590
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return (CapitalizingJdbcSchema) connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
+ CapitalizingJdbcSchema parentSchema = schemaMap.get(parentKey);
+ if (parentSchema == null) {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(getSchemaPath(), schemaName, source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+
+ // if a catalog schema doesn't exist, we'll add this at the top level.
+ schemaMap.put(schema.getName(), schema);
+ } else {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(parentSchema.getSchemaPath(), schemaName,
+ source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+ parentSchema.schemaMap.put(schema.getName(), schema);
+ }
+ added = true;
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ return added;
+ }
+
+
+ @Override
+ public String getTypeName() {
+ return JdbcStorageConfig.NAME;
+ }
+
+ @Override
+ public Schema getDefaultSchema() {
+ return defaultSchema;
+ }
+
+ @Override
+ public CapitalizingJdbcSchema getSubSchema(String name) {
+ return schemaMap.get(name);
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return schemaMap.keySet();
+ }
+
+ @Override
+ public Table getTable(String name) {
+ if (defaultSchema != null) {
+ try {
+ return defaultSchema.getTable(name);
+ } catch (RuntimeException e) {
Review comment:
Migrated code from ```JdbcStoragePlugin```. Since the method from ```try``` doesn't declare checked exceptions the ```catch``` is just enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323751
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java
##########
@@ -465,8 +93,7 @@ public SqlDialect getDialect() {
}
@Override
- public AbstractGroupScan getPhysicalScan(String userName, JSONOptions selection, List<SchemaPath> columns)
- throws IOException {
+ public AbstractGroupScan getPhysicalScan(String userName, JSONOptions selection, List<SchemaPath> columns) {
throw new UnsupportedOperationException();
Review comment:
Please add message to the exception.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323645
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return (CapitalizingJdbcSchema) connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
+ CapitalizingJdbcSchema parentSchema = schemaMap.get(parentKey);
+ if (parentSchema == null) {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(getSchemaPath(), schemaName, source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+
+ // if a catalog schema doesn't exist, we'll add this at the top level.
+ schemaMap.put(schema.getName(), schema);
+ } else {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(parentSchema.getSchemaPath(), schemaName,
+ source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+ parentSchema.schemaMap.put(schema.getName(), schema);
+ }
+ added = true;
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ return added;
+ }
+
+
+ @Override
+ public String getTypeName() {
+ return JdbcStorageConfig.NAME;
+ }
+
+ @Override
+ public Schema getDefaultSchema() {
+ return defaultSchema;
+ }
+
+ @Override
+ public CapitalizingJdbcSchema getSubSchema(String name) {
+ return schemaMap.get(name);
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return schemaMap.keySet();
+ }
+
+ @Override
+ public Table getTable(String name) {
+ if (defaultSchema != null) {
+ try {
+ return defaultSchema.getTable(name);
+ } catch (RuntimeException e) {
Review comment:
Why just runtime exception? I assume you want to ignore all exceptions...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] paul-rogers commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361893329
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
##########
@@ -0,0 +1,213 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+import java.util.function.BooleanSupplier;
+import java.util.function.Supplier;
+
+import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.jdbc.CalciteSchema;
+import org.apache.calcite.jdbc.DynamicSchema;
+import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.prepare.Prepare;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Util;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.metastore.MetadataProviderManager;
+import org.apache.drill.exec.planner.logical.DrillTable;
+import org.apache.drill.exec.planner.sql.SchemaUtilites;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.store.dfs.FileSelection;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheBuilder;
+import org.apache.drill.shaded.guava.com.google.common.cache.CacheLoader;
+import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+/**
+ * Extension of {@link CalciteCatalogReader} to add ability to check for temporary tables first
+ * if schema is not indicated near table name during query parsing
+ * or indicated workspace is default temporary workspace.
+ */
+class DrillCalciteCatalogReader extends CalciteCatalogReader {
+
+ private final DrillConfig drillConfig;
+ private final UserSession session;
+ private final String temporarySchema;
+ private boolean allowTemporaryTables;
+ private final BooleanSupplier useRootSchema;
+ private final Supplier<SchemaPlus> defaultSchemaSupplier;
+
+ private final LoadingCache<DrillTableKey, MetadataProviderManager> tableCache;
+
+ DrillCalciteCatalogReader(SchemaPlus rootSchema,
+ boolean caseSensitive,
+ List<String> defaultSchema,
+ JavaTypeFactory typeFactory,
+ DrillConfig drillConfig,
+ UserSession session,
+ String temporarySchema,
+ BooleanSupplier useRootSchema,
+ Supplier<SchemaPlus> defaultSchemaSupplier) {
+ super(DynamicSchema.from(rootSchema), defaultSchema,
+ typeFactory, getConnectionConfig(caseSensitive));
+ this.drillConfig = drillConfig;
+ this.session = session;
+ this.allowTemporaryTables = true;
+ this.tableCache =
+ CacheBuilder.newBuilder()
+ .build(new CacheLoader<DrillTableKey, MetadataProviderManager>() {
+ @Override
+ public MetadataProviderManager load(DrillTableKey key) {
+ return key.getMetadataProviderManager();
+ }
+ });
+ this.temporarySchema = temporarySchema;
+ this.useRootSchema = useRootSchema;
+ this.defaultSchemaSupplier = defaultSchemaSupplier;
+ }
+
+ /**
+ * Disallow temporary tables presence in sql statement (ex: in view definitions)
+ */
+ void disallowTemporaryTables() {
+ this.allowTemporaryTables = false;
+ }
+
+ List<String> getTemporaryNames(List<String> names) {
+ if (mightBeTemporaryTable(names, session.getDefaultSchemaPath(), drillConfig)) {
+ String tableName = FileSelection.removeLeadingSlash(names.get(names.size() - 1));
+ String temporaryTableName = session.resolveTemporaryTableName(tableName);
+ if (temporaryTableName != null) {
+ List<String> temporaryNames = new ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema));
+ temporaryNames.add(temporaryTableName);
+ return temporaryNames;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * If schema is not indicated (only one element in the list) or schema is default temporary workspace,
+ * we need to check among session temporary tables in default temporary workspace first.
+ * If temporary table is found and temporary tables usage is allowed, its table instance will be returned,
+ * otherwise search will be conducted in original workspace.
+ *
+ * @param names list of schema and table names, table name is always the last element
+ * @return table instance, null otherwise
+ * @throws UserException if temporary tables usage is disallowed
+ */
+ @Override
+ public Prepare.PreparingTable getTable(List<String> names) {
+ checkIfWeCanProceedForTemporaryTable(names);
+ Prepare.PreparingTable table = super.getTable(names);
+ DrillTable drillTable;
+ if (table != null && (drillTable = table.unwrap(DrillTable.class)) != null) {
+ drillTable.setOptions(session.getOptions());
+ drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names, drillTable)));
+ }
+ return table;
+ }
+
+ private void checkIfWeCanProceedForTemporaryTable(List<String> names) {
+ String originalTableName;
+ if (!allowTemporaryTables &&
+ (originalTableName = session.getOriginalTableNameFromTemporaryTable(names.get(names.size() - 1))) != null) {
+ throw UserException
+ .validationError()
+ .message("Temporary tables usage is disallowed. Used temporary table name: [%s].", originalTableName)
+ .build(SqlConverter.logger);
+ }
+ }
+
+ @Override
+ public List<List<String>> getSchemaPaths() {
+ if (useRootSchema.getAsBoolean()) {
+ return ImmutableList.of(ImmutableList.of());
+ }
+ return super.getSchemaPaths();
+ }
+
+ /**
+ * Checks if the schema provided is a valid schema:
+ * <li>schema is not indicated (only one element in the names list)<li/>
+ *
+ * @param names list of schema and table names, table name is always the last element
+ * @throws UserException if the schema is not valid.
+ */
+ void isValidSchema(List<String> names) throws UserException {
+ List<String> schemaPath = Util.skipLast(names);
+
+ for (List<String> currentSchema : getSchemaPaths()) {
+ List<String> fullSchemaPath = new ArrayList<>(currentSchema);
+ fullSchemaPath.addAll(schemaPath);
+ CalciteSchema schema = SqlValidatorUtil.getSchema(getRootSchema(),
+ fullSchemaPath, nameMatcher());
+
+ if (schema != null) {
+ return;
+ }
+ }
+ SchemaUtilites.throwSchemaNotFoundException(defaultSchemaSupplier.get(), schemaPath);
+ }
+
+ /**
+ * We should check if passed table is temporary or not if:
+ * <li>schema is not indicated (only one element in the names list)<li/>
+ * <li>current schema or indicated schema is default temporary workspace<li/>
+ *
+ * Examples (where dfs.tmp is default temporary workspace):
+ * <li>select * from t<li/>
+ * <li>select * from dfs.tmp.t<li/>
+ * <li>use dfs; select * from tmp.t<li/>
+ *
+ * @param names list of schema and table names, table name is always the last element
+ * @param defaultSchemaPath current schema path set using USE command
+ * @param drillConfig drill config
+ * @return true if check for temporary table should be done, false otherwise
+ */
+ private boolean mightBeTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
Review comment:
`needsTempTableCheck` as name? The present name suggests that this does the check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on issue #1940: DRILL-7406: Update
Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on issue #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#issuecomment-569887172
@vvysotskyi , @paul-rogers I have updated the PR, please take a look again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362065570
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
Review comment:
Ok, let's leave method names as they are. My objection regarding their names appeared because when I was reviewing this PR, it wasn't unclear what these methods do (or return) in the places where they are used without referring to their signature, so I advised renaming them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361648349
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcIntermediatePrelConverterRule.java
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.util.function.Predicate;
+
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.convert.ConverterRule;
+import org.apache.drill.exec.planner.logical.DrillRel;
+import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.exec.planner.physical.Prel;
+
+class JdbcIntermediatePrelConverterRule extends ConverterRule {
+
+ JdbcIntermediatePrelConverterRule() {
Review comment:
Since this rule does not preserve any state, can we use it as a singleton, i.e. reuse the same instance?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361671367
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillValidator.java
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.validate.SelectScope;
+import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
+import org.apache.calcite.sql.validate.SqlValidatorImpl;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.Static;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.store.ColumnExplorer;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+
+class DrillValidator extends SqlValidatorImpl {
+
+ private final UserSession session;
+
+ DrillValidator(SqlOperatorTable opTab, SqlValidatorCatalogReader catalogReader,
Review comment:
Please pass `SessionOptionManager` instead of `UserSession`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362043643
##########
File path: contrib/storage-hive/core/src/test/java/org/apache/drill/exec/hive/complex_types/TestHiveStructs.java
##########
@@ -476,4 +476,25 @@ public void strWithUnionField() throws Exception {
.baselineValues(2, mapOf("n", 5, "u", "Text"))
.go();
}
+
+ @Test // DRILL-7386
+ public void countStructColumn() throws Exception {
+ testBuilder()
+ .sqlQuery("SELECT COUNT(str_n0) cnt FROM hive.struct_tbl")
+ .unOrdered()
+ .baselineColumns("cnt")
+ .baselineValues(3L)
+ .go();
+ }
+
+ @Test // DRILL-7386
+ public void typeOfFunctions() throws Exception {
+ testBuilder()
+ .sqlQuery("SELECT sqlTypeOf(%1$s) sto, typeOf(%1$s) to, modeOf(%1$s) mo, drillTypeOf(%1$s) dto " +
+ "FROM hive.struct_tbl LIMIT 1", "str_n0")
+ .unOrdered()
+ .baselineColumns("sto", "to", "mo", "dto")
+ .baselineValues("STRUCT", "MAP", "NOT NULL", "MAP")
Review comment:
I think this case is just enough to show that Calcite's flattener doesn't replace struct input with the first primitive nested field of the struct. More details here: [DRILL-7386](https://issues.apache.org/jira/browse/DRILL-7386) and [CALCITE-3393](https://issues.apache.org/jira/browse/CALCITE-3393).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361660467
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
Review comment:
Please make them static and add `Convertlet` to method names.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361647178
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/DrillJdbcConvention.java
##########
@@ -0,0 +1,82 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcRules;
+import org.apache.calcite.adapter.jdbc.JdbcToEnumerableConverterRule;
+import org.apache.calcite.linq4j.tree.ConstantUntypedNull;
+import org.apache.calcite.plan.RelOptPlanner;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.planner.RuleInstance;
+import org.apache.drill.exec.planner.logical.DrillRelFactories;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet;
+
+/**
+ * Convention with set of rules to register for jdbc plugin
+ */
+class DrillJdbcConvention extends JdbcConvention {
+
+ /**
+ * Unwanted Calcite's JdbcRules are filtered out by the predicate
+ */
+ private static final Predicate<RelOptRule> RULES_TO_EXCLUDE = rule -> {
+ Class<?> ruleClass = rule.getClass();
+ return JdbcToEnumerableConverterRule.class == ruleClass || JdbcRules.JdbcFilterRule.class == ruleClass
+ || JdbcRules.JdbcProjectRule.class == ruleClass;
+ };
+
+ private final ImmutableSet<RelOptRule> rules;
+ private final JdbcStoragePlugin plugin;
+
+ DrillJdbcConvention(SqlDialect dialect, String name, JdbcStoragePlugin plugin) {
+ super(dialect, ConstantUntypedNull.INSTANCE, name);
+ this.plugin = plugin;
+ List<RelOptRule> calciteJdbcRules = JdbcRules.rules(this, DrillRelFactories.LOGICAL_BUILDER).stream()
+ .filter(RULES_TO_EXCLUDE.negate())
Review comment:
If we are using negated predicate only, can we initially create it in the way to avoid negation later?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361661048
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
+ return (cx, call) -> {
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ final List<SqlNode> operands = call.getOperandList();
+ final List<RexNode> exprs = new LinkedList<>();
+
+ String timeUnit = ((SqlIntervalQualifier) operands.get(0)).timeUnitRange.toString();
+
+ RelDataTypeFactory typeFactory = cx.getTypeFactory();
+
+ //RelDataType nullableReturnType =
+
+ for (SqlNode node : operands) {
+ exprs.add(cx.convertExpression(node));
+ }
+
+ final RelDataType returnType;
+ if (call.getOperator() == SqlStdOperatorTable.EXTRACT) {
+ // Legacy code:
+ // The return type is wrong!
+ // Legacy code choose SqlTypeName.BIGINT simply to avoid conflicting against Calcite's inference mechanism
+ // (, which chose BIGINT in validation phase already)
Review comment:
```suggestion
// (which chose BIGINT in validation phase already)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#1940: DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361323595
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcCatalogSchema.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.schema.Schema;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.apache.drill.exec.store.SchemaFactory;
+
+class JdbcCatalogSchema extends AbstractSchema {
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final CapitalizingJdbcSchema defaultSchema;
+
+ JdbcCatalogSchema(String name, DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ super(Collections.emptyList(), name);
+ this.schemaMap = new HashMap<>();
+ String connectionSchemaName = null;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getCatalogs()) {
+ connectionSchemaName = con.getSchema();
+ while (set.next()) {
+ final String catalogName = set.getString(1);
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(
+ getSchemaPath(), catalogName, source, dialect, convention, catalogName, null, caseSensitive);
+ schemaMap.put(schema.getName(), schema);
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ // unable to read catalog list.
+ if (schemaMap.isEmpty()) {
+
+ // try to add a list of schemas to the schema map.
+ boolean schemasAdded = addSchemas(source, dialect, convention, caseSensitive);
+
+ if (!schemasAdded) {
+ // there were no schemas, just create a default one (the jdbc system doesn't support catalogs/schemas).
+ schemaMap.put(SchemaFactory.DEFAULT_WS_NAME, new CapitalizingJdbcSchema(Collections.emptyList(), name, source, dialect, convention, null, null, caseSensitive));
+ }
+ } else {
+ // We already have catalogs. Add schemas in this context of their catalogs.
+ addSchemas(source, dialect, convention, caseSensitive);
+ }
+
+ defaultSchema = determineDefaultSchema(connectionSchemaName);
+ }
+
+ private CapitalizingJdbcSchema determineDefaultSchema(String connectionSchemaName) {
+ CapitalizingJdbcSchema connSchema;
+ if (connectionSchemaName == null ||
+ (connSchema = schemaMap.get(connectionSchemaName.toLowerCase())) == null) {
+ // todo: this selection of default schema isn't correct, users are getting implicit random schema as default
+ connSchema = schemaMap.values().iterator().next();
+ }
+ return (CapitalizingJdbcSchema) connSchema.getDefaultSchema();
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ private boolean addSchemas(DataSource source, SqlDialect dialect, DrillJdbcConvention convention, boolean caseSensitive) {
+ boolean added = false;
+ try (Connection con = source.getConnection();
+ ResultSet set = con.getMetaData().getSchemas()) {
+ while (set.next()) {
+ final String schemaName = set.getString(1);
+ final String catalogName = set.getString(2);
+
+ String parentKey = catalogName == null ? null : catalogName.toLowerCase();
+ CapitalizingJdbcSchema parentSchema = schemaMap.get(parentKey);
+ if (parentSchema == null) {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(getSchemaPath(), schemaName, source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+
+ // if a catalog schema doesn't exist, we'll add this at the top level.
+ schemaMap.put(schema.getName(), schema);
+ } else {
+ CapitalizingJdbcSchema schema = new CapitalizingJdbcSchema(parentSchema.getSchemaPath(), schemaName,
+ source, dialect,
+ convention, catalogName, schemaName, caseSensitive);
+ parentSchema.schemaMap.put(schema.getName(), schema);
+ }
+ added = true;
+ }
+ } catch (SQLException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to load JDBC schema.", e);
+ }
+
+ return added;
+ }
+
+
+ @Override
+ public String getTypeName() {
+ return JdbcStorageConfig.NAME;
+ }
+
+ @Override
+ public Schema getDefaultSchema() {
+ return defaultSchema;
+ }
+
+ @Override
+ public CapitalizingJdbcSchema getSubSchema(String name) {
+ return schemaMap.get(name);
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return schemaMap.keySet();
+ }
+
+ @Override
+ public Table getTable(String name) {
+ if (defaultSchema != null) {
+ try {
+ return defaultSchema.getTable(name);
+ } catch (RuntimeException e) {
+ JdbcStoragePlugin.logger.warn("Failure while attempting to read table '{}' from JDBC source.", name, e);
Review comment:
Same here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362065570
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
Review comment:
Ok, let's leave method names as they are. My objection regarding their names appeared because when I was reviewing this PR, it wasn't unclear what these methods do (or return) in the places where they are used, so I advised renaming them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361643360
##########
File path: contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/CapitalizingJdbcSchema.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.drill.exec.store.jdbc;
+
+import javax.sql.DataSource;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.calcite.adapter.jdbc.JdbcConvention;
+import org.apache.calcite.adapter.jdbc.JdbcSchema;
+import org.apache.calcite.schema.Function;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.exec.store.AbstractSchema;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class CapitalizingJdbcSchema extends AbstractSchema {
+
+ private static final Logger logger = LoggerFactory.getLogger(CapitalizingJdbcSchema.class);
+
+ private final Map<String, CapitalizingJdbcSchema> schemaMap;
+ private final JdbcSchema inner;
+ private final boolean caseSensitive;
+
+ CapitalizingJdbcSchema(List<String> parentSchemaPath, String name, DataSource dataSource,
+ SqlDialect dialect, JdbcConvention convention, String catalog, String schema, boolean caseSensitive) {
+ super(parentSchemaPath, name);
+ this.schemaMap = new HashMap<>();
+ this.inner = new JdbcSchema(dataSource, dialect, convention, catalog, schema);
+ this.caseSensitive = caseSensitive;
+ }
+
+ @Override
+ public String getTypeName() {
+ return JdbcStorageConfig.NAME;
+ }
+
+ @Override
+ public Collection<Function> getFunctions(String name) {
+ return inner.getFunctions(name);
+ }
+
+ @Override
+ public Set<String> getFunctionNames() {
+ return inner.getFunctionNames();
+ }
+
+ @Override
+ public CapitalizingJdbcSchema getSubSchema(String name) {
+ return schemaMap.get(name);
+ }
+
+ void setHolder(SchemaPlus plusOfThis) {
+ for (String s : getSubSchemaNames()) {
+ CapitalizingJdbcSchema inner = getSubSchema(s);
+ SchemaPlus holder = plusOfThis.add(s, inner);
+ inner.setHolder(holder);
+ }
+ }
+
+ @Override
+ public Set<String> getSubSchemaNames() {
+ return schemaMap.keySet();
+ }
+
+ @Override
+ public Set<String> getTableNames() {
+ if (isCatalogSchema()) {
+ return Collections.emptySet();
+ }
+ return inner.getTableNames();
+ }
+
+ @Override
+ public Table getTable(String name) {
+ if (isCatalogSchema()) {
+ logger.warn("Failed attempt to find table '{}' in catalog schema '{}'", name, getName());
+ return null;
+ }
+ Table table = inner.getTable(name);
+ if (table == null
+ && !areTableNamesCaseSensitive()
+ && (table = inner.getTable(name.toUpperCase())) == null // Oracle and H2 changes unquoted identifiers to uppercase.
Review comment:
Assigning variable value inside if condition is a little bit unclear, please revert this change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361668715
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.math.BigDecimal;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.util.DecimalUtility;
+
+class DrillRexBuilder extends RexBuilder {
+ DrillRexBuilder(RelDataTypeFactory typeFactory) {
+ super(typeFactory);
+ }
+
+ /**
+ * Since Drill has different mechanism and rules for implicit casting,
+ * ensureType() is overridden to avoid conflicting cast functions being added to the expressions.
+ */
+ @Override
+ public RexNode ensureType(
+ RelDataType type,
+ RexNode node,
+ boolean matchNullability) {
+ return node;
+ }
+
+ /**
+ * Creates a call to the CAST operator, expanding if possible, and optionally
+ * also preserving nullability.
+ *
+ * <p>Tries to expand the cast, and therefore the result may be something
+ * other than a {@link org.apache.calcite.rex.RexCall} to the CAST operator, such as a
+ * {@link RexLiteral} if {@code matchNullability} is false.
+ *
+ * @param type Type to cast to
+ * @param exp Expression being cast
+ * @param matchNullability Whether to ensure the result has the same
+ * nullability as {@code type}
+ * @return Call to CAST operator
+ */
+ @Override
+ public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability) {
+ if (matchNullability) {
+ return makeAbstractCast(type, exp);
+ }
+ // for the case when BigDecimal literal has a scale or precision
+ // that differs from the value from specified RelDataType, cast cannot be removed
+ // TODO: remove this code when CALCITE-1468 is fixed
+ if (type.getSqlTypeName() == SqlTypeName.DECIMAL && exp instanceof RexLiteral) {
+ if (type.getPrecision() < 1) {
+ throw UserException.validationError()
+ .message("Expected precision greater than 0, but was %s.", type.getPrecision())
+ .build(SqlConverter.logger);
Review comment:
Please use its own logger.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] ihuzenko commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
ihuzenko commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r362168596
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillConvertletTable.java
##########
@@ -138,14 +105,209 @@ public SqlRexConvertlet get(SqlCall call) {
((SqlBasicCall) call).setOperator(wrapper);
return sqlRexConvertlet;
}
-
- if ((convertlet = map.get(call.getOperator())) != null) {
+ if ((convertlet = operatorToConvertletMap.get(call.getOperator())) != null) {
return convertlet;
}
-
return StandardConvertletTable.INSTANCE.get(call);
}
- private DrillConvertletTable() {
+ /**
+ * Custom convertlet to handle extract functions. Optiq rewrites
+ * extract functions as divide and modulo functions, based on the
+ * data type. We cannot do that in Drill since we don't know the data type
+ * till we start scanning. So we don't rewrite extract and treat it as
+ * a regular function.
+ */
+ private SqlRexConvertlet extract() {
Review comment:
ok, renamed. Please let me know whether it is clear now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361668902
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillRexBuilder.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.drill.exec.planner.sql.conversion;
+
+import java.math.BigDecimal;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.util.DecimalUtility;
+
+class DrillRexBuilder extends RexBuilder {
+ DrillRexBuilder(RelDataTypeFactory typeFactory) {
+ super(typeFactory);
+ }
+
+ /**
+ * Since Drill has different mechanism and rules for implicit casting,
+ * ensureType() is overridden to avoid conflicting cast functions being added to the expressions.
+ */
+ @Override
+ public RexNode ensureType(
+ RelDataType type,
+ RexNode node,
+ boolean matchNullability) {
+ return node;
+ }
+
+ /**
+ * Creates a call to the CAST operator, expanding if possible, and optionally
+ * also preserving nullability.
+ *
+ * <p>Tries to expand the cast, and therefore the result may be something
+ * other than a {@link org.apache.calcite.rex.RexCall} to the CAST operator, such as a
+ * {@link RexLiteral} if {@code matchNullability} is false.
+ *
+ * @param type Type to cast to
+ * @param exp Expression being cast
+ * @param matchNullability Whether to ensure the result has the same
+ * nullability as {@code type}
+ * @return Call to CAST operator
+ */
+ @Override
+ public RexNode makeCast(RelDataType type, RexNode exp, boolean matchNullability) {
+ if (matchNullability) {
+ return makeAbstractCast(type, exp);
+ }
+ // for the case when BigDecimal literal has a scale or precision
+ // that differs from the value from specified RelDataType, cast cannot be removed
+ // TODO: remove this code when CALCITE-1468 is fixed
+ if (type.getSqlTypeName() == SqlTypeName.DECIMAL && exp instanceof RexLiteral) {
+ if (type.getPrecision() < 1) {
+ throw UserException.validationError()
+ .message("Expected precision greater than 0, but was %s.", type.getPrecision())
+ .build(SqlConverter.logger);
+ }
+ if (type.getScale() > type.getPrecision()) {
+ throw UserException.validationError()
+ .message("Expected scale less than or equal to precision, " +
+ "but was precision %s and scale %s.", type.getPrecision(), type.getScale())
+ .build(SqlConverter.logger);
+ }
+ RexLiteral literal = (RexLiteral) exp;
+ Comparable value = literal.getValueAs(Comparable.class);
Review comment:
```suggestion
Comparable<?> value = literal.getValueAs(Comparable.class);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] paul-rogers commented on a change in pull request #1940:
DRILL-7406: Update Calcite to 1.21.0
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #1940: DRILL-7406: Update Calcite to 1.21.0
URL: https://github.com/apache/drill/pull/1940#discussion_r361893656
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowTablesHandler.java
##########
@@ -37,17 +32,19 @@
import org.apache.calcite.tools.RelConversionException;
import org.apache.calcite.tools.ValidationException;
import org.apache.calcite.util.NlsString;
-import org.apache.calcite.util.Pair;
import org.apache.calcite.util.Util;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
-import org.apache.drill.exec.planner.sql.SqlConverter;
import org.apache.drill.exec.planner.sql.parser.DrillParserUtil;
import org.apache.drill.exec.planner.sql.parser.SqlShowTables;
import org.apache.drill.exec.store.AbstractSchema;
import org.apache.drill.exec.store.ischema.InfoSchemaTableType;
import org.apache.drill.exec.work.foreman.ForemanSetupException;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.IS_SCHEMA_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_NAME;
+import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_SCHEMA;
Review comment:
What is `SHRD`? Short for `SHARD`? `SHARED`? Something else? Can we use a clearer name?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services