remove some common UTs vs Presto

yma11 · May 5, 2024 · 0f8790c · 0f8790c
1 parent 72768ea
commit 0f8790c
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 321 deletions.
diff --git a/velox/docs/functions/spark/map.rst b/velox/docs/functions/spark/map.rst
@@ -29,7 +29,7 @@ Map Functions
 
 .. spark:function:: map_from_entries(array(struct(K,V))) -> map(K,V)
 
-    Converts an array of entries (key value struct types) to a map of values. All elements in keys should not be null.
+    Returns a map created from the given array of entries. Keys are not allowed to be null or to contain nulls.
     If null entry exists in the array, return null for this whole array.::
 
         SELECT map_from_entries(array(struct(1, 'a'), struct(2, 'null'))); -- {1 -> 'a', 2 -> 'null'}

diff --git a/velox/functions/lib/MapFromEntries.cpp b/velox/functions/lib/MapFromEntries.cpp
@@ -31,7 +31,7 @@ static const char* kIndeterminateKeyErrorMessage =
 static const char* kErrorMessageEntryNotNull = "map entry cannot be null";
 
 /// @tparam throwForNull If true, will return null if input array is null or has
-/// null entry (Spark's behavior), instead of throw execeptions(Presto's
+/// null entries (Spark's behavior), instead of throwing exceptions (Presto's
 /// behavior).
 template <bool throwForNull>
 class MapFromEntriesFunction : public exec::VectorFunction {
@@ -132,7 +132,7 @@ class MapFromEntriesFunction : public exec::VectorFunction {
     });
 
     auto resetSize = [&](vector_size_t row) { mutableSizes[row] = 0; };
-    auto nulls = allocateNulls(decodedRowVector->size(), context.pool());
+    auto nulls = allocateNulls(rows.size(), context.pool());
     auto* mutableNulls = nulls->asMutable<uint64_t>();
 
     if (decodedRowVector->mayHaveNulls() || keyVector->mayHaveNulls() ||
@@ -146,7 +146,6 @@ class MapFromEntriesFunction : public exec::VectorFunction {
           const bool isMapEntryNull = decodedRowVector->isNullAt(offset + i);
           if (isMapEntryNull) {
             if constexpr (!throwForNull) {
-              // Spark: For nulls in the top level row vector, return null.
               bits::setNull(mutableNulls, row);
               resetSize(row);
               break;
@@ -229,28 +228,19 @@ class MapFromEntriesFunction : public exec::VectorFunction {
 
     // For Presto, need construct map vector based on input nulls for possible
     // outer expression like try(). For Spark, use the updated nulls.
-    std::shared_ptr<MapVector> mapVector;
     if constexpr (throwForNull) {
-      mapVector = std::make_shared<MapVector>(
-          context.pool(),
-          outputType,
-          inputArray->nulls(),
-          rows.end(),
-          inputArray->offsets(),
-          sizes,
-          wrappedKeys,
-          wrappedValues);
-    } else {
-      mapVector = std::make_shared<MapVector>(
-          context.pool(),
-          outputType,
-          nulls,
-          rows.end(),
-          inputArray->offsets(),
-          sizes,
-          wrappedKeys,
-          wrappedValues);
+      nulls = inputArray->nulls();
     }
+    auto mapVector = std::make_shared<MapVector>(
+        context.pool(),
+        outputType,
+        nulls,
+        rows.end(),
+        inputArray->offsets(),
+        sizes,
+        wrappedKeys,
+        wrappedValues);
+
     checkDuplicateKeys(mapVector, *remianingRows, context);
     return mapVector;
   }

diff --git a/velox/functions/prestosql/tests/MapFromEntriesTest.cpp b/velox/functions/prestosql/tests/MapFromEntriesTest.cpp
@@ -17,7 +17,6 @@
 #include <optional>
 #include "velox/common/base/tests/GTestUtils.h"
 #include "velox/functions/lib/CheckDuplicateKeys.h"
-// #include "velox/functions/lib/CheckDuplicateKeys.h"
 #include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 #include "velox/vector/tests/TestingDictionaryArrayElementsFunction.h"