Kimahriman commented on code in PR #731:
URL: https://github.com/apache/datafusion-comet/pull/731#discussion_r1698929837


##########
spark/src/main/scala/org/apache/spark/sql/comet/CometBatchScanExec.scala:
##########
@@ -152,3 +153,17 @@ case class CometBatchScanExec(wrapped: BatchScanExec, 
runtimeFilters: Seq[Expres
 
   override def supportsColumnar: Boolean = true
 }
+
+object CometBatchScanExec {
+
+  def isSchemaSupported(schema: StructType): Boolean =

Review Comment:
   Currently it's very hard coded that nearly everything in Comet supports the 
exact same data types. This was an attempt to start breaking out supported data 
types for different functionality within Comet. Just because I want to support 
struct types in expressions, doesn't mean I have to support it in the Scan 
operations. And when the Scans are updated to support each complex type, they 
can then opt-in to supporting them. And CometBatchScanExec and CometScanExec 
don't need to be updated at the same time to support the same types either.
   
   We could get away with just delegating back to the 
CometSparkSessionExtensions check with the new allowComplex flag, but it seems 
to make a lot more sense to let each operator opt-in to exactly what it 
supports.



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to