[
https://issues.apache.org/jira/browse/DRILL-8555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18132970#comment-18132970
]
ASF GitHub Bot commented on DRILL-8555:
---------------------------------------
letian-jiang commented on code in PR #3086:
URL: https://github.com/apache/drill/pull/3086#discussion_r4390300728
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/PlanCacheParameterizer.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * 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;
+
+import java.math.BigDecimal;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlJoin;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOrderBy;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlSelect;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.util.SqlShuttle;
+import org.apache.calcite.util.NlsString;
+import org.apache.drill.exec.planner.sql.conversion.SqlBoundDynamicParam;
+
+/** Extracts SQL literals into typed slots independently of cache eligibility.
*/
+final class PlanCacheParameterizer extends SqlShuttle {
+ private final List<SqlLiteral> literals = new ArrayList<>();
+ private final List<String> types = new ArrayList<>();
+ private final RelDataTypeFactory typeFactory;
+ private boolean preserveProjection;
+
+ private PlanCacheParameterizer(RelDataTypeFactory typeFactory) {
+ this.typeFactory = Objects.requireNonNull(typeFactory, "typeFactory");
+ }
+
+ static Candidate parameterize(SqlNode parsed, RelDataTypeFactory
typeFactory) {
+ Objects.requireNonNull(parsed, "parsed");
+ PlanCacheParameterizer visitor = new PlanCacheParameterizer(typeFactory);
+ SqlNode parameterized = parsed.accept(visitor);
+ if (visitor.literals.isEmpty()) {
+ parameterized = parsed;
+ }
+ return new Candidate(parameterized,
+ Collections.unmodifiableList(visitor.literals),
+ parameterized.toString() + visitor.types);
+ }
+
+ @Override
+ public SqlNode visit(SqlCall call) {
+ int configurationOperand = configurationOperand(call);
+ if (configurationOperand >= 0 && configurationOperand <
call.operandCount()) {
+ // These operands become function names, encodings or resolved plugin
+ // references during conversion. They are not bindable expressions in
+ // the physical plan, so keep their entire subtree in the template key.
+ SqlCall parameterized = (SqlCall) call.clone(call.getParserPosition());
+ for (int i = 0; i < call.operandCount(); i++) {
+ if (i != configurationOperand) {
+ parameterized.setOperand(i, visitNullable(call.operand(i)));
+ }
+ }
+ return parameterized;
+ }
+ if ("COALESCE".equalsIgnoreCase(call.getOperator().getName())
+ && call.getOperandList().stream()
+ .anyMatch(operand -> operand instanceof SqlLiteral
+ && ((SqlLiteral) operand).getTypeName() == SqlTypeName.NULL)) {
+ // Calcite cannot infer a dynamic parameter's type alongside an untyped
+ // NULL. Keep this call in the key; explicit casts still allow slots.
+ return call;
+ }
+ if (call instanceof SqlJoin) {
+ SqlJoin join = (SqlJoin) call;
+ return new SqlJoin(join.getParserPosition(),
visitNullable(join.getLeft()),
+ join.isNaturalNode(), join.getJoinTypeNode(),
visitNullable(join.getRight()),
+ join.getConditionTypeNode(), visitNullable(join.getCondition()));
+ }
+ if (call.getKind() == SqlKind.ITEM && call.operand(1) instanceof
SqlLiteral) {
+ // A map/array key identifies a field in the input schema. Changing it
can
+ // change the scan projection and the HBase column used by pushdown.
+ return call.getOperator().createCall(call.getParserPosition(),
+ visitNullable(call.operand(0)), call.operand(1));
+ }
+ // These literals control rows, ordinals, window frames or operator
+ // configuration. Keep them verbatim in the template key.
+ if (call.getKind() == SqlKind.VALUES || call.getKind() == SqlKind.ORDER_BY
+ || call.getKind() == SqlKind.OVER) {
+ if (call instanceof SqlOrderBy) {
+ SqlOrderBy orderBy = (SqlOrderBy) call;
+ boolean previous = preserveProjection;
+ preserveProjection = true;
+ SqlNode query;
+ try {
+ query = orderBy.query.accept(this);
+ } finally {
+ preserveProjection = previous;
+ }
+ return new SqlOrderBy(call.getParserPosition(), query,
orderBy.orderList,
+ orderBy.offset, orderBy.fetch);
+ }
+ return call;
+ }
+ if (call instanceof SqlSelect) {
+ SqlSelect select = (SqlSelect) call;
+ // Preserve matching expressions across SELECT, GROUP BY, HAVING and
+ // QUALIFY. Parameterizing only some occurrences breaks validation or
rewrites.
+ boolean structuralProjection = preserveProjection
Review Comment:
I now preserve literals in SELECT, HAVING and QUALIFY unconditionally. For
example, `SELECT x + 1` keeps the `1` in the template key. I expect most of the
benefit to come from rebinding WHERE predicates, so this conservative
restriction seems reasonable. Added tests for QUALIFY without an outer ORDER BY
and inline windows.
##########
contrib/format-iceberg/src/main/java/org/apache/drill/exec/store/iceberg/plan/DrillExprToIcebergTranslator.java:
##########
@@ -103,7 +145,7 @@ public Expression visitFunctionCall(FunctionCall call, Void
value) throws Runtim
}
case FunctionNames.GE: {
LogicalExpression nameRef = call.args().get(0);
- Expression expression = call.args().get(0).accept(this, null);
+ Expression expression = call.args().get(1).accept(this, null);
Review Comment:
Added translator and cache-off query tests for `>=`, AND/OR and temporal
values. Also fixed NULL handling and retained Drill filtering when
`ignoreResiduals=true`.
##########
contrib/format-iceberg/src/main/java/org/apache/drill/exec/store/iceberg/plan/IcebergPluginImplementor.java:
##########
@@ -67,6 +70,10 @@ public void implement(PluginFilterRel filter) throws
IOException {
new
DrillParseContext(PrelUtil.getPlannerSettings(filter.getCluster().getPlanner())),
filter.getInput(),
condition);
+ if (groupScan.getCondition() != null) {
Review Comment:
Added an implementor regression test for two stacked filters, such as `id >=
2 AND id < 4`. It checks values inside and outside the range to verify that
both bounds are retained.
> Physical plan cache for parameterized SQL queries
> -------------------------------------------------
>
> Key: DRILL-8555
> URL: https://issues.apache.org/jira/browse/DRILL-8555
> Project: Apache Drill
> Issue Type: New Feature
> Components: Query Planning & Optimization
> Reporter: Letian Jiang
> Priority: Major
>
> h2. Motivation
> Repeated queries that differ only in literals are a common workload, yet each
> query currently requires full planning. This adds unnecessary latency and
> wastes CPU time.
> h2. Brief Design
> Add a per-Drillbit, in-memory plan cache. Drill automatically parameterizes
> eligible SQL queries and reuses a cached physical plan with the current
> literal values, without changes to client queries.
> h2. Plan reuse safety
> * *Engine:* matches the SQL template and checks relevant option changes,
> plugin configuration changes, and whether base table versions still match.
> * *Storage plugin:* explicitly opts in, provides a plan-compatible table
> version, and is responsible for rebuilding its predicate-dependent scan work
> from the rebound expressions.
> h2. Expected benefit
> Short, frequently executed queries such as HBase reads can benefit
> substantially. Analytical queries with significant planning cost can benefit
> as well.
> h2. Implementation status
> A working implementation is available, including HBase, Paimon and Iceberg
> integrations. A pull request will follow.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
