Merge pull request #4158 from rgsl888prabhu/4152_merge_issue_with_emp…

…ty_table [REVIEW] Fix merge issue with empty table return if one of the two tables are empty
rapidsai · Feb 14, 2020 · 8e6cbba · 8e6cbba
2 parents 16ebc35 + e7b325a
commit 8e6cbba
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -105,6 +105,7 @@
 - PR #4125 Fix type enum to account for added Dictionary type in `types.hpp`
 - PR #4137 Update Java for mutating fill and rolling window changes
 - PR #4141 Fix NVStrings test_convert failure in 10.2 build
+- PR #4158 Fix merge issue with empty table return if one of the two tables are empty
 
 
 # cuDF 0.12.0 (04 Feb 2020)

diff --git a/cpp/src/table/table.cpp b/cpp/src/table/table.cpp
@@ -45,7 +45,7 @@ table::table(std::vector<std::unique_ptr<column>>&& columns)
 
 // Copy the contents of a `table_view`
 table::table(table_view view, cudaStream_t stream,
-             rmm::mr::device_memory_resource* mr) {
+             rmm::mr::device_memory_resource* mr) : _num_rows{view.num_rows()} {
   _columns.reserve(view.num_columns());
   for (auto const& c : view) {
     _columns.emplace_back(std::make_unique<column>(c, stream, mr));

diff --git a/cpp/tests/merge/merge_test.cpp b/cpp/tests/merge/merge_test.cpp
@@ -46,6 +46,31 @@ class MergeTest_ : public cudf::test::BaseFixture {};
 
 TYPED_TEST_CASE(MergeTest_, cudf::test::FixedWidthTypes);
 
+TYPED_TEST(MergeTest_, MergeIsZeroWhenShouldNotBeZero) {
+    using columnFactoryT = cudf::test::fixed_width_column_wrapper<TypeParam>;
+
+    columnFactoryT leftColWrap1{1,2,3,4,5};
+    columnFactoryT rightColWrap1{};
+
+    std::vector<cudf::size_type> key_cols{0};
+    std::vector<cudf::order> column_order;
+    column_order.push_back(cudf::order::ASCENDING);
+    std::vector<cudf::null_order> null_precedence(column_order.size(), cudf::null_order::AFTER);
+
+    cudf::table_view left_view{{leftColWrap1}};
+    cudf::table_view right_view{{rightColWrap1}};
+    cudf::table_view expected{{leftColWrap1}};
+
+    auto result = cudf::experimental::merge({left_view, right_view},
+                                           key_cols,
+                                           column_order,
+                                           null_precedence);
+
+    int expected_len = 5;
+    ASSERT_EQ(result->num_rows(), expected_len);
+    cudf::test::expect_tables_equal(expected, result->view());
+}
+
 TYPED_TEST(MergeTest_, MismatchedNumColumns) {
     using columnFactoryT = cudf::test::fixed_width_column_wrapper<TypeParam>;
 

diff --git a/cpp/tests/table/table_tests.cpp b/cpp/tests/table/table_tests.cpp
@@ -51,6 +51,23 @@ TEST_F(TableTest, EmptyColumnedTable)
     EXPECT_EQ(input.num_columns(), expected);
 }
 
+TEST_F(TableTest, ValidateConstructorTableViewToTable)
+{
+    column_wrapper <int8_t>  col1{{1,2,3,4}};
+    column_wrapper <int8_t>  col2{{1,2,3,4}};
+
+    CVector cols;
+    cols.push_back(col1.release());
+    cols.push_back(col2.release());
+
+    Table input_table(std::move(cols));
+
+    Table out_table(input_table.view());
+
+    EXPECT_EQ(input_table.num_columns(), out_table.num_columns());
+    EXPECT_EQ(input_table.num_rows(), out_table.num_rows());
+}
+
 TEST_F(TableTest, GetTableWithSelectedColumns)
 {
   column_wrapper <int8_t>  col1{{1,2,3,4}};