comphead commented on code in PR #6455:
URL: https://github.com/apache/datafusion-comet/pull/6455#discussion_r4146867168
##########
spark/src/test/scala/org/apache/comet/CometCodegenSuite.scala:
##########
@@ -2346,6 +2393,44 @@ object CometCodegenSuite {
class NotSerializableTarget {
def twice(s: UTF8String): UTF8String = UTF8String.fromString(s.toString +
s.toString)
}
+
+ /**
+ * DSv2 function catalog for the #6425 test. `as_money` declares
`DECIMAL(10, 2)` and
+ * `as_wide_money` declares `DECIMAL(20, 12)`.
+ */
+ class DecimalFunctionCatalog extends FunctionCatalog {
+ private val functions = Map(
+ "as_money" -> new ScaleZeroDecimalFunction(10, 2),
+ "as_wide_money" -> new ScaleZeroDecimalFunction(20, 12))
+ private var catalogName: String = _
+
+ override def initialize(name: String, options: CaseInsensitiveStringMap):
Unit =
+ catalogName = name
+
+ override def name(): String = catalogName
+
+ override def listFunctions(namespace: Array[String]): Array[Identifier] =
+ functions.keys.map(Identifier.of(namespace, _)).toArray
+
+ override def loadFunction(ident: Identifier): UnboundFunction =
+ functions.getOrElse(ident.name(), throw new
NoSuchFunctionException(ident))
+ }
+
+ /**
+ * Returns its `INT` argument as `Decimal(v)`, at scale 0, whatever scale it
declares. `invoke`
+ * is an instance method, so Spark lowers a call to `Invoke`. The function
binds to itself.
+ */
+ class ScaleZeroDecimalFunction(precision: Int, scale: Int)
+ extends UnboundFunction
+ with ScalarFunction[Decimal] {
+ override def name(): String = "scale_zero_decimal"
+ override def description(): String = s"int -> decimal($precision, $scale),
at scale 0"
+ override def bind(inputType: StructType): BoundFunction = this
+ override def inputTypes(): Array[DataType] = Array(IntegerType)
+ override def resultType(): DataType = DecimalType(precision, scale)
+ def invoke(v: Int): Decimal = Decimal(v)
Review Comment:
It might be worth covering the opposite direction too, since `Decimal(v)`
only scales up and the `ROUND_HALF_UP` path of `changePrecision` is not
reached. Would a function that returns `v` with three fractional digits (so
`-1005` means `-1.005`) into a `DECIMAL(10, 2)` make sense? I'd expect `-1.01`
for `-1005`, `1.00` for `1004` and `0.01` for `5`. The existing rows also stop
at `-7` on the negative side, so a `-100000000` row would cover the negative
overflow boundary.
--
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]
