Copilot commented on code in PR #2546:
URL: https://github.com/apache/phoenix/pull/2546#discussion_r3471827796
##########
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java:
##########
@@ -1894,7 +1894,7 @@ private static ObjectNode scanAttrs(String scanType,
String table, String keys)
ObjectNode n = defaultAttrs();
n.put("iteratorTypeAndScanSize", "PARALLEL <N>-WAY");
n.put("consistency", "STRONG");
- n.put("explainScanType", scanType);
+ n.put("explainScanType", scanType.trim());
Review Comment:
The `scanAttrs` Javadoc still says `scanType` includes a required trailing
space (e.g. "FULL SCAN "), but the method now trims `scanType` before storing
it. This is now misleading for future test authors.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java:
##########
@@ -344,10 +344,10 @@ protected void explain(String prefix, List<String>
planSteps,
if (OrderBy.REV_ROW_KEY_ORDER_BY.equals(orderBy)) {
explainPlanAttributesBuilder.setClientSortedBy("REVERSE");
}
- explainPlanAttributesBuilder.setExplainScanType(scanTypeDetails);
+ explainPlanAttributesBuilder.setExplainScanType(scanTypeDetails.trim());
explainPlanAttributesBuilder.setTableName(tableName);
if (!scanRanges.isPointLookup()) {
- explainPlanAttributesBuilder.setKeyRanges(appendKeyRanges());
+ explainPlanAttributesBuilder.setKeyRanges(appendKeyRanges().trim());
}
Review Comment:
`ExplainPlanAttributes.explainScanType` and `keyRanges` are now trimmed,
which changes their serialized JSON values and will break existing assertions
that expect leading/trailing spaces (e.g.,
phoenix-core/src/it/java/org/apache/phoenix/end2end/CostBasedDecisionIT.java:127-129
and TenantSpecificViewIndexIT.java:210-215) as well as ExplainPlanTestUtil's
`keyRanges()` helper
(phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTestUtil.java:336-340).
Please update those tests/helpers (or adjust them to trim) so the suite stays
consistent with the new canonical JSON output.
--
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]
