This is an automated email from the ASF dual-hosted git repository.
zhouyuan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gluten.git
The following commit(s) were added to refs/heads/main by this push:
new e526b209fd [VL] Preserve ordinals for nested Substrait field
references (#12821)
e526b209fd is described below
commit e526b209fd96fa778f116abfa2ed21fe336a5535
Author: Mohammad Linjawi <[email protected]>
AuthorDate: Tue Aug 25 11:24:36 2026 +0300
[VL] Preserve ordinals for nested Substrait field references (#12821)
---
cpp/velox/substrait/SubstraitParser.cc | 19 +++
cpp/velox/substrait/SubstraitParser.h | 4 +
cpp/velox/substrait/SubstraitToVeloxExpr.cc | 51 +++++++-
cpp/velox/substrait/SubstraitToVeloxPlan.cc | 9 +-
.../substrait/SubstraitToVeloxPlanValidator.cc | 15 +--
.../tests/Substrait2VeloxPlanConversionTest.cc | 137 +++++++++++++++++++++
.../tests/Substrait2VeloxPlanValidatorTest.cc | 89 +++++++++++++
cpp/velox/tests/SubstraitVeloxExprConverterTest.cc | 67 ++++++++++
8 files changed, 382 insertions(+), 9 deletions(-)
diff --git a/cpp/velox/substrait/SubstraitParser.cc
b/cpp/velox/substrait/SubstraitParser.cc
index e31ea8fa4e..869c42b62c 100644
--- a/cpp/velox/substrait/SubstraitParser.cc
+++ b/cpp/velox/substrait/SubstraitParser.cc
@@ -164,6 +164,25 @@ bool SubstraitParser::parseReferenceSegment(
}
}
+bool SubstraitParser::isTopLevelFieldSelection(const ::substrait::Expression&
expression) {
+ if (!expression.has_selection()) {
+ return false;
+ }
+
+ const auto& selection = expression.selection();
+ if (!selection.has_direct_reference() || selection.has_expression() ||
selection.has_outer_reference()) {
+ return false;
+ }
+
+ const auto& reference = selection.direct_reference();
+ if (!reference.has_struct_field()) {
+ return false;
+ }
+
+ const auto& field = reference.struct_field();
+ return field.field() >= 0 && !field.has_child();
+}
+
std::vector<std::string> SubstraitParser::makeNames(const std::string& prefix,
int size) {
std::vector<std::string> names;
names.reserve(size);
diff --git a/cpp/velox/substrait/SubstraitParser.h
b/cpp/velox/substrait/SubstraitParser.h
index 5097783f9b..8587c1e5f4 100644
--- a/cpp/velox/substrait/SubstraitParser.h
+++ b/cpp/velox/substrait/SubstraitParser.h
@@ -52,6 +52,10 @@ class SubstraitParser {
/// field.
static bool parseReferenceSegment(const
::substrait::Expression::ReferenceSegment& refSegment, uint32_t& fieldIndex);
+ /// Return true if the expression selects a non-negative top-level field from
+ /// the input row.
+ static bool isTopLevelFieldSelection(const ::substrait::Expression&
expression);
+
/// Make names in the format of {prefix}_{index}.
static std::vector<std::string> makeNames(const std::string& prefix, int
size);
diff --git a/cpp/velox/substrait/SubstraitToVeloxExpr.cc
b/cpp/velox/substrait/SubstraitToVeloxExpr.cc
index 68a245c97b..1fe143597a 100755
--- a/cpp/velox/substrait/SubstraitToVeloxExpr.cc
+++ b/cpp/velox/substrait/SubstraitToVeloxExpr.cc
@@ -208,6 +208,55 @@ makeFieldAccessExpr(const std::string& name, const
TypePtr& type, core::FieldAcc
return std::make_shared<core::FieldAccessTypedExpr>(type, name);
}
+core::TypedExprPtr
+makeOrdinalFieldReferenceExpr(uint32_t index, const RowTypePtr& inputType,
core::TypedExprPtr input) {
+ const auto& type = inputType->childAt(index);
+ if (input) {
+ return std::make_shared<core::DereferenceTypedExpr>(type,
std::move(input), index);
+ }
+
+ return std::make_shared<core::FieldAccessTypedExpr>(type,
inputType->nameOf(index));
+}
+
+core::TypedExprPtr toVeloxOrdinalFieldReferenceExpr(
+ const ::substrait::Expression::FieldReference& substraitField,
+ const RowTypePtr& inputType) {
+ auto typeCase = substraitField.reference_type_case();
+ switch (typeCase) {
+ case
::substrait::Expression::FieldReference::ReferenceTypeCase::kDirectReference: {
+ const auto& directRef = substraitField.direct_reference();
+ core::TypedExprPtr fieldReference{nullptr};
+ const auto* tmp = &directRef.struct_field();
+
+ auto inputColumnType = inputType;
+ for (;;) {
+ auto idx = tmp->field();
+ VELOX_USER_CHECK(
+ idx >= 0 && static_cast<uint32_t>(idx) < inputColumnType->size(),
+ "Field reference index {} is out of range for the {}-field row
type.",
+ idx,
+ inputColumnType->size());
+ const TypePtr childType = inputColumnType->childAt(idx);
+ fieldReference =
+ makeOrdinalFieldReferenceExpr(static_cast<uint32_t>(idx),
inputColumnType, std::move(fieldReference));
+
+ if (!tmp->has_child()) {
+ break;
+ }
+
+ inputColumnType = asRowType(childType);
+ VELOX_USER_CHECK_NOT_NULL(
+ inputColumnType,
+ "Nested field reference into a non-struct type (e.g. an array or
map element) is not supported.");
+ tmp = &tmp->child().struct_field();
+ }
+ return fieldReference;
+ }
+ default:
+ VELOX_NYI("Substrait conversion not supported for Reference '{}'",
std::to_string(typeCase));
+ }
+}
+
} // namespace
using facebook::velox::variantToVector;
@@ -651,7 +700,7 @@ core::TypedExprPtr SubstraitVeloxExprConverter::toVeloxExpr(
case ::substrait::Expression::RexTypeCase::kScalarFunction:
return toVeloxExpr(substraitExpr.scalar_function(), inputType);
case ::substrait::Expression::RexTypeCase::kSelection:
- return toVeloxExpr(substraitExpr.selection(), inputType);
+ return toVeloxOrdinalFieldReferenceExpr(substraitExpr.selection(),
inputType);
case ::substrait::Expression::RexTypeCase::kCast:
return toVeloxExpr(substraitExpr.cast(), inputType);
case ::substrait::Expression::RexTypeCase::kIfThen:
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlan.cc
b/cpp/velox/substrait/SubstraitToVeloxPlan.cc
index 2cff687dd2..21320519b0 100644
--- a/cpp/velox/substrait/SubstraitToVeloxPlan.cc
+++ b/cpp/velox/substrait/SubstraitToVeloxPlan.cc
@@ -596,6 +596,7 @@ core::PlanNodePtr
SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
if (substraitAggMask.ByteSizeLong() > 0) {
mask = std::dynamic_pointer_cast<const core::FieldAccessTypedExpr>(
exprConverter_->toVeloxExpr(substraitAggMask, inputType));
+ VELOX_USER_CHECK(mask && mask->isInputColumn(), "Aggregation Operator
only supports a top-level field mask.");
}
}
const auto& aggFunction = measure.measure();
@@ -935,7 +936,13 @@ core::PlanNodePtr
SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
for (const auto& projectExpr : projections.switching_field().duplicates())
{
if (projectExpr.has_selection()) {
- auto expression = exprConverter_->toVeloxExpr(projectExpr.selection(),
inputType);
+ VELOX_USER_CHECK(
+ SubstraitParser::isTopLevelFieldSelection(projectExpr),
+ "Expand Operator only supports a top-level field or literal.");
+ auto expression = std::dynamic_pointer_cast<const
core::FieldAccessTypedExpr>(
+ exprConverter_->toVeloxExpr(projectExpr, inputType));
+ VELOX_USER_CHECK(
+ expression && expression->isInputColumn(), "Expand Operator only
supports a top-level field or literal.");
projectExprs.emplace_back(expression);
} else if (projectExpr.has_literal()) {
auto expression = exprConverter_->toVeloxExpr(projectExpr.literal());
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
index f1bd84b671..6dcce60359 100644
--- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
+++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
@@ -629,6 +629,11 @@ bool SubstraitToVeloxPlanValidator::validate(const
::substrait::ExpandRel& expan
const auto& typeCase = projectExpr.rex_type_case();
switch (typeCase) {
case ::substrait::Expression::RexTypeCase::kSelection:
+ if (!SubstraitParser::isTopLevelFieldSelection(projectExpr)) {
+ LOG_VALIDATION_MSG("Expand Operator only supports a top-level
field or literal.");
+ return false;
+ }
+ break;
case ::substrait::Expression::RexTypeCase::kLiteral:
break;
default:
@@ -1296,13 +1301,9 @@ bool SubstraitToVeloxPlanValidator::validate(const
::substrait::AggregateRel& ag
if (smea.has_filter()) {
::substrait::Expression aggRelMask = smea.filter();
if (aggRelMask.ByteSizeLong() > 0) {
- auto typeCase = aggRelMask.rex_type_case();
- switch (typeCase) {
- case ::substrait::Expression::RexTypeCase::kSelection:
- break;
- default:
- LOG_VALIDATION_MSG("Only field is supported in aggregate filter
expression.");
- return false;
+ if (!SubstraitParser::isTopLevelFieldSelection(aggRelMask)) {
+ LOG_VALIDATION_MSG("Aggregation Operator only supports a top-level
field mask.");
+ return false;
}
}
}
diff --git a/cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc
b/cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc
index 76fe6d79ec..d002f34a14 100644
--- a/cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc
+++ b/cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc
@@ -289,4 +289,141 @@ TEST_F(Substrait2VeloxPlanConversionTest, filterUpper) {
planNode->toString(true, true));
}
+TEST_F(Substrait2VeloxPlanConversionTest, expandSelectionMustBeTopLevelField) {
+ const auto makeExpandRel = [](bool nestedSelection) {
+ ::substrait::Rel rel;
+ auto* expand = rel.mutable_expand();
+ expand->mutable_common()->mutable_direct();
+
+ auto* read = expand->mutable_input()->mutable_read();
+ read->mutable_common()->mutable_direct();
+ auto* schema = read->mutable_base_schema();
+ for (const auto* name : {"nested", "mask", "value"}) {
+ schema->add_names(name);
+ }
+
+ auto* nestedType =
schema->mutable_struct_()->add_types()->mutable_struct_();
+
nestedType->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ nestedType->add_names("");
+ nestedType->add_names("");
+
nestedType->add_types()->mutable_i64()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+
nestedType->add_types()->mutable_bool_()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_bool_()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_i64()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+
+ auto* field = expand->add_fields()
+ ->mutable_switching_field()
+ ->add_duplicates()
+ ->mutable_selection()
+ ->mutable_direct_reference()
+ ->mutable_struct_field();
+ field->set_field(nestedSelection ? 0 : 1);
+ if (nestedSelection) {
+ field->mutable_child()->mutable_struct_field()->set_field(1);
+ }
+ return rel;
+ };
+
+ const auto makeConverter = [&] {
+ return std::make_shared<SubstraitToVeloxPlanConverter>(
+ pool(),
+ veloxCfg_.get(),
+ std::vector<std::shared_ptr<ResultIterator>>{},
+ VeloxConnectorIds{.hive =
facebook::velox::exec::test::kHiveConnectorId},
+ std::nullopt,
+ std::nullopt,
+ /*validationMode=*/true);
+ };
+
+ auto plan =
makeConverter()->toVeloxPlan(makeExpandRel(/*nestedSelection=*/false));
+ auto expand = std::dynamic_pointer_cast<const core::ExpandNode>(plan);
+ ASSERT_NE(expand, nullptr);
+ ASSERT_EQ(expand->projections().size(), 1);
+ ASSERT_EQ(expand->projections().front().size(), 1);
+ auto field = std::dynamic_pointer_cast<const
core::FieldAccessTypedExpr>(expand->projections().front().front());
+ ASSERT_NE(field, nullptr);
+ EXPECT_TRUE(field->isInputColumn());
+ EXPECT_EQ(field->name(), "n0_1");
+
+ VELOX_ASSERT_USER_THROW(
+ makeConverter()->toVeloxPlan(makeExpandRel(/*nestedSelection=*/true)),
+ "Expand Operator only supports a top-level field or literal.");
+}
+
+TEST_F(Substrait2VeloxPlanConversionTest, aggregateMaskMustBeTopLevelField) {
+ const auto makeAggregateRel = [](bool nestedMask) {
+ ::substrait::Rel rel;
+ auto* aggregate = rel.mutable_aggregate();
+ aggregate->mutable_common()->mutable_direct();
+
+ auto* read = aggregate->mutable_input()->mutable_read();
+ read->mutable_common()->mutable_direct();
+ auto* schema = read->mutable_base_schema();
+ for (const auto* name : {"nested", "mask", "value"}) {
+ schema->add_names(name);
+ }
+
+ auto* nestedType =
schema->mutable_struct_()->add_types()->mutable_struct_();
+
nestedType->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ nestedType->add_names("");
+ nestedType->add_names("");
+
nestedType->add_types()->mutable_i64()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+
nestedType->add_types()->mutable_bool_()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_bool_()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_i64()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+
+ auto* measure = aggregate->add_measures();
+ auto* maskField =
+
measure->mutable_filter()->mutable_selection()->mutable_direct_reference()->mutable_struct_field();
+ maskField->set_field(nestedMask ? 0 : 1);
+ if (nestedMask) {
+ maskField->mutable_child()->mutable_struct_field()->set_field(1);
+ }
+
+ auto* function = measure->mutable_measure();
+ function->set_function_reference(1);
+ function->set_phase(::substrait::AGGREGATION_PHASE_INITIAL_TO_RESULT);
+
function->set_invocation(::substrait::AggregateFunction::AGGREGATION_INVOCATION_ALL);
+ function->add_arguments()
+ ->mutable_value()
+ ->mutable_selection()
+ ->mutable_direct_reference()
+ ->mutable_struct_field()
+ ->set_field(2);
+
function->mutable_output_type()->mutable_i64()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ return rel;
+ };
+
+ const auto makeConverter = [&] {
+ auto converter = std::make_shared<SubstraitToVeloxPlanConverter>(
+ pool(),
+ veloxCfg_.get(),
+ std::vector<std::shared_ptr<ResultIterator>>{},
+ VeloxConnectorIds{.hive =
facebook::velox::exec::test::kHiveConnectorId},
+ std::nullopt,
+ std::nullopt,
+ /*validationMode=*/true);
+ converter->constructFunctionMap(std::unordered_map<uint64_t,
std::string>{{1, "sum:opt_i64"}});
+ return converter;
+ };
+
+ auto plan =
makeConverter()->toVeloxPlan(makeAggregateRel(/*nestedMask=*/false));
+ auto aggregation = std::dynamic_pointer_cast<const
core::AggregationNode>(plan);
+ ASSERT_NE(aggregation, nullptr);
+ ASSERT_EQ(aggregation->aggregates().size(), 1);
+ ASSERT_NE(aggregation->aggregates().front().mask, nullptr);
+ EXPECT_TRUE(aggregation->aggregates().front().mask->isInputColumn());
+ EXPECT_EQ(aggregation->aggregates().front().mask->name(), "n0_1");
+
+ // A nested selection converts to a DereferenceTypedExpr, which cannot be an
+ // AggregationNode mask. Reject it instead of silently dropping the filter.
+ VELOX_ASSERT_USER_THROW(
+ makeConverter()->toVeloxPlan(makeAggregateRel(/*nestedMask=*/true)),
+ "Aggregation Operator only supports a top-level field mask.");
+}
+
} // namespace gluten
diff --git a/cpp/velox/tests/Substrait2VeloxPlanValidatorTest.cc
b/cpp/velox/tests/Substrait2VeloxPlanValidatorTest.cc
index 2476e2a2f8..a1147be798 100644
--- a/cpp/velox/tests/Substrait2VeloxPlanValidatorTest.cc
+++ b/cpp/velox/tests/Substrait2VeloxPlanValidatorTest.cc
@@ -35,6 +35,28 @@ using namespace facebook::velox::connector::hive;
using namespace facebook::velox::exec;
namespace gluten {
+namespace {
+
+void addNestedInputSchema(::substrait::ReadRel* read) {
+ read->mutable_common()->mutable_direct();
+ auto* schema = read->mutable_base_schema();
+ for (const auto* name : {"nested", "mask", "value"}) {
+ schema->add_names(name);
+ }
+
+ auto* nestedType = schema->mutable_struct_()->add_types()->mutable_struct_();
+
nestedType->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ nestedType->add_names("");
+ nestedType->add_names("");
+
nestedType->add_types()->mutable_i64()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+
nestedType->add_types()->mutable_bool_()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_bool_()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ schema->mutable_struct_()->add_types()->mutable_i64()->set_nullability(
+ ::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+}
+
+} // namespace
class Substrait2VeloxPlanValidatorTest : public
exec::test::HiveConnectorTestBase {
protected:
@@ -61,4 +83,71 @@ TEST_F(Substrait2VeloxPlanValidatorTest, group) {
ASSERT_FALSE(validatePlan(substraitPlan));
}
+TEST_F(Substrait2VeloxPlanValidatorTest, expandSelectionMustBeTopLevelField) {
+ const auto makePlan = [](bool nestedSelection) {
+ ::substrait::Plan plan;
+ auto* expand = plan.add_relations()->mutable_rel()->mutable_expand();
+ expand->mutable_common()->mutable_direct();
+ addNestedInputSchema(expand->mutable_input()->mutable_read());
+
+ auto* field = expand->add_fields()
+ ->mutable_switching_field()
+ ->add_duplicates()
+ ->mutable_selection()
+ ->mutable_direct_reference()
+ ->mutable_struct_field();
+ field->set_field(nestedSelection ? 0 : 1);
+ if (nestedSelection) {
+ field->mutable_child()->mutable_struct_field()->set_field(1);
+ }
+ return plan;
+ };
+
+ auto topLevelPlan = makePlan(/*nestedSelection=*/false);
+ EXPECT_TRUE(validatePlan(topLevelPlan));
+
+ auto nestedPlan = makePlan(/*nestedSelection=*/true);
+ EXPECT_FALSE(validatePlan(nestedPlan));
+}
+
+TEST_F(Substrait2VeloxPlanValidatorTest, aggregateMaskMustBeTopLevelField) {
+ const auto makePlan = [](bool nestedMask) {
+ ::substrait::Plan plan;
+ auto* extension = plan.add_extensions()->mutable_extension_function();
+ extension->set_function_anchor(1);
+ extension->set_name("sum:opt_i64");
+
+ auto* aggregate = plan.add_relations()->mutable_rel()->mutable_aggregate();
+ aggregate->mutable_common()->mutable_direct();
+ addNestedInputSchema(aggregate->mutable_input()->mutable_read());
+
+ auto* measure = aggregate->add_measures();
+ auto* maskField =
+
measure->mutable_filter()->mutable_selection()->mutable_direct_reference()->mutable_struct_field();
+ maskField->set_field(nestedMask ? 0 : 1);
+ if (nestedMask) {
+ maskField->mutable_child()->mutable_struct_field()->set_field(1);
+ }
+
+ auto* function = measure->mutable_measure();
+ function->set_function_reference(1);
+ function->set_phase(::substrait::AGGREGATION_PHASE_INITIAL_TO_RESULT);
+
function->set_invocation(::substrait::AggregateFunction::AGGREGATION_INVOCATION_ALL);
+ function->add_arguments()
+ ->mutable_value()
+ ->mutable_selection()
+ ->mutable_direct_reference()
+ ->mutable_struct_field()
+ ->set_field(2);
+
function->mutable_output_type()->mutable_i64()->set_nullability(::substrait::Type_Nullability_NULLABILITY_NULLABLE);
+ return plan;
+ };
+
+ auto topLevelPlan = makePlan(/*nestedMask=*/false);
+ EXPECT_TRUE(validatePlan(topLevelPlan));
+
+ auto nestedPlan = makePlan(/*nestedMask=*/true);
+ EXPECT_FALSE(validatePlan(nestedPlan));
+}
+
} // namespace gluten
diff --git a/cpp/velox/tests/SubstraitVeloxExprConverterTest.cc
b/cpp/velox/tests/SubstraitVeloxExprConverterTest.cc
index 784ba0c13d..32b94da2b5 100644
--- a/cpp/velox/tests/SubstraitVeloxExprConverterTest.cc
+++ b/cpp/velox/tests/SubstraitVeloxExprConverterTest.cc
@@ -18,12 +18,18 @@
#include "substrait/SubstraitToVeloxExpr.h"
#include "velox/common/base/tests/GTestUtils.h"
+#include "velox/core/QueryConfig.h"
+#include "velox/exec/tests/utils/AssertQueryBuilder.h"
+#include "velox/exec/tests/utils/OperatorTestBase.h"
+#include "velox/exec/tests/utils/PlanBuilder.h"
#include "velox/type/Type.h"
using namespace facebook::velox;
namespace gluten {
+class SubstraitVeloxExprConverterExecutionTest : public
exec::test::OperatorTestBase {};
+
// Regression test for a SIGSEGV in
// SubstraitVeloxExprConverter::toVeloxExpr(Expression::FieldReference, ...).
// The direct-reference loop descends one nested struct_field at a time with
@@ -66,4 +72,65 @@ TEST(SubstraitVeloxExprConverterTest,
fieldReferenceIndexOutOfRangeThrows) {
VELOX_ASSERT_USER_THROW(SubstraitVeloxExprConverter::toVeloxExpr(fieldReference,
inputType), "out of range");
}
+TEST_F(SubstraitVeloxExprConverterExecutionTest,
ordinalFieldReferenceIntoNonStructThrows) {
+ auto inputType = ROW({"arr"}, {ARRAY(INTEGER())});
+
+ ::substrait::Expression substraitExpr;
+ auto* structField =
substraitExpr.mutable_selection()->mutable_direct_reference()->mutable_struct_field();
+ structField->set_field(0);
+ structField->mutable_child()->mutable_struct_field()->set_field(0);
+
+ const std::unordered_map<uint64_t, std::string> functionMap;
+ SubstraitVeloxExprConverter converter(pool(), functionMap);
+ VELOX_ASSERT_THROW(converter.toVeloxExpr(substraitExpr, inputType), "Nested
field reference into a non-struct type");
+}
+
+TEST_F(SubstraitVeloxExprConverterExecutionTest,
ordinalFieldReferenceIndexOutOfRangeThrows) {
+ auto inputType = ROW({"a", "b"}, {INTEGER(), INTEGER()});
+ const std::unordered_map<uint64_t, std::string> functionMap;
+ SubstraitVeloxExprConverter converter(pool(), functionMap);
+
+ for (const auto index : {-1, 5}) {
+ SCOPED_TRACE(index);
+ ::substrait::Expression substraitExpr;
+
substraitExpr.mutable_selection()->mutable_direct_reference()->mutable_struct_field()->set_field(index);
+ VELOX_ASSERT_USER_THROW(converter.toVeloxExpr(substraitExpr, inputType),
"out of range");
+ }
+}
+
+TEST_F(SubstraitVeloxExprConverterExecutionTest,
nestedFieldReferenceUsesOrdinalForUnnamedFields) {
+ auto accumulator = makeRowVector(
+ {"", ""}, {makeFlatVector<int128_t>({12345, 67890}, DECIMAL(22, 2)),
makeFlatVector<bool>({false, true})});
+ auto input = makeRowVector({"acc"}, {accumulator});
+
+ // Nested ROW fields can have duplicate or empty names. A name-based lookup
+ // would bind both references to field 0 and return HUGEINT for field 1.
+ ::substrait::Expression substraitExpr;
+ auto* structField =
substraitExpr.mutable_selection()->mutable_direct_reference()->mutable_struct_field();
+ structField->set_field(0);
+ structField->mutable_child()->mutable_struct_field()->set_field(1);
+
+ const std::unordered_map<uint64_t, std::string> functionMap;
+ SubstraitVeloxExprConverter converter(pool(), functionMap);
+ auto expression = converter.toVeloxExpr(substraitExpr,
asRowType(input->type()));
+ auto dereference = std::dynamic_pointer_cast<const
core::DereferenceTypedExpr>(expression);
+ ASSERT_NE(dereference, nullptr);
+ EXPECT_EQ(dereference->index(), 1);
+ EXPECT_EQ(dereference->type()->kind(), TypeKind::BOOLEAN);
+
+ auto inputField = std::dynamic_pointer_cast<const
core::FieldAccessTypedExpr>(dereference->inputs().front());
+ ASSERT_NE(inputField, nullptr);
+ EXPECT_TRUE(inputField->isInputColumn());
+ EXPECT_EQ(inputField->name(), "acc");
+
+ auto plan =
exec::test::PlanBuilder().values({input}).projectExpressions({expression}).planNode();
+ for (const bool simplified : {false, true}) {
+ SCOPED_TRACE(simplified ? "simplified=true" : "simplified=false");
+ auto result = exec::test::AssertQueryBuilder(plan)
+ .config(core::QueryConfig::kExprEvalSimplified,
simplified ? "true" : "false")
+ .copyResults(pool());
+ test::assertEqualVectors(makeFlatVector<bool>({false, true}),
result->childAt(0));
+ }
+}
+
} // namespace gluten
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
