You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Omega359 (via GitHub)" <gi...@apache.org> on 2024/09/09 18:17:11 UTC
[PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Omega359 opened a new pull request, #12400:
URL: https://github.com/apache/datafusion/pull/12400
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #11822
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
Add support for timestamps and steps less than a day for timestamps.
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Code, tests, doc.
## Are these changes tested?
Yes.
## Are there any user-facing changes?
No.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1754801798
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper_bound: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "Arguments length must be 3 for {}",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper_bound)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper_bound)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = start_tz_opt
+ .clone()
+ .map_or_else(TimestampNanosecondBuilder::new, |start_tz_str| {
+ TimestampNanosecondBuilder::new().with_timezone(start_tz_str)
+ });
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(exec_datafusion_err!(
+ "Cannot generate timestamp range where start + step overflows"
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for stop: {}: {:?}",
+ stop,
+ stop_tz
+ ),
+ )?;
+
+ let mut current = start;
+ let mut current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for start: {}: {:?}",
+ current,
+ start_tz
+ ),
+ )?;
+
+ let values = from_fn(|| {
+ if (include_upper_bound
+ && ((neg && current_dt < stop_dt) || (!neg && current_dt > stop_dt)))
+ || (!include_upper_bound
+ && ((neg && current_dt <= stop_dt)
+ || (!neg && current_dt >= stop_dt)))
+ {
+ return None;
+ }
+
+ let prev_current = current;
+
+ if let Some(ts) = TSNT::add_month_day_nano(current, step, start_tz) {
+ current = ts;
+ current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz)?;
+
+ Some(Some(prev_current))
+ } else {
+ // we failed to parse the timestamp here so terminate the series
+ None
+ }
+ });
+
+ list_builder.append_value(values);
+ }
+
+ let arr = Arc::new(list_builder.finish());
+
+ Ok(arr)
+}
+
+fn cast_timestamp_arg(
Review Comment:
My programming roots are showing through :) I can take a look at that
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750834562
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(DataFusionError::Execution(
Review Comment:
```suggestion
.ok_or(exec_datafusion_error!(
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750829756
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -109,8 +121,11 @@ impl ScalarUDFImpl for Range {
match args[0].data_type() {
Review Comment:
we can probably factor our this block to avoid make a modifications twice
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750830706
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
Review Comment:
```suggestion
"RANGE"
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#issuecomment-2342002409
I plan to review this tomorrow -- thank you @Omega359
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#issuecomment-2338964572
Thanks @comphead for the review, much appreciated. I'll go through the proposed changes and comments tonight or tomorrow and push updates.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750836169
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(DataFusionError::Execution(
+ "Cannot generate timestamp range where start + step overflows"
+ .to_string(),
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ DataFusionError::Execution(format!(
+ "Cannot generate timestamp for stop: {}: {:?}",
+ stop, stop_tz
+ )),
+ )?;
+
+ let mut current = start;
+ let mut current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz).ok_or(
+ DataFusionError::Execution(format!(
+ "Cannot generate timestamp for start: {}: {:?}",
+ current, start_tz
+ )),
+ )?;
+
+ let values = from_fn(|| {
+ if (include_upper
+ && ((neg && current_dt < stop_dt) || (!neg && current_dt > stop_dt)))
+ || (!include_upper
+ && ((neg && current_dt <= stop_dt)
+ || (!neg && current_dt >= stop_dt)))
+ {
+ return None;
+ }
+
+ let prev_current = current;
+
+ if let Some(ts) = TSNT::add_month_day_nano(current, step, start_tz) {
+ current = ts;
+ current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz)?;
+
+ Some(Some(prev_current))
+ } else {
+ // we failed to parse the timestamp here so terminate the series
+ None
+ }
+ });
+
+ list_builder.append_value(values);
+ }
+
+ let arr = Arc::new(list_builder.finish());
+
+ Ok(arr)
+}
+
+fn cast_timestamp_arg(
+ arg: &ArrayRef,
+ include_upper: bool,
+) -> Result<(&TimestampNanosecondArray, &Option<Arc<str>>)> {
+ match arg.data_type() {
+ Timestamp(Nanosecond, tz_opt) => {
+ Ok((as_timestamp_nanosecond_array(arg)?, tz_opt))
+ }
+ _ => {
+ internal_err!(
+ "Unexpected argument type for {} : {}",
+ if include_upper {
Review Comment:
this error block is third time showing up, perhaps we can factor it out
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#issuecomment-2346601498
🚀 - thanks again @comphead @Omega359 and @Abdullahsab3
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750835005
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(DataFusionError::Execution(
+ "Cannot generate timestamp range where start + step overflows"
+ .to_string(),
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ DataFusionError::Execution(format!(
Review Comment:
exec_datafusion_error!(
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(DataFusionError::Execution(
+ "Cannot generate timestamp range where start + step overflows"
+ .to_string(),
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ DataFusionError::Execution(format!(
+ "Cannot generate timestamp for stop: {}: {:?}",
+ stop, stop_tz
+ )),
+ )?;
+
+ let mut current = start;
+ let mut current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz).ok_or(
+ DataFusionError::Execution(format!(
+ "Cannot generate timestamp for start: {}: {:?}",
Review Comment:
exec_datafusion_error!(
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750830343
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
Review Comment:
```suggestion
"GENERATE_SERIES"
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1751139469
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -109,8 +121,11 @@ impl ScalarUDFImpl for Range {
match args[0].data_type() {
Review Comment:
There is so much duplication in these UDF's. I think it's best to push the cleanup of that to a separate PR and keep this one as small as possible.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #12400:
URL: https://github.com/apache/datafusion/pull/12400
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750832286
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
Review Comment:
that looks somewhat weird, do we really need so many indirections 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750831420
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Review Comment:
```suggestion
fn gen_range_timestamp(args: &[ArrayRef], include_upper_bound: bool) -> Result<ArrayRef> {
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1754716549
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper_bound: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "Arguments length must be 3 for {}",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper_bound)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper_bound)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = start_tz_opt
+ .clone()
+ .map_or_else(TimestampNanosecondBuilder::new, |start_tz_str| {
+ TimestampNanosecondBuilder::new().with_timezone(start_tz_str)
+ });
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(exec_datafusion_err!(
+ "Cannot generate timestamp range where start + step overflows"
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for stop: {}: {:?}",
+ stop,
+ stop_tz
+ ),
+ )?;
+
+ let mut current = start;
+ let mut current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for start: {}: {:?}",
+ current,
+ start_tz
+ ),
+ )?;
+
+ let values = from_fn(|| {
+ if (include_upper_bound
+ && ((neg && current_dt < stop_dt) || (!neg && current_dt > stop_dt)))
+ || (!include_upper_bound
+ && ((neg && current_dt <= stop_dt)
+ || (!neg && current_dt >= stop_dt)))
+ {
+ return None;
+ }
+
+ let prev_current = current;
+
+ if let Some(ts) = TSNT::add_month_day_nano(current, step, start_tz) {
+ current = ts;
+ current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz)?;
+
+ Some(Some(prev_current))
+ } else {
+ // we failed to parse the timestamp here so terminate the series
+ None
+ }
+ });
+
+ list_builder.append_value(values);
+ }
+
+ let arr = Arc::new(list_builder.finish());
+
+ Ok(arr)
+}
+
+fn cast_timestamp_arg(
+ arg: &ArrayRef,
+ include_upper: bool,
+) -> Result<(&TimestampNanosecondArray, &Option<Arc<str>>)> {
+ match arg.data_type() {
+ Timestamp(Nanosecond, tz_opt) => {
+ Ok((as_timestamp_nanosecond_array(arg)?, tz_opt))
+ }
+ _ => {
+ internal_err!(
+ "Unexpected argument type for {} : {}",
+ if include_upper {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ },
+ arg.data_type()
+ )
+ }
+ }
+}
+
+fn parse_tz(tz: &Option<Arc<str>>) -> Result<Tz> {
+ let tz = tz.as_ref().map_or_else(|| "+00", |s| s);
+
+ Tz::from_str(tz)
+ .map_err(|op| exec_datafusion_err!("failed on timezone {tz}: {:?}", op))
Review Comment:
```suggestion
.map_err(|op| exec_datafusion_err!("failed to parse timezone {tz}: {:?}", op))
```
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper_bound: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "Arguments length must be 3 for {}",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper_bound)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper_bound)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = start_tz_opt
+ .clone()
+ .map_or_else(TimestampNanosecondBuilder::new, |start_tz_str| {
+ TimestampNanosecondBuilder::new().with_timezone(start_tz_str)
+ });
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper_bound {
+ "GENERATE_SERIES"
+ } else {
+ "RANGE"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(exec_datafusion_err!(
+ "Cannot generate timestamp range where start + step overflows"
+ ))?
+ .cmp(&start)
+ == Ordering::Less;
+
+ let stop_dt = as_datetime_with_timezone::<TSNT>(stop, stop_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for stop: {}: {:?}",
+ stop,
+ stop_tz
+ ),
+ )?;
+
+ let mut current = start;
+ let mut current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz).ok_or(
+ exec_datafusion_err!(
+ "Cannot generate timestamp for start: {}: {:?}",
+ current,
+ start_tz
+ ),
+ )?;
+
+ let values = from_fn(|| {
+ if (include_upper_bound
+ && ((neg && current_dt < stop_dt) || (!neg && current_dt > stop_dt)))
+ || (!include_upper_bound
+ && ((neg && current_dt <= stop_dt)
+ || (!neg && current_dt >= stop_dt)))
+ {
+ return None;
+ }
+
+ let prev_current = current;
+
+ if let Some(ts) = TSNT::add_month_day_nano(current, step, start_tz) {
+ current = ts;
+ current_dt = as_datetime_with_timezone::<TSNT>(current, start_tz)?;
+
+ Some(Some(prev_current))
+ } else {
+ // we failed to parse the timestamp here so terminate the series
+ None
+ }
+ });
+
+ list_builder.append_value(values);
+ }
+
+ let arr = Arc::new(list_builder.finish());
+
+ Ok(arr)
+}
+
+fn cast_timestamp_arg(
Review Comment:
nit:
You could potentially use [`as_timestamp_nanosecond_array`](https://docs.rs/datafusion/latest/datafusion/common/cast/fn.as_timestamp_second_array.html) (the datafusion version) from: https://docs.rs/datafusion/latest/datafusion/common/cast/index.html
And then call [`timezone`](https://docs.rs/arrow/latest/arrow/array/struct.PrimitiveArray.html#tymethod.timezone) to get the timezone from the array
I personally found the use of the word `cast` somewhat confusing as function doesn't actually call the arrow `cast` kernel, but instead does the rust `as_any` stuff (which is like a cast in C or Java)
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -109,8 +121,11 @@ impl ScalarUDFImpl for Range {
match args[0].data_type() {
Int64 => make_scalar_function(|args| gen_range_inner(args, false))(args),
Date32 => make_scalar_function(|args| gen_range_date(args, false))(args),
- _ => {
- exec_err!("unsupported type for range")
+ Timestamp(_, _) => {
+ make_scalar_function(|args| gen_range_timestamp(args, false))(args)
+ }
+ dt => {
+ exec_err!("unsupported type for RANGE. Expected Int64, Date32 or Timestamp, got: {dt}")
Review Comment:
❤️ for a much better error
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -5866,50 +5902,124 @@ select generate_series(5),
----
[0, 1, 2, 3, 4, 5] [2, 3, 4, 5] [2, 5, 8] [1, 2, 3, 4, 5] [5, 4, 3, 2, 1] [10, 7, 4] [1992-09-01, 1992-10-01, 1992-11-01, 1992-12-01, 1993-01-01, 1993-02-01, 1993-03-01] [1993-02-01, 1993-01-31, 1993-01-30, 1993-01-29, 1993-01-28, 1993-01-27, 1993-01-26, 1993-01-25, 1993-01-24, 1993-01-23, 1993-01-22, 1993-01-21, 1993-01-20, 1993-01-19, 1993-01-18, 1993-01-17, 1993-01-16, 1993-01-15, 1993-01-14, 1993-01-13, 1993-01-12, 1993-01-11, 1993-01-10, 1993-01-09, 1993-01-08, 1993-01-07, 1993-01-06, 1993-01-05, 1993-01-04, 1993-01-03, 1993-01-02, 1993-01-01] [1989-04-01, 1990-04-01, 1991-04-01, 1992-04-01]
-query error DataFusion error: Execution error: Cannot generate date range less than 1 day\.
-select generate_series('2021-01-01'::timestamp, '2021-01-02'::timestamp, INTERVAL '1' HOUR);
+query ?
+select generate_series('2021-01-01'::timestamp, '2021-01-01T15:00:00'::timestamp, INTERVAL '1' HOUR);
+----
+[2021-01-01T00:00:00, 2021-01-01T01:00:00, 2021-01-01T02:00:00, 2021-01-01T03:00:00, 2021-01-01T04:00:00, 2021-01-01T05:00:00, 2021-01-01T06:00:00, 2021-01-01T07:00:00, 2021-01-01T08:00:00, 2021-01-01T09:00:00, 2021-01-01T10:00:00, 2021-01-01T11:00:00, 2021-01-01T12:00:00, 2021-01-01T13:00:00, 2021-01-01T14:00:00, 2021-01-01T15:00:00]
+
+query ?
+select generate_series('2021-01-01T00:00:00EST'::timestamp, '2021-01-01T15:00:00-12:00'::timestamp, INTERVAL '1' HOUR);
Review Comment:
is it worth testing when the end timestamp is not an exact increment?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#issuecomment-2339458817
> For tests its also good to have examples like
>
> series of date2 less than date1 with positive interval
I think that may already be in there for range and generate_series
```
query ?
select range(TIMESTAMP '1993-03-01', TIMESTAMP '1989-04-01', INTERVAL '1' YEAR)
----
[]
query ?
select generate_series(TIMESTAMP '1993-03-01', TIMESTAMP '1989-04-01', INTERVAL '1' YEAR)
----
[]
```
> fractional intervals - which might be tricky
I added this one - I think this may be along the lines of what you are thinking of:
```
query ?
select generate_series(arrow_cast('2021-01-01T00:00:00', 'Timestamp(Nanosecond, Some("-05:00"))'), arrow_cast('2021-01-01T06:00:00', 'Timestamp(Nanosecond, Some("-05:00"))'), INTERVAL '1 HOUR 30 MINUTE -5500000000 NANOSECOND');
----
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1750833953
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
Review Comment:
better to output function names in upper cases so it improves the readability to the user
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1751140090
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
+ } else {
+ TimestampNanosecondBuilder::new()
+ };
+ let mut list_builder = ListBuilder::new(values_builder);
+
+ for idx in 0..start_arr.len() {
+ if start_arr.is_null(idx) || stop_arr.is_null(idx) || step_arr.is_null(idx) {
+ list_builder.append_null();
+ continue;
+ }
+
+ let start = start_arr.value(idx);
+ let stop = stop_arr.value(idx);
+ let step = step_arr.value(idx);
+
+ let (months, days, ns) = IntervalMonthDayNanoType::to_parts(step);
+ if months == 0 && days == 0 && ns == 0 {
+ return exec_err!(
+ "Interval argument to {} must not be 0",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ let neg = TSNT::add_month_day_nano(start, step, start_tz)
+ .ok_or(DataFusionError::Execution(
Review Comment:
Ah, I was looking for something like that but missed that one. Thx
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1751138765
##########
datafusion/functions-nested/src/range.rs:
##########
@@ -394,3 +412,136 @@ fn gen_range_date(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
Ok(arr)
}
+
+fn gen_range_timestamp(args: &[ArrayRef], include_upper: bool) -> Result<ArrayRef> {
+ if args.len() != 3 {
+ return exec_err!(
+ "arguments length must be 3 for {}",
+ if include_upper {
+ "generate_series"
+ } else {
+ "range"
+ }
+ );
+ }
+
+ // coerce_types fn should coerce all types to Timestamp(Nanosecond, tz)
+ let (start_arr, start_tz_opt) = cast_timestamp_arg(&args[0], include_upper)?;
+ let (stop_arr, stop_tz_opt) = cast_timestamp_arg(&args[1], include_upper)?;
+ let step_arr = as_interval_mdn_array(&args[2])?;
+ let start_tz = parse_tz(start_tz_opt)?;
+ let stop_tz = parse_tz(stop_tz_opt)?;
+
+ // values are timestamps
+ let values_builder = if let Some(start_tz_str) = start_tz_opt {
+ TimestampNanosecondBuilder::new().with_timezone(&**start_tz_str)
Review Comment:
As it was written, yes, it was required. I changed it up to use map_or_else to accomplish the same thing.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support timestamps and steps of less than a day for range/generate_series [datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on code in PR #12400:
URL: https://github.com/apache/datafusion/pull/12400#discussion_r1754796330
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -5866,50 +5902,124 @@ select generate_series(5),
----
[0, 1, 2, 3, 4, 5] [2, 3, 4, 5] [2, 5, 8] [1, 2, 3, 4, 5] [5, 4, 3, 2, 1] [10, 7, 4] [1992-09-01, 1992-10-01, 1992-11-01, 1992-12-01, 1993-01-01, 1993-02-01, 1993-03-01] [1993-02-01, 1993-01-31, 1993-01-30, 1993-01-29, 1993-01-28, 1993-01-27, 1993-01-26, 1993-01-25, 1993-01-24, 1993-01-23, 1993-01-22, 1993-01-21, 1993-01-20, 1993-01-19, 1993-01-18, 1993-01-17, 1993-01-16, 1993-01-15, 1993-01-14, 1993-01-13, 1993-01-12, 1993-01-11, 1993-01-10, 1993-01-09, 1993-01-08, 1993-01-07, 1993-01-06, 1993-01-05, 1993-01-04, 1993-01-03, 1993-01-02, 1993-01-01] [1989-04-01, 1990-04-01, 1991-04-01, 1992-04-01]
-query error DataFusion error: Execution error: Cannot generate date range less than 1 day\.
-select generate_series('2021-01-01'::timestamp, '2021-01-02'::timestamp, INTERVAL '1' HOUR);
+query ?
+select generate_series('2021-01-01'::timestamp, '2021-01-01T15:00:00'::timestamp, INTERVAL '1' HOUR);
+----
+[2021-01-01T00:00:00, 2021-01-01T01:00:00, 2021-01-01T02:00:00, 2021-01-01T03:00:00, 2021-01-01T04:00:00, 2021-01-01T05:00:00, 2021-01-01T06:00:00, 2021-01-01T07:00:00, 2021-01-01T08:00:00, 2021-01-01T09:00:00, 2021-01-01T10:00:00, 2021-01-01T11:00:00, 2021-01-01T12:00:00, 2021-01-01T13:00:00, 2021-01-01T14:00:00, 2021-01-01T15:00:00]
+
+query ?
+select generate_series('2021-01-01T00:00:00EST'::timestamp, '2021-01-01T15:00:00-12:00'::timestamp, INTERVAL '1' HOUR);
Review Comment:
There is a test for -5.5sec (in nanoseconds) somewhere in that file.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org