luis4a0 commented on code in PR #12963:
URL: https://github.com/apache/gluten/pull/12963#discussion_r4072750104
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -1538,6 +1538,22 @@ core::PlanNodePtr
SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
!readRel.common().has_emit(), "Emit not supported for ValuesNode and
TableScanNode related Substrait plans.");
}
+ // Virtual tables are self-contained and do not have Spark file splits.
+ if (readRel.has_virtual_table()) {
+ std::vector<TypePtr> veloxTypes;
+ if (readRel.has_base_schema()) {
+ const bool asLowerCase = !veloxCfg_->get<bool>(kCaseSensitive, false);
+ veloxTypes = SubstraitParser::parseNamedStruct(readRel.base_schema(),
asLowerCase);
+ }
+
+ std::vector<std::string> outputNames;
+ outputNames.reserve(veloxTypes.size());
+ for (int32_t idx = 0; idx < veloxTypes.size(); ++idx) {
+ outputNames.emplace_back(SubstraitParser::makeNodeName(planNodeId_,
idx));
+ }
+ return toVeloxPlan(readRel, ROW(std::move(outputNames),
std::move(veloxTypes)));
+ }
Review Comment:
Addressed in d74055ff8d5dafc69134910fb1cc660fcae7b17c. The converter now
explicitly rejects any virtual-table row group containing fields when
base_schema is absent, before cardinality derivation. Added a regression test
covering both validation modes; the focused 8-test virtual-table suite and full
60-test native plan-conversion binary pass.
##########
cpp/velox/tests/Substrait2VeloxValuesNodeConversionTest.cc:
##########
@@ -65,4 +68,208 @@ TEST_F(Substrait2VeloxValuesNodeConversionTest, valuesNode)
{
assertQuery(veloxPlan, "SELECT * FROM tmp");
}
+TEST_F(Substrait2VeloxValuesNodeConversionTest, zeroColumnOneRowValuesNode) {
+ auto planPath =
FilePathGenerator::getDataFilePath("substrait_virtualTable_emptySchema.json");
+
+ ::substrait::Plan substraitPlan;
+ JsonToProtoConverter::readFromFile(planPath, substraitPlan);
+ auto veloxCfg =
std::make_shared<facebook::velox::config::ConfigBase>(std::unordered_map<std::string,
std::string>());
Review Comment:
Addressed in d74055ff8d5dafc69134910fb1cc660fcae7b17c by adding the direct
<unordered_map> include. The native target rebuild and clang-format-15 check
both pass.
##########
gluten-substrait/src/main/java/org/apache/gluten/substrait/rel/RelBuilder.java:
##########
@@ -171,6 +171,17 @@ public static RelNode makeReadRelForInputIterator(
return new InputIteratorRelNode(typeList, nameList, iteratorIndex);
}
+ public static RelNode makeVirtualTableReadRel(
+ List<TypeNode> types,
+ List<String> names,
+ List<List<Expression.Literal>> rows,
+ SubstraitContext context,
+ Long operatorId) {
+ RelNode node = new VirtualTableRelNode(types, names, rows);
+ context.registerRelToOperator(operatorId);
+ return node;
+ }
Review Comment:
Addressed in d74055ff8d5dafc69134910fb1cc660fcae7b17c by changing the new
builder parameter to primitive long. This API does not exist on origin/main and
is introduced only by this unmerged PR, so no released boxed-signature
compatibility contract exists. Direct compilation verified the J descriptor and
the Scala OneRow caller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
