This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch PHOENIX-7876-feature in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit 6b49dbd6acd87fffd0b01f93294fe8e8635b6c18 Author: Andrew Purtell <[email protected]> AuthorDate: Wed Jun 17 16:11:51 2026 -0700 PHOENIX-7930: Address review feedback for EXPLAIN improvements (#2537) Co-authored-by: Claude Opus 4.8[1m] <[email protected]> --- .../phoenix/compile/ExplainPlanAttributes.java | 15 +++-- .../org/apache/phoenix/compile/QueryCompiler.java | 2 +- .../apache/phoenix/compile/StatementContext.java | 62 ++++++++++++------- .../org/apache/phoenix/compile/UnionCompiler.java | 10 +-- .../org/apache/phoenix/iterate/ExplainTable.java | 71 ++++++++++------------ .../apache/phoenix/optimize/QueryOptimizer.java | 1 - .../apache/phoenix/end2end/index/IndexUsageIT.java | 30 ++++----- .../phoenix/end2end/json/JsonFunctionsIT.java | 2 +- .../apache/phoenix/compile/QueryOptimizerTest.java | 43 +++++++++++++ .../phoenix/query/explain/ExplainPlanTest.java | 2 +- .../phoenix/query/explain/ExplainPlanTestUtil.java | 14 ++--- 11 files changed, 153 insertions(+), 99 deletions(-) diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java index 29be54c6e0..a068ec5fb4 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -186,7 +187,9 @@ public class ExplainPlanAttributes { this.scanEstimatedSizeInBytes = b.scanEstimatedSizeInBytes; this.serverWhereFilter = b.serverWhereFilter; this.serverDistinctFilter = b.serverDistinctFilter; - this.serverMergeColumns = b.serverMergeColumns; + this.serverMergeColumns = (b.serverMergeColumns == null || b.serverMergeColumns.isEmpty()) + ? null + : Collections.unmodifiableSet(new LinkedHashSet<>(b.serverMergeColumns)); this.serverParsedProjections = copyServerParsedProjections(b.serverParsedProjections); this.serverProject = (b.serverProject == null || b.serverProject.isEmpty()) ? null @@ -222,12 +225,16 @@ public class ExplainPlanAttributes { : Collections.unmodifiableList(new ArrayList<>(b.clientSteps)); this.lhsJoinQueryExplainPlan = b.lhsJoinQueryExplainPlan; this.rhsJoinQueryExplainPlan = b.rhsJoinQueryExplainPlan; - this.subPlans = b.subPlans; + this.subPlans = (b.subPlans == null || b.subPlans.isEmpty()) + ? null + : Collections.unmodifiableList(new ArrayList<>(b.subPlans)); this.dynamicServerFilter = b.dynamicServerFilter; this.afterJoinFilter = b.afterJoinFilter; this.joinScannerLimit = b.joinScannerLimit; this.sortMergeSkipMerge = b.sortMergeSkipMerge; - this.regionLocations = b.regionLocations; + this.regionLocations = (b.regionLocations == null || b.regionLocations.isEmpty()) + ? null + : Collections.unmodifiableList(new ArrayList<>(b.regionLocations)); this.regionLocationsTotalSize = b.regionLocationsTotalSize; this.numRegionLocationLookups = b.numRegionLocationLookups; } @@ -924,7 +931,7 @@ public class ExplainPlanAttributes { } public ExplainPlanAttributesBuilder setServerMergeColumns(Set<PColumn> columns) { - this.serverMergeColumns = columns; + this.serverMergeColumns = columns == null ? null : new LinkedHashSet<>(columns); return this; } diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/QueryCompiler.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/QueryCompiler.java index 6d38fb51bb..8b187042aa 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/QueryCompiler.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/QueryCompiler.java @@ -762,7 +762,7 @@ public class QueryCompiler { QueryPlan innerPlan = compileSubquery(innerSelect, false, context); if (innerPlan instanceof UnionPlan) { UnionCompiler.optimizeUnionOrderByIfPossible((UnionPlan) innerPlan, select, - this::createStatementContext); + this::createStatementContext, context); } RowProjector innerQueryPlanRowProjector = innerPlan.getProjector(); TupleProjector tupleProjector = new TupleProjector(innerQueryPlanRowProjector); diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java index 4ce383aed9..6767035b3c 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java @@ -175,18 +175,9 @@ public class StatementContext { this.totalSegmentsValue = context.totalSegmentsValue; this.hasRowSizeFunction = context.hasRowSizeFunction; this.hasRawRowSizeFunction = context.hasRawRowSizeFunction; - this.appliedRewrites = context.appliedRewrites; - this.derivedTableFlattenCount = context.derivedTableFlattenCount; - this.appliedIndexExpressionMatches = context.appliedIndexExpressionMatches; - this.appliedIndexExpressionPairs = context.appliedIndexExpressionPairs; - this.functionalIndexNames = context.functionalIndexNames; - this.partialIndexCheckedSet = context.partialIndexCheckedSet; - this.serverParsedProjections = context.serverParsedProjections; this.parentContext = context.parentContext; this.explainOptions = context.explainOptions; - this.predicateOrigins = context.predicateOrigins; - this.decorrelatedSubqueryAlias = context.decorrelatedSubqueryAlias; - this.ignoredHints = context.ignoredHints; + copyRewriteStateFrom(context); } /** @@ -253,8 +244,8 @@ public class StatementContext { this.derivedTableFlattenCount = 0; this.appliedIndexExpressionMatches = Maps.newLinkedHashMap(); this.appliedIndexExpressionPairs = Maps.newLinkedHashMap(); - this.functionalIndexNames = Sets.newHashSet(); - this.partialIndexCheckedSet = Sets.newHashSet(); + this.functionalIndexNames = Sets.newLinkedHashSet(); + this.partialIndexCheckedSet = Sets.newLinkedHashSet(); this.serverParsedProjections = null; this.parentContext = null; this.predicateOrigins = new IdentityHashMap<>(); @@ -541,22 +532,47 @@ public class StatementContext { } /** - * Adopt the rewrite breadcrumb accumulator (and related diagnostic state) of another context by - * sharing its references. Used to carry breadcrumbs recorded by the early + * Adopt the rewrite breadcrumb accumulator (and related diagnostic state) of another context. + * Used to carry breadcrumbs recorded by the early * {@link SubselectRewriter}/{@link SubqueryRewriter} pass on a pre-built context across the - * rewrite boundary onto the actual compilation context. + * rewrite boundary onto the actual compilation context. Fresh collections are allocated (see + * {@link #copyRewriteStateFrom}) so this context and {@code source} continue to mutate + * independently. */ public void adoptRewriteState(StatementContext source) { - this.appliedRewrites = source.appliedRewrites; + copyRewriteStateFrom(source); + } + + /** + * Copy the rewrite breadcrumb accumulators and related diagnostic state from {@code source} into + * this context, allocating fresh collections so the two contexts never share mutable backing + * collections. Shared by the copy constructor and {@link #adoptRewriteState}, both of which hand + * the resulting context to a compile pass that continues to mutate this state. + */ + private void copyRewriteStateFrom(StatementContext source) { + this.appliedRewrites = + source.appliedRewrites == null ? new ArrayList<>() : new ArrayList<>(source.appliedRewrites); this.derivedTableFlattenCount = source.derivedTableFlattenCount; - this.appliedIndexExpressionMatches = source.appliedIndexExpressionMatches; - this.appliedIndexExpressionPairs = source.appliedIndexExpressionPairs; - this.functionalIndexNames = source.functionalIndexNames; - this.partialIndexCheckedSet = source.partialIndexCheckedSet; + this.appliedIndexExpressionMatches = Maps.newLinkedHashMap(); + for (Map.Entry<String, List<String>> entry : source.appliedIndexExpressionMatches.entrySet()) { + this.appliedIndexExpressionMatches.put(entry.getKey(), new ArrayList<>(entry.getValue())); + } + this.appliedIndexExpressionPairs = Maps.newLinkedHashMap(); + for (Map.Entry<String, Map<String, String>> entry : source.appliedIndexExpressionPairs + .entrySet()) { + this.appliedIndexExpressionPairs.put(entry.getKey(), new LinkedHashMap<>(entry.getValue())); + } + this.functionalIndexNames = Sets.newLinkedHashSet(source.functionalIndexNames); + this.partialIndexCheckedSet = Sets.newLinkedHashSet(source.partialIndexCheckedSet); + // serverParsedProjections is nullable and only ever replaced wholesale via its setter (never + // mutated in place), so the reference can be carried over directly. this.serverParsedProjections = source.serverParsedProjections; - this.predicateOrigins = source.predicateOrigins; - this.decorrelatedSubqueryAlias = source.decorrelatedSubqueryAlias; - this.ignoredHints = source.ignoredHints; + this.predicateOrigins = new IdentityHashMap<>(); + for (Map.Entry<Expression, Set<String>> entry : source.predicateOrigins.entrySet()) { + this.predicateOrigins.put(entry.getKey(), new LinkedHashSet<>(entry.getValue())); + } + this.decorrelatedSubqueryAlias = new IdentityHashMap<>(source.decorrelatedSubqueryAlias); + this.ignoredHints = new EnumMap<>(source.ignoredHints); } public void incrementDerivedTableFlattenCount() { diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/UnionCompiler.java b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/UnionCompiler.java index f99c9346d6..646d1cd587 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/compile/UnionCompiler.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/compile/UnionCompiler.java @@ -235,8 +235,8 @@ public class UnionCompiler { * perform any special processing on the output of the subqueries. */ static void optimizeUnionOrderByIfPossible(UnionPlan innerUnionPlan, - SelectStatement outerSelectStatement, Supplier<StatementContext> statementContextCreator) - throws SQLException { + SelectStatement outerSelectStatement, Supplier<StatementContext> statementContextCreator, + StatementContext breadcrumbContext) throws SQLException { innerUnionPlan.enableCheckSupportOrderByOptimize(); if (!innerUnionPlan.isSupportOrderByOptimize()) { return; @@ -250,8 +250,10 @@ public class UnionCompiler { // not perform any special processing on the output of the subqueries. innerUnionPlan.disableSupportOrderByOptimize(); } else { - // The order-by merge optimization is preserved. Record a top-of-plan breadcrumb. - statementContextCreator.get().addAppliedRewrite("UNION ORDER BY MERGE"); + // The order-by merge optimization is preserved. Record a top-of-plan breadcrumb on the outer + // query's compilation context (the throwaway contexts produced by statementContextCreator are + // only used for trial group-by/order-by compilation and are discarded). + breadcrumbContext.addAppliedRewrite("UNION ORDER BY MERGE"); } } diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java b/phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java index 1045d492fc..115e88d329 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java @@ -75,12 +75,9 @@ import org.apache.phoenix.util.CDCUtil; import org.apache.phoenix.util.MetaDataUtil; import org.apache.phoenix.util.ScanUtil; import org.apache.phoenix.util.StringUtil; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public abstract class ExplainTable { - private static final Logger LOGGER = LoggerFactory.getLogger(ExplainTable.class); private static final List<KeyRange> EVERYTHING = Collections.singletonList(KeyRange.EVERYTHING_RANGE); public static final String POINT_LOOKUP_ON_STRING = "POINT LOOKUP ON "; @@ -801,46 +798,40 @@ public abstract class ExplainTable { if (regionLocationsFromResultIterator == null) { return ""; } - try { - StringBuilder buf = new StringBuilder().append(REGION_LOCATIONS); - Set<String> regionBoundaries = new LinkedHashSet<>(); - List<HRegionLocation> regionLocations = new ArrayList<>(); - for (HRegionLocation regionLocation : regionLocationsFromResultIterator) { - String regionBoundary = Bytes.toStringBinary(regionLocation.getRegion().getStartKey()) + ":" - + Bytes.toStringBinary(regionLocation.getRegion().getEndKey()); - if (regionBoundaries.add(regionBoundary)) { - regionLocations.add(regionLocation); - } + StringBuilder buf = new StringBuilder().append(REGION_LOCATIONS); + Set<String> regionBoundaries = new LinkedHashSet<>(); + List<HRegionLocation> regionLocations = new ArrayList<>(); + for (HRegionLocation regionLocation : regionLocationsFromResultIterator) { + String regionBoundary = Bytes.toStringBinary(regionLocation.getRegion().getStartKey()) + ":" + + Bytes.toStringBinary(regionLocation.getRegion().getEndKey()); + if (regionBoundaries.add(regionBoundary)) { + regionLocations.add(regionLocation); + } + } + int maxLimitRegionLoc = context.getConnection().getQueryServices().getConfiguration().getInt( + QueryServices.MAX_REGION_LOCATIONS_SIZE_EXPLAIN_PLAN, + QueryServicesOptions.DEFAULT_MAX_REGION_LOCATIONS_SIZE_EXPLAIN_PLAN); + if (regionLocations.size() > maxLimitRegionLoc) { + int originalSize = regionLocations.size(); + List<HRegionLocation> trimmedRegionLocations = regionLocations.subList(0, maxLimitRegionLoc); + if (explainPlanAttributesBuilder != null) { + explainPlanAttributesBuilder + .setRegionLocations(Collections.unmodifiableList(trimmedRegionLocations)) + .setRegionLocationsTotalSize(originalSize); } - int maxLimitRegionLoc = context.getConnection().getQueryServices().getConfiguration().getInt( - QueryServices.MAX_REGION_LOCATIONS_SIZE_EXPLAIN_PLAN, - QueryServicesOptions.DEFAULT_MAX_REGION_LOCATIONS_SIZE_EXPLAIN_PLAN); - if (regionLocations.size() > maxLimitRegionLoc) { - int originalSize = regionLocations.size(); - List<HRegionLocation> trimmedRegionLocations = - regionLocations.subList(0, maxLimitRegionLoc); - if (explainPlanAttributesBuilder != null) { - explainPlanAttributesBuilder - .setRegionLocations(Collections.unmodifiableList(trimmedRegionLocations)) - .setRegionLocationsTotalSize(originalSize); - } - buf.append(trimmedRegionLocations); - buf.append("...total size = "); - buf.append(originalSize); - } else { - buf.append(regionLocations); - if (explainPlanAttributesBuilder != null) { - explainPlanAttributesBuilder - .setRegionLocations(Collections.unmodifiableList(regionLocations)) - .setRegionLocationsTotalSize(regionLocations.size()); - } + buf.append(trimmedRegionLocations); + buf.append("...total size = "); + buf.append(originalSize); + } else { + buf.append(regionLocations); + if (explainPlanAttributesBuilder != null) { + explainPlanAttributesBuilder + .setRegionLocations(Collections.unmodifiableList(regionLocations)) + .setRegionLocationsTotalSize(regionLocations.size()); } - buf.append(") "); - return buf.toString(); - } catch (Exception e) { - LOGGER.error("Explain table unable to add region locations.", e); - return ""; } + buf.append(") "); + return buf.toString(); } @SuppressWarnings("rawtypes") diff --git a/phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java b/phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java index fd5bfaa498..20711d5412 100644 --- a/phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java +++ b/phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java @@ -177,7 +177,6 @@ public class QueryOptimizer { if (stmt.getWhere() != null && stmt.getWhere().hasSubquery()) { StatementContext context = new StatementContext(statement, resolver, new Scan(), new SequenceManager(statement)); - ; ParseNode dummyWhere = GenSubqueryParamValuesRewriter.replaceWithDummyValues(stmt.getWhere(), context); stmt = FACTORY.select(stmt, dummyWhere); diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java index de405ef9f5..2ca74c462f 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexUsageIT.java @@ -135,7 +135,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { .iteratorType("PARALLEL 1-WAY").serverFirstKeyOnlyProjection(true) .serverAggregate("SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY " + "[TO_BIGINT(\"(A.INT_COL1 + B.INT_COL2)\")]") - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(A.INT_COL1 + B.INT_COL2)").indexRejectedNone(); if (localIndex) { basePlan.scanType("RANGE SCAN") .table("INDEX_TEST." + indexName + "(" + fullDataTableName + ")") @@ -192,7 +192,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { "SERVER DISTINCT PREFIX FILTER OVER " + "[TO_BIGINT(\"(A.INT_COL1 + 1)\")]") .serverAggregate( "SERVER AGGREGATE INTO ORDERED DISTINCT ROWS BY " + "[TO_BIGINT(\"(A.INT_COL1 + 1)\")]") - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(A.INT_COL1 + 1)").indexRejectedNone(); if (localIndex) { basePlan.table("INDEX_TEST." + indexName + "(" + fullDataTableName + ")") .keyRanges(" [1,0] - [1,*]").clientSortAlgo("CLIENT MERGE SORT"); @@ -245,9 +245,9 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { conn.createStatement().execute(ddl); String sql = "SELECT int_col1+1 FROM " + fullDataTableName + " where int_col1+1 IN (2)"; - ExplainPlanTestUtil.ExplainPlanAssert basePlan = - assertPlan(conn, sql).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") - .serverFirstKeyOnlyProjection(true).indexRuleStartsWith("matches").indexRejectedNone(); + ExplainPlanTestUtil.ExplainPlanAssert basePlan = assertPlan(conn, sql) + .iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN").serverFirstKeyOnlyProjection(true) + .indexRuleMatches("(A.INT_COL1 + 1)").indexRejectedNone(); if (localIndex) { basePlan.table("INDEX_TEST." + indexName + "(" + fullDataTableName + ")") .keyRanges(" [1,2]").clientSortAlgo("CLIENT MERGE SORT"); @@ -301,7 +301,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { ExplainPlanTestUtil.ExplainPlanAssert basePlan = assertPlan(conn, sql).iteratorType("PARALLEL 1-WAY").serverFirstKeyOnlyProjection(true) - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(A.INT_COL1 + 1)").indexRejectedNone(); if (localIndex) { basePlan.scanType("RANGE SCAN") .table("INDEX_TEST." + indexName + "(" + fullDataTableName + ")").keyRanges(" [1]") @@ -376,7 +376,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { + " WHERE (\"V1\" || '_' || \"v2\") = 'x_1'"; ExplainPlanTestUtil.ExplainPlanAssert basePlan = assertPlan(conn, query).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(\"cf1\".\"V1\" || '_' || \"CF2\".\"v2\")").indexRejectedNone(); if (localIndex) { basePlan.table(indexName + "(" + dataTableName + ")").keyRanges(" [1,'x_1']") .clientSortAlgo("CLIENT MERGE SORT"); @@ -401,7 +401,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { "SELECT \"V1\", \"V1\" as foo1, (\"V1\" || '_' || \"v2\") as foo, (\"V1\" || '_' || \"v2\") as \"Foo1\", (\"V1\" || '_' || \"v2\") FROM " + dataTableName + " ORDER BY foo"; basePlan = assertPlan(conn, query).iteratorType("PARALLEL 1-WAY") - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(\"cf1\".\"V1\" || '_' || \"CF2\".\"v2\")").indexRejectedNone(); if (localIndex) { basePlan.scanType("RANGE SCAN").table(indexName + "(" + dataTableName + ")") .keyRanges(" [1]").clientSortAlgo("CLIENT MERGE SORT"); @@ -482,7 +482,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { basePlan.scanType("RANGE SCAN") .table("INDEX_TEST." + indexName + "(" + fullDataTableName + ")").keyRanges(" [1,2]") .clientSortAlgo("CLIENT MERGE SORT").serverFirstKeyOnlyProjection(true) - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(A.INT_COL1 + 1)").indexRejectedNone(); } else { basePlan.scanType("FULL SCAN").table(fullDataTableName).clientSortAlgo(null) .serverWhereFilter("SERVER FILTER BY (A.INT_COL1 + 1) = 2") @@ -537,7 +537,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { String query = "SELECT k1, k2, k3, s1, s2 FROM " + viewName + " WHERE k1+k2+k3 = 173.0"; ExplainPlanTestUtil.ExplainPlanAssert basePlan = assertPlan(conn, query).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("(K1 + K2 + K3)").indexRejectedNone(); if (local) { basePlan.table(indexName1 + "(" + dataTableName + ")").keyRanges(" [1,173]") .clientSortAlgo("CLIENT MERGE SORT"); @@ -560,7 +560,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { query = "SELECT k1, k2, s1||'_'||s2 FROM " + viewName + " WHERE (s1||'_'||s2)='foo2_bar2'"; basePlan = assertPlan(conn, query).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") - .serverFirstKeyOnlyProjection(true).indexRuleStartsWith("matches") + .serverFirstKeyOnlyProjection(true).indexRuleMatches("(S1 || '_' || S2)") .indexRejectedCount(1); if (local) { basePlan.table(indexName2 + "(" + dataTableName + ")") @@ -625,7 +625,7 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { assertPlan(conn, query).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") .table(indexName2).keyRanges(" [1,'abc_cab','foo']").serverFirstKeyOnlyProjection(true) - .indexRuleStartsWith("matches").indexRejectedCount(1); + .indexRuleMatches("(S2 || '_' || S3)").indexRejectedCount(1); rs = conn.createStatement().executeQuery(query); assertTrue(rs.next()); @@ -722,9 +722,9 @@ public class IndexUsageIT extends ParallelStatsDisabledIT { query = "SELECT k FROM " + dataTableName + " WHERE REGEXP_SUBSTR(v,'id:\\\\w+') = 'id:id1'"; - ExplainPlanTestUtil.ExplainPlanAssert basePlan = - assertPlan(conn, query).iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN") - .serverFirstKeyOnlyProjection(true).indexRuleStartsWith("matches").indexRejectedNone(); + ExplainPlanTestUtil.ExplainPlanAssert basePlan = assertPlan(conn, query) + .iteratorType("PARALLEL 1-WAY").scanType("RANGE SCAN").serverFirstKeyOnlyProjection(true) + .indexRuleMatches("REGEXP_SUBSTR(V,'id:\\\\w+')").indexRejectedNone(); if (localIndex) { basePlan.table(indexName + "(" + dataTableName + ")").keyRanges(" [1,'id:id1']") .clientSortAlgo("CLIENT MERGE SORT"); diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java index e527e76d12..d6eddcda1e 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/json/JsonFunctionsIT.java @@ -369,7 +369,7 @@ public class JsonFunctionsIT extends ParallelStatsDisabledIT { "SELECT JSON_VALUE(JSONCOL,'$.type'), " + "JSON_VALUE(JSONCOL,'$.info.address.town') FROM " + tableName + " WHERE JSON_VALUE(JSONCOL,'$.type') = 'Basic'"; assertPlan(conn, selectSql).scanType("RANGE SCAN").table(indexName) - .indexRuleStartsWith("matches").indexRejectedNone(); + .indexRuleMatches("JSON_VALUE(JSONCOL.JSONCOL,'$.type')").indexRejectedNone(); // Validate the total count of rows String countSql = "SELECT COUNT(1) FROM " + tableName; ResultSet rs = conn.createStatement().executeQuery(countSql); diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.java index abd027ca1a..29d609ba1c 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryOptimizerTest.java @@ -36,12 +36,15 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Properties; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.phoenix.compile.OrderByCompiler.OrderBy; +import org.apache.phoenix.expression.Expression; +import org.apache.phoenix.expression.LiteralExpression; import org.apache.phoenix.jdbc.PhoenixPreparedStatement; import org.apache.phoenix.jdbc.PhoenixResultSet; import org.apache.phoenix.jdbc.PhoenixStatement; @@ -956,4 +959,44 @@ public class QueryOptimizerTest extends BaseConnectionlessQueryTest { assertNull(scan.getAttribute(MIN_QUALIFIER)); assertNull(scan.getAttribute(MAX_QUALIFIER)); } + + /** + * Test the deep copy contract of the {@link StatementContext} copy constructor and + * {@link StatementContext#adoptRewriteState}. + */ + @Test + public void testAdoptRewriteStateDoesNotBleedSourceMutations() throws Exception { + Connection conn = DriverManager.getConnection(getUrl()); + PhoenixStatement stmt = conn.createStatement().unwrap(PhoenixStatement.class); + + StatementContext source = new StatementContext(stmt); + source.addAppliedRewrite("X"); + source.recordAppliedIndexExpressionMatches("IDX", Collections.singletonList("A + B")); + Expression predicate = LiteralExpression.newConstant(1); + source.tagPredicate(predicate, "WHERE"); + + // Copy via the copy constructor, which shares copyRewriteStateFrom() with adoptRewriteState(). + StatementContext copy = new StatementContext(source); + + // The copy observed the breadcrumbs recorded before the copy. + assertTrue(copy.getAppliedRewrites().contains("X")); + assertEquals(Collections.singletonList("A + B"), copy.getAppliedIndexExpressionMatches("IDX")); + assertTrue(copy.getPredicateOrigins(predicate).contains("WHERE")); + + // Mutating the source after the copy must not leak into the destination. + source.addAppliedRewrite("foo"); + source.recordAppliedIndexExpressionMatches("IDX", Collections.singletonList("C + D")); + source.tagPredicate(predicate, "JOIN ON"); + + assertFalse(copy.getAppliedRewrites().contains("foo")); + assertFalse(copy.getAppliedIndexExpressionMatches("IDX").contains("C + D")); + assertFalse(copy.getPredicateOrigins(predicate).contains("JOIN ON")); + + // adoptRewriteState() onto a fresh destination has the same isolation guarantee. + StatementContext adopted = new StatementContext(stmt); + adopted.adoptRewriteState(source); + assertTrue(adopted.getAppliedRewrites().contains("foo")); + source.addAppliedRewrite("bar"); + assertFalse(adopted.getAppliedRewrites().contains("bar")); + } } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java index 12fa1bc43b..72fa018b75 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java @@ -1020,7 +1020,7 @@ public class ExplainPlanTest extends BaseConnectionlessQueryTest { // The functional index is chosen and the rule comment names the matched expression. ExplainPlanTestUtil.assertPlan(conn, query).indexName(idx) - .indexRuleStartsWith("matches BSON_VALUE(PAYLOAD,'k','VARCHAR')") + .indexRuleMatches("BSON_VALUE(PAYLOAD,'k','VARCHAR')") // Exactly one breadcrumb (one applied substitution; no eager per-PK-column emissions). .rewriteCount(1).rewrite(0, "INDEX EXPRESSION BSON_VALUE(PAYLOAD,'k','VARCHAR') AS \":BSON_VALUE(PAYLOAD,'k','VARCHAR')\""); diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java b/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java index b239b9af50..711f5dcd53 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java @@ -32,6 +32,7 @@ import org.apache.phoenix.compile.ExplainPlan; import org.apache.phoenix.compile.ExplainPlanAttributes; import org.apache.phoenix.compile.QueryPlan; import org.apache.phoenix.jdbc.PhoenixPreparedStatement; +import org.apache.phoenix.optimize.OptimizerReasons; import org.apache.phoenix.optimize.RejectedIndexEntry; import org.apache.phoenix.parse.ExplainOptions; import org.apache.phoenix.parse.UpsertStatement.OnDuplicateKeyType; @@ -255,16 +256,11 @@ public final class ExplainPlanTestUtil { } /** - * Assert the index-selection rule label starts with {@code prefix}. Useful for the functional - * index {@code "matches <expr>"} rule whose suffix is expression dependent. + * Assert the optimizer chose a functional index whose rule label is exactly + * {@code "matches <expression>"} (see {@link OptimizerReasons#matches(String)}). */ - public ExplainPlanAssert indexRuleStartsWith(String prefix) { - String actual = attributes.getIndexRule(); - assertNotNull(at("indexRule") + " must not be null", actual); - assertTrue( - at("indexRule") + " expected to start with '" + prefix + "' but was '" + actual + "'", - actual.startsWith(prefix)); - return this; + public ExplainPlanAssert indexRuleMatches(String expression) { + return indexRule(OptimizerReasons.matches(expression)); } /** Assert the number of rejected index candidates recorded for this plan. */
