Join pushdown on case sensitive varchar columns for SqlServer

trinodb · Feb 24, 2023 · 028e4ae · 028e4ae
1 parent ddfcbd8
commit 028e4ae
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 7 deletions.
diff --git a/plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java b/plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java
@@ -998,10 +998,24 @@ protected boolean isSupportedJoinCondition(ConnectorSession session, JdbcJoinCon
             return false;
         }
 
-        // Remote database can be case insensitive.
-        return Stream.of(joinCondition.getLeftColumn(), joinCondition.getRightColumn())
+        boolean isVarcharJoinColumn = Stream.of(joinCondition.getLeftColumn(), joinCondition.getRightColumn())
                 .map(JdbcColumnHandle::getColumnType)
-                .noneMatch(type -> type instanceof CharType || type instanceof VarcharType);
+                .allMatch(type -> type instanceof CharType || type instanceof VarcharType);
+        if (isVarcharJoinColumn) {
+            JoinCondition.Operator operator = joinCondition.getOperator();
+            return switch (operator) {
+                case LESS_THAN, LESS_THAN_OR_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL -> false;
+                case EQUAL, NOT_EQUAL -> isCaseSensitiveVarchar(joinCondition.getLeftColumn()) && isCaseSensitiveVarchar(joinCondition.getRightColumn());
+                default -> false;
+            };
+        }
+
+        return true;
+    }
+
+    private boolean isCaseSensitiveVarchar(JdbcColumnHandle columnHandle)
+    {
+        return columnHandle.getJdbcTypeHandle().getCaseSensitivity().orElse(CASE_INSENSITIVE) == CASE_SENSITIVE;
     }
 
     @Override

diff --git a/...n/trino-sqlserver/src/test/java/io/trino/plugin/sqlserver/BaseSqlServerConnectorTest.java b/...n/trino-sqlserver/src/test/java/io/trino/plugin/sqlserver/BaseSqlServerConnectorTest.java
@@ -52,9 +52,9 @@ public abstract class BaseSqlServerConnectorTest
     protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
     {
         switch (connectorBehavior) {
+            case SUPPORTS_JOIN_PUSHDOWN_WITH_VARCHAR_EQUALITY:
             case SUPPORTS_PREDICATE_PUSHDOWN_WITH_VARCHAR_EQUALITY:
                 return true;
-            case SUPPORTS_JOIN_PUSHDOWN_WITH_VARCHAR_EQUALITY:
             case SUPPORTS_PREDICATE_PUSHDOWN_WITH_VARCHAR_INEQUALITY:
                 return false;
 
@@ -351,9 +351,9 @@ public void testPredicatePushdown()
             assertThat(query(joinPushdownEnabled, "SELECT c.name, n.name FROM customer c JOIN nation n ON c.custkey = n.nationkey WHERE n.name = 'POLAND'"))
                     .isFullyPushedDown();
 
-            // join on varchar columns is not pushed down
-            assertThat(query(joinPushdownEnabled, "SELECT c.name, n.name FROM customer c JOIN nation n ON c.address = n.name"))
-                    .joinIsNotFullyPushedDown();
+            // join on varchar columns
+            assertThat(query(joinPushdownEnabled, "SELECT n.name, n2.regionkey FROM nation n JOIN nation n2 ON n.name = n2.name"))
+                    .isFullyPushedDown();
         }
     }
 
@@ -385,6 +385,36 @@ public void testNoPushdownOnCaseInsensitiveVarcharColumn()
         }
     }
 
+    @Test
+    public void testNoJoinPushdownOnCaseInsensitiveVarcharColumn()
+    {
+        // if collation on column is caseinsensitive we should not apply join pushdown
+        try (TestTable testTable = new TestTable(
+                onRemoteDatabase(),
+                "test_join_collate",
+                "(collate_column_1 varchar(25) COLLATE Latin1_General_CI_AS, collate_column_2 varchar(25) COLLATE Latin1_General_CI_AS)",
+                List.of("'Collation', 'Collation'", "'collation', 'collation'"))) {
+            assertThat(query(format("SELECT n.collate_column_1, n2.collate_column_2 FROM %1$s n JOIN %1$s n2 ON n.collate_column_1 = n2.collate_column_2", testTable.getName())))
+                    .matches("VALUES " +
+                            "((CAST('Collation' AS varchar(25))), (CAST('Collation' AS varchar(25)))), " +
+                            "((CAST('collation' AS varchar(25))), (CAST('collation' AS varchar(25))))")
+                    .joinIsNotFullyPushedDown();
+            assertThat(query(format("SELECT n.collate_column_1, n2.collate_column_2 FROM %1$s n JOIN %1$s n2 ON n.collate_column_1 != n2.collate_column_2", testTable.getName())))
+                    .matches("VALUES " +
+                            "((CAST('collation' AS varchar(25))), (CAST('Collation' AS varchar(25)))), " +
+                            "((CAST('Collation' AS varchar(25))), (CAST('collation' AS varchar(25))))")
+                    .joinIsNotFullyPushedDown();
+            assertThat(query(format("SELECT n.collate_column_1, n2.collate_column_2 FROM %1$s n JOIN %1$s n2 ON n.collate_column_1 = n2.collate_column_2 WHERE n.collate_column_1 = 'Collation'", testTable.getName())))
+                    .matches("VALUES " +
+                            "((CAST('Collation' AS varchar(25))), (CAST('Collation' AS varchar(25))))")
+                    .joinIsNotFullyPushedDown();
+            assertThat(query(format("SELECT n.collate_column_1, n2.collate_column_2 FROM %1$s n JOIN %1$s n2 ON n.collate_column_1 != n2.collate_column_2 WHERE n.collate_column_1 != 'collation'", testTable.getName())))
+                    .matches("VALUES " +
+                            "((CAST('Collation' AS varchar(25))), (CAST('collation' AS varchar(25))))")
+                    .joinIsNotFullyPushedDown();
+        }
+    }
+
     @Override
     @Test
     public void testDeleteWithVarcharInequalityPredicate()