sunchao commented on code in PR #6851:
URL: https://github.com/apache/datafusion-comet/pull/6851#discussion_r4237796386
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteDetectionSuite.scala:
##########
@@ -1500,95 +1500,25 @@ class CometIcebergWriteDetectionSuite extends
CometTestBase with CometIcebergTes
}
}
- test("fall-back: identity partition on a float or double column") {
- // iceberg-rust groups float partition values with an equality that treats
-0.0 and 0.0 as one
- // value, where iceberg-java keeps them apart (#6138).
+ test("Compatible when partitioned by identity on a float or double column") {
+ // iceberg-rust grouped float partition values with an equality that
treated -0.0 and 0.0 as
+ // one value, so these writes fell back (#6138) until
apache/iceberg-rust#3327 kept the two
+ // apart as iceberg-java does. A nested source field resolves the same way.
withDetectionCatalog { _ =>
- Seq("float" -> "FLOAT", "double" -> "DOUBLE").foreach { case (typeName,
sqlType) =>
- val table = s"part_$typeName"
- spark.sql(s"""
- CREATE TABLE $catalog.$ns.$table (id INT, v $sqlType)
- USING iceberg PARTITIONED BY (v)
- """)
- val writeExec = captureWriteExec(table, allowWriteFailure = false) {
- spark.sql(s"INSERT INTO $catalog.$ns.$table VALUES (1, CAST(1.5 AS
$sqlType))")
- }
- assertUnsupportedContains(writeExec, table, "partition field v",
typeName, "-0.0")
- }
- }
- }
-
- test("fall-back: identity partition on a nested double field") {
- // The source of a partition field can be nested inside a struct.
`Schema.findField` resolves
- // a nested id too, so the rule must not fail open for it.
- withDetectionCatalog { _ =>
- spark.sql(s"""
- CREATE TABLE $catalog.$ns.part_nested (id INT, s STRUCT<v: DOUBLE>)
- USING iceberg PARTITIONED BY (s.v)
- """)
- val writeExec = captureWriteExec("part_nested", allowWriteFailure =
false) {
- spark.sql(s"INSERT INTO $catalog.$ns.part_nested VALUES (1,
named_struct('v', 1.5D))")
- }
- assertUnsupportedContains(writeExec, "part_nested", "partition field
s.v", "double", "-0.0")
- }
- }
-
- test("fall-back: double identity partition beside a dropped partition
field") {
- // A format-version-1 spec keeps a dropped partition field as a `void`
transform, and that
- // field's source column can be dropped afterwards. The surviving double
field must still be
- // found, whatever the dropped one does to the spec's partition type.
- withDetectionCatalog { dir =>
- createTable(
- dir,
- "part_dropped",
- partitionSpec = "PARTITIONED BY (region, amount)",
- properties = Some("'format-version'='1'"))
- // Loaded afresh for each change: the insert in between commits through
another handle.
- def table: org.apache.iceberg.Table =
- loadIcebergTable(spark, catalog, ns, "part_dropped")
- .asInstanceOf[org.apache.iceberg.Table]
- table.updateSpec().removeField("region").commit()
- spark.sql(s"REFRESH TABLE $catalog.$ns.part_dropped")
- assertUnsupportedContains("part_dropped", "partition field amount",
"double", "-0.0")
-
- // Iceberg before 1.11 cannot plan a write once the `void` field's
source column is gone.
- // On 1.11 iceberg-java plans it but cannot build a partition key for a
spec that mixes that
- // field with a live one, so the write itself fails on either path; only
the gate's decision
- // is checked.
- if (icebergVersionAtLeast(1, 11)) {
- table.updateSchema().deleteColumn("region").commit()
- spark.sql(s"REFRESH TABLE $catalog.$ns.part_dropped")
- val writeExec = captureWriteExec("part_dropped", allowWriteFailure =
true) {
- spark.sql(s"INSERT INTO $catalog.$ns.part_dropped VALUES (2, 2.0)")
- }
- assertUnsupportedContains(
- writeExec,
- "part_dropped",
- "partition field amount",
- "double",
- "-0.0")
+ Seq(
+ "part_float" -> ("v FLOAT", "v", "CAST(1.5 AS FLOAT)"),
Review Comment:
[P2] Construct these entries as explicit nested tuples. The three new
`"part_*" -> (...)` expressions trigger Scala 2.12’s adapted-argument warning
because `->` accepts one argument and the compiler implicitly creates a
three-tuple. The supported Spark 3.5 strict-warnings build promotes this
warning to an error, so `test-compile` fails before tests can run. Using
`("part_float", ("v FLOAT", "v", "CAST(1.5 AS FLOAT)"))`, and the equivalent
form for the other entries, avoids the adaptation and restores compilation.
Evidence: Exact-head CI job
https://github.com/apache/datafusion-comet/actions/runs/38055061563/job/114222370917
fails at lines 1509–1511 with `Adapting argument list by creating a 3-tuple`
while running `./mvnw -B test-compile -Pspark-3.5 -Pstrict-warnings
-DskipTests`. Independently compiling the three entries with Scala 2.12.18 and
`-Xlint:adapted-args -Xfatal-warnings` exited 1. The explicit nested-tuple
version exited 0.
--
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]
