[SPARK-58965][SQL][PYTHON] Add gcd and lcm math functions - #58242
[SPARK-58965][SQL][PYTHON] Add gcd and lcm math functions#58242Vivek1106-04 wants to merge 5 commits into
Conversation
|
@uros-b ! can you review this, added two new functions which are standard functions in other databases. |
|
Thank you for @Vivek1106-04! I'll take a look soon. Also, cc @bojana-db to help with reviewing this PR. |
| */ | ||
| def factorial(e: Column): Column = Column.fn("factorial", e) | ||
|
|
||
| /** |
There was a problem hiding this comment.
gcd/lcm sit between factorial (5443) and floor (5488). The section is strictly alphabetical (cos, cosh, cot, csc, exp, expm1, factorial, floor, greatest, hex, hypot, least, ln, log), so gcd belongs after the floor overloads / before greatest (~5526) and lcm after the last hypot overload / before least (~5696).
__init__.py and functions.rst place both correctly, so this file is the lone inconsistency.
|
I also double-checked the other places the functions are registered/exported: |
|
|
||
| override def dataType: DataType = LongType | ||
|
|
||
| // The result overflows for inputs whose divisor is -Long.MinValue, which is null in ANSI mode. |
There was a problem hiding this comment.
| // The result overflows for inputs whose divisor is -Long.MinValue, which is null in ANSI mode. | |
| // The result overflows for inputs whose divisor is -Long.MinValue, which is null in non-ANSI mode. |
| }) | ||
| } | ||
|
|
||
| override def prettyName: String = "gcd" |
There was a problem hiding this comment.
This is redundant. Single word function names are correctly handled by the base class. (Same for lcm)
| checkEvaluation(Gcd(Literal(24L), Literal(36L)), 12L) | ||
| checkEvaluation(Gcd(Literal(36L), Literal(24L)), 12L) | ||
| checkEvaluation(Gcd(Literal(17L), Literal(5L)), 1L) | ||
| // The result is never negative, whichever inputs are. |
There was a problem hiding this comment.
| // The result is never negative, whichever inputs are. | |
| // The result is non-negative regardless of the inputs' signs. |
(Same for other occurrences.)
bojana-db
left a comment
There was a problem hiding this comment.
LGTM, thank you for the feature!
Left some suggestions for grammatically awkward comments.
| -- Gcd | ||
| SELECT gcd(24, 36); | ||
| SELECT gcd(17, 5); | ||
| -- The result is never negative, whichever inputs are. |
There was a problem hiding this comment.
| -- The result is never negative, whichever inputs are. | |
| -- The result is non-negative regardless of the inputs' signs. |
| -- Lcm | ||
| SELECT lcm(4, 6); | ||
| SELECT lcm(17, 5); | ||
| -- The result is never negative, whichever inputs are. |
There was a problem hiding this comment.
| -- The result is never negative, whichever inputs are. | |
| -- The result is non-negative regardless of the inputs' signs. |
|
Thank you @Vivek1106-04 and @bojana-db! |
|
Let's loop in @Yicong-Huang and/or @HyukjinKwon for PySpark as well ^^ |
|
cc @cloud-fan |
|
Also, please resolve conflicts @Vivek1106-04 |
Spark SQL has no expression, and no combination of existing built-ins, that computes the greatest common divisor or the least common multiple of two integers, so users have to fall back to a UDF. This adds `gcd(expr1, expr2)` and `lcm(expr1, expr2)`. Both arguments are implicitly cast to BIGINT and the result is BIGINT, matching the existing `factorial` expression. The result is never negative, `gcd(0, 0)` and `lcm(0, 0)` are 0, and a NULL argument yields NULL. The greatest common divisor is computed with the Euclidean algorithm. The least common multiple divides by it before multiplying, so a result that is representable does not overflow on the way there. Results that are genuinely unrepresentable raise ARITHMETIC_OVERFLOW under ANSI mode and return NULL otherwise, as elsewhere in Spark. Both helpers live in MathUtils and are shared by the interpreted and codegen paths. PostgreSQL 13+ and DuckDB provide the same two functions.
….scala The math_funcs section of functions.scala is ordered alphabetically. Place gcd after the floor overloads (before greatest) and lcm after the last hypot overload (before least). Pure code movement, no behavior change.
- Fix the Gcd nullable comment: overflow yields null in non-ANSI mode, not ANSI mode (ANSI raises). - Drop the redundant prettyName overrides on Gcd and Lcm; the base Expression.prettyName already lowercases the single-word node name. - Reword the sign-related comments in the gcd/lcm tests.
- Reword the sign-related comments in math.sql to match the wording already applied in MathExpressionsSuite. - Use the same "non-negative" wording in the public Scala and Python docs for gcd and lcm. - Collapse the Gcd and Lcm nullable comments to the fact they explain: overflow yields null in non-ANSI mode. - Drop the note that MathUtils.gcd/lcm are shared by the eval and codegen paths; that is what a shared helper is for.
The lcm scaladoc line exceeded the 98-column scalafmt limit for sql/api, failing the Scala linter check. Rewrap it to match the gcd scaladoc.
c51ef70 to
182c0be
Compare
What changes were proposed in this pull request?
This PR adds two new built-in math functions,
gcdandlcm:Both arguments are implicitly cast to BIGINT and the result is BIGINT, matching the existing
factorialexpression, which likewise takes an integral argument and returns BIGINT.Semantics:
gcdlcmabs(other)ARITHMETIC_OVERFLOWunder ANSI mode, NULL otherwiseARITHMETIC_OVERFLOWunder ANSI mode, NULL otherwiseThe greatest common divisor is computed with the Euclidean algorithm. The least common multiple
divides by the greatest common divisor before multiplying, so a representable result never
overflows on the way there — for example
lcm(4611686018427387904, 2)returns4611686018427387904even though the naive product
4611686018427387904 * 2would not fit.Overflow arises in exactly two places, and in both PostgreSQL raises as well:
gcdwhere the result would be-Long.MinValue, which is not representable — that is, theinput pairs
(0, x),(x, 0)and(x, x)forx = Long.MinValue.lcmwhereabs(a) / gcd(a, b) * abs(b)exceedsLong.MaxValue.Both are reported with the existing
ARITHMETIC_OVERFLOWerror condition, so no new errorcondition is introduced. Overflow is gated on ANSI mode as it is elsewhere in Spark —
convin thesame file takes the same approach — raising under ANSI mode and returning NULL otherwise.
Both helpers live in
MathUtilsand are shared by the interpreted and codegen paths so the twocannot diverge.
The functions are exposed through SQL, the Scala/Java
functionsAPI, PySpark, and Spark Connect.Why are the changes needed?
Spark SQL currently has no way to compute either value. There is no expression for it, and no
combination of existing built-ins produces the result, so users have to fall back to a UDF — which
for PySpark means a Python round trip per row and no whole-stage codegen.
Both functions are standard in comparable engines:
gcd(a, b),lcm(a, b)forinteger,bigintandnumericgcd(a, b),lcm(a, b)(aliasesgreatest_common_divisor,least_common_multiple)Common uses include reducing fractions and ratios to lowest terms, aligning batch or partition
sizes, computing the repeat period of overlapping schedules, and normalizing denominators before
aggregation.
Does this PR introduce any user-facing change?
Yes. Two new built-in functions are available in SQL, the Scala/Java
functionsAPI, PySpark andSpark Connect. Previously
gcdandlcmwere unresolved function names:No existing behavior changes; the change is purely additive.
How was this patch tested?
New tests:
MathExpressionsSuite— unit tests for both expressions covering ordinary values, negativeinputs, zero, NULL, the boundary values around
Long.MinValue/Long.MaxValue, and overflow inboth ANSI and non-ANSI mode, plus interpreted-vs-codegen consistency checks.
math.sqlgolden-file tests, regenerated for the ANSI and non-ANSI results and the analyzerresults.
PlanGenerationTestSuitefunction tests, with the regenerated Spark Connect plan and explaingolden files.
gcdandlcm.Existing suites re-run and passing:
MathExpressionsSuite,SQLQueryTestSuite(math.sql),ExpressionsSchemaSuite,PlanGenerationTestSuite,ProtoToParsedPlanTestSuite.dev/lint-scalaanddev/lint-pythonboth pass.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)