[GitHub] [doris] morrySnow commented on a diff in pull request #13091: [feature](nereids) dump physical tree and memo

[GitBox <gi...@apache.org>]

morrySnow commented on code in PR #13091:
URL: https://github.com/apache/doris/pull/13091#discussion_r990582108


##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -200,6 +200,7 @@ public class SessionVariable implements Serializable, Writable {
     public static final String ENABLE_NEREIDS_RUNTIME_FILTER = "enable_nereids_runtime_filter";
 
     public static final String NEREIDS_STAR_SCHEMA_SUPPORT = "nereids_star_schema_support";
+    public static final String ENABLE_NEREIDS_DEBUG = "enable_nereids_debug";

Review Comment:
   debug -> trace



##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatsDeriveResult.java:
##########
@@ -146,4 +146,13 @@ public StatsDeriveResult merge(StatsDeriveResult other) {
     public StatsDeriveResult copy() {
         return new StatsDeriveResult(this);
     }
+
+    @Override
+    public String toString() {
+        StringBuilder builder = new StringBuilder();
+        builder.append(" stats=(rows:").append(rowCount)
+                .append(" isReduced:").append(isReduced)
+                .append(" width:").append(width).append(")");

Review Comment:
   i think we'd better to keep string format like other class.
   ```
   xxx (k = v, ...)
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashJoin.java:
##########
@@ -88,6 +88,16 @@ public <R, C> R accept(PlanVisitor<R, C> visitor, C context) {
 
     @Override
     public String toString() {
+        if (statsDeriveResult != null) {
+            return Utils.toSqlString("PhysicalHashJoin",
+                    "type", joinType,
+                    "hashJoinCondition", hashJoinConjuncts,
+                    "otherJoinCondition", otherJoinCondition,
+                    "estimateRows", statsDeriveResult.getRowCount(),
+                    "isReduced", statsDeriveResult.isReduced,
+                    "width", statsDeriveResult.width

Review Comment:
   why print stats info in plans? i don't think this is a good idea



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org