Skip to content

Commit 79d6458

Browse files
authored
GH-48062: [C++] Fix null pointer dereference in MakeExecBatch (#48063)
### Rationale for this change The `arrow::compute::MakeExecBatch` function calls `DataType::id()` on `partial.type()` which could be a null pointer when `partial` is a datum representing a table. `MakeExecBatch` fails to check for this and thus null pointer dereference indeed happen. ### What changes are included in this PR? This patch adds a simple check in `MakeExecBatch` before calling `DataType::id()` to make sure it won't gets called on a null pointer. ### Are these changes tested? Yes. This patch updates the unit test `ExpressionUtils.MakeExecBatch` and includes a case that calls `MakeExecBatch` with a table. ### Are there any user-facing changes? No. This PR indeed resolve a crash problem, but I'm not quite sure whether this should be classified as a "critical fix". Resolve #48062 . * GitHub Issue: #48062 Authored-by: Sirui Mu <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
1 parent 60b976b commit 79d6458

File tree

2 files changed

+5
-1
lines changed

2 files changed

+5
-1
lines changed

cpp/src/arrow/compute/expression.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,8 @@ Result<ExecBatch> MakeExecBatch(const Schema& full_schema, const Datum& partial,
687687
}
688688

689689
// wasteful but useful for testing:
690-
if (partial.type()->id() == Type::STRUCT) {
690+
const auto& partial_type = partial.type();
691+
if (partial_type && partial_type->id() == Type::STRUCT) {
691692
if (partial.is_array()) {
692693
ARROW_ASSIGN_OR_RAISE(auto partial_batch,
693694
RecordBatch::FromStructArray(partial.make_array()));

cpp/src/arrow/compute/expression_test.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,9 @@ TEST(ExpressionUtils, MakeExecBatch) {
224224
auto duplicated_names =
225225
RecordBatch::Make(schema({GetField("i32"), GetField("i32")}), kNumRows, {i32, i32});
226226
ASSERT_RAISES(Invalid, MakeExecBatch(*kBoringSchema, duplicated_names));
227+
228+
ASSERT_OK_AND_ASSIGN(auto boring_table, Table::MakeEmpty(kBoringSchema));
229+
ASSERT_RAISES(NotImplemented, MakeExecBatch(*kBoringSchema, boring_table));
227230
}
228231

229232
class WidgetifyOptions : public compute::FunctionOptions {

0 commit comments

Comments
 (0)