malinjawi commented on code in PR #12214:
URL: https://github.com/apache/gluten/pull/12214#discussion_r3426989935
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/AggregateFunctionsBuilder.scala:
##########
@@ -62,4 +66,14 @@ object AggregateFunctionsBuilder {
}
}
}
+
+ private def isPortableDeltaBitmapAggregator(aggregateFunc:
AggregateFunction): Boolean = {
+ aggregateFunc.prettyName == ExpressionNames.BITMAP_AGGREGATOR &&
+ Try {
+ aggregateFunc.getClass
+ .getMethod("serializationFormatString")
+ .invoke(aggregateFunc)
+ .asInstanceOf[String]
+ }.toOption.contains("Portable")
+ }
Review Comment:
Yeah, dug into this and the reflection wasn't actually buying us anything,
so I dropped it it's just a prettyName match now (cb2f28772).
The Portable check was me being paranoid. Delta only ever builds this
aggregator in one spot (DMLWithDeletionVectorsHelper) and always with the
Portable format, so it never actually filtered anything. And if a weird
non-Portable payload ever did show up, the native deserialize already throws on
the magic number so the real check is on the C++ side anyway. Bonus:
gluten-substrait no longer needs to know anything about delta-spark.
--
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]
