!1088 Memory Connector: fix column is null predicate query

Merge pull request !1088 from farhan3/mem-con-fix-null-val

Author: it-is-a-robot

presto-memory/pom.xml

@@ -137,6 +137,12 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>io.hetu.core</groupId>
+            <artifactId>presto-tpcds</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>io.airlift.tpch</groupId>
             <artifactId>tpch</artifactId>

presto-memory/src/main/java/io/prestosql/plugin/memory/data/LogicalPart.java

@@ -283,6 +283,11 @@ List<Page> getPages(TupleDomain<ColumnHandle> predicate)
             int expressionColumnIndex = ((MemoryColumnHandle) e.getKey()).getColumnIndex();
             List<Range> ranges = ((SortedRangeSet) e.getValue().getValues()).getOrderedRanges();
 
+            // e.g. column=null
+            if (ranges.isEmpty()) {
+                continue;
+            }
+
             if (minMaxIdx.containsKey(expressionColumnIndex)) {
                 minmaxChannelsToRangesMap.put(expressionColumnIndex, ranges);
             }
@@ -341,6 +346,11 @@ List<Page> getPages(
                             noMatches++;
                         }
                     }
+                    else {
+                        // the lookup value isn't comparable, we can't do filtering, e.g. if it's null
+                        LOG.warn("Lookup value is not Comparable. MinMax index could not be used.");
+                        return getPages();
+                    }
                 }
                 else {
                     // <, <=, >=, >, BETWEEN
@@ -368,6 +378,11 @@ List<Page> getPages(
                                 }
                             }
                         }
+                        else {
+                            // the lookup value isn't comparable, we can't do filtering, e.g. if it's null
+                            LOG.warn("Lookup value is not Comparable. MinMax index could not be used.");
+                            return getPages();
+                        }
                     }
                     else if (!highBoundless && lowBoundless) {
                         // <= or <
@@ -388,6 +403,11 @@ else if (!highBoundless && lowBoundless) {
                                 }
                             }
                         }
+                        else {
+                            // the lookup value isn't comparable, we can't do filtering, e.g. if it's null
+                            LOG.warn("Lookup value is not Comparable. MinMax index could not be used.");
+                            return getPages();
+                        }
                     }
                     else if (!highBoundless && !lowBoundless) {
                         // BETWEEN
@@ -401,6 +421,11 @@ else if (!highBoundless && !lowBoundless) {
                                 noMatches++;
                             }
                         }
+                        else {
+                            // the lookup value isn't comparable, we can't do filtering, e.g. if it's null
+                            LOG.warn("Lookup value is not Comparable. MinMax index could not be used.");
+                            return getPages();
+                        }
                     }
                 }
             }

presto-memory/src/test/java/io/prestosql/plugin/memory/MemoryQueryRunner.java

@@ -21,6 +21,7 @@
 import io.hetu.core.filesystem.HetuFileSystemClientPlugin;
 import io.hetu.core.metastore.HetuMetastorePlugin;
 import io.prestosql.Session;
+import io.prestosql.plugin.tpcds.TpcdsPlugin;
 import io.prestosql.plugin.tpch.TpchPlugin;
 import io.prestosql.tests.DistributedQueryRunner;
 
@@ -122,6 +123,8 @@ public static DistributedQueryRunner createQueryRunner(int nodes, Map<String, St
 
             queryRunner.installPlugin(new TpchPlugin());
             queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of());
+            queryRunner.installPlugin(new TpcdsPlugin());
+            queryRunner.createCatalog("tpcds", "tpcds", ImmutableMap.of());
 
             if (createTpchTables) {
                 copyTpchTables(queryRunner, "tpch", TINY_SCHEMA_NAME, session, TpchTable.getTables());

presto-memory/src/test/java/io/prestosql/plugin/memory/TestMemorySelection.java

@@ -53,6 +53,24 @@ public void dropAllTables()
         }
     }
 
+    @Test
+    public void testNullSelect()
+    {
+        getQueryRunner().execute("CREATE TABLE test_null_noidx AS SELECT * FROM tpcds.tiny.customer");
+        getQueryRunner().execute("CREATE TABLE test_null_sort WITH (sorted_by=ARRAY['c_birth_year']) AS SELECT * FROM tpcds.tiny.customer");
+        getQueryRunner().execute("CREATE TABLE test_null_idx WITH (index_columns=ARRAY['c_birth_year']) AS SELECT * FROM tpcds.tiny.customer");
+
+        Object expectedCount = getSingleResult("SELECT count(*) FROM tpcds.tiny.customer where c_birth_year is null");
+        Object noidxCount = getSingleResult("SELECT count(*) FROM test_null_noidx where c_birth_year is null");
+        Object sortCount = getSingleResult("SELECT count(*) FROM test_null_sort where c_birth_year is null");
+        Object idxCount = getSingleResult("SELECT count(*) FROM test_null_idx where c_birth_year is null");
+
+        assertTrue((long) expectedCount > 0, "expected null count is 0, testcase should be updated");
+        assertEquals(noidxCount, expectedCount);
+        assertEquals(sortCount, expectedCount);
+        assertEquals(idxCount, expectedCount);
+    }
+
     @Test
     public void testSortedBySelect()
     {