Fix crash when map subfield is pruned but we still want to add non-nu…

…ll filter on its keys Differential Revision: D56642855
Yuhta · Apr 26, 2024 · 8ea17ca · 8ea17ca
1 parent 97160cd
commit 8ea17ca
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 3 deletions.
diff --git a/velox/connectors/hive/HiveConnectorUtil.cpp b/velox/connectors/hive/HiveConnectorUtil.cpp
@@ -323,9 +323,10 @@ namespace {
 
 void filterOutNullMapKeys(const Type& rootType, common::ScanSpec& rootSpec) {
   rootSpec.visit(rootType, [](const Type& type, common::ScanSpec& spec) {
-    if (type.isMap()) {
-      spec.childByName(common::ScanSpec::kMapKeysFieldName)
-          ->addFilter(common::IsNotNull());
+    if (type.isMap() && !spec.isConstant()) {
+      auto* keys = spec.childByName(common::ScanSpec::kMapKeysFieldName);
+      VELOX_CHECK_NOT_NULL(keys);
+      keys->addFilter(common::IsNotNull());
     }
   });
 }

diff --git a/velox/connectors/hive/tests/HiveConnectorTest.cpp b/velox/connectors/hive/tests/HiveConnectorTest.cpp
@@ -417,6 +417,25 @@ TEST_F(HiveConnectorTest, makeScanSpec_filterPartitionKey) {
   ASSERT_FALSE(scanSpec->childByName("ds")->projectOut());
 }
 
+TEST_F(HiveConnectorTest, makeScanSpec_prunedMapNonNullMapKey) {
+  auto rowType =
+      ROW({"c0"},
+          {ROW(
+              {{"c0c0", MAP(BIGINT(), MAP(BIGINT(), BIGINT()))},
+               {"c0c1", BIGINT()}})});
+  auto scanSpec = makeScanSpec(
+      rowType,
+      groupSubfields(makeSubfields({"c0.c0c1"})),
+      {},
+      nullptr,
+      {},
+      {},
+      pool_.get());
+  auto* c0 = scanSpec->childByName("c0");
+  ASSERT_EQ(c0->children().size(), 2);
+  ASSERT_TRUE(c0->childByName("c0c0")->isConstant());
+}
+
 TEST_F(HiveConnectorTest, extractFiltersFromRemainingFilter) {
   core::QueryCtx queryCtx;
   exec::SimpleExpressionEvaluator evaluator(&queryCtx, pool_.get());

diff --git a/velox/dwio/common/ScanSpec.h b/velox/dwio/common/ScanSpec.h
@@ -410,6 +410,10 @@ class ScanSpec {
 template <typename F>
 void ScanSpec::visit(const Type& type, F&& f) {
   f(type, *this);
+  if (isConstant()) {
+    // Child specs are not populated in this case.
+    return;
+  }
   switch (type.kind()) {
     case TypeKind::ROW:
       for (auto& child : children_) {