luis4a0 commented on code in PR #12963:
URL: https://github.com/apache/gluten/pull/12963#discussion_r4081806289
##########
backends-velox/src/test/scala/org/apache/gluten/execution/MiscOperatorSuite.scala:
##########
@@ -791,16 +798,322 @@ class MiscOperatorSuite extends
VeloxWholeStageTransformerSuite with AdaptiveSpa
}
test("test OneRowRelation") {
- val df = sql("SELECT 1")
- checkAnswer(df, Row(1))
- val plan = df.queryExecution.executedPlan
- if (isSparkVersionGE("4.1")) {
- assert(plan.find(_.getClass.getSimpleName ==
"OneRowRelationExec").isDefined)
- } else {
- assert(plan.find(_.isInstanceOf[RDDScanExec]).isDefined)
+ val testCases = Seq(
+ "SELECT 1" -> Seq(Row(1)),
+ "SELECT 'x', 42, CAST(NULL AS INT)" -> Seq(Row("x", 42, null)),
+ "SELECT (SELECT 1)" -> Seq(Row(1)),
+ "SELECT (SELECT 1), (SELECT 1)" -> Seq(Row(1, 1)),
+ "SELECT 1 UNION ALL SELECT 2" -> Seq(Row(1), Row(2))
+ )
+
+ def checkOneRowRelation(query: String, expected: Seq[Row]): Unit = {
+ val df = sql(query)
+ checkAnswer(df, expected)
+ assert(df.count() == expected.size)
Review Comment:
Addressed in
https://github.com/apache/gluten/commit/ec3ed82e6d6ddb6cf852da81f1e575fda553f1e0.
The test now calls the existing upstream `GlutenQueryTest.checkAnswer` with
`checkToRDD = false` and removes the separate `count()`. That performs one
`collect()` action while comparing the complete result, including cardinality.
The corrected source compiles against the existing backend and
`gluten-substrait` test dependencies.
##########
cpp/velox/tests/Substrait2VeloxValuesNodeConversionTest.cc:
##########
@@ -49,7 +53,7 @@ TEST_F(Substrait2VeloxValuesNodeConversionTest, valuesNode) {
VeloxConnectorIds{},
std::nullopt,
std::nullopt,
- true);
+ false);
Review Comment:
Addressed in
https://github.com/apache/gluten/commit/ec3ed82e6d6ddb6cf852da81f1e575fda553f1e0.
The non-empty-schema `valuesNode` test now constructs a fresh converter and
executes `assertQuery` for both `validationMode = false` and `validationMode =
true`. All 8 `Substrait2VeloxValuesNodeConversionTest` cases pass.
##########
cpp/velox/tests/Substrait2VeloxValuesNodeConversionTest.cc:
##########
@@ -65,4 +69,231 @@ 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>());
+ auto planConverter = std::make_shared<SubstraitToVeloxPlanConverter>(
+ pool_.get(),
+ veloxCfg.get(),
+ std::vector<std::shared_ptr<ResultIterator>>{},
+ VeloxConnectorIds{},
+ std::nullopt,
+ std::nullopt,
+ false);
+ auto veloxPlan = planConverter->toVeloxPlan(substraitPlan);
+
+ auto valuesNode = std::dynamic_pointer_cast<const
core::ValuesNode>(veloxPlan);
+ ASSERT_NE(valuesNode, nullptr);
+ ASSERT_TRUE(valuesNode->outputType()->equivalent(*ROW({})));
+ ASSERT_EQ(valuesNode->values().size(), 1);
+ ASSERT_EQ(valuesNode->values().front()->childrenSize(), 0);
+ ASSERT_EQ(valuesNode->values().front()->size(), 1);
+ ASSERT_EQ(planConverter->splitInfos().at(valuesNode->id())->leafType,
SplitInfo::LeafType::TRIVIAL_LEAF);
Review Comment:
Addressed in
https://github.com/apache/gluten/commit/ec3ed82e6d6ddb6cf852da81f1e575fda553f1e0.
The test now uses `find()`, asserts that the entry exists and is non-null, and
only then checks `TRIVIAL_LEAF`, so missing metadata produces focused assertion
diagnostics. All 8 values-node conversion tests pass.
--
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]
