From fae73800a7791e386c2b0c6bce3de06f85b525d3 Mon Sep 17 00:00:00 2001 From: Laksh Singla <30999375+LakshSingla@users.noreply.github.com> Date: Wed, 12 Jan 2022 15:11:17 +0530 Subject: [PATCH] Set plannerContext error when cannot query external datasources and when insert is not supported. (#12136) This PR aims to add plannerContext.setPlanningError whenever external table scan rule is invoked, without the queryMaker having the ability to do so. --- .../external/ExternalTableScanRule.java | 1 + .../sql/calcite/planner/DruidPlanner.java | 33 ++++---- .../druid/sql/calcite/CalciteQueryTest.java | 9 +++ .../external/ExternalTableScanRuleTest.java | 77 +++++++++++++++++++ 4 files changed, 104 insertions(+), 16 deletions(-) create mode 100644 sql/src/test/java/org/apache/druid/sql/calcite/external/ExternalTableScanRuleTest.java diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalTableScanRule.java b/sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalTableScanRule.java index 786837eb87d..5e74ca02b94 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalTableScanRule.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalTableScanRule.java @@ -46,6 +46,7 @@ public class ExternalTableScanRule extends RelOptRule if (plannerContext.getQueryMaker().feature(QueryFeature.CAN_READ_EXTERNAL_DATA)) { return super.matches(call); } else { + plannerContext.setPlanningError("SQL query requires scanning external datasources that is not suported."); return false; } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java index 621653591ed..924e56b53c6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java @@ -213,24 +213,25 @@ public class DruidPlanner implements Closeable // Not a CannotPlanningException, rethrow without trying with bindable throw e; } - if (parsed.getInsertNode() != null) { - // Cannot INSERT with BINDABLE. - throw e; - } - // Try again with BINDABLE convention. Used for querying Values and metadata tables. - try { - return planWithBindableConvention(rootQueryRel, parsed.getExplainNode()); - } - catch (Exception e2) { - e.addSuppressed(e2); - Logger logger = log; - if (!QueryContexts.isDebug(plannerContext.getQueryContext())) { - logger = log.noStackTrace(); + + // If there isn't any INSERT clause, then we should try again with BINDABLE convention. And return without + // any error, if it is plannable by the bindable convention + if (parsed.getInsertNode() == null) { + // Try again with BINDABLE convention. Used for querying Values and metadata tables. + try { + return planWithBindableConvention(rootQueryRel, parsed.getExplainNode()); + } + catch (Exception e2) { + e.addSuppressed(e2); } - String errorMessage = buildSQLPlanningErrorMessage(cannotPlanException); - logger.warn(e, errorMessage); - throw new UnsupportedSQLQueryException(errorMessage); } + Logger logger = log; + if (!QueryContexts.isDebug(plannerContext.getQueryContext())) { + logger = log.noStackTrace(); + } + String errorMessage = buildSQLPlanningErrorMessage(cannotPlanException); + logger.warn(e, errorMessage); + throw new UnsupportedSQLQueryException(errorMessage); } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 688649a3673..326c17421c6 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13480,4 +13480,13 @@ public class CalciteQueryTest extends BaseCalciteQueryTest ImmutableList.of(new Object[]{"A", "10.1"}) ); } + + @Test + public void testSurfaceErrorsWhenInsertingThroughIncorrectSelectStatment() + { + assertQueryIsUnplannable( + "INSERT INTO druid.dst SELECT dim2, dim1, m1 FROM foo2 UNION SELECT dim1, dim2, m1 FROM foo", + "Possible error: SQL requires 'UNION' but only 'UNION ALL' is supported." + ); + } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/external/ExternalTableScanRuleTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/external/ExternalTableScanRuleTest.java new file mode 100644 index 00000000000..128ae0a44c2 --- /dev/null +++ b/sql/src/test/java/org/apache/druid/sql/calcite/external/ExternalTableScanRuleTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.sql.calcite.external; + +import com.google.common.collect.ImmutableMap; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.rel.RelRoot; +import org.apache.calcite.schema.SchemaPlus; +import org.apache.calcite.tools.ValidationException; +import org.apache.druid.query.QueryRunnerFactoryConglomerate; +import org.apache.druid.query.QuerySegmentWalker; +import org.apache.druid.sql.calcite.planner.PlannerConfig; +import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.apache.druid.sql.calcite.schema.DruidSchema; +import org.apache.druid.sql.calcite.schema.DruidSchemaCatalog; +import org.apache.druid.sql.calcite.schema.NamedDruidSchema; +import org.apache.druid.sql.calcite.schema.NamedViewSchema; +import org.apache.druid.sql.calcite.schema.ViewSchema; +import org.apache.druid.sql.calcite.util.CalciteTests; +import org.easymock.EasyMock; +import org.junit.Assert; +import org.junit.Test; + +public class ExternalTableScanRuleTest +{ + @Test + public void testMatchesWhenExternalScanUnsupported() throws ValidationException + { + + final PlannerContext plannerContext = PlannerContext.create( + "DUMMY", // The actual query isn't important for this test + CalciteTests.createOperatorTable(), + CalciteTests.createExprMacroTable(), + CalciteTests.getJsonMapper(), + new PlannerConfig(), + new DruidSchemaCatalog( + EasyMock.createMock(SchemaPlus.class), + ImmutableMap.of( + "druid", new NamedDruidSchema(EasyMock.createMock(DruidSchema.class), "druid"), + NamedViewSchema.NAME, new NamedViewSchema(EasyMock.createMock(ViewSchema.class)) + ) + ), + ImmutableMap.of() + ); + plannerContext.setQueryMaker( + CalciteTests.createMockQueryMakerFactory( + EasyMock.createMock(QuerySegmentWalker.class), + EasyMock.createMock(QueryRunnerFactoryConglomerate.class) + ) + .buildForSelect(EasyMock.createMock(RelRoot.class), plannerContext) + ); + + ExternalTableScanRule rule = new ExternalTableScanRule(plannerContext); + rule.matches(EasyMock.createMock(RelOptRuleCall.class)); + Assert.assertEquals( + "SQL query requires scanning external datasources that is not suported.", + plannerContext.getPlanningError() + ); + } +}