edmondop commented on code in PR #805: URL: https://github.com/apache/datafusion-comet/pull/805#discussion_r1724944483
########## spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala: ########## @@ -1210,6 +1210,58 @@ object QueryPlanSerde extends Logging with ShimQueryPlanSerde with CometExprShim None } + case StructsToJson(options, child, timezoneId) => + if (options.nonEmpty) { + withInfo(expr, "StructsToJson with options is not supported") + None + } else { + + def isSupportedType(dt: DataType): Boolean = { + dt match { + case StructType(fields) => + fields.forall(f => isSupportedType(f.dataType)) + case DataTypes.BooleanType | DataTypes.ByteType | DataTypes.ShortType | + DataTypes.IntegerType | DataTypes.LongType | DataTypes.FloatType | + DataTypes.DoubleType | DataTypes.StringType => + true + case DataTypes.DateType | DataTypes.TimestampType => + // TODO implement these types with tests for formatting options and timezone + false + case _ => false + } + } + + val isSupported = child.dataType match { + case s: StructType => + s.fields.forall(f => isSupportedType(f.dataType)) + case _ => Review Comment: I think @eejbyfeldt point is correct. Map and Arrays can effectively be jsonified, but they are not `StructType` and we need to extend the pattern matching to support those -- 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