Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check column type equality, handling nested types correctly. #14531

Merged
merged 39 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
39d14c8
Check column types with a utility that handles nesting correctly.
bdice Mar 11, 2024
2e3979f
Merge remote-tracking branch 'upstream/branch-24.04' into nested-colu…
bdice Mar 14, 2024
57b3b50
Merge remote-tracking branch 'upstream/branch-24.06' into nested-colu…
bdice Mar 26, 2024
5f7f1f8
Add column_scalar_types_equal.
bdice Mar 26, 2024
d880838
Use column_scalar_types_equal.
bdice Mar 26, 2024
8b152dc
Fix bugs and update tests.
bdice Mar 26, 2024
5cf4769
Add scalar_types_equal.
bdice Mar 27, 2024
8d938bb
Fix tests.
bdice Mar 27, 2024
f7d8c41
Merge remote-tracking branch 'upstream/branch-24.06' into nested-colu…
bdice Mar 27, 2024
5430397
Add data_type_errors.
bdice Mar 27, 2024
8e02603
Fix tests.
bdice Mar 27, 2024
a1bbecb
Fix remaining type checks that should use new utilities.
bdice Mar 27, 2024
0de9f9b
Add docs section on comparing data types.
bdice Mar 27, 2024
fc7b821
Fix typo.
bdice Mar 27, 2024
55e0c9f
Merge remote-tracking branch 'upstream/branch-24.06' into nested-colu…
bdice Apr 1, 2024
faa92c8
Use cudf::is_fixed_point.
bdice Apr 1, 2024
6556266
Fix missing paren.
bdice Apr 1, 2024
6531a53
list column, not lists column
bdice Apr 4, 2024
454d413
Rename functions to be overloads of cudf::types_equal, update and exp…
bdice Apr 9, 2024
c91df10
Rename function.
bdice Apr 9, 2024
c742253
Rename to have_same_types, apply PR feedback.
bdice Apr 10, 2024
8a44004
Merge remote-tracking branch 'upstream/branch-24.06' into nested-colu…
bdice Apr 10, 2024
fce091f
Add back deprecated methods.
bdice Apr 12, 2024
cafdf6f
Fix verb/subject agreement.
bdice Apr 12, 2024
cda4e0d
Update docs for all_have_same_types.
bdice Apr 12, 2024
ec740c9
Un-deprecate column_types_equivalent.
bdice Apr 12, 2024
ec0eeff
Apply patch from Lawrence.
bdice Apr 12, 2024
5afc5e2
Use std::equal.
bdice Apr 12, 2024
18cdd52
Switch back to column_types_equivalent.
bdice Apr 12, 2024
7c05d95
Fix tests that had incorrect nested types.
bdice Apr 13, 2024
7ba2664
Merge branch 'nested-column-type-checks' of github.com:bdice/cudf int…
bdice Apr 13, 2024
9904431
Merge remote-tracking branch 'upstream/branch-24.06' into nested-colu…
bdice Apr 30, 2024
15d05e0
Fix clang-format.
bdice Apr 30, 2024
57c1f64
Move table_view overload from table_view.hpp to type_checks.hpp.
bdice May 1, 2024
25c5ad9
Mirror scalar/column call.
bdice May 1, 2024
d764f7e
Minor cleanup.
bdice May 1, 2024
e0732b9
Improve dispatching to existing overloads.
bdice May 1, 2024
fc0dada
Merge branch 'nested-column-type-checks' of github.com:bdice/cudf int…
bdice May 1, 2024
32444c5
Merge branch 'branch-24.06' into nested-column-type-checks
bdice May 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix remaining type checks that should use new utilities.
  • Loading branch information
bdice committed Mar 27, 2024
commit a1bbecbb410a94f65d04ec4caca287938f8eaa67
9 changes: 5 additions & 4 deletions cpp/doxygen/developer_guide/DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -921,13 +921,14 @@ Use the `CUDF_EXPECTS` macro to enforce runtime conditions necessary for correct
Example usage:

```c++
CUDF_EXPECTS(lhs.type() == rhs.type(), "Column type mismatch");
CUDF_EXPECTS(cudf::column_types_equal(lhs, rhs), "Column type mismatch", cudf::data_type_error);
```

The first argument is the conditional expression expected to resolve to `true` under normal
conditions. If the conditional evaluates to `false`, then an error has occurred and an instance of
`cudf::logic_error` is thrown. The second argument to `CUDF_EXPECTS` is a short description of the
error that has occurred and is used for the exception's `what()` message.
conditions. The second argument to `CUDF_EXPECTS` is a short description of the error that has
occurred and is used for the exception's `what()` message. If the conditional evaluates to
`false`, then an error has occurred and an instance of the exception class in the third argument
(or the default, `cudf::logic_error`) is thrown.

There are times where a particular code path, if reached, should indicate an error no matter what.
For example, often the `default` case of a `switch` statement represents an invalid alternative.
Expand Down
14 changes: 10 additions & 4 deletions cpp/src/strings/slice.cu
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <cudf/strings/string_view.cuh>
#include <cudf/strings/strings_column_view.hpp>
#include <cudf/utilities/default_stream.hpp>
#include <cudf/utilities/error.hpp>
#include <cudf/utilities/type_checks.hpp>

#include <rmm/cuda_stream_view.hpp>

Expand Down Expand Up @@ -226,13 +228,17 @@ std::unique_ptr<column> slice_strings(strings_column_view const& strings,
"Parameter starts must have the same number of rows as strings.");
CUDF_EXPECTS(stops_column.size() == strings_count,
"Parameter stops must have the same number of rows as strings.");
CUDF_EXPECTS(starts_column.type() == stops_column.type(),
"Parameters starts and stops must be of the same type.");
CUDF_EXPECTS(cudf::column_types_equal(starts_column, stops_column),
"Parameters starts and stops must be of the same type.",
cudf::data_type_error);
CUDF_EXPECTS(starts_column.null_count() == 0, "Parameter starts must not contain nulls.");
CUDF_EXPECTS(stops_column.null_count() == 0, "Parameter stops must not contain nulls.");
CUDF_EXPECTS(starts_column.type().id() != data_type{type_id::BOOL8}.id(),
"Positions values must not be bool type.");
CUDF_EXPECTS(is_fixed_width(starts_column.type()), "Positions values must be fixed width type.");
"Positions values must not be bool type.",
cudf::data_type_error);
CUDF_EXPECTS(is_fixed_width(starts_column.type()),
"Positions values must be fixed width type.",
cudf::data_type_error);

auto strings_column = column_device_view::create(strings.parent(), stream);
auto starts_iter = cudf::detail::indexalator_factory::make_input_iterator(starts_column);
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/table/table_view.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018-2023, NVIDIA CORPORATION.
* Copyright (c) 2018-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -164,7 +164,7 @@ bool is_relationally_comparable(TableView const& lhs, TableView const& rhs)
return std::all_of(thrust::counting_iterator<size_type>(0),
thrust::counting_iterator<size_type>(lhs.num_columns()),
[lhs, rhs](auto const i) {
return lhs.column(i).type() == rhs.column(i).type() and
return cudf::column_types_equal(lhs.column(i), rhs.column(i)) and
cudf::is_relationally_comparable(lhs.column(i).type());
});
}
Expand Down