Copilot commented on code in PR #2526:
URL: https://github.com/apache/phoenix/pull/2526#discussion_r3407509407
##########
phoenix-core/src/test/java/org/apache/phoenix/query/explain/ExplainPlanTest.java:
##########
@@ -973,11 +974,176 @@ public void testIndexRejectedRendered() throws Exception
{
stmt.execute("CREATE LOCAL INDEX " + idx + " ON " + base + " (c1)");
String query =
"SELECT c1, max(rowkey), max(c2) FROM " + base + " WHERE rowkey <= 'z'
GROUP BY c1";
+ // The structured indexRejected attribute is populated regardless of
EXPLAIN mode.
ExplainPlanTestUtil.assertPlan(conn, query).indexName(base)
.indexRule(OptimizerReasons.RULE_MORE_BOUND_PK_COLUMNS).indexRejectedCount(1)
.indexRejected(0, idx, OptimizerReasons.REASON_NO_PK_PREFIX_BOUND);
assertPlanContainsLine(conn, query, " INDEX " + base + " /* more
bound PK columns */");
- assertPlanContainsLine(conn, query, " /* !INDEX " + idx + " -- no PK
prefix bound */");
+ // The !INDEX rejection comment text is VERBOSE-only.
+ assertNoPlanLineContains(conn, query, "!INDEX");
+ List<String> verboseSteps =
+ ExplainPlanTestUtil.getPlanSteps(conn, query, ExplainOptions.VERBOSE);
+ assertTrue(
+ "expected VERBOSE plan to contain the !INDEX rejection comment but was
" + verboseSteps,
+ verboseSteps.contains(" /* !INDEX " + idx + " -- no PK prefix bound
*/"));
+ }
+ }
+
+ @Test
+ public void testVerboseProjectLine() throws Exception {
+ try (Connection conn = DriverManager.getConnection(getUrl(),
defaultProps());
+ java.sql.Statement stmt = conn.createStatement()) {
+ String t = generateUniqueName();
+ stmt
+ .execute("CREATE TABLE " + t + " (k VARCHAR PRIMARY KEY, a VARCHAR, b
VARCHAR, c VARCHAR)");
+ String query = "SELECT a, b FROM " + t;
+ ExplainPlanTestUtil.assertPlanWithVerbose(conn,
query).serverProject("A", "B");
+ List<String> verboseSteps =
+ ExplainPlanTestUtil.getPlanSteps(conn, query, ExplainOptions.VERBOSE);
+ assertTrue("expected VERBOSE plan to contain the PROJECT line but was "
+ verboseSteps,
+ verboseSteps.contains(" PROJECT A, B"));
+ // Plain EXPLAIN carries no PROJECT line and no serverProject attribute.
+ ExplainPlanTestUtil.assertPlan(conn, query).serverProjectNone();
+ assertNoPlanLineContains(conn, query, "PROJECT ");
+ }
+ }
+
+ @Test
+ public void testVerboseServerFilterWhereFanout() throws Exception {
+ try (Connection conn = DriverManager.getConnection(getUrl(),
defaultProps());
+ java.sql.Statement stmt = conn.createStatement()) {
+ String t = generateUniqueName();
+ stmt
+ .execute("CREATE TABLE " + t + " (k VARCHAR PRIMARY KEY, a VARCHAR, b
VARCHAR, c VARCHAR)");
+ String query = "SELECT a FROM " + t + " WHERE b = 'x' AND c = 'y'";
+ ExplainPlanTestUtil.assertPlanWithVerbose(conn,
query).serverFilterCount(2)
+ .serverFilterOrigin(0, "WHERE").serverFilterPathTest(0,
null).serverFilterOrigin(1, "WHERE")
+ .serverFilterPathTest(1, null);
Review Comment:
This chained assertion line exceeds the project's 100-character Checkstyle
`LineLength` limit, which will fail the build. Please wrap the fluent calls
onto separate lines.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/iterate/ExplainTable.java:
##########
@@ -562,6 +585,165 @@ private void emitServerProjection(List<String> planSteps,
}
}
+ /** Emit the VERBOSE-only {@code PROJECT <cols>} line and populate {@code
serverProject}. */
+ private void emitProject(List<String> planSteps,
+ ExplainPlanAttributesBuilder explainPlanAttributesBuilder, boolean
verbose) {
+ if (!verbose) {
+ return;
+ }
+ RowProjector projector = getProjector();
+ if (projector == null) {
+ return;
+ }
+ List<? extends ColumnProjector> columnProjectors =
projector.getColumnProjectors();
+ if (columnProjectors == null || columnProjectors.isEmpty()) {
+ return;
+ }
+ List<String> columns = new ArrayList<>(columnProjectors.size());
+ for (ColumnProjector columnProjector : columnProjectors) {
+ String name = columnProjector.getName();
+ if (name == null || name.isEmpty()) {
+ name = String.valueOf(columnProjector.getExpression());
+ }
+ columns.add(name);
+ }
+ planSteps.add(" PROJECT " + String.join(", ", columns));
+ if (explainPlanAttributesBuilder != null) {
+ explainPlanAttributesBuilder.setServerProject(columns);
+ }
+ }
+
+ /**
+ * Emit the VERBOSE-only ignored-hint comments, one {@code /*- HINT(args) --
reason *}{@code /}
+ * line per hint the planner intentionally ignored.
+ */
+ private void emitIgnoredHints(List<String> planSteps,
+ ExplainPlanAttributesBuilder explainPlanAttributesBuilder, boolean
verbose) {
+ if (!verbose) {
+ return;
+ }
+ Map<Hint, String> ignoredHints = context.getIgnoredHints();
+ if (ignoredHints == null || ignoredHints.isEmpty()) {
+ return;
+ }
+ for (Map.Entry<Hint, String> entry : ignoredHints.entrySet()) {
+ Hint ignoredHint = entry.getKey();
+ String args = hint == null ? null : hint.getHint(ignoredHint);
+ String rendered = ignoredHint.name() + (args == null ? "" : args);
+ planSteps.add(" /*- " + rendered + " -- " + entry.getValue() + " */");
+ if (explainPlanAttributesBuilder != null) {
+ explainPlanAttributesBuilder.addIgnoredHint(ignoredHint.name(),
entry.getValue());
+ }
+ }
+ }
+
+ /**
+ * Emit the VERBOSE-only per-predicate {@code SERVER FILTER BY <expr> --
<origin>} lines and
+ * populate {@code serverFilters}. When the top-level filter is an {@link
AndExpression} whose
+ * children all carry origin tags, one line is emitted per child. Otherwise
a single line carries
+ * the comma-separated union of origins.
+ */
+ private void emitServerFilters(List<String> planSteps,
+ ExplainPlanAttributesBuilder explainPlanAttributesBuilder, Expression
whereExpression,
+ String whereFilterStr) {
+ List<ServerFilter> filters = renderVerboseFilters(context,
whereExpression, whereFilterStr,
+ " SERVER FILTER BY", planSteps);
+ if (explainPlanAttributesBuilder != null) {
+ explainPlanAttributesBuilder.setServerFilters(filters);
+ }
+ }
+
+ /**
+ * Render the VERBOSE per-predicate filter breakdown. Each emitted line is
appended to
+ * {@code planSteps}.
+ * @param context the statement context carrying the
predicate-origin tags.
+ * @param filterExpression the residual filter expression (may be {@code
null} when only the
+ * rendered string is available, e.g. an
index-serialized filter).
+ * @param combinedFilterStr the combined filter string used for the
single-line fallback.
+ * @param linePrefix the full leading token including any
indentation, e.g.
+ * {@code " SERVER FILTER BY"} or {@code "CLIENT
FILTER BY"}.
+ * @param planSteps the plan-steps list to append rendered lines to.
+ * @return the structured filters in render order (never {@code null}; never
empty).
+ */
+ public static List<ServerFilter> renderVerboseFilters(StatementContext
context,
+ Expression filterExpression, String combinedFilterStr, String linePrefix,
+ List<String> planSteps) {
+ List<ServerFilter> filters = new ArrayList<>();
+ if (filterExpression instanceof AndExpression) {
+ List<Expression> children = filterExpression.getChildren();
+ boolean allTagged = !children.isEmpty();
+ for (Expression child : children) {
+ if (context.getPredicateOrigins(child).isEmpty()) {
+ allTagged = false;
+ break;
+ }
+ }
+ if (allTagged) {
+ for (Expression child : children) {
+ filters.add(buildServerFilter(context, "(" + child + ")", child));
+ }
+ }
+ }
+ if (filters.isEmpty()) {
+ filters.add(buildServerFilter(context, combinedFilterStr,
filterExpression));
+ }
+ for (ServerFilter filter : filters) {
+ StringBuilder line = new StringBuilder(linePrefix).append("
").append(filter.getExpr());
+ String comment = renderOriginComment(filter);
+ if (comment != null) {
+ line.append(" -- ").append(comment);
+ }
+ planSteps.add(line.toString());
+ }
+ return filters;
+ }
+
+ private static ServerFilter buildServerFilter(StatementContext context,
String exprStr,
+ Expression expression) {
+ List<String> origins =
+ expression == null ? null : new
ArrayList<>(context.getPredicateOrigins(expression));
+ String pathTestSubtag = detectPathTestSubtag(expression);
+ return new ServerFilter(exprStr, origins, pathTestSubtag);
+ }
Review Comment:
`renderVerboseFilters` falls back to a single combined line when the
top-level filter is an `AndExpression` but not all children are tagged. In that
fallback path, `buildServerFilter` only reads origin tags from the parent
expression, so the origin attribution comment can be empty even when some child
predicates were tagged. This contradicts the method Javadoc/PR intent (“union
of origins”) and can hide useful attribution in VERBOSE output.
Consider unioning origin tags from conjunct children when the parent has no
tags (or when falling back), so the combined line includes the expected origin
set.
##########
phoenix-core-client/src/main/java/org/apache/phoenix/compile/ExplainPlanAttributes.java:
##########
@@ -581,6 +619,59 @@ public static ExplainPlanAttributes
getDefaultExplainPlan() {
return EXPLAIN_PLAN_INSTANCE;
}
+ /** A single VERBOSE-mode server filter predicate. */
+ @JsonPropertyOrder({ "expr", "origin", "pathTestSubtag" })
+ public static class ServerFilter {
Review Comment:
The new `ExplainPlanAttributes.ServerFilter` type is used for both
`serverFilters` and `clientFilters`. Naming it `ServerFilter` is misleading in
client contexts and makes the API harder to understand for callers/JSON
consumers.
Consider renaming it to something neutral like
`FilterPredicate`/`ExplainFilter` and using that for both fields.
--
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]
