Make split_to_map throw when more than one delimiter is found

Summary: Presto throws when there are more than one delimiter so Velox should do the same. Differential Revision: D60995768
DanielHunte · Aug 8, 2024 · 86467e0 · 86467e0
1 parent c6c67f1
commit 86467e0
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 16 deletions.
diff --git a/velox/functions/prestosql/SplitToMap.h b/velox/functions/prestosql/SplitToMap.h
@@ -88,11 +88,9 @@ struct SplitToMapFunction {
         Status::UserError(
             "entryDelimiter and keyValueDelimiter must not be the same: {}",
             entryDelimiter));
-
     if (input.empty()) {
       return Status::OK();
     }
-
     size_t pos = 0;
 
     folly::F14FastMap<std::string_view, std::string_view> keyValuePairs;
@@ -140,7 +138,12 @@ struct SplitToMapFunction {
     VELOX_RETURN_IF(
         delimiterPos == std::string::npos,
         Status::UserError(
-            "Key-value delimiter must appear exactly once in each entry. Bad input: '{}'",
+            "No delimiter found. Key-value delimiter must appear exactly once in each entry. Bad input: '{}'",
+            entry));
+    VELOX_RETURN_IF(
+        entry.find(keyValueDelimiter, delimiterPos + 1) != std::string::npos,
+        Status::UserError(
+            "More than one delimiter found. Key-value delimiter must appear exactly once in each entry. Bad input: '{}'",
             entry));
 
     const auto key = std::string_view(entry.data(), delimiterPos);

diff --git a/velox/functions/prestosql/tests/SplitToMapTest.cpp b/velox/functions/prestosql/tests/SplitToMapTest.cpp
@@ -88,15 +88,17 @@ TEST_F(SplitToMapTest, invalidInput) {
   });
 
   auto splitToMap = [&](const std::string& entryDelimiter,
-                        const std::string& keyValueDelimiter) {
+                        const std::string& keyValueDelimiter,
+                        RowVectorPtr data) {
     evaluate(
         fmt::format(
             "split_to_map(c0, '{}', '{}')", entryDelimiter, keyValueDelimiter),
         data);
   };
 
   auto trySplitToMap = [&](const std::string& entryDelimiter,
-                           const std::string& keyValueDelimiter) {
+                           const std::string& keyValueDelimiter,
+                           RowVectorPtr data) {
     SCOPED_TRACE(fmt::format("{} {}", entryDelimiter, keyValueDelimiter));
     auto result = evaluate(
         fmt::format(
@@ -109,24 +111,34 @@ TEST_F(SplitToMapTest, invalidInput) {
   };
 
   VELOX_ASSERT_THROW(
-      splitToMap(".", "."),
+      splitToMap(".", ".", data),
       "entryDelimiter and keyValueDelimiter must not be the same");
-  trySplitToMap(".", ".");
+  trySplitToMap(".", ".", data);
 
-  VELOX_ASSERT_THROW(splitToMap(".", ""), "keyValueDelimiter is empty");
-  trySplitToMap(".", "");
+  VELOX_ASSERT_THROW(splitToMap(".", "", data), "keyValueDelimiter is empty");
+  trySplitToMap(".", "", data);
 
-  VELOX_ASSERT_THROW(splitToMap("", "."), "entryDelimiter is empty");
-  trySplitToMap("", ".");
+  VELOX_ASSERT_THROW(splitToMap("", ".", data), "entryDelimiter is empty");
+  trySplitToMap("", ".", data);
 
   VELOX_ASSERT_THROW(
-      splitToMap(":", ","),
-      "Key-value delimiter must appear exactly once in each entry. Bad input: '1'");
-  trySplitToMap(":", ",");
+      splitToMap(":", ",", data),
+      "No delimiter found. Key-value delimiter must appear exactly once in each entry. Bad input: '1'");
+  trySplitToMap(":", ",", data);
 
   VELOX_ASSERT_THROW(
-      splitToMap(",", ":"), "Duplicate keys (1) are not allowed.");
-  trySplitToMap(",", ":");
+      splitToMap(",", ":", data), "Duplicate keys (1) are not allowed.");
+  trySplitToMap(",", ":", data);
+
+  data = makeRowVector({
+      makeFlatVector<std::string>({
+          "1::10,2:20,1:30",
+      }),
+  });
+  VELOX_ASSERT_THROW(
+      splitToMap(",", ":", data),
+      "More than one delimiter found. Key-value delimiter must appear exactly once in each entry");
+  trySplitToMap(",", ":", data);
 }
 
 TEST_F(SplitToMapTest, lambda) {