felipepessoto commented on code in PR #13136:
URL: https://github.com/apache/gluten/pull/13136#discussion_r4319362437
##########
cpp/velox/tests/Substrait2VeloxPlanConversionTest.cc:
##########
@@ -290,6 +385,153 @@ TEST_F(Substrait2VeloxPlanConversionTest, filterUpper) {
planNode->toString(true, true));
}
+TEST_F(Substrait2VeloxPlanConversionTest, derivedFileSchemaUsesRegularSlots) {
+ auto read = makeRead(
+ ROW({"file_name", "p", "file_name", "row_index", "keep"}, {BIGINT(),
INTEGER(), VARCHAR(), BIGINT(), BOOLEAN()}),
+ {::substrait::NamedStruct::NORMAL_COL,
+ ::substrait::NamedStruct::PARTITION_COL,
+ ::substrait::NamedStruct::METADATA_COL,
+ ::substrait::NamedStruct::ROWINDEX_COL,
+ ::substrait::NamedStruct::NORMAL_COL});
+
read.mutable_filter()->mutable_selection()->mutable_direct_reference()->mutable_struct_field()->set_field(4);
+ auto scan = convertRead(read, makeParquetFiles({"/unused.parquet"}));
+ auto table = std::dynamic_pointer_cast<const
HiveTableHandle>(scan->tableHandle());
+ ASSERT_NE(table, nullptr);
+ EXPECT_EQ(table->dataColumns()->toString(),
"ROW<file_name:BIGINT,keep:BOOLEAN>");
+ EXPECT_EQ(scan->outputType()->size(), 5);
+ EXPECT_EQ(scan->assignments().size(), 5);
+ const std::vector<ColumnType> roles{
+ ColumnType::kRegular,
+ ColumnType::kPartitionKey,
+ ColumnType::kSynthesized,
+ ColumnType::kRowIndex,
+ ColumnType::kRegular};
+ for (int i = 0; i < roles.size(); ++i) {
+ auto column =
+ std::dynamic_pointer_cast<const
HiveColumnHandle>(scan->assignments().at(scan->outputType()->nameOf(i)));
+ ASSERT_NE(column, nullptr);
+ EXPECT_EQ(column->columnType(), roles[i]);
+ }
+ EXPECT_EQ(scan->outputType()->childAt(0)->kind(), TypeKind::BIGINT);
+ EXPECT_EQ(scan->outputType()->childAt(2)->kind(), TypeKind::VARCHAR);
+ auto filter = std::dynamic_pointer_cast<const
core::FieldAccessTypedExpr>(table->remainingFilter());
+ ASSERT_NE(filter, nullptr);
+ EXPECT_EQ(filter->name(), "keep");
+}
+
+TEST_F(Substrait2VeloxPlanConversionTest, emptyAndUntaggedFileSchemas) {
+ const auto type = ROW({"file_name", "row_index"}, {VARCHAR(), BIGINT()});
+ auto scan = convertRead(
+ makeRead(type, {::substrait::NamedStruct::METADATA_COL,
::substrait::NamedStruct::ROWINDEX_COL}),
+ makeParquetFiles({"/unused.parquet"}));
+ auto table = std::dynamic_pointer_cast<const
HiveTableHandle>(scan->tableHandle());
+ ASSERT_NE(table, nullptr);
+ ASSERT_NE(table->dataColumns(), nullptr);
+ EXPECT_EQ(table->dataColumns()->size(), 0);
+ EXPECT_EQ(scan->outputType()->size(), 2);
+ EXPECT_EQ(scan->assignments().size(), 2);
+
+ scan = convertRead(makeRead(type), makeParquetFiles({"/unused.parquet"}));
+ table = std::dynamic_pointer_cast<const
HiveTableHandle>(scan->tableHandle());
+ ASSERT_NE(table, nullptr);
+ EXPECT_TRUE(table->dataColumns()->equivalent(*type));
+}
+
+TEST_F(Substrait2VeloxPlanConversionTest,
explicitFileSchemaPreservesPhysicalFields) {
+ auto read = makeRead(
+ ROW({"FILE_NAME", "P", "ROW_INDEX", "VALUE"}, {VARCHAR(), INTEGER(),
BIGINT(), BIGINT()}),
+ {::substrait::NamedStruct::METADATA_COL,
+ ::substrait::NamedStruct::PARTITION_COL,
+ ::substrait::NamedStruct::ROWINDEX_COL,
+ ::substrait::NamedStruct::NORMAL_COL});
+ auto tableSchema = ROW({"FILE_NAME", "VALUE", "ROW_INDEX", "P"}, {BIGINT(),
BIGINT(), VARCHAR(), INTEGER()});
+ auto scan = convertRead(read, makeParquetFiles({"/unused.parquet"},
tableSchema));
+ auto table = std::dynamic_pointer_cast<const
HiveTableHandle>(scan->tableHandle());
+ ASSERT_NE(table, nullptr);
+ EXPECT_EQ(table->dataColumns()->toString(),
"ROW<file_name:BIGINT,value:BIGINT,row_index:VARCHAR>");
+ EXPECT_EQ(scan->outputType()->size(), 4);
+ EXPECT_EQ(scan->outputType()->childAt(0)->kind(), TypeKind::VARCHAR);
+ EXPECT_EQ(scan->outputType()->childAt(2)->kind(), TypeKind::BIGINT);
+}
+
+TEST_F(Substrait2VeloxPlanConversionTest, parquetMetadataOnlyPreservesRows) {
+ writeParquet("/first.parquet", makeRowVector({"file_name"},
{makeFlatVector<int64_t>({10, 20})}));
+ writeParquet("/second.parquet", makeRowVector({"file_name"},
{makeFlatVector<int64_t>({30})}));
+ const auto files = makeParquetFiles({"/first.parquet", "/second.parquet"});
+ auto scan = convertRead(makeRead(ROW({"file_name"}, {VARCHAR()}),
{::substrait::NamedStruct::METADATA_COL}), files);
+ EXPECT_EQ(planConverter_->splitInfos().at(scan->id())->columnMappingMode,
dwio::common::ColumnMappingMode::kName);
Review Comment:
The execution path already adds the temporary-directory prefix:
`makeSplits()` constructs the connector path with `fmt::format("{}{}",
tmpDir_->getPath(), paths[i])` (line 77), matching `writeParquet()` (line 158).
All three Parquet execution tests use `.splits(makeSplits(scan))`; the
`/unused.parquet` cases only convert plans and never open files. Passing fully
qualified paths without changing `makeSplits()` would add the prefix twice, so
I am keeping the existing path handling.
##########
gluten-ut/spark41/src/test/scala/org/apache/spark/sql/execution/datasources/GlutenFileMetadataStructSuite.scala:
##########
@@ -86,7 +127,10 @@ class GlutenFileMetadataStructSuite extends
FileMetadataStructSuite with GlutenS
.listFiles()
.filter(_.getName.endsWith(s".$testFileFormat"))
.head
- f(df, getMetadataForFile(realF0), getMetadataForFile(realF1))
+ assert(realF0.length() < 1024 * 1024 && realF1.length() < 1024 *
1024)
+ withSQLConf(SQLConf.FILES_MAX_PARTITION_BYTES.key -> "1m") {
+ f(df, getMetadataForFile(realF0), getMetadataForFile(realF1))
+ }
Review Comment:
Keeping this assertion as an explicit fixture precondition. These files
contain only a few fixed rows, and the block-metadata expectations assume one
split per file (`file_block_start = 0`, `file_block_length = file size`). The
assertion makes an unexpected change to that assumption visible. Raising the
fixed limit would only move the boundary; there is no failing size case here
that requires it.
--
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]
