[WIP] decimal meta adjustment for GPU runtime #976
Conversation
|
Can one of the admins verify this patch? |
| @@ -532,6 +532,20 @@ object GpuOverrides { | |||
| // There are so many of these that we don't need to print them out. | |||
| override def print(append: StringBuilder, depth: Int, all: Boolean): Unit = {} | |||
| }), | |||
| // TODO: maybe we should apply the conversion between decimal32 and decimal64 to promote precision? | |||
| expr[PromotePrecision]( | |||
There was a problem hiding this comment.
This is the same as KnownFloatingPointNormalized or KnownNotNull. In those cases we don't throw it away, we replace it with s GPU version that is essentially a noop. It has little performance impact and lets us keep the same structure that spark has.
There was a problem hiding this comment.
Yes! And this PR has been refactored.
| override def convertToGpu(child: Expression): GpuExpression = { | ||
| child match { | ||
| case c: GpuCast => c.child.asInstanceOf[GpuExpression] | ||
| case c => throw new IllegalStateException( |
There was a problem hiding this comment.
Is this guaranteed 100% of the time that the child will be a cast? If not then we need to figure out better way to handle this?
There was a problem hiding this comment.
So far, it is 100% guaranteed since case class PrecisionPromotionwill only be created inside private method DecimalPrecision.promotePrecisionwith Cast Op.
sperlingxx
changed the title
sperlingxx
changed the title
Yes. Because initial work for decimal is W.I.P. in terms of cuDF java library, I am trying to work on something which is independent on |
|
I am fine with you wanting to work ahead. I just wasn't aware that this patch is still a work in progress. Sorry for the confusion. |
Signed-off-by: sperlingxx <lovedreamf@gmail.com>
sperlingxx
changed the title
sperlingxx
changed the title
…IDIA#976) Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Catalyst analyzer will apply
promotePrecisionandcheckOverflowon binary arithmetic expressions if data in both sides of expressions are decimal types (refer to link).Considering GPU_MAX_PRECISION of decimal type can only be up to 19 (half of MAX_PRECISION in spark), we need to reapply precision and scale adjustment with GPU constraints. To achieve this, we introduce two new expression-rules
PromotePrecisionExprMetaandCheckOverflowExprMetainto GpuOverrides.In order to keep align with decimal overflow controlling behavior of spark, we follow the same strategy as catalyst with GPU constraints replacement. But in spark catalyst runtime, overflow decimal value will be replaced by null. (Otherwise, overflow exception will be thrown.) In cuDF runtime, overflow check for decimal operations is still missing.