Normalize symbol names

Replace any sequence of non-alphanumeric with underscore.
trinodb · Mar 25, 2024 · d24ed81 · d24ed81
1 parent 7e84eea
commit d24ed81
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 18 deletions.
diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/SymbolAllocator.java b/core/trino-main/src/main/java/io/trino/sql/planner/SymbolAllocator.java
@@ -13,6 +13,7 @@
  */
 package io.trino.sql.planner;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.primitives.Ints;
 import io.trino.spi.type.Type;
 import io.trino.sql.analyzer.Field;
@@ -30,6 +31,11 @@
 
 public class SymbolAllocator
 {
+    public static final CharMatcher EXCLUDED_CHARACTERS = CharMatcher.inRange('a', 'z')
+            .or(CharMatcher.inRange('0', '9'))
+            .negate()
+            .precomputed();
+
     private final Map<String, Symbol> symbols;
     private int nextId;
 
@@ -54,8 +60,7 @@ public Symbol newSymbol(String nameHint, Type type)
         requireNonNull(nameHint, "nameHint is null");
         requireNonNull(type, "type is null");
 
-        // TODO: workaround for the fact that QualifiedName lowercases parts
-        nameHint = nameHint.toLowerCase(ENGLISH);
+        nameHint = EXCLUDED_CHARACTERS.trimAndCollapseFrom(nameHint.toLowerCase(ENGLISH), '_');
 
         // don't strip the tail if the only _ is the first character
         int index = nameHint.lastIndexOf("_");

diff --git a/...in/src/test/java/io/trino/sql/planner/iterative/rule/TestPushProjectionIntoTableScan.java b/...in/src/test/java/io/trino/sql/planner/iterative/rule/TestPushProjectionIntoTableScan.java
@@ -155,8 +155,6 @@ public void testPushProjection()
                             Map.Entry::getValue,
                             e -> column(e.getValue(), types.get(e.getKey()))));
 
-            String name = newNames.get(dereference);
-            String name1 = newNames.get(identity);
             ruleTester.assertThat(createRule(ruleTester))
                     .withSession(MOCK_SESSION)
                     .on(p -> {
@@ -195,15 +193,7 @@ public void testPushProjection()
                                             Optional.of(ImmutableList.copyOf(expectedColumns.values())))::equals,
                                     TupleDomain.all(),
                                     expectedColumns.entrySet().stream()
-                                            .collect(toImmutableMap(Map.Entry::getKey, e -> e.getValue()::equals)),
-                                    Optional.of(PlanNodeStatsEstimate.builder()
-                                            .setOutputRowCount(42)
-                                            .addSymbolStatistics(new Symbol(UNKNOWN, name1), SymbolStatsEstimate.builder()
-                                                    .setDistinctValuesCount(33)
-                                                    .setNullsFraction(0)
-                                                    .build())
-                                            .addSymbolStatistics(new Symbol(UNKNOWN, name), SymbolStatsEstimate.unknown())
-                                            .build())::equals)));
+                                            .collect(toImmutableMap(Map.Entry::getKey, e -> e.getValue()::equals)))));
         }
     }
 

diff --git a/.../trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java b/.../trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
@@ -2152,7 +2152,7 @@ public void testProjectionPushdownExplain()
         assertExplain(
                 "EXPLAIN SELECT root.f2 FROM " + tableName,
                 "TableScan\\[table = (.*)]",
-                "root#f2 := root#f2:bigint:REGULAR");
+                "(.*) := (.*):bigint:REGULAR");
 
         Session sessionWithoutPushdown = Session.builder(getSession())
                 .setCatalogSessionProperty(getSession().getCatalog().orElseThrow(), "projection_pushdown_enabled", "false")
@@ -2178,12 +2178,12 @@ public void testProjectionPushdownNonPrimitiveTypeExplain()
         assertExplain(
                 "EXPLAIN SELECT id, _row.child, _array[1].child, _map[1] FROM " + tableName,
                 "ScanProject\\[table = (.*)]",
-                "expr(.*) := system\\.builtin\\.\\$operator\\$subscript\\(_array_.*, bigint '1'\\).0",
+                "expr(.*) := system\\.builtin\\.\\$operator\\$subscript\\(.*, bigint '1'\\).0",
                 "id(.*) := id:bigint:REGULAR",
                 // _array:array\\(row\\(child bigint\\)\\) is a symbol name, not a dereference expression.
-                "_array(.*) := _array:array\\(row\\(child bigint\\)\\):REGULAR",
-                "_map(.*) := _map:map\\(bigint, bigint\\):REGULAR",
-                "_row#child := _row#child:bigint:REGULAR");
+                "(.*) := _array:array\\(row\\(child bigint\\)\\):REGULAR",
+                "(.*) := _map:map\\(bigint, bigint\\):REGULAR",
+                "(.*) := _row#child:bigint:REGULAR");
     }
 
     @Test