Skip to content

[GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions) #12849

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.

Sign up for GitHub

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

Open
nielspardon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[GLUTEN-12597][CORE] Migrate ReadRel.VirtualTable to Substrait 0.98 (values -> expressions) #12849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 18 additions & 16 deletions cpp/velox/substrait/SubstraitToVeloxPlan.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,26 +1681,23 @@ core::PlanNodePtr SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
core::PlanNodePtr SubstraitToVeloxPlanConverter::toVeloxPlan(
const ::substrait::ReadRel& readRel,
const RowTypePtr& type) {
::substrait::ReadRel_VirtualTable readVirtualTable = readRel.virtual_table();
int64_t numVectors = readVirtualTable.values_size();
int64_t numColumns = type->size();
int64_t valueFieldNums = readVirtualTable.values(numVectors - 1).fields_size();
const ::substrait::ReadRel_VirtualTable& readVirtualTable = readRel.virtual_table();
const int64_t numVectors = readVirtualTable.expressions_size();
const int64_t numColumns = type->size();
std::vector<RowVectorPtr> vectors;
vectors.reserve(numVectors);

int64_t batchSize;
// For the empty vectors, eg,vectors = makeRowVector(ROW({}, {}), 1).
if (numColumns == 0) {
batchSize = 1;
} else {
batchSize = valueFieldNums / numColumns;
}

for (int64_t index = 0; index < numVectors; ++index) {
std::vector<VectorPtr> children;
::substrait::Expression_Literal_Struct rowValue = readRel.virtual_table().values(index);
auto fieldSize = rowValue.fields_size();
VELOX_CHECK_EQ(fieldSize, batchSize * numColumns);
// Each Nested.Struct holds one row group, laid out column-major. Row groups need not all
// carry the same number of rows, so derive the batch size per struct rather than once for
// the whole table.
const ::substrait::Expression_Nested_Struct& rowValue = readVirtualTable.expressions(index);
const int64_t fieldSize = rowValue.fields_size();
// For the empty vectors, eg,vectors = makeRowVector(ROW({}, {}), 1).
const int64_t batchSize = numColumns == 0 ? 1 : fieldSize / numColumns;
VELOX_USER_CHECK_EQ(
fieldSize, batchSize * numColumns, "ReadRel.VirtualTable field count must be a multiple of the column count.");

for (int64_t col = 0; col < numColumns; ++col) {
const TypePtr& outputChildType = type->childAt(col);
Expand All @@ -1709,7 +1706,12 @@ core::PlanNodePtr SubstraitToVeloxPlanConverter::toVeloxPlan(
for (int64_t batchId = 0; batchId < batchSize; batchId++) {
// each value in the batch
auto fieldIdx = col * batchSize + batchId;
::substrait::Expression_Literal field = rowValue.fields(fieldIdx);
// Substrait models virtual table values as Expressions; Gluten only ever emits literals,
// so unwrap back to the Literal the downstream conversion expects. This converter is also
// reachable from the JSON-plan test and benchmark paths, so reject anything else loudly.
const ::substrait::Expression& fieldExpr = rowValue.fields(fieldIdx);
VELOX_USER_CHECK(fieldExpr.has_literal(), "ReadRel.VirtualTable expressions must be literals.");
const ::substrait::Expression_Literal& field = fieldExpr.literal();

auto expr = exprConverter_->toVeloxExpr(field);
if (auto constantExpr = std::dynamic_pointer_cast<const core::ConstantTypedExpr>(expr)) {
Expand Down
8 changes: 4 additions & 4 deletions cpp/velox/substrait/VeloxToSubstraitExpr.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ template <TypeKind kind>
void convertVectorValue(
google::protobuf::Arena& arena,
const velox::VectorPtr& vectorValue,
::substrait::Expression_Literal_Struct* litValue,
::substrait::Expression_Nested_Struct* litValue,
::substrait::Expression_Literal* substraitField) {
const TypePtr& childType = vectorValue->type();

Expand All @@ -303,7 +303,7 @@ void convertVectorValue(
// Get the batchSize and convert each value in it.
vector_size_t flatVecSize = childToFlatVec->size();
for (int64_t i = 0; i < flatVecSize; i++) {
substraitField = litValue->add_fields();
substraitField = litValue->add_fields()->mutable_literal();
if (childToFlatVec->isNullAt(i)) {
// Process the null value.
substraitField->MergeFrom(toSubstraitNullLiteral(arena, childType->kind()));
Expand Down Expand Up @@ -512,7 +512,7 @@ const ::substrait::Expression& VeloxToSubstraitExprConvertor::toSubstraitExpr(
const ::substrait::Expression_Literal& VeloxToSubstraitExprConvertor::toSubstraitExpr(
google::protobuf::Arena& arena,
const std::shared_ptr<const core::ConstantTypedExpr>& constExpr,
::substrait::Expression_Literal_Struct* litValue) {
::substrait::Expression_Nested_Struct* litValue) {
if (constExpr->hasValueVector()) {
return toSubstraitLiteral(arena, constExpr->valueVector(), litValue);
} else {
Expand Down Expand Up @@ -595,7 +595,7 @@ const ::substrait::Expression_Literal& VeloxToSubstraitExprConvertor::toSubstrai
const ::substrait::Expression_Literal& VeloxToSubstraitExprConvertor::toSubstraitLiteral(
google::protobuf::Arena& arena,
const velox::VectorPtr& vectorValue,
::substrait::Expression_Literal_Struct* litValue) {
::substrait::Expression_Nested_Struct* litValue) {
::substrait::Expression_Literal* substraitField =
google::protobuf::Arena::CreateMessage<::substrait::Expression_Literal>(&arena);
if (vectorValue->isScalar()) {
Expand Down
10 changes: 6 additions & 4 deletions cpp/velox/substrait/VeloxToSubstraitExpr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,14 @@ class VeloxToSubstraitExprConvertor {
/// Literal Expression.
/// @param arena Arena to use for allocating Substrait plan objects.
/// @param constExpr Velox Constant expression needed to be converted.
/// @param litValue The Struct that returned literal expression belong to.
/// @param litValue The Nested.Struct the converted literals are appended to, each wrapped in an
/// Expression. Substrait models struct-valued expressions as Expression.Nested.Struct.
/// @return A pointer to Substrait Literal expression object allocated on
/// the arena and representing the input Velox Constant expression.
const ::substrait::Expression_Literal& toSubstraitExpr(
google::protobuf::Arena& arena,
const std::shared_ptr<const core::ConstantTypedExpr>& constExpr,
::substrait::Expression_Literal_Struct* litValue = nullptr);
::substrait::Expression_Nested_Struct* litValue = nullptr);

/// Convert Velox FieldAccessTypedExpr to Substrait FieldReference Expression.
const ::substrait::Expression_FieldReference& toSubstraitExpr(
Expand All @@ -64,11 +65,12 @@ class VeloxToSubstraitExprConvertor {
const std::shared_ptr<const core::DereferenceTypedExpr>& derefExpr,
const RowTypePtr& inputType);

/// Convert Velox vector to Substrait literal.
/// Convert Velox vector to Substrait literal. One literal per row is appended to litValue,
/// each wrapped in an Expression so it fits Substrait's Expression.Nested.Struct container.
const ::substrait::Expression_Literal& toSubstraitLiteral(
google::protobuf::Arena& arena,
const velox::VectorPtr& vectorValue,
::substrait::Expression_Literal_Struct* litValue);
::substrait::Expression_Nested_Struct* litValue);

private:
/// Convert Velox Cast Expression to Substrait Cast Expression.
Expand Down
18 changes: 12 additions & 6 deletions cpp/velox/substrait/VeloxToSubstraitPlan.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,19 @@ void VeloxToSubstraitPlanConvertor::toSubstrait(
::substrait::ReadRel_VirtualTable* virtualTable = readRel->mutable_virtual_table();

for (const auto& vector : valuesNode->values()) {
::substrait::Expression_Literal_Struct* litValue = virtualTable->add_values();

// Substrait models a virtual table row group as an Expression.Nested.Struct laid out
// column-major; toSubstraitLiteral appends one Expression per value into it.
::substrait::Expression_Nested_Struct* nested = virtualTable->add_expressions();
for (const auto& column : vector->children()) {
::substrait::Expression_Literal* substraitField =
google::protobuf::Arena::CreateMessage<::substrait::Expression_Literal>(&arena);

substraitField->MergeFrom(exprConvertor_->toSubstraitLiteral(arena, column, litValue));
const int expectedFields = nested->fields_size() + vector->size();
exprConvertor_->toSubstraitLiteral(arena, column, nested);
// Only scalar columns are appended; complex-typed ones are returned by value instead, which
// would silently shorten the row group and transpose the table the consumer decodes from it.
VELOX_USER_CHECK_EQ(
nested->fields_size(),
expectedFields,
"Unsupported virtual table column type: {}",
column->type()->toString());
}
}

Expand Down
122 changes: 76 additions & 46 deletions cpp/velox/tests/data/substrait_virtualTable.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,83 +55,113 @@
}
},
"virtual_table": {
"values": [
"expressions": [
{
"fields": [
{
"nullable": false,
"i64": "2499109626526694126"
"literal": {
"nullable": false,
"i64": "2499109626526694126"
}
},
{
"nullable": false,
"i64": "2342493223442167775"
"literal": {
"nullable": false,
"i64": "2342493223442167775"
}
},
{
"nullable": false,
"i64": "4077358421272316858"
"literal": {
"nullable": false,
"i64": "4077358421272316858"
}
},
{
"nullable": false,
"i32": 581869302
"literal": {
"nullable": false,
"i32": 581869302
}
},
{
"nullable": false,
"i32": -708632711
"literal": {
"nullable": false,
"i32": -708632711
}
},
{
"nullable": false,
"i32": -133711905
"literal": {
"nullable": false,
"i32": -133711905
}
},
{
"nullable": false,
"fp64": 0.90579193414549275
"literal": {
"nullable": false,
"fp64": 0.90579193414549275
}
},
{
"nullable": false,
"fp64": 0.96886777112423139
"literal": {
"nullable": false,
"fp64": 0.96886777112423139
}
},
{
"nullable": false,
"fp64": 0.63235925003444637
"literal": {
"nullable": false,
"fp64": 0.63235925003444637
}
},
{
"nullable": false,
"boolean": true
"literal": {
"nullable": false,
"boolean": true
}
},
{
"nullable": false,
"boolean": false
"literal": {
"nullable": false,
"boolean": false
}
},
{
"nullable": false,
"boolean": false
"literal": {
"nullable": false,
"boolean": false
}
},
{
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
"literal": {
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
}
},
{
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
"literal": {
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
}
},
{
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
"literal": {
"null": {
"i32": {
"type_variation_reference": 0,
"nullability": "NULLABILITY_NULLABLE"
}
},
"nullable": true
}
}
]
}
Expand Down
7 changes: 5 additions & 2 deletions gluten-substrait/src/main/resources/substrait/proto/substrait/algebra.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,12 @@ message ReadRel {
substrait.extensions.AdvancedExtension advanced_extension = 10;
}

// A table composed of literals.
// A table composed of expressions.
message VirtualTable {
repeated Expression.Literal.Struct values = 1;
reserved 1;
reserved "values";

repeated Expression.Nested.Struct expressions = 2;
}

// A stub type that can be used to extend/introduce new table types outside
Expand Down
Loading
Loading