u70b3 commented on code in PR #23723:
URL: https://github.com/apache/datafusion/pull/23723#discussion_r3680740474


##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -80,7 +80,12 @@ pub trait SeriesValue: fmt::Debug + Clone + Send + Sync + 
'static {
     fn should_stop(&self, end: Self, step: &Self::StepType, include_end: bool) 
-> bool;
 
     /// Advance to the next value in the series
-    fn advance(&mut self, step: &Self::StepType) -> Result<()>;
+    ///
+    /// If advancing would overflow the value range, `end` is updated so that
+    /// the series terminates after the current value (matching the behavior
+    /// of PostgreSQL and DuckDB, which return the reachable values instead of
+    /// erroring).
+    fn advance(&mut self, end: &mut Self, step: &Self::StepType) -> Result<()>;

Review Comment:
   Thanks @kosiew — addressed in 49bd63d27. I restored the existing `advance` 
method signature and added a default `advance_with_end` hook, which `i64` 
overrides for the overflow-clamping behavior. This preserves compatibility for 
downstream `SeriesValue` implementations. I had overlooked the earlier 
cargo-semver-checks result when I introduced the `end` parameter; thank you for 
catching it in review.



-- 
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]

Reply via email to