[
https://issues.apache.org/jira/browse/DRILL-8555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18132972#comment-18132972
]
ASF GitHub Bot commented on DRILL-8555:
---------------------------------------
letian-jiang commented on code in PR #3086:
URL: https://github.com/apache/drill/pull/3086#discussion_r4390307593
##########
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java:
##########
@@ -58,6 +65,26 @@ public boolean supportsRead() {
return true;
}
+ @Override
+ public boolean supportPlanCache(DrillTableSelection selection) {
+ return selection instanceof HBaseScanSpec;
+ }
+
+ @Override
+ public PlanCacheTable planCacheTable(DrillTableSelection selection) throws
IOException {
+ if (!supportPlanCache(selection)) {
+ return null;
+ }
+ String identifier = ((HBaseScanSpec) selection).getTableName();
+ try (Table table =
getConnection().getTable(TableName.valueOf(identifier))) {
+ String version = Hashing.sha256().hashBytes(
+
TableDescriptorBuilder.toByteArray(table.getDescriptor())).toString();
Review Comment:
Agreed. I’ll leave further metadata reuse and the existing Admin lifetime
issue for a follow-up optimization.
##########
contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseGroupScan.java:
##########
@@ -108,18 +116,44 @@ public HBaseGroupScan(@JsonProperty("userName") String
userName,
@JsonProperty("storage") HBaseStoragePluginConfig
storagePluginConfig,
@JsonProperty("columns") List<SchemaPath> columns,
@JsonProperty("maxRecords") int maxRecords,
+ @JsonProperty("baseScanSpec") HBaseScanSpec
baseScanSpec,
+ @JsonProperty("pushedFilter") String pushedFilter,
+ @JsonProperty("pushedFilterFullyConverted") boolean
pushedFilterFullyConverted,
@JacksonInject StoragePluginRegistry pluginRegistry)
throws IOException, ExecutionSetupException {
- this (userName, pluginRegistry.resolve(storagePluginConfig,
HBaseStoragePlugin.class), hbaseScanSpec, columns, maxRecords);
+ this(userName, pluginRegistry.resolve(storagePluginConfig,
HBaseStoragePlugin.class),
+ hbaseScanSpec, columns, maxRecords, baseScanSpec,
+ pushedFilter == null ? null :
LogicalExpressionParser.parse(pushedFilter),
+ pushedFilterFullyConverted, true);
}
public HBaseGroupScan(String userName, HBaseStoragePlugin storagePlugin,
HBaseScanSpec scanSpec,
List<SchemaPath> columns, int maxRecords) {
+ this(userName, storagePlugin, scanSpec, columns, maxRecords, null, null,
false, false);
+ }
+
+ HBaseGroupScan(String userName, HBaseStoragePlugin storagePlugin,
HBaseScanSpec scanSpec,
+ List<SchemaPath> columns, int maxRecords, HBaseScanSpec baseScanSpec,
+ LogicalExpression pushedFilter, boolean pushedFilterFullyConverted,
boolean rebuild) {
super(userName);
this.storagePlugin = storagePlugin;
this.storagePluginConfig = storagePlugin.getConfig();
- this.hbaseScanSpec = scanSpec;
+ this.baseScanSpec = baseScanSpec;
+ this.pushedFilter = pushedFilter;
+ this.pushedFilterFullyConverted = pushedFilterFullyConverted;
+ this.hbaseScanSpec = rebuild && pushedFilter != null ? baseScanSpec :
scanSpec;
this.columns = columns == null ? ALL_COLUMNS : columns;
this.maxRecords = maxRecords;
+ if (rebuild && pushedFilter != null) {
+ if (baseScanSpec == null) {
+ throw new IllegalArgumentException("Cached HBase filter has no base
scan specification");
+ }
+ HBaseFilterBuilder builder = new HBaseFilterBuilder(this, pushedFilter);
+ HBaseScanSpec rebuilt = builder.parseTree();
+ if (rebuilt == null || (pushedFilterFullyConverted &&
!builder.isAllExpressionsConverted())) {
Review Comment:
Cache hits now require scan reconstruction; ordinary physical-plan JSON uses
its saved scan specification. Added tests for JSON replay, region splits,
partial filtering and parameter changes that require replanning.
##########
logical/src/main/java/org/apache/drill/common/expression/LiteralExpression.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.common.expression;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
+/** A literal that may carry a bound SQL parameter slot in a cached plan. */
+public abstract class LiteralExpression extends LogicalExpressionBase {
+ private int dynamicParamIndex = -1;
+
+ protected LiteralExpression(ExpressionPosition position) {
+ super(position);
+ }
+
+ @JsonIgnore
+ public int getDynamicParamIndex() {
+ return dynamicParamIndex;
+ }
+
+ /** Whether this literal is bound to a zero-based SQL parameter slot. */
+ @JsonIgnore
+ public boolean isDynamicParam() {
+ return dynamicParamIndex >= 0;
+ }
+
+ public void setDynamicParamIndex(int dynamicParamIndex) {
Review Comment:
Thanks for pointing this out. The current binding paths allocate fresh
literals and use VARDECIMAL for bound decimals. Given that these cases seem
unlikely in the current implementation, I’d prefer to keep the design simple
for now and revisit the additional hardening if those paths expand.
> 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)
