Skip to content

Commit 0262e90

Browse files
committed
fix: Native Scan assert is irrelevant once in a different Comet native plan
1 parent ad17997 commit 0262e90

2 files changed

Lines changed: 14 additions & 10 deletions

File tree

spark/src/main/scala/org/apache/spark/sql/comet/operators.scala

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,12 @@ abstract class CometNativeExec extends CometExec {
219219

220220
// For each relation in a CometNativeScan generate a hadoopConf,
221221
// for each file path in a relation associate with hadoopConf
222-
val cometNativeScans: Seq[CometNativeScanExec] = this
223-
.collectLeaves()
224-
.filter(_.isInstanceOf[CometNativeScanExec])
225-
.map(_.asInstanceOf[CometNativeScanExec])
222+
// This is done per native plan, so only count scans until a comet input is reached.
223+
val cometNativeScans = mutable.ArrayBuffer.empty[CometNativeScanExec]
224+
foreachUntilCometInput(this) {
225+
case scan: CometNativeScanExec => cometNativeScans += scan
226+
case _ => // no-op
227+
}
226228
assert(
227229
cometNativeScans.size <= 1,
228230
"We expect one native scan in a Comet plan since we will broadcast one hadoopConf.")

spark/src/test/scala/org/apache/spark/sql/comet/ParquetEncryptionITCase.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,17 @@ package org.apache.spark.sql.comet
2222
import java.io.{File, RandomAccessFile}
2323
import java.nio.charset.StandardCharsets
2424
import java.util.Base64
25-
2625
import org.junit.runner.RunWith
2726
import org.scalactic.source.Position
2827
import org.scalatest.Tag
2928
import org.scalatestplus.junit.JUnitRunner
30-
3129
import org.apache.parquet.crypto.DecryptionPropertiesFactory
3230
import org.apache.parquet.crypto.keytools.{KeyToolkit, PropertiesDrivenCryptoFactory}
3331
import org.apache.parquet.crypto.keytools.mocks.InMemoryKMS
3432
import org.apache.spark.{DebugFilesystem, SparkConf}
35-
import org.apache.spark.sql.{CometTestBase, SQLContext}
33+
import org.apache.spark.sql.{CometTestBase, SQLContext, functions}
3634
import org.apache.spark.sql.internal.SQLConf
3735
import org.apache.spark.sql.test.SQLTestUtils
38-
3936
import org.apache.comet.{CometConf, IntegrationTestSuite}
4037
import org.apache.comet.CometConf.{SCAN_NATIVE_COMET, SCAN_NATIVE_DATAFUSION, SCAN_NATIVE_ICEBERG_COMPAT}
4138

@@ -394,11 +391,16 @@ class ParquetEncryptionITCase extends CometTestBase with SQLTestUtils {
394391
val parquetDF2 = spark.read.parquet(parquetDir2)
395392

396393
val unionDF = parquetDF1.union(parquetDF2)
394+
// Since the union has its own executeColumnar, problems would not surface if it is the last operator
395+
// If we add another comet aggregate after the union, we see the need for the
396+
// foreachUntilCometInput() in operator.scala
397+
// as we would error on multiple native scan execs despite no longer being in the same plan at all
398+
val aggDf = unionDF.agg(functions.sum("id"))
397399

398400
if (CometConf.COMET_ENABLED.get(conf)) {
399-
checkSparkAnswerAndOperator(unionDF)
401+
checkSparkAnswerAndOperator(aggDf)
400402
} else {
401-
checkSparkAnswer(unionDF)
403+
checkSparkAnswer(aggDf)
402404
}
403405
}
404406
}

0 commit comments

Comments
 (0)