Skip to content

Commit f68d00b

Browse files
authored
Avoid leaving multi-rank execution in a bad state in cudf-polars tests (NVIDIA#23340)
If a cudf-polars query raises an exception during execution, it is possible that it can leave a dangling collective that then tears down one process. This is racy because it depends on tasks being cancelled in Python and then dropping C++ objects in a particular order. The one test in the cudf-polars tests suite that could do this is fixed on main, so backport the relevant changes (NVIDIA#23235), and un-xfail the test. Authors: - Lawrence Mitchell (https://github.com/wence-) - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Richard (Rick) Zamora (https://github.com/rjzamora) - Matthew Murray (https://github.com/Matt711) - Bradley Dice (https://github.com/bdice) - Vyas Ramasubramani (https://github.com/vyasr) URL: NVIDIA#23340
1 parent a405005 commit f68d00b

6 files changed

Lines changed: 94 additions & 31 deletions

File tree

‎cpp/include/cudf/join/join.hpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ full_join(cudf::table_view const& left_keys,
248248
*
249249
* The cross join returns the cartesian product of rows from each table.
250250
*
251+
* The result has `left.num_rows() * right.num_rows()` rows and
252+
* `left.num_columns() + right.num_columns()` columns. Either operand may have
253+
* zero columns and still contribute its row count.
254+
*
251255
* @note Warning: This function can easily cause out-of-memory errors. The size of the output is
252256
* equal to `left.num_rows() * right.num_rows()`. Use with caution.
253257
*
@@ -256,8 +260,9 @@ full_join(cudf::table_view const& left_keys,
256260
* Right b: {3, 4, 5}
257261
* Result: { a: {0, 0, 0, 1, 1, 1, 2, 2, 2}, b: {3, 4, 5, 3, 4, 5, 3, 4, 5} }
258262
* @endcode
259-
260-
* @throw cudf::logic_error if the number of columns in either `left` or `right` table is 0
263+
*
264+
* @throw std::overflow_error if `left.num_rows() * right.num_rows()` exceeds the maximum
265+
* number of rows a column can hold.
261266
*
262267
* @param left The left table
263268
* @param right The right table

‎cpp/src/join/cross_join.cu‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -19,7 +19,10 @@
1919

2020
#include <rmm/cuda_stream_view.hpp>
2121

22+
#include <cstdint>
23+
#include <limits>
2224
#include <memory>
25+
#include <stdexcept>
2326

2427
namespace cudf {
2528
namespace detail {
@@ -33,9 +36,6 @@ std::unique_ptr<cudf::table> cross_join(cudf::table_view const& left,
3336
rmm::cuda_stream_view stream,
3437
rmm::device_async_resource_ref mr)
3538
{
36-
CUDF_EXPECTS(0 != left.num_columns(), "Left table is empty");
37-
CUDF_EXPECTS(0 != right.num_columns(), "Right table is empty");
38-
3939
// If left or right table has no rows, return an empty table with all columns
4040
if ((0 == left.num_rows()) || (0 == right.num_rows())) {
4141
auto empty_left_columns = empty_like(left)->release();
@@ -46,6 +46,12 @@ std::unique_ptr<cudf::table> cross_join(cudf::table_view const& left,
4646
return std::make_unique<table>(std::move(empty_left_columns));
4747
}
4848

49+
auto const out_num_rows =
50+
static_cast<int64_t>(left.num_rows()) * static_cast<int64_t>(right.num_rows());
51+
CUDF_EXPECTS(out_num_rows <= std::numeric_limits<size_type>::max(),
52+
"Cross join result exceeds the column size limit",
53+
std::overflow_error);
54+
4955
// Repeat left table
5056
auto left_repeated = detail::repeat(left, right.num_rows(), stream, mr);
5157

@@ -59,7 +65,8 @@ std::unique_ptr<cudf::table> cross_join(cudf::table_view const& left,
5965
right_tiled_columns.end(),
6066
std::back_inserter(left_repeated_columns));
6167

62-
return std::make_unique<table>(std::move(left_repeated_columns));
68+
return std::make_unique<table>(std::move(left_repeated_columns),
69+
static_cast<size_type>(out_num_rows));
6370
}
6471
} // namespace detail
6572

‎cpp/tests/join/cross_join_tests.cpp‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: Copyright (c) 2020-2025, NVIDIA CORPORATION.
2+
* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

@@ -14,6 +14,9 @@
1414
#include <cudf/table/table.hpp>
1515
#include <cudf/table/table_view.hpp>
1616

17+
#include <limits>
18+
#include <stdexcept>
19+
1720
template <typename T, typename SourceT = T>
1821
using column_wrapper = cudf::test::fixed_width_column_wrapper<T, SourceT>;
1922

@@ -67,26 +70,47 @@ TYPED_TEST(CrossJoinTypeTests, CrossJoin)
6770
CUDF_TEST_EXPECT_TABLES_EQUAL(join_table->view(), table_expect);
6871
}
6972

70-
class CrossJoinInvalidInputs : public cudf::test::BaseFixture {};
73+
class CrossJoinZeroColumnOperand : public cudf::test::BaseFixture {};
7174

72-
TEST_F(CrossJoinInvalidInputs, EmptyTable)
75+
TEST_F(CrossJoinZeroColumnOperand, PreservesRowCount)
7376
{
7477
auto b_0 = column_wrapper<int32_t>{10, 20, 20, 50};
7578
auto b_1 = column_wrapper<float>{5.0, .7, .7, .7};
7679
auto b_2 = column_wrapper<int8_t>{90, 75, 62, 41};
7780
auto b_3 = cudf::test::strings_column_wrapper({"quick", "words", "result", ""});
7881

79-
auto column_a = std::vector<std::unique_ptr<cudf::column>>{};
80-
auto table_a = cudf::table(std::move(column_a));
81-
auto table_b = cudf::table_view{{b_0, b_1, b_2, b_3}};
82-
83-
//
84-
// table_a has no columns, table_b has columns
85-
// Let's check different permutations of passing table
86-
// with no columns to verify that exceptions are thrown
87-
//
88-
EXPECT_THROW(cudf::cross_join(table_a, table_b), cudf::logic_error);
89-
EXPECT_THROW(cudf::cross_join(table_b, table_a), cudf::logic_error);
82+
// A zero-column operand with a non-zero row count is valid: it contributes
83+
// only to the output row count, not to the output columns.
84+
auto table_a = cudf::table_view{std::vector<cudf::column_view>{}, 3};
85+
auto table_b = cudf::table_view{{b_0, b_1, b_2, b_3}};
86+
87+
auto join_ab = cudf::cross_join(table_a, table_b);
88+
EXPECT_EQ(join_ab->num_columns(), table_b.num_columns());
89+
EXPECT_EQ(join_ab->num_rows(), table_a.num_rows() * table_b.num_rows());
90+
91+
auto join_ba = cudf::cross_join(table_b, table_a);
92+
EXPECT_EQ(join_ba->num_columns(), table_b.num_columns());
93+
EXPECT_EQ(join_ba->num_rows(), table_a.num_rows() * table_b.num_rows());
94+
95+
// Both operands zero-column: result is a zero-column table whose row count is
96+
// the product of the operands' row counts.
97+
auto table_c = cudf::table_view{std::vector<cudf::column_view>{}, 5};
98+
auto join_ac = cudf::cross_join(table_a, table_c);
99+
EXPECT_EQ(join_ac->num_columns(), 0);
100+
EXPECT_EQ(join_ac->num_rows(), table_a.num_rows() * table_c.num_rows());
101+
}
102+
103+
TEST_F(CrossJoinZeroColumnOperand, OverflowThrows)
104+
{
105+
// A cross join whose row-count product exceeds size_type::max() throws
106+
// std::overflow_error. Using zero-column operands makes the check happen
107+
// before any output is allocated.
108+
auto table_a =
109+
cudf::table_view{std::vector<cudf::column_view>{}, std::numeric_limits<cudf::size_type>::max()};
110+
auto table_b = cudf::table_view{std::vector<cudf::column_view>{}, 2};
111+
112+
EXPECT_THROW(cudf::cross_join(table_a, table_b), std::overflow_error);
113+
EXPECT_THROW(cudf::cross_join(table_b, table_a), std::overflow_error);
90114
}
91115

92116
class CrossJoinEmptyResult : public cudf::test::BaseFixture {};

‎python/cudf/cudf/core/join/join.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,20 @@ def perform_merge(self) -> DataFrame:
371371

372372
if self.how == "cross":
373373
lib_table = plc.join.cross_join(
374-
plc.Table([col.plc_column for col in self.lhs._columns]),
375-
plc.Table([col.plc_column for col in self.rhs._columns]),
374+
plc.Table(
375+
[col.plc_column for col in self.lhs._columns],
376+
num_rows=len(self.lhs),
377+
),
378+
plc.Table(
379+
[col.plc_column for col in self.rhs._columns],
380+
num_rows=len(self.rhs),
381+
),
376382
)
377383
columns = lib_table.columns()
384+
# A cross join of two column-less operands is a column-less table
385+
# whose row count is the product of the operands' row counts.
386+
# ``columns`` cannot carry that count, so remember it explicitly.
387+
cross_num_rows = lib_table.num_rows()
378388
num_left_cols = len(self.lhs._column_names)
379389
left_result = DataFrame._from_data(
380390
{
@@ -444,6 +454,13 @@ def perform_merge(self) -> DataFrame:
444454
result = DataFrame._from_data(
445455
*self._merge_results(left_result, right_result)
446456
)
457+
if self.how == "cross" and result._num_columns == 0:
458+
from cudf.core.index import RangeIndex
459+
460+
# Both operands were column-less, so the cross product is a
461+
# column-less table whose row count an empty ColumnAccessor cannot
462+
# carry. Rebuild the result with the row count captured above.
463+
result = DataFrame._from_data({}, index=RangeIndex(cross_num_rows))
447464

448465
if self.sort:
449466
result = self._sort_result(result)

‎python/cudf/cudf/tests/reshape/test_merge.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,6 +1577,24 @@ def test_merge_invalid_input(param):
15771577
cudf.merge(left["a"], param)
15781578

15791579

1580+
@pytest.mark.parametrize("left_cols", [True, False])
1581+
@pytest.mark.parametrize("right_cols", [True, False])
1582+
def test_cross_merge_zero_column_operand(left_cols, right_cols):
1583+
# A cross merge where one or both operands have no columns must still
1584+
# produce ``len(left) * len(right)`` rows (matching pandas), rather than
1585+
# dropping the row count of the column-less operand.
1586+
ldata = {"x": [1, 2, 3]} if left_cols else {}
1587+
rdata = {"y": [10, 20, 30, 40]} if right_cols else {}
1588+
pleft = pd.DataFrame(ldata, index=range(3))
1589+
pright = pd.DataFrame(rdata, index=range(4))
1590+
gleft = cudf.DataFrame(ldata, index=range(3))
1591+
gright = cudf.DataFrame(rdata, index=range(4))
1592+
1593+
expected = pleft.merge(pright, how="cross")
1594+
result = gleft.merge(gright, how="cross")
1595+
assert_eq(result, expected)
1596+
1597+
15801598
def test_merge_natural_join_key_order_matches_left_frame():
15811599
# A merge without ``on`` joins on the common columns in left-frame
15821600
# column order, like pandas. Building the key list from an unordered

‎python/cudf_polars/tests/streaming/test_select.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
assert_gpu_result_equal,
2323
)
2424
from cudf_polars.testing.engine_utils import warns_on_spmd
25-
from cudf_polars.utils.versions import POLARS_VERSION_LT_141
2625

2726

2827
@pytest.fixture
@@ -181,13 +180,6 @@ def test_select_mean_with_decimals(engine):
181180
assert_gpu_result_equal(q, engine=engine)
182181

183182

184-
@pytest.mark.xfail(
185-
condition=not POLARS_VERSION_LT_141,
186-
reason=(
187-
"len() row count lost in zero-column streaming chunks "
188-
"(https://github.com/rapidsai/cudf/issues/21428)"
189-
),
190-
)
191183
def test_select_with_len(streaming_engine_factory):
192184
engine = streaming_engine_factory(
193185
StreamingOptions(max_rows_per_partition=3, fallback_mode="warn"),

0 commit comments

Comments
 (0)