Make `scan::execute` return a lazy iterator #340

OussamaSaoudi-db · 2024-09-17T22:02:43Z

Currently, scan::execute computes all ScanResults ahead of time and returns a vector. This PR changes it to return an iterator that lazily fetches scan files, reads parquet data, and applies deletion vectors. This avoids a large upfront performance cost when executing a scan.

Closes: #123

codecov · 2024-09-17T23:17:57Z

Codecov Report

Attention: Patch coverage is 78.57143% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.55%. Comparing base (c4be634) to head (ae9e45a).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
acceptance/src/data.rs	42.85%	0 Missing and 4 partials ⚠️
kernel/src/scan/mod.rs	90.47%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #340      +/-   ##
==========================================
- Coverage   76.56%   76.55%   -0.01%     
==========================================
  Files          45       45              
  Lines        9374     9384      +10     
  Branches     9374     9384      +10     
==========================================
+ Hits         7177     7184       +7     
  Misses       1799     1799              
- Partials      398      401       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kernel/examples/read-table-single-threaded/src/main.rs

nicklan

Wow, nice work getting through all the iterator madness.

We've ended up with a crazy amount of iterator nesting here because we have so many levels of iterators. At some point I begin to wonder if it's worth creating our own iterator type that stores the inner iterators and does the flatting itself. It would avoid the like 3 chained flatten_oks which looks crazy and makes the code a bit hard to reason about. Thoughts?

acceptance/src/data.rs

kernel/examples/read-table-single-threaded/src/main.rs

kernel/src/scan/mod.rs

nicklan · 2024-09-24T16:24:20Z

kernel/src/scan/mod.rs

@@ -311,11 +316,13 @@ impl Scan {
                        mask: sv,
                    };
                    selection_vector = rest;
-                    Ok(result)
+                    Ok::<ScanResult, Error>(result)


On line 296 above you can help the compiler by telling what type the closure will return:

Ok(read_result_iter.into_iter().map(move |read_result| -> DeltaResult<_> {

Then you don't need a turbofish here:

Suggested change

Ok::<ScanResult, Error>(result)

Ok(result)

Any suggestions to wrap my head around these types? The return type of this closure (according to my LSP) is DeltaResult<Map<Box<dyn Iterator<Item = Result<Box<dyn EngineData>, Error>> + Send>, impl FnMut(Result<Box<dyn EngineData>, Error>) -> Result<ScanResult, Error>>> 😅. I'd like to do a DeltaResult<Iterator<Item = DeltaResult<ScanResult>>>, but that yields a pretty ugly compiler error.

The types are super complex because of all the nested iterators and so on. Essentially we have 3 layers of iterators:

The outer iterator: scan_data, each element here is basically one file in the log, so a 0000000..0002.json for example

We iterate over each chunk of data, which can contain multiple add actions each, and produce a scan_files_iter which tells us which files we need to scan

Each scan file we read produces an iterator because the parquet reader is allowed to return things in "chunks"

So ultimately we have an iterator inside an iterator inside an iterator, so we need to flatten it out three time. I don't even bother trying to unravel the types because most of them are actually adapters (like the return from a map say, and it's better to just think of them as "just another iterator".

Regarding the turbofish and the type annotation. Say you have an iterator over results, and you want to map to another result, you can do something like:

some_iter.map(|result| { let thing = result?.do_something(); let other_thing = fallible_call()?; Ok(thing) })

but now the compiler needs to know what the error type is. result might be a Result<_, ErrorOne> and fallible_call() might return Result<_, ErrorTwo>. We have intos for everything to go into a delta_kernel::Error type, but the compiler doesn't know that's what we want it to use when we do ?, so it complains. You can fix this by saying Ok::<_, delta_kernel::Error> which is just telling it fully what the return type is. But turbofish is ugly, so we use the other way to tell it what the return type is by adding a type annotation to the closure:

some_iter.map(|result| -> Result<_, UnifiedErrorType> { let thing = result?.do_something(); let other_thing = fallible_call()?; Ok(thing) })

(note that DeltaResult<_> is just a type alias for Result<_, delta_kernel::Error>)

That helps a lot, thank you!

kernel/src/scan/mod.rs

kernel/tests/dv.rs

kernel/tests/read.rs

acceptance/src/data.rs

OussamaSaoudi-db · 2024-09-24T20:39:29Z

Wow, nice work getting through all the iterator madness.

We've ended up with a crazy amount of iterator nesting here because we have so many levels of iterators. At some point I begin to wonder if it's worth creating our own iterator type that stores the inner iterators and does the flatting itself. It would avoid the like 3 chained flatten_oks which looks crazy and makes the code a bit hard to reason about. Thoughts?

I think this would be cool if we get to avoid flatten_ok in multiple areas in the code. Doing a quick grep shows that all the occurrences of flatten_ok are in execute. Not too many other instances of flatten either. If the iterator is only being used for this, I think it would add more maintenance overhead than is worth.

zachschuermann

looking great - couple questions and I do think it will be useful to make a follow-up issue for a custom iterator for us to at least consider in the future

acceptance/src/data.rs

zachschuermann · 2024-09-27T17:09:04Z

kernel/examples/read-table-single-threaded/src/main.rs

+    for res in scan.execute(engine.as_ref())? {
+        let res = res?;


could maybe use a try_for_each? not sure if it's any cleaner though

try_for_each only returns results returned in the body, it doesn't fix the issue of getting results in the iterator

What about process_results?

process_results ends up having a lot of nesting errors. We get Results in, for which process_result outputs DeltaResult<Output>, but because we also have errors inside the loop, the ouptut itself is an error DeltaResult<DeltaResult<_>>. I noticed that we are collecting batches, so I went with a map_ok, flatten_ok, and try_collect.

zachschuermann · 2024-09-27T17:16:46Z

kernel/src/scan/mod.rs

+                let scan_files = vec![];
+                state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)
+                    .map(IntoIterator::into_iter)


was thinking about whether or not we could 'push down' the iterator here so that we don't have to materialize everything into a vector. but since the visitor eagerly extracts, probably best we can do is this for now?

Yep, ScanFileVisitor iterates through all the rows eagerly, and calls the callback in the loop. I could make an issue to make that lazy as well?

yea let's have an issue to at least consider it :)

Is there some specific performance or simplicity benefit to making the row visitor lazy at row level?

Right now, engine data is the unit of work (we don't want to force engines to do row-by-row execution)
Probably not much performance benefit to making it lazy within any one engine data, because we already paid the compute and I/O to materialize it?

This makes sense to me. Probably okay to just collect these up since we're just iterating over some in memory data and not doing anything expensive.

zachschuermann · 2024-09-27T17:17:40Z

kernel/src/scan/mod.rs

+            .flatten_ok()
+            .flatten_ok();


agree with @nicklan that there's probably some room for improvement on flatten_ok()'s. how about we just make a follow-up issue to track it and not block this PR it?

Let's keep comments like this open for posterity -- they're really hard to find if resolved.

kernel/src/scan/mod.rs

kernel/tests/read.rs

scovich · 2024-09-28T04:26:04Z

acceptance/src/data.rs

            }
        })
-        .collect();
+        .collect::<DeltaResult<Vec<RecordBatch>>>()?;


If you use itertools::Itertools, can do just:

Suggested change

.collect::<DeltaResult<Vec<RecordBatch>>>()?;

.try_collect()?;

Using itertools would require that I add a new dependency to the crate. Is it better to avoid new dependencies, or avoid turbofish?

also could just provide the type at the variable bind?

Variable bind type hint unfortunately doesn't help :(

let batches: Vec<RecordBatch> = scan_res // ... .collect()?;

176 | .collect()?; | ^^^^^^^ cannot infer type of the type parameter `B` declared on the method `collect`

Just going to go with try_collect

Variable bind type hint unfortunately doesn't help :(

Yeah, ? is too flexible so the compiler can't infer what type it's dealing with.

Using itertools would require that I add a new dependency to the crate.

The rest of kernel already has that dep, so it seems ok to include it here as well?

Yeah, ? is too flexible so the compiler can't infer what type it's dealing with.

TIL, thank you!

The rest of kernel already has that dep, so it seems ok to include it here as well?

Sounds good!

scovich · 2024-09-28T04:31:08Z

kernel/src/scan/mod.rs

+            .flatten_ok()
+            .flatten_ok();


Let's keep comments like this open for posterity -- they're really hard to find if resolved.

kernel/src/scan/mod.rs

scovich · 2024-09-28T04:41:56Z

kernel/src/scan/mod.rs

+                let scan_files = vec![];
+                state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)
+                    .map(IntoIterator::into_iter)


Is there some specific performance or simplicity benefit to making the row visitor lazy at row level?

Right now, engine data is the unit of work (we don't want to force engines to do row-by-row execution)
Probably not much performance benefit to making it lazy within any one engine data, because we already paid the compute and I/O to materialize it?

kernel/tests/read.rs

scovich · 2024-09-28T05:01:49Z

kernel/src/scan/mod.rs

+            .flatten_ok()
+            .flatten_ok();


We need one flatten_ok() to for the vec we created... but what is the other one collapsing?

First we have scan_data, which is Iterator<DeltaResult<_>>. We use map_ok to map each Ok to DeltaResult<Iterator<ScanFile>>. So this becomes: Iterator<DeltaResult<DeltaResult<Iterator<ScanFile>>>>.

First flatten_ok:
Iterator<DeltaResult<Iterator<ScanFile>>>
Second flatten_ok:
Iterator<DeltaResult<ScanFile>>

Oh! I didn't realize that flatten_ok unpacked Result in addition to Iterator!

It might be helpful, when there are multiple flatten calls like this, to add code comments explaining what each one does, e.g.

Suggested change

.flatten_ok()

.flatten_ok();

.flatten_ok() // Iterator<DeltaResult<DeltaResult<_>>> to Iterator<DeltaResult<_>>

.flatten_ok(); // Iterator<DeltaResult<Iterator<_>>> to Iterator<DeltaResult<_>>

Actually... I'm not completely sure this is correct code.

The flattening works because because of impl IntoIterator for Result, but:

The iterator yields one value if the result is Result::Ok, otherwise none.

I think that means Ok(Err(..)) would simply disappear (empty iterator), rather than short-circuiting to surface the error as we almost certainly want?

It seems like we really want Itertools::map_ok to behave like Result::and_then, but it doesn't. I almost wonder if it might be worth defining our own map_and_then helper method?

trait MapAndThen : Iterator { fn map_and_then<F, T, U, E>(self, mut f: F) -> impl Iterator<Item = Result<U, E>> where Self: Iterator<Item = Result<T, E>> + Sized, F: FnMut(T) -> Result<U, E>, { // NOTE: FnMut is-a FnOnce, but using it that way consumes `f`. Fortunately, // &mut FnMut is _also_ FnOnce, and using it that way does _not_ consume `f`. self.map(move |result| result.and_then(&mut f)) } } impl<T: Iterator> MapAndThen for T {}

Darn you're right. Here's a minimal example:

let a = vec![Ok(1), Err("error"), Ok(4)].into_iter(); let b = vec![Ok(5), Ok(6)].into_iter(); let x = vec![Ok(a), Err("error"), Ok(b)].into_iter(); for y in x.flatten_ok().flatten_ok() { println!("y: {:?}", y); }

Results in:

y: Ok(1) y: Ok(4) y: Err("error") y: Ok(5) y: Ok(6)

One would expect to instead see two errors. I'll look into patching this. Thank you so much for flagging this @scovich!

I've patched the PR to use map_and_then in scan. I updated a bunch of other places where I was using map_ok to instead do map(|res| res.and_then(|value| {...}). The reason I don't use map_and_then is that it is defined in the kernel crate, and is not accessible in test crates. I didn't make it public because it doesn't seem relevant to what the kernel does, and it shouldn't be something users depend on.

Also for those looking to sanity check, here's what map_and_then looks like in action:

let a = vec![Ok(1), Err("error"), Ok(4)].into_iter(); let b = vec![Ok(5), Ok(6)].into_iter(); let x = vec![Ok(a), Err("error"), Ok(b)].into_iter(); let iter = x .flatten_ok() // Iterator<Result<Iterator<Result<usize>>>> to Iterator<Result<Result<usize>>> .map_and_then(identity); // Iterator<Result<Result<usize>>> to Iterator<Result<usize>> for y in iter { println!("y: {:?}", y); }

Returns

y: Ok(1) y: Err("error") y: Ok(4) y: Err("error") y: Ok(5) y: Ok(6)

scovich · 2024-09-28T05:08:07Z

kernel/src/scan/mod.rs

            .flatten_ok()
-            .try_collect()?
+            .flatten_ok()
+            .flatten_ok())


Similar question here -- what are the three levels of flattening? Each scan file produces multiple chunks, and each chunk produces multiple rows... which seems like two levels?

Nick has a great explanation here.

scovich · 2024-09-28T05:11:50Z

kernel/tests/golden_tables.rs

+        .map(|sr| -> DeltaResult<_> {
+            let sr = sr?;


Would map_ok simplify things, by not requiring Ok at L171 and L173 below?
Actually, since we're materializing, process_result might be even better?

For a similar reason to my comment here, the Result input and Result output leads to process_results becoming DeltaResult<DeltaResult<_>>, requiring a ?? at the end. It also felt harder to reason about.

kernel/tests/golden_tables.rs

kernel/src/scan/mod.rs

OussamaSaoudi-db · 2024-10-01T01:17:47Z

kernel/tests/dv.rs

 use test_log::test;

+fn count_valid_rows(stream: impl Iterator<Item = DeltaResult<ScanResult>>) -> DeltaResult<usize> {


I wanted to flag this helper function I made, since I noticed duplicate code. This handles res being a DeltaResult, avoids let res = res?. Instead of having mutable state that a map_ok accesses, I made it into a sum over each RecordBatch.

kernel/examples/read-table-single-threaded/src/main.rs

scovich · 2024-10-01T17:02:28Z

kernel/src/scan/mod.rs

+            .flatten_ok()
+            .flatten_ok();


It might be helpful, when there are multiple flatten calls like this, to add code comments explaining what each one does, e.g.

Suggested change

.flatten_ok()

.flatten_ok();

.flatten_ok() // Iterator<DeltaResult<DeltaResult<_>>> to Iterator<DeltaResult<_>>

.flatten_ok(); // Iterator<DeltaResult<Iterator<_>>> to Iterator<DeltaResult<_>>

scovich · 2024-10-01T17:10:02Z

kernel/src/scan/mod.rs

+            .flatten_ok()
+            .flatten_ok();


Actually... I'm not completely sure this is correct code.

The flattening works because because of impl IntoIterator for Result, but:

The iterator yields one value if the result is Result::Ok, otherwise none.

I think that means Ok(Err(..)) would simply disappear (empty iterator), rather than short-circuiting to surface the error as we almost certainly want?

It seems like we really want Itertools::map_ok to behave like Result::and_then, but it doesn't. I almost wonder if it might be worth defining our own map_and_then helper method?

trait MapAndThen : Iterator { fn map_and_then<F, T, U, E>(self, mut f: F) -> impl Iterator<Item = Result<U, E>> where Self: Iterator<Item = Result<T, E>> + Sized, F: FnMut(T) -> Result<U, E>, { // NOTE: FnMut is-a FnOnce, but using it that way consumes `f`. Fortunately, // &mut FnMut is _also_ FnOnce, and using it that way does _not_ consume `f`. self.map(move |result| result.and_then(&mut f)) } } impl<T: Iterator> MapAndThen for T {}

nicklan

nice. this is a lot cleaner with the new trait. just a couple of nits from me.

nicklan · 2024-10-03T00:20:52Z

kernel/src/utils.rs

@@ -8,5 +8,18 @@ macro_rules! require {
        }
    };
 }
+pub(crate) trait MapAndThen: Iterator {


nit: doc comment

Just added one. How's it look?

kernel/examples/read-table-single-threaded/src/main.rs

nicklan · 2024-10-03T00:28:25Z

kernel/src/scan/mod.rs

+                let scan_files = vec![];
+                state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)
+                    .map(IntoIterator::into_iter)


This makes sense to me. Probably okay to just collect these up since we're just iterating over some in memory data and not doing anything expensive.

kernel/examples/read-table-single-threaded/src/main.rs

kernel/src/scan/mod.rs

kernel/tests/dv.rs

scovich · 2024-10-03T16:17:00Z

kernel/src/scan/mod.rs

+            .map_and_then(|(data, vec)| {
+                let scan_files = vec![];
+                state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)
+            })


Sorry if this is going in circles, but this code equivalent?

Suggested change

.map_and_then(|(data, vec)| {

let scan_files = vec![];

state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)

})

.map(|result| -> DeltaResult<_> {

let (data, vec) = result?

let scan_files = vec![];

state::visit_scan_files(data?.as_ref(), &vec, scan_files, scan_data_callback)

})

More generally, is this:

iter.map_and_then(foo)

equivalent to this?

iter.map(|result| -> DeltaResult<_> { foo(result?) })

... assuming foo is-a FnMut(_) -> DeltaResult<_>?

If so, I wonder if we're being "too clever" with all these iterator adaptors, and should really just write the extra line of direct code?

Either that, or we're not being clever enough, and we actually need to define a try_flat_map extension method so we no longer need a flatten_ok call afterward, e.g.

let scan_files_iter = scan_data .try_flat_map(|(data, vec)| { let (data, vec) = data?; let scan_files = vec![]; state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)?; });

where the definition fo try_flat_map would be:

Given Self::Item = Result<Outer, OuterErr: Into<E>>
and mapping function Outer -> Result<Mapped: IntoIterator, MapErr: Into<E>>
and Mapped::Item = Result<T, E>:

Attempts a flat_map over Ok items of this iterator for which the supplied mapping function also returns Ok. The result is an Iterator<Item = Result<T, E>>. Any Err produced by this iterator or by the mapping function propagates as an error in the resulting iterator's output.

It can be implemented in about 40 LoC.

Pro: One function to chain instead of two (also: avoids the complicated type signatures)
Pro: The mapping function is simple to use, because all the error handling is encapsulated

Con: The semantics are complicated to explain (also: impl needs seven template params).
Con: Our test crates still wouldn't be able to use it, unless we export it semi-publicly

My two cents is that clever solutions (especially home grown ones) are hard to keep standard. I'm not sure if every developer would remember to use try_flat_map or a map_and_then, but I think every developer would use/understand something like let sr = sr?. I think there's value in the clarity and simplicity.

Agree with staying simple.

More generally, is this:

iter.map_and_then(foo)

equivalent to this?

iter.map(|result| -> DeltaResult<_> { foo(result?) })

... assuming foo is-a FnMut(_) -> DeltaResult<_>?

As I understand it, yes. Also sorry for the round and round here, but seems like we can actually remove the new trait and keep things pretty simple by just doing a ? at the top of the closures you're currently passing to map_and_then .

The identity one is pretty nice too:

iter.map(|x| x?)

zachschuermann

looks great to me! just had a few nits

kernel/src/scan/mod.rs

zachschuermann · 2024-10-07T05:37:28Z

kernel/src/scan/mod.rs

+                let scan_files = vec![];
+                state::visit_scan_files(data.as_ref(), &vec, scan_files, scan_data_callback)
+            })
+            .flatten_ok(); // Iterator<DeltaResult<Vec<ScanFile>>> to Iterator<DeltaResult<ScanFile>>


nit: newline? or put on the line above?

Just put the comment on a newline above flatten_ok. How's it look?

ah yea might be the best we can do now unless we split across 2 lines? generally i still try to stick to <100 char wide even for docs

kernel/src/scan/mod.rs

acceptance/src/data.rs

kernel/examples/read-table-single-threaded/src/main.rs

kernel/tests/dv.rs

zachschuermann · 2024-10-07T05:47:26Z

kernel/tests/dv.rs

+                .map_or(0, |mask| mask.iter().filter(|&&m| !m).count());
+            Ok(data.length() - deleted_rows)
+        })
+        .fold_ok(0, Add::add)


very nice :)

Iteration over scan_files and processing them is now lazy Fixed failing tests

This commit introduces the ability to scan files lazily, only doing so when more ScanResults are requested

Co-authored-by: Ryan Johnson <[email protected]>

Co-authored-by: Zach Schuermann <[email protected]>

scovich

LGTM

nicklan

lgtm! Thanks for iterating on this!

OussamaSaoudi-db force-pushed the lazy_scan_execute branch from 3b09714 to 13628a5 Compare September 17, 2024 22:35

OussamaSaoudi-db commented Sep 17, 2024

View reviewed changes

kernel/examples/read-table-single-threaded/src/main.rs Outdated Show resolved Hide resolved

OussamaSaoudi-db changed the title ~~[WIP] Make scan::execute return a lazy iterator~~ Make scan::execute return a lazy iterator Sep 18, 2024

OussamaSaoudi-db marked this pull request as ready for review September 18, 2024 17:22

OussamaSaoudi-db force-pushed the lazy_scan_execute branch 2 times, most recently from c794261 to 901c0e5 Compare September 19, 2024 20:47

zachschuermann requested review from nicklan and zachschuermann September 20, 2024 17:38

nicklan reviewed Sep 24, 2024

View reviewed changes

OussamaSaoudi-db commented Sep 24, 2024

View reviewed changes

acceptance/src/data.rs Outdated Show resolved Hide resolved

OussamaSaoudi-db requested a review from nicklan September 25, 2024 20:50

OussamaSaoudi-db force-pushed the lazy_scan_execute branch from fcd9a8c to 7ce4593 Compare September 26, 2024 21:17

zachschuermann reviewed Sep 27, 2024

View reviewed changes

OussamaSaoudi-db mentioned this pull request Sep 27, 2024

Create a custom iterator to manage nested DeltaResult Iterators #361

Open

scovich reviewed Sep 28, 2024

View reviewed changes

OussamaSaoudi-db commented Oct 1, 2024

View reviewed changes

scovich reviewed Oct 1, 2024

View reviewed changes

OussamaSaoudi-db requested a review from zachschuermann October 2, 2024 21:51

nicklan requested changes Oct 3, 2024

View reviewed changes

OussamaSaoudi-db requested review from nicklan and scovich October 3, 2024 00:32

scovich reviewed Oct 3, 2024

View reviewed changes

zachschuermann approved these changes Oct 7, 2024

View reviewed changes

zachschuermann added the breaking-change Change that will require a version bump label Oct 7, 2024

OussamaSaoudi-db force-pushed the lazy_scan_execute branch from 5e0e133 to 782e71d Compare October 7, 2024 16:57

OussamaSaoudi-db added 3 commits October 7, 2024 09:59

First stage of moving scan::execute to be lazy

1ca141d

Iteration over scan_files and processing them is now lazy Fixed failing tests

Implement lazy scanning of files

c4f40bd

This commit introduces the ability to scan files lazily, only doing so when more ScanResults are requested

Clean up tests affected by the move to lazy execute

7e5ad45

OussamaSaoudi-db and others added 24 commits October 7, 2024 09:59

Move itertools to alphabetical order

3b56037

Address PR comments

14caf61

remove itertools dependency

b5bc921

Address feedback

95e1e8b

One more map_ok

b08992d

Respond to feedback

47710d2

use map_and_then

04ca9fe

Remove incorrect usage of map_ok

49f19ad

Style improvements

62f0f6e

Add clarifying comments

673bf50

More style changes

7d5d591

remove unused import

cc3772c

Address PR comments

454af5e

Small doc comment fix

fcb3253

Update kernel/tests/dv.rs

f52eaf9

Co-authored-by: Ryan Johnson <[email protected]>

apply pr changes

35e09b2

simplify execute

c51fa2f

Update acceptance/src/data.rs

faf5531

Co-authored-by: Zach Schuermann <[email protected]>

Update acceptance/src/data.rs

e9d6eda

Co-authored-by: Zach Schuermann <[email protected]>

Update kernel/examples/read-table-single-threaded/src/main.rs

b7174d4

Co-authored-by: Zach Schuermann <[email protected]>

Apply suggestions from code review

8f1d39f

Co-authored-by: Zach Schuermann <[email protected]>

Apply suggestions from code review

155e967

Co-authored-by: Zach Schuermann <[email protected]>

Address nits

0e0d1f5

fix docs

ae9e45a

OussamaSaoudi-db force-pushed the lazy_scan_execute branch from 782e71d to ae9e45a Compare October 7, 2024 17:00

OussamaSaoudi-db requested a review from scovich October 7, 2024 17:01

scovich approved these changes Oct 7, 2024

View reviewed changes

nicklan approved these changes Oct 7, 2024

View reviewed changes

OussamaSaoudi-db merged commit 0fdb40a into delta-io:main Oct 7, 2024
12 checks passed

OussamaSaoudi-db deleted the lazy_scan_execute branch October 7, 2024 20:10

	.collect::<DeltaResult<Vec<RecordBatch>>>()?;
	.try_collect()?;

		use test_log::test;

		fn count_valid_rows(stream: impl Iterator<Item = DeltaResult<ScanResult>>) -> DeltaResult<usize> {

Make scan::execute return a lazy iterator #340

Make scan::execute return a lazy iterator #340

Conversation

OussamaSaoudi-db commented Sep 17, 2024

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

nicklan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicklan Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OussamaSaoudi-db commented Sep 24, 2024

zachschuermann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OussamaSaoudi-db Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OussamaSaoudi-db Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicklan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zachschuermann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OussamaSaoudi-db Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scovich left a comment

Choose a reason for hiding this comment

nicklan left a comment

Choose a reason for hiding this comment

Make `scan::execute` return a lazy iterator #340

Make `scan::execute` return a lazy iterator #340

codecov bot commented Sep 17, 2024 •

edited

Loading

nicklan Sep 26, 2024 •

edited

Loading

OussamaSaoudi-db Sep 30, 2024 •

edited

Loading

OussamaSaoudi-db Oct 1, 2024 •

edited

Loading

OussamaSaoudi-db Oct 7, 2024 •

edited

Loading