You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org> on 2026/09/29 13:01:50 UTC
[PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
SEPURI-SAI-KRISHNA opened a new pull request, #59135:
URL: https://github.com/apache/spark/pull/59135
### What changes were proposed in this pull request?
This PR makes `date_add` and `date_sub` (`DateAdd` / `DateSub`) fail with `DATETIME_OVERFLOW` when the resulting number of days does not fit in an `Int`, instead of silently wrapping around. It adds `DateTimeUtils.dateAddDays` / `dateSubtractDays`, which use `Math.addExact` / `Math.subtractExact`, and uses them in both the interpreted and the generated code.
This also covers the operators and functions that resolve to these expressions: `date + int`, `date - int`, `dateadd`, and `try_add` / `try_subtract` on dates, which now return `NULL` on overflow as documented.
### Why are the changes needed?
The days were added with plain `Int` arithmetic, so an overflowing result wrapped around to a wrong but valid-looking date, with ANSI mode on and off:
```sql
SELECT date_add(DATE'2020-01-01', 2147483647); -- -5877591-06-21
SELECT date_sub(DATE'1900-01-01', 2147483647); -- +5881510-07-14
SELECT try_add(DATE'2020-01-01', 2147483647); -- -5877591-06-21 instead of NULL
```
Similar operations already fail in this case, e.g. `add_months(DATE'2020-01-01', 2147483647)` raises `ARITHMETIC_OVERFLOW` and `DATE'2020-01-01' + make_dt_interval(2147483647, 0, 0, 0)` raises `INTERVAL_ARITHMETIC_OVERFLOW`. The error class follows `TIMESTAMPADD`, which raises `DATETIME_OVERFLOW` since SPARK-38481.
### Does this PR introduce _any_ user-facing change?
Yes. When the result of `date_add` / `date_sub` does not fit in the date range, it now fails instead of returning a wrong date:
```
[DATETIME_OVERFLOW] Datetime operation overflow: add 2147483647 days to DATE '2020-01-01'. SQLSTATE: 22008
```
`try_add` / `try_subtract` on dates return `NULL` in that case. Results that fit are unchanged.
### How was this patch tested?
- New tests in `DateExpressionsSuite` for overflow in both directions and for the largest values that still fit, in both interpreted and codegen mode. The existing consistency checks for `IntegerType` now allow both paths to throw. The new tests fail without this change.
- Ran the queries above on a table (not constant folded) with whole-stage codegen and interpreted mode, with ANSI mode on and off.
- Ran `DateExpressionsSuite`, `DateTimeUtilsSuite`, `DateFunctionsSuite`, and the date, timestamp, interval and try_arithmetic tests in `SQLQueryTestSuite`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4284811138
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala:
##########
@@ -510,7 +510,20 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
DateAdd(Literal(Date.valueOf("2016-02-28")), negativeIntLit), -15910)
checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, ByteType)
checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, ShortType)
- checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, IntegerType)
+ checkConsistencyBetweenInterpretedAndCodegenAllowingException(DateAdd, DateType, IntegerType)
+ }
+
+ test("SPARK-59859: date_add fails on overflow instead of wrapping around") {
Review Comment:
Added `try_add(date'2021-01-01', 2147483647)`, `try_add(2147483647, date'2021-01-01')` and `try_subtract(date'1900-01-01', 2147483647)` to `try_arithmetic.sql`, plus a non-overflowing `try_subtract(date'2021-01-01', 1)` under a new `-- Date - Integer` block. I regenerated the golden files on JDK 17 and then on JDK 21, so `nonansi/try_arithmetic.sql.out.java21` has the new cases too. Without the fix, the new cases return the wrapped date `-5877590-06-22`.
I also fixed the sentence in the PR description: it now says the failure happens only when the day count overflows an `Int`, and that `date_add(DATE'9999-12-31', 1)` still returns `+10000-01-01`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub) via reviews" <re...@spark.apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4476997468
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -534,13 +534,12 @@ case class DateAdd(startDate: Expression, days: Expression)
override def dataType: DataType = DateType
Review Comment:
Thanks @stevomitric! Yes, `datediff` has the same issue. It subtracts the two day counts as an
`Int`, so the result wraps around with ANSI mode on or off, in both interpreted and generated
code. I checked it on master and on 4.0.3, on values read from a table:
```sql
datediff(date_from_unix_date(2147483647), date_from_unix_date(-1)) -- -2147483648
datediff(date_from_unix_date(2147483647), date_from_unix_date(-2147483648)) -- -1
```
On the same values, subtracting dates (`d1 - d2`), `timestampdiff(DAY, ...)` and
`months_between` fail with a raw `java.lang.ArithmeticException` instead of an error class.
I'll file a separate JIRA and open a follow-up PR for these.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4281264122
##########
docs/sql-migration-guide.md:
##########
@@ -48,6 +48,7 @@ license: |
- Since Spark 4.4, `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` defaults to `true`. A V2 scan that reports a keyed partitioning but no explicit ordering now also reports itself sorted by its partition key expressions, because every row of such a partition evaluates them to the same value. Partition transforms such as `days(ts)` or `bucket(8, id)` are left out of that ordering. This lets Spark drop a `Sort` it would otherwise place above the scan. An aggregate that groups by a leading run of the other key expressions, in key order, may also be planned as a sort aggregate rather than a hash aggregate. This is because `spark.sql.execution.replaceHashWithSortAgg` keys off the reported ordering. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` to `false`.
- Since Spark 4.4, `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` defaults to `true`. When `GroupPartitionsExec` merges several input partitions that share one partition key value into a single output partition, it now keeps sort orders over the partition key expressions in the ordering it reports, since those expressions are constant within the merged partition. Sort orders over partition transforms such as `days(ts)` or `bucket(8, id)` are left out. Orders over other columns are still dropped, as the concatenation invalidates them, and a join that reduced the partition keys onto a common key space (see `spark.sql.sources.v2.bucketing.allowCompatibleTransforms.enabled`) reports no order, since the merged partitions then share only the reduced key. This can remove a `Sort` downstream of the merge. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` to `false`.
- Since Spark 4.4, the value columns of `UNPIVOT` and `Dataset.unpivot` merge the nested nullability (array element, map value, and struct field nullability) of all unpivoted values. Previously, a value column took the nested nullability of the first listed value, so a `NULL` array element, map value, or struct field of a later value could be returned as a non-`NULL` value such as `0`, and `IS NULL` on it returned `false`. A view created by an earlier Spark version over such an `UNPIVOT` may have recorded the non-nullable nested type in its schema and can fail to read; recreate it with `CREATE OR REPLACE VIEW`. To restore the previous behavior, set `spark.sql.legacy.unpivot.useFirstValueNestedNullability` to `true`.
+- Since Spark 4.4, `date_add`, `date_sub`, `dateadd`, and adding or subtracting an integer number of days to or from a date (for example `date + 1` or `date - 1`) fail with `DATETIME_OVERFLOW` when the number of days since 1970-01-01 in the result does not fit in an `INT`, regardless of `spark.sql.ansi.enabled`. Previously, the result silently wrapped around; for example, `date_add(DATE'2020-01-01', 2147483647)` returned a date in year -5877591. `try_add` and `try_subtract` on dates now return `NULL` in this case instead of the wrapped date.
Review Comment:
**Nit (P3):** This entry lists `date_add`, `date_sub`, `dateadd`, and `date + 1` / `date - 1`. The check lives in `DateAdd`, though, and `WindowEvaluatorFactoryBase.createBoundOrdering` also builds `DateAdd(expr, boundOffset)` for a `RANGE` frame ordered by a DATE column with an integer offset. Take this query:
```sql
SELECT d, count(*) OVER (ORDER BY d RANGE BETWEEN CURRENT ROW AND 2147483647 FOLLOWING)
FROM VALUES (DATE'2020-01-01') AS t(d)
```
It used to compute a wrapped, wrong frame bound. It now fails with `DATETIME_OVERFLOW`, regardless of `spark.sql.ansi.enabled`. Someone upgrading wouldn't connect that failure to this entry, since they never wrote `date + n` themselves. Could you add a clause for DATE-ordered RANGE frames with integer offsets? Day-interval forms (`date + INTERVAL n DAY` and DAY-interval frame offsets) don't need a mention. A DAY interval is at most 106751991 days, so they can't overflow for dates in the documented range.
**Verification:**
- **Inspection:** The rendered migration entry explicitly mentions RANGE window frames over DATE columns with integer offsets. Its remaining claims still match the implementation: it fails only on Int day-count overflow, regardless of ANSI mode, and try_add/try_subtract return NULL.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala:
##########
@@ -510,7 +510,20 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
DateAdd(Literal(Date.valueOf("2016-02-28")), negativeIntLit), -15910)
checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, ByteType)
checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, ShortType)
- checkConsistencyBetweenInterpretedAndCodegen(DateAdd, DateType, IntegerType)
+ checkConsistencyBetweenInterpretedAndCodegenAllowingException(DateAdd, DateType, IntegerType)
+ }
+
+ test("SPARK-59859: date_add fails on overflow instead of wrapping around") {
Review Comment:
**Nit (P3):** These tests cover `DateAdd`/`DateSub` directly. The PR also says `try_add`/`try_subtract` on a date now return NULL on overflow, and that depends on a separate composition. `TryAdd`'s non-numeric branch builds `TryEval(Add(..., EvalMode.ANSI))`, the resolver lowers the inner `Add` to `DateAdd`, and `TryEval` turns the exception into NULL. No test runs that path with an overflowing date, and the existing date cases in `try_arithmetic.sql` don't overflow. That branch also carries `// TODO: support TRY eval mode on datetime arithmetic expressions.`. If someone later drops the `TryEval` wrapper while implementing it, `try_add(DATE'2020-01-01', 2147483647)` would start throwing and no test would fail. A couple of cases next to the `-- Date + Integer` block in `try_arithmetic.sql` would pin this down, for example `try_add(date'2021-01-01', 2147483647)` and `try_subtract(date'1900-01-01', 2147483647)`. Note that `inputs/nonansi/try_arithmetic.sql` imports that file. That me
ans `results/nonansi/try_arithmetic.sql.out.java21` also has to be regenerated, on JDK 21. A JDK 17 run leaves it stale, and that has needed follow-up commits for this file before.
**Verification:**
- **Behavior:** SQL-level try_add and try_subtract on a date with a day count that overflows the Int range return NULL, while the existing non-overflowing date cases keep their current results.
- **Regression:** The new cases distinguish the fixed behavior from both failure modes: a wrapped date (the pre-PR behavior) and a propagated DATETIME_OVERFLOW (lost TryEval wrapping).
- **Regression:** SQLQueryTestSuite passes for try_arithmetic.sql and nonansi/try_arithmetic.sql on JDK 21 as well as JDK 17, which shows the .java21 golden file was regenerated with the new cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub) via reviews" <re...@spark.apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4483664732
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -534,13 +534,12 @@ case class DateAdd(startDate: Expression, days: Expression)
override def dataType: DataType = DateType
Review Comment:
Filed SPARK-60910 and opened #60134 for `datediff` / `date_diff` and date subtraction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "stevomitric (via GitHub) via reviews" <re...@spark.apache.org>.
stevomitric commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4470872344
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -534,13 +534,12 @@ case class DateAdd(startDate: Expression, days: Expression)
override def dataType: DataType = DateType
Review Comment:
not suggesting to do it in this PR, but could you check if we have the same issue for `datediff(date_from_unix_date(2147483647), date_from_unix_date(-1))` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub) via reviews" <re...@spark.apache.org>.
SEPURI-SAI-KRISHNA commented on PR #59135:
URL: https://github.com/apache/spark/pull/59135#issuecomment-6664516176
Thanks @uros-b. The only failure was `StarRocksIntegrationSuite` in the Docker integration tests:
it timed out connecting to the StarRocks container (the other 674 tests in that job passed), and
this PR does not touch JDBC. I pushed a commit with the window frame test from the review, so CI
is running again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4284801866
##########
docs/sql-migration-guide.md:
##########
@@ -48,6 +48,7 @@ license: |
- Since Spark 4.4, `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` defaults to `true`. A V2 scan that reports a keyed partitioning but no explicit ordering now also reports itself sorted by its partition key expressions, because every row of such a partition evaluates them to the same value. Partition transforms such as `days(ts)` or `bucket(8, id)` are left out of that ordering. This lets Spark drop a `Sort` it would otherwise place above the scan. An aggregate that groups by a leading run of the other key expressions, in key order, may also be planned as a sort aggregate rather than a hash aggregate. This is because `spark.sql.execution.replaceHashWithSortAgg` keys off the reported ordering. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` to `false`.
- Since Spark 4.4, `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` defaults to `true`. When `GroupPartitionsExec` merges several input partitions that share one partition key value into a single output partition, it now keeps sort orders over the partition key expressions in the ordering it reports, since those expressions are constant within the merged partition. Sort orders over partition transforms such as `days(ts)` or `bucket(8, id)` are left out. Orders over other columns are still dropped, as the concatenation invalidates them, and a join that reduced the partition keys onto a common key space (see `spark.sql.sources.v2.bucketing.allowCompatibleTransforms.enabled`) reports no order, since the merged partitions then share only the reduced key. This can remove a `Sort` downstream of the merge. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` to `false`.
- Since Spark 4.4, the value columns of `UNPIVOT` and `Dataset.unpivot` merge the nested nullability (array element, map value, and struct field nullability) of all unpivoted values. Previously, a value column took the nested nullability of the first listed value, so a `NULL` array element, map value, or struct field of a later value could be returned as a non-`NULL` value such as `0`, and `IS NULL` on it returned `false`. A view created by an earlier Spark version over such an `UNPIVOT` may have recorded the non-nullable nested type in its schema and can fail to read; recreate it with `CREATE OR REPLACE VIEW`. To restore the previous behavior, set `spark.sql.legacy.unpivot.useFirstValueNestedNullability` to `true`.
+- Since Spark 4.4, `date_add`, `date_sub`, `dateadd`, and adding or subtracting an integer number of days to or from a date (for example `date + 1` or `date - 1`) fail with `DATETIME_OVERFLOW` when the number of days since 1970-01-01 in the result does not fit in an `INT`, regardless of `spark.sql.ansi.enabled`. Previously, the result silently wrapped around; for example, `date_add(DATE'2020-01-01', 2147483647)` returned a date in year -5877591. `try_add` and `try_subtract` on dates now return `NULL` in this case instead of the wrapped date.
Review Comment:
Thanks, good catch. I checked it: before this change that query counted 0 rows in the frame instead of 1, and now it fails with `DATETIME_OVERFLOW` with ANSI mode on and off. A `PRECEDING` offset of 2147483647 still works, and so does a DAY-interval offset. I added a sentence on DATE-ordered `RANGE` frames with an integer offset to the migration guide entry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub) via reviews" <re...@spark.apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4483617240
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -539,13 +539,12 @@ case class DateAdd(startDate: Expression, days: Expression)
override def dataType: DataType = DateType
override def nullSafeEval(start: Any, d: Any): Any = {
- start.asInstanceOf[Int] + d.asInstanceOf[Number].intValue()
+ DateTimeUtils.dateAddDays(start.asInstanceOf[Int], d.asInstanceOf[Number].intValue())
Review Comment:
Thanks, added `SPARK-59859: range frame ordered by a date fails when an integer offset overflows`
to `DataFrameWindowFramesSuite`, with ANSI mode on and off:
- `ORDER BY d RANGE BETWEEN CURRENT ROW AND 2147483647 FOLLOWING` fails with `DATETIME_OVERFLOW`.
- `ORDER BY d DESC RANGE BETWEEN 2147483647 PRECEDING AND CURRENT ROW` fails with
`DATETIME_OVERFLOW`, since its bound is `d + 2147483647`.
- `ORDER BY d RANGE BETWEEN 2147483647 PRECEDING AND CURRENT ROW` returns the expected counts.
Without the fix, both overflowing frames count 0 rows for every row, with ANSI mode on and off,
although each frame contains at least the current row.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #59135:
URL: https://github.com/apache/spark/pull/59135#issuecomment-5930045836
Thanks @uros-b, added an entry to the 4.3 to 4.4 section. It says "does not fit in an INT" rather than out of the DATE range on purpose, since `date_add(DATE'9999-12-31', 1)` still returns `+10000-01-01`; only the day count overflowing fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "uros-b (via GitHub) via reviews" <re...@spark.apache.org>.
uros-b commented on PR #59135:
URL: https://github.com/apache/spark/pull/59135#issuecomment-6656338652
@SEPURI-SAI-KRISHNA Please make the CI green here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59859][SQL] Fail on overflow in date_add and date_sub instead of wrapping around [spark]
Posted by "spark-test (via GitHub) via reviews" <re...@spark.apache.org>.
spark-test commented on code in PR #59135:
URL: https://github.com/apache/spark/pull/59135#discussion_r4480692436
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala:
##########
@@ -539,13 +539,12 @@ case class DateAdd(startDate: Expression, days: Expression)
override def dataType: DataType = DateType
override def nullSafeEval(start: Any, d: Any): Any = {
- start.asInstanceOf[Int] + d.asInstanceOf[Number].intValue()
+ DateTimeUtils.dateAddDays(start.asInstanceOf[Int], d.asInstanceOf[Number].intValue())
Review Comment:
**Nit (P3):** This check also makes `RANGE` window frames ordered by a `DATE` column with an integer offset fail on overflow, because `WindowEvaluatorFactoryBase.createBoundOrdering` builds the frame bound as `DateAdd(orderExpr, offset)`, negating the offset for descending order. The migration entry documents that a frame such as `RANGE BETWEEN CURRENT ROW AND 2147483647 FOLLOWING` now fails with `DATETIME_OVERFLOW` instead of using a wrapped bound, and the PR description reports a manual run of that query. The new tests, though, only evaluate `DateAdd`/`DateSub` directly and run `try_add`/`try_subtract`. If the bound computation ever stopped going through the checked `DateAdd`, the old wrong frames would come back without any test failing. Could you add a window frame test for this, in `DataFrameWindowFramesSuite` or `window.sql`?
**Verification:**
- **Regression:** A window count over a single DATE'2020-01-01' row with RANGE BETWEEN CURRENT ROW AND 2147483647 FOLLOWING fails with DATETIME_OVERFLOW instead of counting 0 rows, with ANSI mode on and off.
- **Regression:** A descending DATE-ordered RANGE frame whose integer-offset bound overflows, such as 2147483647 PRECEDING with ORDER BY d DESC, also fails with DATETIME_OVERFLOW.
- **Behavior:** A DATE-ordered RANGE frame with 2147483647 PRECEDING over DATE'2020-01-01' in ascending order returns the expected frame.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org