[
https://issues.apache.org/jira/browse/SPARK-60202?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Varun Bhandary updated SPARK-60202:
-----------------------------------
Description:
h3. Problem and reproduction
I found that scaled {{floor}} and {{ceil}} can round Double inputs in the wrong
direction while investigating rounding behavior:
{code:sql}
SELECT
floor(CAST('0.9999999999999999' AS DOUBLE)) AS unary_floor,
floor(CAST('0.9999999999999999' AS DOUBLE), 0) AS scaled_floor,
ceil(CAST('0.9000000000000001' AS DOUBLE), 1) AS scaled_ceil;
{code}
Baseline numerical results: {{(0, 1, 0.9)}}. Expected: {{(0, 0, 1.0)}}. The
first input is below one; the last is above 0.9.
Catalyst casts Double to {{decimal(30,15)}} and Float to {{decimal(14,7)}}
before applying scaled floor/ceil. This fixed-scale conversion can cross a
rounding boundary before the operation runs. Runtime columns also reproduce the
issue. It affects the shared SQL implementation used by Classic and Connect,
and can change bucket or interval assignments.
h3. Proposed behavior
* Avoid the premature fixed-scale cast in scaled floor/ceil. Retain existing
decimal result precision/scale, output-scale caps and input-range checks; unary
floor/ceil remain unchanged.
* Follow [Spark's existing round/bround conversion
convention|https://github.com/apache/spark/blob/f04e6d60ebf9131fd83f306627697370ef52a309/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala]:
widen Float to Double, then use its canonical decimal representation before
directional rounding. This is not an exact-binary arithmetic contract.
* For result overflow, raise an error with ANSI enabled and return null with
ANSI disabled, consistently for Float, Double and Decimal inputs to scaled
floor/ceil. This deliberately includes correcting existing Decimal
result-overflow behavior in these functions. Changes to round/bround/truncate
are outside this proposal.
h3. Compatibility and current status
The Float policy changes ordinary inputs too: {{ceil(CAST('0.1' AS FLOAT), 1)}}
changes from {{0.1}} to {{0.2}}; the Double form remains {{0.1}}. This follows
the conversion convention above and needs compatibility review. SQL standards
and other engines do not establish one required conversion policy for Spark's
scaled, decimal-returning overload.
For {{ceil(CAST('1e-20' AS DOUBLE), -38)}}, the old cast loses the input to
zero. Preserving it exposes a result of {{1e38}}, outside {{decimal(38,0)}}.
The current patch throws in both ANSI modes; the proposed ANSI-disabled null
behavior is *not implemented yet*.
The defect was reproduced in 4.0.2, 4.1.2 and master
{{f04e6d60ebf9131fd83f306627697370ef52a309}}. Two new Double regressions failed
before the fix and passed afterward. [PR
#59408|https://github.com/apache/spark/pull/59408] records implementation and
validation. The overflow change, additional compatibility/schema tests and
migration documentation remain pending; the contribution is not yet ready to
merge. No new public API, setting or dependency is proposed.
was:
h3. Problem and reproduction
I found that two-argument {{floor}} and {{ceil}} can round floating-point
inputs in the wrong direction while investigating rounding behavior. Catalyst
implicitly casts Double to {{decimal(30,15)}} and Float to {{decimal(14,7)}}
before directional rounding. That cast can move an input across the requested
boundary.
{code:sql}
SELECT
floor(CAST('0.9999999999999999' AS DOUBLE)) AS unary_floor,
floor(CAST('0.9999999999999999' AS DOUBLE), 0) AS scaled_floor,
ceil(CAST('0.9000000000000001' AS DOUBLE), 1) AS scaled_ceil;
{code}
Current numerical results: {{(0, 1, 0.9)}}. Expected: {{(0, 0, 1.0)}}. Unary
floor remains a long; scaled results remain decimals.
|| Explicit Double expression || Current || Expected ||
| {{floor(CAST('0.8999999999999999' AS DOUBLE), 1)}} | 0.9 | 0.8 |
| {{floor(CAST('1.7999999999999998' AS DOUBLE), 1)}} | 1.8 | 1.7 |
The supplied Double values are already on the appropriate side of the boundary;
the intermediate cast changes that. Runtime-column regressions also reproduce
this, so it is not confined to constant folding or Spark Connect. Changed
rounding can affect interval and bucket assignments.
h3. Proposed correction and Spark precedent
Avoid the premature fixed-scale cast locally in
[RoundFloor/RoundCeil|https://github.com/apache/spark/blob/f04e6d60ebf9131fd83f306627697370ef52a309/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala].
Retain existing decimal result precision/scale, explicit casts,
integral/decimal input behavior, null/non-finite handling and input-range
checks. No public API, setting, dependency or global coercion change is
proposed.
Decimal output is intentional: see
[SPARK-38604|https://issues.apache.org/jira/browse/SPARK-38604] and [the
original implementation
discussion|https://github.com/apache/spark/pull/34729#discussion_r789493919].
[SPARK-59373|https://issues.apache.org/jira/browse/SPARK-59373] clarified the
return-type documentation without changing behavior.
The candidate follows Spark's existing {{round}}/{{bround}} conversion
convention: widen Float to Double, then use decimal conversion before
directional rounding at the retained output scale. This is not exact IEEE-value
arithmetic for every input. The Float conversion policy and compatibility
effects below need explicit review.
h3. Compatibility and ANSI considerations
*Ordinary Float inputs can change.* These are observed candidate results, not a
claim that every engine must produce them:
|| Expression || Previous path || Candidate ||
| {{ceil(CAST('0.1' AS FLOAT), 1)}} | 0.1 | 0.2 |
| {{floor(CAST('0.9' AS FLOAT), 1)}} | 0.9 | 0.8 |
The widened Float values lie just above 0.1 and below 0.9. For Double {{0.1}},
the canonical decimal conversion yields {{0.1}}, so ceil at scale 1 remains
{{0.1}}. Explicit decimal casts continue to let callers choose normalization.
*Input-range compatibility does not imply unchanged result overflow.* For
{{ceil(CAST('1e-20' AS DOUBLE), -38)}}, the old cast loses the input to zero.
The candidate instead reaches {{1e38}}, which cannot fit {{decimal(38,0)}}, and
throws in both ANSI modes through the existing decimal negative-scale path;
scale -37 succeeds. [Spark's general ANSI
guidance|https://spark.apache.org/docs/latest/sql-ref-ansi-compliance.html#arithmetic-operations]
describes null on decimal overflow with ANSI disabled. This discrepancy needs
an explicit decision and tests; the proposal does not claim unchanged overflow
outcomes or silently redefine global overflow rules. It is separate from
[SPARK-58052|https://issues.apache.org/jira/browse/SPARK-58052].
*Standards and other engines guide the review, but do not settle this
overload.* [ISO SQL's 2003 corrigendum, section
6.2|https://cdn.standards.iteh.ai/samples/37833/600f10511a6d4d70b305701fd69ed417/ISO-IEC-9075-2-1999-Amd-1-2001-Cor-1-2003.pdf]
specifies unary floor/ceiling direction and an exception for unrepresentable
results, with different result categories for exact and approximate inputs. It
does not establish Spark's scaled Decimal-returning contract. [Snowflake's
scaled CEIL|https://docs.snowflake.com/en/sql-reference/functions/ceil] returns
FLOAT for FLOAT input, unlike Spark. This is a focused correctness proposal,
not a claim of ANSI certification or identical floating-point results across
engines.
h3. Validation and remaining review work
Recorded Catalyst reproductions cover 4.0.2 and 4.1.2. On master
{{f04e6d60ebf9131fd83f306627697370ef52a309}}, two new regressions fail before
the fix and pass afterward. Four relevant suites pass (242 tests), including
runtime SQL with code generation and ANSI each enabled/disabled; three style
checks pass. Local tests used a temporary Netty 4.2.18 override because the
proxy blocks 4.2.19; no dependency files changed. Default-dependency CI results
are tracked on the linked PR.
[PR #59408|https://github.com/apache/spark/pull/59408] contains the candidate.
Follow-up remains to add ordinary-Float and result-overflow regressions in both
ANSI/execution modes, independent expected-schema assertions, and a [migration
note|https://spark.apache.org/contributing.html#behavior-changes] explaining
changed results and potential recomputation of persisted buckets. The Float
policy and ANSI-disabled overflow behavior remain review decisions.
> Preserve floating-point boundaries in scaled floor and ceil
> -----------------------------------------------------------
>
> Key: SPARK-60202
> URL: https://issues.apache.org/jira/browse/SPARK-60202
> Project: Spark
> Issue Type: Bug
> Components: SQL
> Affects Versions: 4.0.2, 4.1.2
> Reporter: Varun Bhandary
> Priority: Major
> Labels: correctness, pull-request-available
>
> h3. Problem and reproduction
> I found that scaled {{floor}} and {{ceil}} can round Double inputs in the
> wrong direction while investigating rounding behavior:
> {code:sql}
> SELECT
> floor(CAST('0.9999999999999999' AS DOUBLE)) AS unary_floor,
> floor(CAST('0.9999999999999999' AS DOUBLE), 0) AS scaled_floor,
> ceil(CAST('0.9000000000000001' AS DOUBLE), 1) AS scaled_ceil;
> {code}
> Baseline numerical results: {{(0, 1, 0.9)}}. Expected: {{(0, 0, 1.0)}}. The
> first input is below one; the last is above 0.9.
> Catalyst casts Double to {{decimal(30,15)}} and Float to {{decimal(14,7)}}
> before applying scaled floor/ceil. This fixed-scale conversion can cross a
> rounding boundary before the operation runs. Runtime columns also reproduce
> the issue. It affects the shared SQL implementation used by Classic and
> Connect, and can change bucket or interval assignments.
> h3. Proposed behavior
> * Avoid the premature fixed-scale cast in scaled floor/ceil. Retain existing
> decimal result precision/scale, output-scale caps and input-range checks;
> unary floor/ceil remain unchanged.
> * Follow [Spark's existing round/bround conversion
> convention|https://github.com/apache/spark/blob/f04e6d60ebf9131fd83f306627697370ef52a309/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala]:
> widen Float to Double, then use its canonical decimal representation before
> directional rounding. This is not an exact-binary arithmetic contract.
> * For result overflow, raise an error with ANSI enabled and return null with
> ANSI disabled, consistently for Float, Double and Decimal inputs to scaled
> floor/ceil. This deliberately includes correcting existing Decimal
> result-overflow behavior in these functions. Changes to round/bround/truncate
> are outside this proposal.
> h3. Compatibility and current status
> The Float policy changes ordinary inputs too: {{ceil(CAST('0.1' AS FLOAT),
> 1)}} changes from {{0.1}} to {{0.2}}; the Double form remains {{0.1}}. This
> follows the conversion convention above and needs compatibility review. SQL
> standards and other engines do not establish one required conversion policy
> for Spark's scaled, decimal-returning overload.
> For {{ceil(CAST('1e-20' AS DOUBLE), -38)}}, the old cast loses the input to
> zero. Preserving it exposes a result of {{1e38}}, outside {{decimal(38,0)}}.
> The current patch throws in both ANSI modes; the proposed ANSI-disabled null
> behavior is *not implemented yet*.
> The defect was reproduced in 4.0.2, 4.1.2 and master
> {{f04e6d60ebf9131fd83f306627697370ef52a309}}. Two new Double regressions
> failed before the fix and passed afterward. [PR
> #59408|https://github.com/apache/spark/pull/59408] records implementation and
> validation. The overflow change, additional compatibility/schema tests and
> migration documentation remain pending; the contribution is not yet ready to
> merge. No new public API, setting or dependency is proposed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
