Fix pow() to check exponent integrality by value, not representation - #38385
Open
antiguru wants to merge 1 commit into
Open
Fix pow() to check exponent integrality by value, not representation#38385antiguru wants to merge 1 commit into
antiguru wants to merge 1 commit into
Conversation
`power_numeric` decided that a negative base was being raised to a non-integer power by testing `b.exponent() < 0`. That reads the representation rather than the value: an integral exponent that still carries trailing fractional zeroes, such as the `2.0` produced by `1.5 + 0.5` (coefficient 20, exponent -1), tripped the complex-number guard even though PostgreSQL returns 4 for `power(-2, 2.0)`. This is the same hazard class as the `round(numeric, scale)` fix in #38277. `Row` encoding reduces numerics, folding trailing zeroes into the exponent, so equal values reach the function in different representations depending on whether they made a round trip through a `Row`. Whether the function errors must not depend on which one arrives: the abstract interpreter reads its datums back out of a `Row`, so it would call the expression infallible while the evaluator failed on the inline representation, which is exactly the unsoundness persist filter pushdown relies on not happening. Reduce the exponent before reading it, so integrality is a property of the value. `decNumberPower` itself already recognises `2.0` as an integral exponent, so only the guard needed fixing. Adds a `mz-expr` unit test covering several representations of the same integral exponent, and a `numeric.slt` case that computes the exponent at runtime from a table column so the value reaches `pow` unreduced. Closes: CPU-214
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.
Motivation
The
pow()function rejects negative bases with non-integral exponents to avoid complex results. However, the integrality check was examining the exponent's representation (specifically, whetherexponent < 0) rather than its actual value.This caused inconsistent behavior: the same integral value could be represented differently depending on whether it was computed inline or read back from a
Row. For example,2.0might arrive as coefficient2with exponent0, or as coefficient20with exponent-1afterRowencoding folds trailing zeroes into the exponent. The old check would incorrectly rejectpow(-2, 2.0)in the latter case.Description
This PR introduces
is_integral_numeric()which determines whether a numeric value is integral by reducing it first, making integrality a property of the value rather than its representation. The function:The
power_numeric()function now usesis_integral_numeric(b)instead ofb.exponent() < 0to validate the exponent before computing the power.Verification
power_numeric_integral_exponent_representations()that verifies the same integral exponent value (represented multiple ways:"2","2.0","2.000","2E+0","0.2E+1") all produce consistent resultshttps://claude.ai/code/session_01MkQaXN6bFv9JxFeHpRtof7