Copilot commented on code in PR #12726:
URL: https://github.com/apache/gluten/pull/12726#discussion_r4063525216
##########
gluten-iceberg/src/test/scala/org/apache/gluten/execution/IcebergSuite.scala:
##########
@@ -836,4 +895,127 @@ abstract class IcebergSuite extends
WholeStageTransformerSuite {
}
}
}
+
+ test("case-sensitive mode: data column named Input_File_Name is not confused
with metadata") {
+ // Regression test for the IcebergScanTransformer fix.
+ // A user data column whose name equals "input_file_name" when lowercased
must NOT be
+ // misclassified as an Iceberg metadata column under caseSensitive=true.
+ // Before the fix, readSchemaFields and inputFileRelatedMetadataColumns
both lowercased
+ // names unconditionally, so "Input_File_Name" would hash-collide with the
metadata
+ // constant "input_file_name" and could be injected as a metadata column,
shadowing the
+ // user data.
+ withSQLConf("spark.sql.caseSensitive" -> "true") {
+ withTable("iceberg_col_collision") {
+ spark.sql("""
+ |CREATE TABLE iceberg_col_collision
+ | (id INT, `Input_File_Name` STRING)
+ |USING iceberg
+ |""".stripMargin)
+ spark.sql("""
+ |INSERT INTO iceberg_col_collision VALUES
+ |(1, 'user-data-value'), (2, 'another-value')
+ |""".stripMargin)
+
+ // Data column must return the user value, not a file path. This
exercises the
+ // readSchemaFields and inputFileRelatedMetadataColumns paths fixed in
this patch.
+ val dfData = runAndCompare("""
+ |SELECT id, `Input_File_Name`
+ |FROM iceberg_col_collision
+ |ORDER BY id
+ |""".stripMargin)
+ checkGlutenPlan[IcebergScanTransformer](dfData)
+ val dataRows = dfData.collect()
+ assert(dataRows.length == 2, s"Expected 2 rows, got
${dataRows.length}")
+ assert(
+ dataRows(0).getString(1) == "user-data-value",
+ s"Row 0 data column value wrong: ${dataRows(0).getString(1)}")
+ assert(
+ dataRows(1).getString(1) == "another-value",
+ s"Row 1 data column value wrong: ${dataRows(1).getString(1)}")
+ }
+ }
+ }
+
+ test("case-sensitive mode: Input_File_Name data column and input_file_name
metadata") {
+ // Regression test for PushDownInputFileExpression.PostOffload. Under
caseSensitive=true,
+ // the user data column `Input_File_Name` and generated metadata attribute
`input_file_name`
+ // are distinct Spark attributes and both must remain available for
binding.
+ withSQLConf("spark.sql.caseSensitive" -> "true") {
+ withTable("iceberg_input_file_projection") {
+ spark.sql("""
+ |CREATE TABLE iceberg_input_file_projection
+ | (id INT, `Input_File_Name` STRING)
+ |USING iceberg
+ |""".stripMargin)
+ spark.sql("""
+ |INSERT INTO iceberg_input_file_projection VALUES
+ |(1, 'user-data-value'), (2, 'another-value')
+ |""".stripMargin)
+
+ val df = runAndCompare("""
+ |SELECT id, `Input_File_Name`,
input_file_name() AS fname
+ |FROM iceberg_input_file_projection
+ |ORDER BY id
+ |""".stripMargin)
+ checkGlutenPlan[IcebergScanTransformer](df)
+ val rows = df.collect()
+ assert(rows.length == 2, s"Expected 2 rows, got ${rows.length}")
+ assert(rows(0).getString(1) == "user-data-value")
+ assert(rows(1).getString(1) == "another-value")
+ assert(
+ rows.forall(r => !r.isNullAt(2) && r.getString(2).nonEmpty),
+ s"Expected non-empty input_file_name values, got: ${rows.mkString(",
")}")
+ }
+ }
+ }
+
+ test("case-sensitive mode: lowercase input_file_name as data column is
rejected by Iceberg") {
+ // Iceberg reserves the field name "input_file_name" as a Spark metadata
expression name.
+ // While Iceberg's MetadataColumns does not list it in META_COLUMNS by
that exact string,
+ // Spark itself may reject or mishandle a user column with this exact name
because
+ // input_file_name() resolves to an AttributeReference with that name in
the plan.
+ // This test documents the platform behavior: if Iceberg rejects the
schema, that is
+ // expected and is not a Gluten defect. If it succeeds, the data value
must be returned.
+ withSQLConf("spark.sql.caseSensitive" -> "true") {
+ withTable("iceberg_exact_collision") {
+ val created =
+ try {
+ spark.sql("""
+ |CREATE TABLE iceberg_exact_collision
+ | (id INT, input_file_name STRING)
+ |USING iceberg
+ |""".stripMargin)
+ true
+ } catch {
+ case _: Exception => false
+ }
+ if (created) {
+ // If Iceberg allowed the schema, insert and verify Gluten handles
it correctly.
+ val inserted =
+ try {
+ spark.sql("""
+ |INSERT INTO iceberg_exact_collision VALUES (1,
'exact-value')
+ |""".stripMargin)
+ true
+ } catch {
+ case _: Exception => false
+ }
+ if (inserted) {
+ val df = runAndCompare("""
+ |SELECT id, input_file_name FROM
iceberg_exact_collision
+ |""".stripMargin)
+ checkGlutenPlan[IcebergScanTransformer](df)
+ val rows = df.collect()
+ assert(rows.length == 1)
+ assert(
+ rows(0).getString(1) == "exact-value",
+ s"Expected 'exact-value', got: ${rows(0).getString(1)}")
+ }
+ // If insert failed (Spark resolves input_file_name as expression),
that is
+ // expected platform behavior, not a Gluten defect.
+ }
+ // If CREATE TABLE failed, Iceberg correctly rejects reserved names.
+ }
+ }
Review Comment:
This test can pass without executing any assertion: both table creation and
insertion failures are caught and treated as expected, so a regression that
makes either operation fail will remain undetected. Either remove this test if
the exact lowercase name is intentionally unsupported, or assert the expected
platform behavior explicitly and fail on unexpected exceptions; if the schema
is supported, the test should not swallow creation or insertion errors.
--
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]
