refactor: gate CometUnaryMinus input types in getSupportLevel#4759
Merged
andygrove merged 2 commits intoJun 30, 2026
Conversation
UnaryMinus was the only arithmetic serde still relying on a convert-time fallback for unsupported input types after apache#4674. Add a getSupportLevel override mirroring the other math expressions (via mathDataTypeSupportLevel) so the unsupported-datatype reason surfaces in EXPLAIN and the generated compatibility docs, and interval inputs fall back with a clear reason. Numeric types (including decimal) are verified to stay native; interval types fall back to Spark.
comphead
reviewed
Jun 29, 2026
| // must stay native; interval types are unsupported and fall back to Spark. | ||
| withParquetTable( | ||
| (1 to 5).map(i => | ||
| (i.toByte, i.toShort, i, i.toLong, i.toFloat, i.toDouble, BigDecimal(i * 3, 2))), |
Contributor
There was a problem hiding this comment.
perhaps its covered with sql tests, but maybe we can test also extreme cases? min/max, nans, nulls, inf. feel free to ignore it if such tests already exist
Member
Author
There was a problem hiding this comment.
Added unary minus on float/double special values and nulls covering NaN, ±Infinity, signed zero (0.0/-0.0), the float/double range limits, and null, with dictionary on and off — all match Spark, since negation is an IEEE sign flip.
Integer Max/MinValue and the ANSI overflow behavior are already covered by the existing unary negative integer overflow test, so I left those out to avoid duplication.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Relates to #4500 (item 1: lift
convert-time fallbacks inarithmetic.scalaintogetSupportLevel).Rationale for this change
PR #4674 moved the arithmetic and math support checks into
getSupportLevelso that unsupported input types are reported at planning time, where the reason surfaces in EXPLAIN output and is picked up by the generated compatibility docs.CometUnaryMinuswas the only arithmetic serde left relying on aconvert-time fallback for unsupported input types: an interval input would serialize its child, fail, and fall back with a generic child-level reason rather than a clear datatype reason.This change completes that pattern for
UnaryMinus.What changes are included in this PR?
CometUnaryMinusnow mixes inMathBaseand overridesgetSupportLevelwithmathDataTypeSupportLevel(expr.child.dataType), matching the other arithmetic serdes. Every SparkNumericType(including decimal) stays native; interval types are reported asUnsupportedwith a datatype reason and fall back to Spark.CometExpressionSuiteregression test covering unary minus over byte, short, int, long, float, double, and decimal.How are these changes tested?
New test
unary minus across numeric typesinCometExpressionSuite, passing on Spark 3.5 and 4.0. Behavior was validated with theaudit-comet-expressionskill: numeric types (including decimal) run natively and match Spark, while day-time and year-month interval inputs fall back to Spark.