feat: Implement Spark-compatible CAST from String to Decimal #615
Conversation
We currently delegate to DataFusion when casting from string to decimal and there are some differences in behavior compared to Spark. This PR implements a Spark-compatible cast from String to Decimal.
|
@kevinmingtarja, thank you for helping me with a draft. Could you please take a look? |
|
@andygrove @vaibhawvipul Could you please take a look? |
Apologies @sujithjay I had missed this ping. I will review this early next week. |
| Ok(Some(cast)) | ||
| } | ||
|
|
||
| /// Copied from arrow-rs, modified to replicate Spark's behavior |
There was a problem hiding this comment.
This comment is useful, thanks. Could you also update the comment to give a brief summary of what needed to change to replicate Spark's behavior?
|
This is looking good @sujithjay. CI is failing due to clippy warnings. If you run clippy locally you should be able to see the same warnings as well as suggestions for fixing. |
|
@sujithjay I took the liberty of fixing the clippy issues and fixing the conflicts |
| let result: SparkResult<ArrayRef> = Ok(Arc::new(cast_array.finish()) as ArrayRef); | ||
| result? |
There was a problem hiding this comment.
No need to wrap in a result and then unwrap.
| let result: SparkResult<ArrayRef> = Ok(Arc::new(cast_array.finish()) as ArrayRef); | |
| result? | |
| Arc::new(cast_array.finish()) as ArrayRef |
|
@sujithjay I will close this PR for now since it has been inactive for a while. Feel free to reopen this or create a new PR if you continue work on this. |
Which issue does this PR close?
Closes #325.
Rationale for this change
We currently delegate to DataFusion when casting from string to decimal and there are some differences in behavior compared to Spark.
This PR implements a Spark-compatible cast from String to Decimal.
What changes are included in this PR?
An implementation of
parse_decimalcopied fromarrow-rs, modified to be compatible with Spark's behavior.How are these changes tested?
CometCastSuitenow passes.cast.rs.