Improve duplicated key error message (#9374)

Summary: So the user has more context. Differential Revision: D55779686
facebookincubator · Apr 5, 2024 · f20defe · f20defe
1 parent 776ab24
commit f20defe
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 4 deletions.
diff --git a/velox/dwio/dwrf/test/ColumnWriterTest.cpp b/velox/dwio/dwrf/test/ColumnWriterTest.cpp
@@ -1215,22 +1215,41 @@ TEST_F(ColumnWriterTest, TestMapWriterInt64Key) {
   testMapWriterNumericKey<int64_t>(/* useFlatMap */ true, /* useStruct */ true);
 }
 
+TEST_F(ColumnWriterTest, TestMapWriterDuplicatedInt64Key) {
+  using T = int64_t;
+  using b = MapBuilder<T, T>;
+
+  auto pool = memory::memoryManager()->addLeafPool();
+  auto batch = b::create(
+      *pool, {typename b::row{typename b::pair{5, 3}, typename b::pair{5, 4}}});
+
+  EXPECT_THAT(
+      ([&]() { testMapWriter<T, T>(*pool, batch, /* useFlatMap */ true); }),
+      Throws<
+          facebook::velox::dwio::common::exception::LoggedException>(Property(
+          &facebook::velox::dwio::common::exception::LoggedException::message,
+          HasSubstr("Duplicated key in map: 5"))));
+}
+
 TEST_F(ColumnWriterTest, TestMapWriterInt32Key) {
   testMapWriterNumericKey<int32_t>(/* useFlatMap */ false);
   testMapWriterNumericKey<int32_t>(/* useFlatMap */ true);
-  testMapWriterNumericKey<int32_t>(/* useFlatMap */ true, /* useStruct */ true);
+  testMapWriterNumericKey<int32_t>(
+      /* useFlatMap */ true, /* useStruct */ true);
 }
 
 TEST_F(ColumnWriterTest, TestMapWriterInt16Key) {
   testMapWriterNumericKey<int16_t>(/* useFlatMap */ false);
   testMapWriterNumericKey<int16_t>(/* useFlatMap */ true);
-  testMapWriterNumericKey<int16_t>(/* useFlatMap */ true, /* useStruct */ true);
+  testMapWriterNumericKey<int16_t>(
+      /* useFlatMap */ true, /* useStruct */ true);
 }
 
 TEST_F(ColumnWriterTest, TestMapWriterInt8Key) {
   testMapWriterNumericKey<int8_t>(/* useFlatMap */ false);
   testMapWriterNumericKey<int8_t>(/* useFlatMap */ true);
-  testMapWriterNumericKey<int8_t>(/* useFlatMap */ true, /* useStruct */ true);
+  testMapWriterNumericKey<int8_t>(
+      /* useFlatMap */ true, /* useStruct */ true);
 }
 
 TEST_F(ColumnWriterTest, TestMapWriterStringKey) {
@@ -1250,6 +1269,26 @@ TEST_F(ColumnWriterTest, TestMapWriterStringKey) {
       *pool_, batch, /* useFlatMap */ true, true, /* useStruct */ true);
 }
 
+TEST_F(ColumnWriterTest, TestMapWriterDuplicatedStringKey) {
+  using keyType = StringView;
+  using valueType = StringView;
+  using b = MapBuilder<keyType, valueType>;
+
+  auto pool = memory::memoryManager()->addLeafPool();
+  auto batch = b::create(
+      *pool_,
+      {b::row{b::pair{"2", "5"}, b::pair{"2", "8"}}}); // Duplicated key: 2
+
+  EXPECT_THAT(
+      ([&]() {
+        testMapWriter<keyType, valueType>(*pool_, batch, /* useFlatMap */ true);
+      }),
+      Throws<
+          facebook::velox::dwio::common::exception::LoggedException>(Property(
+          &facebook::velox::dwio::common::exception::LoggedException::message,
+          HasSubstr("Duplicated key in map: 2"))));
+}
+
 TEST_F(ColumnWriterTest, TestMapWriterDifferentNumericKeyValue) {
   using keyType = float;
   using valueType = int32_t;

diff --git a/velox/dwio/dwrf/writer/FlatMapColumnWriter.h b/velox/dwio/dwrf/writer/FlatMapColumnWriter.h
@@ -120,7 +120,13 @@ class ValueWriter {
 
   void addOffset(uint64_t offset, uint64_t inMapIndex) {
     if (UNLIKELY(inMapBuffer_[inMapIndex])) {
-      DWIO_RAISE("Duplicate key in map");
+      std::string duplicatedKey = "<unknown>";
+      if (keyInfo_.has_byteskey()) {
+        duplicatedKey = keyInfo_.byteskey();
+      } else if (keyInfo_.has_intkey()) {
+        duplicatedKey = std::to_string(keyInfo_.intkey());
+      }
+      DWIO_RAISE("Duplicated key in map: ", duplicatedKey);
     }
 
     ranges_.add(offset, offset + 1);