kosiew commented on code in PR #23723:
URL: https://github.com/apache/datafusion/pull/23723#discussion_r3663830007
##########
datafusion/sqllogictest/test_files/table_functions.slt:
##########
@@ -197,6 +197,43 @@ SELECT * FROM generate_series(1, 2, 3, 4)
statement error DataFusion error: Error during planning: Argument \#1 must be
an INTEGER, TIMESTAMP, DATE or NULL, got Utf8
SELECT * FROM generate_series('foo', 'bar')
+# Regression test for https://github.com/apache/datafusion/issues/22208
+# A step that would overflow i64 after the last reachable value must return the
+# reachable values instead of panicking, matching PostgreSQL/DuckDB behavior.
+query I
+SELECT * FROM generate_series(9223372036854775806, 9223372036854775807, 2)
+----
+9223372036854775806
+
+# Same, in the descending direction
+query I
+SELECT * FROM generate_series(-9223372036854775806, -9223372036854775808, -2)
+----
+-9223372036854775806
+-9223372036854775808
+
+# Landing exactly on i64::MAX must include it
+query I
+SELECT * FROM generate_series(9223372036854775805, 9223372036854775807, 2)
+----
+9223372036854775805
+9223372036854775807
+
+# Same overflow behavior for `range` (end exclusive)
+query I
+SELECT * FROM range(9223372036854775806, 9223372036854775807, 2)
+----
+9223372036854775806
+
+# Regression test for https://github.com/apache/datafusion/issues/22193
Review Comment:
It would be helpful to add positive boundary tests for the maximum
representable date and timestamp, alongside the existing out-of-range error
cases. That would lock in the expected behaviour of returning all reachable
values at the edge of the nanosecond range.
##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -417,7 +433,15 @@ impl<T: SeriesValue> LazyBatchGenerator for
GenericSeriesState<T> {
.should_stop(self.end.clone(), &self.step, self.include_end)
{
buf.push(self.current.to_value_type());
- self.current.advance(&self.step)?;
+ if self
+ .current
+ .should_stop(self.end.clone(), &self.step, false)
+ {
+ self.current.advance(&mut self.end, &self.step)?;
Review Comment:
It looks like the generator still advances after emitting the inclusive end
value. For date and timestamp series near the supported upper bound, this can
turn a valid one-row query into an error.
For example:
```sql
SELECT * FROM generate_series(
DATE '2262-04-11',
DATE '2262-04-11',
INTERVAL '1' DAY
);
```
This should return the single reachable row, but it currently fails with
`Failed to add interval ... to timestamp 9223286400000000000`.
Could we apply the same terminal-state rule used for integer overflow and
avoid calling `advance` once the current value is the last value to emit? One
option would be to have `advance` report whether a next value exists. Another
would be to break before advancing when the exclusive stop condition has
already been reached.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
