Fix issue 1323

kuzudb · Mar 1, 2023 · 48faead · 48faead
1 parent d151554
commit 48faead
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 3 deletions.
diff --git a/dataset/copy-fault-tests/null-pk/schema.cypher b/dataset/copy-fault-tests/null-pk/schema.cypher
@@ -0,0 +1 @@
+create node table person (fName STRING, PRIMARY KEY (fName));
diff --git a/dataset/copy-fault-tests/null-pk/vPerson.csv b/dataset/copy-fault-tests/null-pk/vPerson.csv
@@ -0,0 +1,3 @@
+alice
+
+bob
diff --git a/dataset/copy-special-char-test/vPerson.csv b/dataset/copy-special-char-test/vPerson.csv
@@ -5,3 +5,4 @@
 4|-only one ## should be recognized-|2013-05-01
 5|this is a ##plain## #string|2013-05-01
 6|this is another ##plain## #string with \|2013-05-01
+7|NA|2022-01-11
diff --git a/src/storage/copy_arrow/copy_node_arrow.cpp b/src/storage/copy_arrow/copy_node_arrow.cpp
@@ -200,6 +200,9 @@ void CopyNodeArrow::populatePKIndex(
     InMemColumn* column, HashIndexBuilder<T>* pkIndex, offset_t startOffset, uint64_t numValues) {
     for (auto i = 0u; i < numValues; i++) {
         auto offset = i + startOffset;
+        if (column->isNullAtNodeOffset(offset)) {
+            throw ReaderException("Primary key cannot be null.");
+        }
         if constexpr (std::is_same<T, int64_t>::value) {
             auto key = (int64_t*)column->getElement(offset);
             if (!pkIndex->append(*key, offset)) {
@@ -257,15 +260,15 @@ void CopyNodeArrow::putPropsOfLineIntoColumns(
                 column->setElement(nodeOffset, reinterpret_cast<uint8_t*>(&val));
             } break;
             case DOUBLE: {
-                double_t val = TypeUtils::convertStringToNumber<double_t>(data);
+                auto val = TypeUtils::convertStringToNumber<double_t>(data);
                 column->setElement(nodeOffset, reinterpret_cast<uint8_t*>(&val));
             } break;
             case FLOAT: {
                 auto val = TypeUtils::convertStringToNumber<float_t>(data);
                 column->setElement(nodeOffset, reinterpret_cast<uint8_t*>(&val));
             } break;
             case BOOL: {
-                bool val = TypeUtils::convertToBoolean(data);
+                auto val = TypeUtils::convertToBoolean(data);
                 column->setElement(nodeOffset, reinterpret_cast<uint8_t*>(&val));
             } break;
             case DATE: {

diff --git a/src/storage/copy_arrow/copy_structures_arrow.cpp b/src/storage/copy_arrow/copy_structures_arrow.cpp
@@ -187,19 +187,21 @@ arrow::Status CopyStructuresArrow::initCSVReader(
     ARROW_ASSIGN_OR_RAISE(arrow_input_stream, arrow::io::ReadableFile::Open(filePath));
     auto arrowRead = arrow::csv::ReadOptions::Defaults();
     arrowRead.block_size = CopyConstants::CSV_READING_BLOCK_SIZE;
-
     if (!copyDescription.csvReaderConfig->hasHeader) {
         arrowRead.autogenerate_column_names = true;
     }
 
     auto arrowConvert = arrow::csv::ConvertOptions::Defaults();
     arrowConvert.strings_can_be_null = true;
+    // Only the empty string is treated as NULL.
+    arrowConvert.null_values = {""};
     arrowConvert.quoted_strings_can_be_null = false;
 
     auto arrowParse = arrow::csv::ParseOptions::Defaults();
     arrowParse.delimiter = copyDescription.csvReaderConfig->delimiter;
     arrowParse.escape_char = copyDescription.csvReaderConfig->escapeChar;
     arrowParse.quote_char = copyDescription.csvReaderConfig->quoteChar;
+    arrowParse.ignore_empty_lines = false;
     arrowParse.escaping = true;
 
     ARROW_ASSIGN_OR_RAISE(

diff --git a/test/copy/copy_fault_test.cpp b/test/copy/copy_fault_test.cpp
@@ -42,6 +42,12 @@ class CopyInvalidNumberTest : public CopyFaultTest {
     }
 };
 
+class CopyNullPKTest : public CopyFaultTest {
+    std::string getInputDir() override {
+        return TestHelper::appendKuzuRootPath("dataset/copy-fault-tests/null-pk/");
+    }
+};
+
 TEST_F(CopyDuplicateIDTest, DuplicateIDsError) {
     validateCopyException(
         "COPY person FROM \"" +
@@ -154,3 +160,10 @@ TEST_F(CopyInvalidNumberTest, InvalidNumberError) {
             TestHelper::appendKuzuRootPath("dataset/copy-fault-tests/invalid-number/vMovie.csv\""),
         "Invalid number: 312abc.");
 }
+
+TEST_F(CopyNullPKTest, NullPKErrpr) {
+    validateCopyException(
+        "COPY person FROM \"" +
+            TestHelper::appendKuzuRootPath("dataset/copy-fault-tests/null-pk/vPerson.csv\""),
+        "Reader exception: Primary key cannot be null.");
+}
diff --git a/test/copy/copy_test.cpp b/test/copy/copy_test.cpp
@@ -221,6 +221,7 @@ TEST_F(CopySpecialCharTest, CopySpecialChars) {
     EXPECT_EQ("only one # should be recognized", col->readValue(4).strVal);
     EXPECT_EQ("this is a #plain# string", col->readValue(5).strVal);
     EXPECT_EQ("this is another #plain# string with \\", col->readValue(6).strVal);
+    EXPECT_EQ("NA", col->readValue(7).strVal);
 
     tableID = catalog->getReadOnlyVersion()->getTableID("organisation");
     propertyIdx = catalog->getReadOnlyVersion()->getNodeProperty(tableID, "name");