[
https://issues.apache.org/jira/browse/DRILL-8555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18123653#comment-18123653
]
ASF GitHub Bot commented on DRILL-8555:
---------------------------------------
letian-jiang commented on code in PR #3086:
URL: https://github.com/apache/drill/pull/3086#discussion_r4193014085
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java:
##########
@@ -295,9 +301,116 @@ private static PhysicalPlan getQueryPlan(QueryContext
context, String sql, Point
context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL,
true);
}
+ if (context.getOptions().getOption(PlannerSettings.PLAN_CACHE)
+ && !context.getSession().hasTemporaryTables()
+ && !hasMutableAliases(context)
+ && PlanCacheEligibility.isSafeToCache(sqlNode,
context.getFunctionRegistry())) {
+ try {
+ PlanCacheParameterizer.Candidate candidate =
PlanCacheParameterizer.parameterize(sqlNode);
+ PlanCache cache = context.getDrillbitContext().getPlanCache();
+ String key = context.getQueryUserName() + '\n'
+ + context.getSession().getDefaultSchemaPath() + '\n'
+ + candidate.template;
+ PlanCache.Entry entry = cache.get(key);
+ String optionsFingerprint = null;
+ if (entry != null) {
+ try {
+ optionsFingerprint = optionFingerprint(context);
+ if (entry.matchesContext(optionsFingerprint,
context.getStorage())) {
+ PhysicalPlan bound = entry.bind(candidate.literals,
+ context.getDrillbitContext().getPlanReader());
+ if (textPlan != null) {
+ textPlan.value =
PlanCache.textWithBindings(entry.getTextPlan(), candidate.literals);
+ }
+ cache.recordBind();
+ context.setPlanCacheHit();
+ return bound;
+ }
+ } catch (RuntimeException | IOException e) {
+ logger.debug("Cached plan could not be rebound", e);
+ }
+ cache.invalidate(key);
+ }
+ // Capture the context before planning. Changes during planning
invalidate
+ // this entry on its first hit.
+ PlanCache.ContextSnapshot snapshot = PlanCache.ContextSnapshot.resolve(
+ parser.getDefaultSchema(), sqlNode,
+ context.getStorage());
+ if (snapshot != null) {
+ if (optionsFingerprint == null) {
+ optionsFingerprint = optionFingerprint(context);
+ }
+ snapshot = snapshot.withOptionsFingerprint(optionsFingerprint);
+ }
+ PhysicalPlan planned = handler.getPlan(candidate.sql);
Review Comment:
Thanks for catching this. The context snapshot is now resolved from the
original SQL before parameterization, including checking that all participating
plugins support plan caching for their table selections. If the snapshot cannot
be established, planning uses the original SQL with its literals intact.
Parameterized planning is only used when a valid snapshot is available.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java:
##########
@@ -295,9 +301,116 @@ private static PhysicalPlan getQueryPlan(QueryContext
context, String sql, Point
context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL,
true);
}
+ if (context.getOptions().getOption(PlannerSettings.PLAN_CACHE)
+ && !context.getSession().hasTemporaryTables()
+ && !hasMutableAliases(context)
+ && PlanCacheEligibility.isSafeToCache(sqlNode,
context.getFunctionRegistry())) {
+ try {
+ PlanCacheParameterizer.Candidate candidate =
PlanCacheParameterizer.parameterize(sqlNode);
+ PlanCache cache = context.getDrillbitContext().getPlanCache();
+ String key = context.getQueryUserName() + '\n'
+ + context.getSession().getDefaultSchemaPath() + '\n'
+ + candidate.template;
+ PlanCache.Entry entry = cache.get(key);
+ String optionsFingerprint = null;
+ if (entry != null) {
+ try {
+ optionsFingerprint = optionFingerprint(context);
+ if (entry.matchesContext(optionsFingerprint,
context.getStorage())) {
+ PhysicalPlan bound = entry.bind(candidate.literals,
+ context.getDrillbitContext().getPlanReader());
+ if (textPlan != null) {
+ textPlan.value =
PlanCache.textWithBindings(entry.getTextPlan(), candidate.literals);
+ }
+ cache.recordBind();
+ context.setPlanCacheHit();
+ return bound;
+ }
+ } catch (RuntimeException | IOException e) {
+ logger.debug("Cached plan could not be rebound", e);
+ }
+ cache.invalidate(key);
+ }
+ // Capture the context before planning. Changes during planning
invalidate
+ // this entry on its first hit.
+ PlanCache.ContextSnapshot snapshot = PlanCache.ContextSnapshot.resolve(
+ parser.getDefaultSchema(), sqlNode,
+ context.getStorage());
+ if (snapshot != null) {
+ if (optionsFingerprint == null) {
+ optionsFingerprint = optionFingerprint(context);
+ }
+ snapshot = snapshot.withOptionsFingerprint(optionsFingerprint);
+ }
+ PhysicalPlan planned = handler.getPlan(candidate.sql);
+ final String templateTextPlan = textPlan == null ? null :
textPlan.value;
+ if (textPlan != null) {
+ textPlan.value = PlanCache.textWithBindings(templateTextPlan,
candidate.literals);
+ }
+ if (snapshot != null && cache.canCache(planned)) {
+ final org.apache.drill.exec.planner.PhysicalPlanReader cacheReader =
+ context.getDrillbitContext().getPlanReader();
+ final PlanCache.ContextSnapshot cacheContext = snapshot;
+ context.setPendingPlanCacheInsert(() -> cache.writeAfterSuccess(key,
planned,
+ templateTextPlan, cacheReader, cacheContext));
+ }
+ return planned;
+ } catch (Exception e) {
Review Comment:
Agreed. I moved `handler.getPlan(...)` outside the cache fallback block, so
cache lookup or binding failures can fall back before planning, while planning
failures propagate without a cache-induced retry.
> 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
(v8.20.10#820010)
