Copilot commented on code in PR #2523:
URL: https://github.com/apache/phoenix/pull/2523#discussion_r3405220810
##########
phoenix-core-client/src/main/java/org/apache/phoenix/parse/IndexExpressionParseNodeRewriter.java:
##########
@@ -90,9 +113,32 @@ public IndexExpressionParseNodeRewriter(PTable index,
String alias, PhoenixConne
@Override
protected ParseNode leaveCompoundNode(CompoundParseNode node,
List<ParseNode> children,
CompoundNodeFactory factory) {
- return indexedParseNodeToColumnParseNodeMap.containsKey(node)
- ? indexedParseNodeToColumnParseNodeMap.get(node)
- : super.leaveCompoundNode(node, children, factory);
+ ParseNode replacement = indexedParseNodeToColumnParseNodeMap.get(node);
+ if (replacement == null) {
+ return super.leaveCompoundNode(node, children, factory);
+ }
+ // A functional index substitution actually fired for this query node.
+ String[] functionalColumn = indexedParseNodeToFunctionalColumn.get(node);
+ if (functionalColumn != null) {
+ appliedFunctionalSubstitutions.put(functionalColumn[0],
functionalColumn[1]);
+ }
Review Comment:
When a functional index substitution fires, the code currently records the
index's stored expression string (from metadata) rather than the query path
expression that actually matched. This can diverge in
quoting/casing/qualification and contradicts the intent of surfacing the user's
expression in EXPLAIN. Record the fired query node's SQL string instead, and
keep the first-seen match per index column.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java:
##########
@@ -495,6 +495,7 @@ private AddPlanResult addPlan(PhoenixStatement statement,
SelectStatement select
isHinted, indexSelect, resolver);
}
+ @SuppressWarnings("rawtypes")
private AddPlanResult addPlan(PhoenixStatement statement, SelectStatement
select, PTable index,
Review Comment:
The method-level @SuppressWarnings("rawtypes") looks unnecessary here (the
method body and signature use generics throughout). Keeping it at method scope
can hide future raw-type regressions; it would be better to remove it or scope
the suppression to the specific statement that needs it.
--
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]
