[
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 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.
was:
h3. Problem
The two-argument {{floor}} and {{ceil}} can return a value on the wrong side of
their floating-point input. They implicitly convert Double to
{{decimal(30,15)}} and Float to {{decimal(14,7)}} before applying directional
rounding. The conversion can round across the very boundary that floor/ceil
should preserve.
h3. Reproduction
For example, with explicit Double inputs:
{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}
The current results are {{(0, 1, 0.9)}}. The expected numerical results are
{{(0, 0, 1.0)}}. The unary result remains a long and the scaled results remain
decimals; this report does not propose making their types identical.
|| Explicit Double expression || Current || Expected ||
| {{floor(0.8999999999999999, 1)}} | 0.9 | 0.8 |
| {{floor(1.7999999999999998, 1)}} | 1.8 | 1.7 |
| {{floor(0.9999999999999999, 0)}} | 1 | 0 |
| {{ceil(0.9000000000000001, 1)}} | 0.9 | 1.0 |
This affects directional rounding and interval/bucket boundaries, including
values obtained from calculations rather than SQL literals. Runtime-column
regressions reproduce the problem without relying on constant folding.
h3. Cause and proposed correction
The cause is in Catalyst's [RoundFloor and RoundCeil
implementation|https://github.com/apache/spark/blob/f04e6d60ebf9131fd83f306627697370ef52a309/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala]:
both require decimal input, so analysis inserts the lossy cast before
rounding. This is distinct from ordinary binary floating-point representation
error: the supplied Double values are already on the correct side of the
boundary, but the intermediate cast moves them across it.
The proposed correction is local to scaled floor/ceil: accept Float/Double
directly and defer rounding to the requested output scale. Preserve existing
decimal result precision/scale, explicit user casts, integral/decimal input
behavior, and established null, non-finite and ANSI overflow behavior. Do not
change global numeric-to-decimal coercion.
Float results can also change. For example, {{3.1415f}} represents
{{3.141499996185302734375}}, so floor at scale 4 should produce {{3.1414}}, not
{{3.1415}}. The proposal does not expand the existing maximum output scales or
promise exact binary arithmetic for every input; it retains Spark's existing
Double-to-Decimal conversion convention while removing the premature scale
reduction.
h3. Related work
The decimal return type is intentional, as discussed in
[SPARK-38604|https://issues.apache.org/jira/browse/SPARK-38604] and [the
original scale
implementation|https://github.com/apache/spark/pull/34729#discussion_r789493919].
[SPARK-59373|https://issues.apache.org/jira/browse/SPARK-59373] clarified that
contract without changing behavior. This precision problem is separate from the
overflow work in
[SPARK-58052|https://issues.apache.org/jira/browse/SPARK-58052].
h3. Validation
Recorded Catalyst reproductions cover released 4.0.2 and 4.1.2. On master
{{f04e6d60ebf9131fd83f306627697370ef52a309}}, two new regressions fail before
the local fix and pass afterward. Four relevant suites pass (242 tests),
including SQL runtime columns with code generation enabled/disabled and ANSI
enabled/disabled. Local builds required a session-only Netty 4.2.18 override
because the configured proxy rejects master's 4.2.19 artifacts as "version too
new"; validation with the exact upstream dependency set remains outstanding.
> 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 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.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
