Copilot commented on code in PR #2537:
URL: https://github.com/apache/phoenix/pull/2537#discussion_r3430368710
##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/StatementContext.java:
##########
@@ -541,22 +532,50 @@ public List<String> getAppliedRewrites() {
}
/**
- * 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: after adoption the source is still read and written (e.g.
the optimizer keeps
+ * recording per-index applicability breadcrumbs on the data plan's context
and reconciles them
+ * onto the candidate plans), so sharing the backing collections by
reference would let two
+ * contexts read and write the same lists/maps/sets.
*/
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 (deep enough to cover the
nested lists/maps/sets
+ * that accumulate during compilation) 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 = 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.newHashSet(source.functionalIndexNames);
+ this.partialIndexCheckedSet =
Sets.newHashSet(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<Hint, String>(source.ignoredHints);
Review Comment:
The explicit generic parameters in the constructor are redundant in modern
Java and reduce readability. Consider using the diamond operator (`new
EnumMap<>(...)`) to match typical project conventions and keep the line concise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
