From 16dc5a52a888603ad73e181bf672db32d6941ae5 Mon Sep 17 00:00:00 2001 From: ILuffZhe Date: Fri, 7 Feb 2025 19:37:22 +0800 Subject: [PATCH] [CALCITE-6817] Add string representation of default nulls direction for RelNode --- .../org/apache/calcite/plan/RelOptUtil.java | 18 ++++- .../apache/calcite/rel/RelFieldCollation.java | 11 +++ .../org/apache/calcite/rel/RelWriter.java | 9 +++ .../org/apache/calcite/rel/core/Sort.java | 6 +- .../rel/externalize/RelWriterImpl.java | 11 +++ .../apache/calcite/test/RelBuilderTest.java | 67 +++++++++++++++++++ .../calcite/adapter/druid/DruidQuery.java | 6 +- .../org/apache/calcite/test/Matchers.java | 12 ++++ 8 files changed, 135 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java index 0c73d0c00e1..01e712c9007 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java @@ -2442,20 +2442,32 @@ public static String toString(final RelNode rel) { return toString(rel, SqlExplainLevel.EXPPLAN_ATTRIBUTES); } + /** + * Converts a relational expression to a string, showing just basic + * attributes, and doesn't expand detail info for {@code rel}. + */ + public static String toString( + final RelNode rel, + SqlExplainLevel detailLevel) { + return toString(rel, detailLevel, false); + } + /** * Converts a relational expression to a string; - * returns null if and only if {@code rel} is null. + * returns null if and only if {@code rel} is null, + * returns expanded detail info for {@code rel} if {@code expand} is true. */ public static @PolyNull String toString( final @PolyNull RelNode rel, - SqlExplainLevel detailLevel) { + SqlExplainLevel detailLevel, + boolean expand) { if (rel == null) { return null; } final StringWriter sw = new StringWriter(); final RelWriter planWriter = new RelWriterImpl( - new PrintWriter(sw), detailLevel, false); + new PrintWriter(sw), detailLevel, false, expand); rel.explain(planWriter); return sw.toString(); } diff --git a/core/src/main/java/org/apache/calcite/rel/RelFieldCollation.java b/core/src/main/java/org/apache/calcite/rel/RelFieldCollation.java index 9359f92230d..985ae902cac 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelFieldCollation.java +++ b/core/src/main/java/org/apache/calcite/rel/RelFieldCollation.java @@ -329,4 +329,15 @@ public String shortString() { return direction.shortString; } } + + public String fullString() { + switch (nullDirection) { + case FIRST: + return direction.shortString + "-nulls-first"; + case LAST: + return direction.shortString + "-nulls-last"; + default: + return direction.shortString; + } + } } diff --git a/core/src/main/java/org/apache/calcite/rel/RelWriter.java b/core/src/main/java/org/apache/calcite/rel/RelWriter.java index 17b44227f0e..6af6e111787 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelWriter.java +++ b/core/src/main/java/org/apache/calcite/rel/RelWriter.java @@ -85,4 +85,13 @@ default RelWriter itemIf(String term, @Nullable Object value, boolean condition) default boolean nest() { return false; } + + /** + * Returns whether the writer needs to expand node's detail information when printing plan. + * For example, LogicalSort(sort0=[$0], dir0=[ASC]) will be expanded to + * LogicalSort(sort0=[$0], dir0=[ASC-nulls-last]). + */ + default boolean expand() { + return false; + } } diff --git a/core/src/main/java/org/apache/calcite/rel/core/Sort.java b/core/src/main/java/org/apache/calcite/rel/core/Sort.java index 5f83f32c2ef..9271a621071 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Sort.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Sort.java @@ -269,7 +269,11 @@ public List getSortExps() { } for (Ord ord : Ord.zip(collation.getFieldCollations())) { - pw.item("dir" + ord.i, ord.e.shortString()); + if (!pw.expand()) { + pw.item("dir" + ord.i, ord.e.shortString()); + } else { + pw.item("dir" + ord.i, ord.e.fullString()); + } } } pw.itemIf("offset", offset, offset != null); diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java index 9df7a3c27ea..8d52ffbe380 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelWriterImpl.java @@ -41,6 +41,7 @@ public class RelWriterImpl implements RelWriter { protected final PrintWriter pw; protected final SqlExplainLevel detailLevel; protected final boolean withIdPrefix; + protected final boolean expand; protected final Spacer spacer = new Spacer(); private final List> values = new ArrayList<>(); @@ -53,9 +54,15 @@ public RelWriterImpl(PrintWriter pw) { public RelWriterImpl( PrintWriter pw, SqlExplainLevel detailLevel, boolean withIdPrefix) { + this(pw, detailLevel, withIdPrefix, false); + } + public RelWriterImpl( + PrintWriter pw, SqlExplainLevel detailLevel, + boolean withIdPrefix, boolean expand) { this.pw = pw; this.detailLevel = detailLevel; this.withIdPrefix = withIdPrefix; + this.expand = expand; } //~ Methods ---------------------------------------------------------------- @@ -180,4 +187,8 @@ public String simple() { buf.append(")"); return buf.toString(); } + + @Override public boolean expand() { + return this.expand; + } } diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index 3b0cc5f9863..ba2c10c7019 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -124,6 +124,7 @@ import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import static org.apache.calcite.test.Matchers.hasExpandedTree; import static org.apache.calcite.test.Matchers.hasFieldNames; import static org.apache.calcite.test.Matchers.hasHints; import static org.apache.calcite.test.Matchers.hasTree; @@ -4064,6 +4065,72 @@ private static RelBuilder assertSize(RelBuilder b, assertThat(root, hasTree(expected)); } + /** Test case for [CALCITE-6817] + * Add string representation of default nulls direction for RelNode. */ + @Test void testDescWithDefaultNullDirection() { + // Equivalent SQL: + // SELECT * + // FROM emp + // ORDER BY empno DESC + final RelBuilder builder = RelBuilder.create(config().build()); + final RelNode root = + builder.scan("EMP") + .sort(builder.desc(builder.field(0))) + .build(); + final String expected = + "LogicalSort(sort0=[$0], dir0=[DESC])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + final String expectedExpanded = + "LogicalSort(sort0=[$0], dir0=[DESC-nulls-first])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + assertThat(root, hasTree(expected)); + assertThat(root, hasExpandedTree(expectedExpanded)); + + // Equivalent SQL: + // SELECT * + // FROM emp + // ORDER BY empno DESC NULLS FIRST + final RelNode root2 = + builder.scan("EMP") + .sort(builder.nullsFirst(builder.desc(builder.field(0)))) + .build(); + assertThat(root2, hasTree(expected)); + assertThat(root2, hasExpandedTree(expectedExpanded)); + } + + /** Test case for [CALCITE-6817] + * Add string representation of default nulls direction for RelNode. */ + @Test void testAscWithDefaultNullDirection() { + // Equivalent SQL: + // SELECT * + // FROM emp + // ORDER BY empno ASC + final RelBuilder builder = RelBuilder.create(config().build()); + final RelNode root = + builder.scan("EMP") + .sort(builder.field(0)) + .build(); + final String expected = + "LogicalSort(sort0=[$0], dir0=[ASC])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + final String expectedExpanded = + "LogicalSort(sort0=[$0], dir0=[ASC-nulls-last])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + assertThat(root, hasTree(expected)); + assertThat(root, hasExpandedTree(expectedExpanded)); + + // Equivalent SQL: + // SELECT * + // FROM emp + // ORDER BY empno ASC NULLS LAST + final RelNode root2 = + builder.scan("EMP") + .sort(builder.nullsLast(builder.field(0))) + .build(); + assertThat(root2, hasTree(expected)); + assertThat(root2, hasExpandedTree(expectedExpanded)); + } + @Test void testLimit() { // Equivalent SQL: // SELECT * diff --git a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java index 1012c717ce1..a393d0e0a66 100644 --- a/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java +++ b/druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java @@ -563,7 +563,11 @@ public DruidTable getDruidTable() { } for (Ord ord : Ord.zip(sort.collation.getFieldCollations())) { - pw.item("dir" + ord.i, ord.e.shortString()); + if (!pw.expand()) { + pw.item("dir" + ord.i, ord.e.shortString()); + } else { + pw.item("dir" + ord.i, ord.e.fullString()); + } } pw.itemIf("fetch", sort.fetch, sort.fetch != null); } else { diff --git a/testkit/src/main/java/org/apache/calcite/test/Matchers.java b/testkit/src/main/java/org/apache/calcite/test/Matchers.java index 635750c46a9..d410affe732 100644 --- a/testkit/src/main/java/org/apache/calcite/test/Matchers.java +++ b/testkit/src/main/java/org/apache/calcite/test/Matchers.java @@ -21,6 +21,7 @@ import org.apache.calcite.rel.RelValidityChecker; import org.apache.calcite.rel.hint.Hintable; import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.SqlExplainLevel; import org.apache.calcite.util.TestUtil; import org.apache.calcite.util.Util; @@ -236,6 +237,17 @@ public static Matcher hasTree(final String value) { }); } + /** + * Basically similar to {@link #hasTree(String)}, except for expanding RelNode's detail info. + * For example, default nulls direction will be compared through this. + */ + public static Matcher hasExpandedTree(final String value) { + return compose(Is.is(value), input -> { + // Convert RelNode to a string with Linux line-endings + return Util.toLinux(RelOptUtil.toString(input, SqlExplainLevel.EXPPLAN_ATTRIBUTES, true)); + }); + } + /** * Creates a Matcher that matches a {@link RelNode} if its field * names, converting to a list, are equal to the given {@code value}.