andygrove commented on code in PR #6821:
URL: https://github.com/apache/datafusion-comet/pull/6821#discussion_r4233109670
##########
spark/src/test/scala/org/apache/comet/parquet/CometParquetWriterSuite.scala:
##########
@@ -1340,6 +1340,122 @@ class CometParquetWriterSuite extends
CometParquetWriterTestBase {
}
}
+ test("a native overwrite of a cached path recaches the data") {
+ withTempPath { dir =>
+ val target = new File(dir, "target").getAbsolutePath
+ val source = new File(dir, "source").getAbsolutePath
+ withSQLConf(CometConf.COMET_ENABLED.key -> "false") {
+ spark.range(0, 5).write.parquet(target)
+ spark.range(100, 103).write.parquet(source)
+ }
+ val cached = spark.read.parquet(target).cache()
+ try {
+ assert(cached.count() == 5)
+ withNativeWriter {
+ assertHasCometNativeWriteExec(
+ captureWritePlan(
+
spark.read.parquet(source).write.mode(SaveMode.Overwrite).parquet(target)))
+ }
+ // Spark recaches every cached plan that reads the output path once
the write commits.
+ checkAnswer(cached, (100L until 103L).map(Row(_)))
+ } finally {
+ cached.unpersist()
+ }
+ }
+ }
+
+ test("a native write rejects case-insensitively duplicate column names") {
+ withTempPath { dir =>
+ val source = new File(dir, "source").getAbsolutePath
+ val target = new File(dir, "target")
+ withSQLConf(CometConf.COMET_ENABLED.key -> "false") {
+ spark.range(3).selectExpr("id AS a", "id AS b").write.parquet(source)
+ }
+ withNativeWriter {
+ val e = intercept[AnalysisException] {
Review Comment:
This is the only new test that doesn't check the write actually went native.
It does today. I reverted the `CometNativeWriteExec` change locally on 3.5 and
this test fails with "no exception was thrown". But if this shape ever starts
falling back, Spark's own command throws the same `COLUMN_ALREADY_EXISTS` and
the test stays green without exercising the new check. That's the trap the
comment in the `INSERT INTO ... SELECT` test at line 1333 warns about.
Could `captureWritePlan` also keep the plan from `onFailure`, so this test
can call `assertHasCometNativeWriteExec` on the failed write?
`CometIcebergWriteDetectionSuite.captureWritePlan` already does this with its
`allowWriteFailure` flag.
--
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]
