CNDB-15280: Remove user data from Plan.toString

adelapena · adelapena · commit c51ea1dd2866 · 2025-10-15T17:05:41.000+01:00
diff --git a/src/java/org/apache/cassandra/index/sai/plan/Plan.java b/src/java/org/apache/cassandra/index/sai/plan/Plan.java
@@ -288,16 +288,37 @@ enum ControlFlow { Continue, Break }
      */
     public final String toStringRecursive()
     {
-        TreeFormatter<Plan> formatter = new TreeFormatter<>(Plan::toString, Plan::subplans);
+        return toStringRecursive(true);
+    }
+
+    /**
+     * Formats the whole plan as a pretty tree
+     *
+     * @param redact whether to redact the queried column values.
+     */
+    public final String toStringRecursive(boolean redact)
+    {
+        TreeFormatter<Plan> formatter = new TreeFormatter<>(plan -> plan.toString(redact), Plan::subplans);
         return formatter.format(this);
     }
 
     /**
      * Returns the string representation of this node only
      */
+    @Override
     public final String toString()
     {
-        String title = title();
+        return toString(true);
+    }
+
+    /**
+     * Returns the string representation of this node only
+     *
+     * @param redact whether to redact the queried column values.
+     */
+    public final String toString(boolean redact)
+    {
+        String title = title(redact);
         String description = description();
         return (title.isEmpty())
                ? String.format("%s (%s)\n%s", getClass().getSimpleName(), cost(), description).stripTrailing()
@@ -309,7 +330,7 @@ public final String toString()
      * The information is included in the output of {@link #toString()} and {@link #toStringRecursive()}.
      * It is up to subclasses to implement it.
      */
-    protected String title()
+    protected String title(boolean redact)
     {
         return "";
     }
@@ -345,7 +366,7 @@ protected String description()
     public final Plan optimize()
     {
         if (logger.isTraceEnabled())
-            logger.trace("Optimizing plan:\n{}", this.toStringRecursive());
+            logger.trace("Optimizing plan:\n{}", toStringRecursive());
 
         Plan bestPlanSoFar = this;
         List<Leaf> leaves = nodesOfType(Leaf.class);
@@ -796,7 +817,7 @@ public IndexScan(Factory factory, int id, Expression predicate, long matchingKey
         }
 
         @Override
-        protected final String title()
+        protected final String title(boolean redact)
         {
             return String.format("of %s (sel: %.9f, step: %.1f)",
                                  getIndexName(), selectivity(), access.meanDistance());
@@ -1689,9 +1710,9 @@ protected Filter withAccess(Access access)
         }
 
         @Override
-        protected String title()
+        protected String title(boolean redact)
         {
-            return String.format("%s (sel: %.9f)", filter, selectivity() / source.get().selectivity());
+            return String.format("%s (sel: %.9f)", filter.toCQLString(redact), selectivity() / source.get().selectivity());
         }
     }
 
@@ -1757,7 +1778,7 @@ protected RowsIteration withAccess(Access access)
         }
 
         @Override
-        protected String title()
+        protected String title(boolean redact)
         {
             return "" + limit;
         }
diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java
@@ -419,7 +419,7 @@ Plan buildPlan()
 
         if (Tracing.isTracing())
         {
-            Tracing.trace("Query execution plan:\n" + plan.toStringRecursive());
+            Tracing.trace("Query execution plan:\n" + plan.toStringRecursive(false));
             List<Plan.IndexScan> origIndexScans = keysIterationPlan.nodesOfType(Plan.IndexScan.class);
             List<Plan.IndexScan> selectedIndexScans = plan.nodesOfType(Plan.IndexScan.class);
             Tracing.trace("Selecting {} {} of {} out of {} indexes",
diff --git a/test/unit/org/apache/cassandra/index/sai/plan/PlanTest.java b/test/unit/org/apache/cassandra/index/sai/plan/PlanTest.java
@@ -108,6 +108,7 @@ private static RowFilter.Expression filerPred(String column, Operator operation)
     {
         RowFilter.Expression pred = Mockito.mock(RowFilter.Expression.class);
         Mockito.when(pred.toCQLString(false)).thenReturn(column + ' ' + operation + " X");
+        Mockito.when(pred.toCQLString(true)).thenReturn(column + ' ' + operation + " ?");
         Mockito.when(pred.operator()).thenReturn(operation);
         return pred;
     }
@@ -561,8 +562,6 @@ public void prettyPrint()
         Plan.RowsIteration filter = factory.recheckFilter(RowFilter.builder().add(pred1).add(pred2).add(pred4).build(), fetch);
         Plan.RowsIteration limit = factory.limit(filter, 3);
 
-        String prettyStr = limit.toStringRecursive();
-
         assertEquals("Limit 3 (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
                      " └─ Filter pred1 < X AND pred2 < X AND pred4 < X (sel: 1.000000000) (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
                      "     └─ Fetch (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
@@ -574,7 +573,22 @@ public void prettyPrint()
                      "                 │   └─ NumericIndexScan of pred1_idx (sel: 0.500000000, step: 250.0) (keys: 2000.0, cost/key: 14.6, cost: 4500.0..33701.3)\n" +
                      "                 │      predicate: RANGE(pred1)\n" +
                      "                 └─ LiteralIndexScan of pred4_idx (sel: 0.001000000, step: 1.0) (keys: 1000.0, cost/key: 0.1, cost: 4500.0..4600.0)\n" +
-                     "                    predicate: RANGE(pred4)\n", prettyStr);
+                     "                    predicate: RANGE(pred4)\n",
+                     limit.toStringRecursive(false));
+
+        assertEquals("Limit 3 (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
+                     " └─ Filter pred1 < ? AND pred2 < ? AND pred4 < ? (sel: 1.000000000) (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
+                     "     └─ Fetch (rows: 3.0, cost/row: 3895.8, cost: 44171.3..55858.7)\n" +
+                     "         └─ KeysSort (keys: 3.0, cost/key: 3792.4, cost: 44171.3..55548.4)\n" +
+                     "             └─ Union (keys: 1999.0, cost/key: 14.8, cost: 13500.0..43001.3)\n" +
+                     "                 ├─ Intersection (keys: 1000.0, cost/key: 29.4, cost: 9000.0..38401.3)\n" +
+                     "                 │   ├─ NumericIndexScan of pred2_idx (sel: 0.002000000, step: 1.0) (keys: 2000.0, cost/key: 0.1, cost: 4500.0..4700.0)\n" +
+                     "                 │   │  predicate: RANGE(pred2)\n" +
+                     "                 │   └─ NumericIndexScan of pred1_idx (sel: 0.500000000, step: 250.0) (keys: 2000.0, cost/key: 14.6, cost: 4500.0..33701.3)\n" +
+                     "                 │      predicate: RANGE(pred1)\n" +
+                     "                 └─ LiteralIndexScan of pred4_idx (sel: 0.001000000, step: 1.0) (keys: 1000.0, cost/key: 0.1, cost: 4500.0..4600.0)\n" +
+                     "                    predicate: RANGE(pred4)\n",
+                     limit.toStringRecursive(true));
     }
 
     @Test
@@ -1035,7 +1049,6 @@ public void testLazyAccessPropagation()
         Mockito.when(indexScan1.withAccess(Mockito.any())).thenReturn(indexScan1);
         Mockito.when(indexScan1.estimateCost()).thenReturn(new Plan.KeysIterationCost(20,0.0, 0.5));
         Mockito.when(indexScan1.estimateSelectivity()).thenReturn(0.001);
-        Mockito.when(indexScan1.title()).thenReturn("");
 
         Plan.KeysIteration indexScan2 = factory.indexScan(saiPred2, (long) (0.01 * factory.tableMetrics.rows));
         Plan.KeysIteration indexScan3 = factory.indexScan(saiPred3, (long) (0.5 * factory.tableMetrics.rows));

Original file line number	Diff line number	Diff line change
`@@ -288,16 +288,37 @@ enum ControlFlow { Continue, Break }`
`288`	`288`	`*/`
`289`	`289`	`public final String toStringRecursive()`
`290`	`290`	`{`
`291`		`- TreeFormatter<Plan> formatter = new TreeFormatter<>(Plan::toString, Plan::subplans);`
	`291`	`+ return toStringRecursive(true);`
	`292`	`+ }`
	`293`	`+`
	`294`	`+ /**`
	`295`	`+ * Formats the whole plan as a pretty tree`
	`296`	`+ *`
	`297`	`+ * @param redact whether to redact the queried column values.`
	`298`	`+ */`
	`299`	`+ public final String toStringRecursive(boolean redact)`
	`300`	`+ {`
	`301`	`+ TreeFormatter<Plan> formatter = new TreeFormatter<>(plan -> plan.toString(redact), Plan::subplans);`
`292`	`302`	`return formatter.format(this);`
`293`	`303`	`}`
`294`	`304`
`295`	`305`	`/**`
`296`	`306`	`* Returns the string representation of this node only`
`297`	`307`	`*/`
	`308`	`+ @Override`
`298`	`309`	`public final String toString()`
`299`	`310`	`{`
`300`		`- String title = title();`
	`311`	`+ return toString(true);`
	`312`	`+ }`
	`313`	`+`
	`314`	`+ /**`
	`315`	`+ * Returns the string representation of this node only`
	`316`	`+ *`
	`317`	`+ * @param redact whether to redact the queried column values.`
	`318`	`+ */`
	`319`	`+ public final String toString(boolean redact)`
	`320`	`+ {`
	`321`	`+ String title = title(redact);`
`301`	`322`	`String description = description();`
`302`	`323`	`return (title.isEmpty())`
`303`	`324`	`? String.format("%s (%s)\n%s", getClass().getSimpleName(), cost(), description).stripTrailing()`
`@@ -309,7 +330,7 @@ public final String toString()`
`309`	`330`	`* The information is included in the output of {@link #toString()} and {@link #toStringRecursive()}.`
`310`	`331`	`* It is up to subclasses to implement it.`
`311`	`332`	`*/`
`312`		`- protected String title()`
	`333`	`+ protected String title(boolean redact)`
`313`	`334`	`{`
`314`	`335`	`return "";`
`315`	`336`	`}`
`@@ -345,7 +366,7 @@ protected String description()`
`345`	`366`	`public final Plan optimize()`
`346`	`367`	`{`
`347`	`368`	`if (logger.isTraceEnabled())`
`348`		`- logger.trace("Optimizing plan:\n{}", this.toStringRecursive());`
	`369`	`+ logger.trace("Optimizing plan:\n{}", toStringRecursive());`
`349`	`370`
`350`	`371`	`Plan bestPlanSoFar = this;`
`351`	`372`	`List<Leaf> leaves = nodesOfType(Leaf.class);`
`@@ -796,7 +817,7 @@ public IndexScan(Factory factory, int id, Expression predicate, long matchingKey`
`796`	`817`	`}`
`797`	`818`
`798`	`819`	`@Override`
`799`		`- protected final String title()`
	`820`	`+ protected final String title(boolean redact)`
`800`	`821`	`{`
`801`	`822`	`return String.format("of %s (sel: %.9f, step: %.1f)",`
`802`	`823`	`getIndexName(), selectivity(), access.meanDistance());`
`@@ -1689,9 +1710,9 @@ protected Filter withAccess(Access access)`
`1689`	`1710`	`}`
`1690`	`1711`
`1691`	`1712`	`@Override`
`1692`		`- protected String title()`
	`1713`	`+ protected String title(boolean redact)`
`1693`	`1714`	`{`
`1694`		`- return String.format("%s (sel: %.9f)", filter, selectivity() / source.get().selectivity());`
	`1715`	`+ return String.format("%s (sel: %.9f)", filter.toCQLString(redact), selectivity() / source.get().selectivity());`
`1695`	`1716`	`}`
`1696`	`1717`	`}`
`1697`	`1718`
`@@ -1757,7 +1778,7 @@ protected RowsIteration withAccess(Access access)`
`1757`	`1778`	`}`
`1758`	`1779`
`1759`	`1780`	`@Override`
`1760`		`- protected String title()`
	`1781`	`+ protected String title(boolean redact)`
`1761`	`1782`	`{`
`1762`	`1783`	`return "" + limit;`
`1763`	`1784`	`}`
Original file line number	Diff line number	Diff line change
`@@ -419,7 +419,7 @@ Plan buildPlan()`
`419`	`419`
`420`	`420`	`if (Tracing.isTracing())`
`421`	`421`	`{`
`422`		`- Tracing.trace("Query execution plan:\n" + plan.toStringRecursive());`
	`422`	`+ Tracing.trace("Query execution plan:\n" + plan.toStringRecursive(false));`
`423`	`423`	`List<Plan.IndexScan> origIndexScans = keysIterationPlan.nodesOfType(Plan.IndexScan.class);`
`424`	`424`	`List<Plan.IndexScan> selectedIndexScans = plan.nodesOfType(Plan.IndexScan.class);`
`425`	`425`	`Tracing.trace("Selecting {} {} of {} out of {} indexes",`