Copilot commented on code in PR #1657:
URL: https://github.com/apache/auron/pull/1657#discussion_r2554630151
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/NativeConverters.scala:
##########
@@ -118,6 +118,19 @@ object NativeConverters extends Logging {
}
}
+ def existTimestampType(dataType: DataType): Boolean = {
+ dataType match {
+ case TimestampType =>
+ true
+ case at: ArrayType => existTimestampType(at.elementType)
+ case m: MapType =>
+ existTimestampType(m.keyType) && existTimestampType(m.valueType)
Review Comment:
Logic error: For MapType, the function should use `||` (OR) instead of `&&`
(AND). The current implementation returns true only when BOTH key and value
contain timestamps, but it should return true if EITHER contains a timestamp.
This means a map like `Map[String, Timestamp]` would incorrectly return false.
```suggestion
existTimestampType(m.keyType) || existTimestampType(m.valueType)
```
##########
spark-extension/src/main/scala/org/apache/spark/sql/auron/NativeConverters.scala:
##########
@@ -118,6 +118,19 @@ object NativeConverters extends Logging {
}
}
+ def existTimestampType(dataType: DataType): Boolean = {
+ dataType match {
+ case TimestampType =>
+ true
+ case at: ArrayType => existTimestampType(at.elementType)
+ case m: MapType =>
+ existTimestampType(m.keyType) && existTimestampType(m.valueType)
+ case s: StructType =>
+ s.fields.forall(e => existTimestampType(e.dataType))
Review Comment:
Logic error: For StructType, the function should use `exists` instead of
`forall`. The current implementation returns true only when ALL fields contain
timestamps, but it should return true if ANY field contains a timestamp. This
means a struct like `Struct(name: String, created_at: Timestamp)` would
incorrectly return false.
```suggestion
s.fields.exists(e => existTimestampType(e.dataType))
```
--
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]
