You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ethan-tyler (via GitHub)" <gi...@apache.org> on 2026/02/15 15:15:55 UTC
[PR] feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions [datafusion]
ethan-tyler opened a new pull request, #20369:
URL: https://github.com/apache/datafusion/pull/20369
## Which issue does this PR close?
- Closes #13614
## Rationale for this change
`LazyMemoryExec` currently uses generator closures (`LazyBatchGenerator`) as its partition interface. This replaces with a `LazyPartition` trait that gives partitions a proper abstraction while keeping the old generator path working through a legacy adapter.
`generate_series`/`range` are migrated to native `LazyPartition` as the first consumer. Proto roundtrip is updated to handle both native and legacy forms.
## What changes are included in this PR?
- `LazyPartition` trait as the native partition interface for `LazyMemoryExec`
- `LazyBatchGeneratorPartition` adapter allowing existing generator to work
- Deprecated compatibility APIs preserved: `LazyMemoryExec::try_new`, `LazyMemoryExec::generators`, `GenerateSeriesTable::as_generator`.
- `generate_series`/`range` migrated to native `GenerateSeriesPartition`
- Shared helper for generate series state construction (dedupes native and deprecated paths)
- Proto roundtrip updated for native `GenerateSeriesPartition` with legacy adapter fallback
- Core coop test helper updated to use `LazyPartition`
- Proto regression test coverage added
## Are these changes tested?
yes
- `cargo test -p datafusion-physical-plan lazy_memory_tests --lib`
- `cargo test -p datafusion-functions-table generate_series_tests --lib`
- `cargo test -p datafusion-proto roundtrip_generate_series --test proto_integration`
- `cargo test -p datafusion execution::coop --test core_integration`
- `cargo check -p datafusion-physical-plan -p datafusion-functions-table -p datafusion-proto -p datafusion`
## Are there any user-facing changes?
No. `LazyBatchGenerator` and existing public APIs remain available (deprecated)
--
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] feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions [datafusion]
Posted by "ethan-tyler (via GitHub)" <gi...@apache.org>.
ethan-tyler commented on PR #20369:
URL: https://github.com/apache/datafusion/pull/20369#issuecomment-3914299406
@2010YOUY01 @Jefffrey - this might be of interest if you.
--
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] feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20369:
URL: https://github.com/apache/datafusion/pull/20369#issuecomment-4473682935
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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] feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #20369:
URL: https://github.com/apache/datafusion/pull/20369#discussion_r2888965206
##########
datafusion/functions-table/src/generate_series.rs:
##########
@@ -238,100 +243,213 @@ impl GenerateSeriesTable {
Self { schema, args }
}
+ pub fn as_partition(&self, batch_size: usize) -> Result<Arc<dyn LazyPartition>> {
Review Comment:
`GenerateSeriesTable::as_partition` currently constructs `GenerateSeriesPartition` without validating timestamp timezone input.
Previously `as_generator` parsed timezone eagerly and could fail during planning. Consider validating in `GenerateSeriesPartition::new` (or a `try_new`) to keep error timing consistent and fail earlier.
##########
datafusion/physical-plan/src/memory.rs:
##########
@@ -246,17 +379,24 @@ impl LazyMemoryExec {
.add_orderings(std::iter::once(ordering));
}
+ /// Get the partitions.
+ pub fn partitions(&self) -> &[Arc<dyn LazyPartition>] {
+ &self.partitions
+ }
+
/// Get the batch generators
+ #[deprecated(note = "Use LazyMemoryExec::partitions instead")]
+ #[expect(deprecated)]
pub fn generators(&self) -> &Vec<Arc<RwLock<dyn LazyBatchGenerator>>> {
Review Comment:
Not the fault of this PR but
`generators()` returns `&Vec<_>`. Since this is a compatibility accessor, returning a slice (`&[_]`) would avoid exposing container type and align with other accessor style.
##########
datafusion/physical-plan/src/memory.rs:
##########
@@ -228,11 +361,11 @@ impl LazyMemoryExec {
pub fn try_set_partitioning(&mut self, partitioning: Partitioning) -> Result<()> {
let partition_count = partitioning.partition_count();
- let generator_count = self.batch_generators.len();
+ let generator_count = self.partitions.len();
Review Comment:
`generator_count` although this now counts partitions?
##########
datafusion/physical-plan/src/memory.rs:
##########
@@ -268,10 +408,10 @@ impl DisplayAs for LazyMemoryExec {
write!(
f,
"LazyMemoryExec: partitions={}, batch_generators=[{}]",
Review Comment:
`DisplayAs` still prints `batch_generators=` even though the primary abstraction is now `partitions`?
##########
datafusion/proto/src/physical_plan/mod.rs:
##########
@@ -3451,22 +3457,118 @@ impl protobuf::PhysicalPlanNode {
}
}
+ fn serialize_generate_series_args(
+ args: &GenSeriesArgs,
+ ) -> Result<protobuf::generate_series_node::Args> {
+ match args {
+ GenSeriesArgs::ContainsNull { name } => {
+ Ok(protobuf::generate_series_node::Args::ContainsNull(
+ protobuf::GenerateSeriesArgsContainsNull {
+ name: Self::str_to_generate_series_name(name)? as i32,
+ },
+ ))
+ }
+ GenSeriesArgs::Int64Args {
+ start,
+ end,
+ step,
+ include_end,
+ name,
+ } => Ok(protobuf::generate_series_node::Args::Int64Args(
+ protobuf::GenerateSeriesArgsInt64 {
+ start: *start,
+ end: *end,
+ step: *step,
+ include_end: *include_end,
+ name: Self::str_to_generate_series_name(name)? as i32,
+ },
+ )),
+ GenSeriesArgs::TimestampArgs {
+ start,
+ end,
+ step,
+ tz,
+ include_end,
+ name,
+ } => Ok(protobuf::generate_series_node::Args::TimestampArgs(
+ protobuf::GenerateSeriesArgsTimestamp {
+ start: *start,
+ end: *end,
+ step: Some(datafusion_proto_common::IntervalMonthDayNanoValue {
+ months: step.months,
+ days: step.days,
+ nanos: step.nanoseconds,
+ }),
+ include_end: *include_end,
+ name: Self::str_to_generate_series_name(name)? as i32,
+ tz: tz.as_ref().map(ToString::to_string),
+ },
+ )),
+ GenSeriesArgs::DateArgs {
+ start,
+ end,
+ step,
+ include_end,
+ name,
+ } => Ok(protobuf::generate_series_node::Args::DateArgs(
+ protobuf::GenerateSeriesArgsDate {
+ start: *start,
+ end: *end,
+ step: Some(datafusion_proto_common::IntervalMonthDayNanoValue {
+ months: step.months,
+ days: step.days,
+ nanos: step.nanoseconds,
+ }),
+ include_end: *include_end,
+ name: Self::str_to_generate_series_name(name)? as i32,
+ },
+ )),
+ }
+ }
+
+ #[allow(deprecated)]
fn try_from_lazy_memory_exec(exec: &LazyMemoryExec) -> Result<Option<Self>> {
Review Comment:
The native `GenerateSeriesPartition` path uses `serialize_generate_series_args`, but legacy generator downcasts still manually reconstruct protobuf args.
Could we extract a helper that converts legacy generator variants into `GenSeriesArgs` first, then reuse `serialize_generate_series_args` to reduce duplicated conversion logic?
--
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] feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #20369: feat: add LazyPartition trait for LazyMemoryExec, migrate generate_series to partitions
URL: https://github.com/apache/datafusion/pull/20369
--
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