Adding predicate to protocol and metadata log replay query

[OussamaSaoudi-db]

Pass a predicate hint to Engine in read_metadata so that log files are filtered to contain either metadata or protocol columns.

The motivation of this change is to filter log data at the engine level instead of needlessly passing EngineData back to the kernel.

In order to verify that an engine gets the predicate passed to log replay, I added a debug print statement in the sync engine that logs the predicate. The scan::tests::test_scan_data uses the sync engine and you can observe that the predicate is received. For example, running the test with RUST_LOG=DEBUG yields the following log:

 Reading json files: 
     ...
    with predicate Some(
        VariadicOperation {
            op: Or,
            exprs: [
                UnaryOperation {
                    op: Not,
                    expr: UnaryOperation {
                        op: IsNull,
                        expr: Column(
                            "metaData.id",
                        ),
                    },
                },
                UnaryOperation {
                    op: Not,
                    expr: UnaryOperation {
                        op: IsNull,
                        expr: Column(
                            "protocol.min_reader_version",
                        ),
                    },
                },
            ],
        },
    )

The predicate checks protocol and metadata leaf fields metaData.id and protocol.min_reader_version instead of checking for metaData and protocol. This is done to leverage row group skipping, since statistics are kept for leaf fields, and not internal nodes. This is especially useful for large checkpoint files which will have exactly one row group containing protocol and metadata info.

Closes: #74

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.88%. Comparing base (ed1919a) to head (beb9850).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
kernel/src/engine/sync/json.rs	50.00%	0 Missing and 1 partial ⚠️
kernel/src/engine/sync/parquet.rs	50.00%	0 Missing and 1 partial ⚠️
kernel/src/snapshot.rs	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #336      +/-   ##
==========================================
+ Coverage   73.86%   73.88%   +0.02%     
==========================================
  Files          43       43              
  Lines        8078     8085       +7     
  Branches     8078     8085       +7     
==========================================
+ Hits         5967     5974       +7     
  Misses       1732     1732              
  Partials      379      379              

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

[zachschuermann]

awesome this is a great start :) one nit: we can use Expression::{whatever} and reduce the need for prepending Expression:: to everything. and we can loop back on testing!

[OussamaSaoudi-db]

In order to verify that an engine gets the predicate passed to log replay, I added a debug print statement in the default engine that logs the predicate. The reader acceptance tests use the default engine and you can observe that the predicate is received.

For example, running the reader_test::basic_append test yields the following print statement:

[2024-09-17T20:20:02Z DEBUG delta_kernel::engine::default::json] Reading json files with predicate: Some(VariadicOperation { op: Or, exprs: [UnaryOperation { op: Not, expr: UnaryOperation { op: IsNull, expr: Column("metaData.id") } }, UnaryOperation { op: Not, expr: UnaryOperation { op: IsNull, expr: Column("protocol.min_reader_version") } }] })
[2024-09-17T20:20:02Z DEBUG delta_kernel::engine::default::parquet] Reading parquet files with predicate: Some(VariadicOperation { op: Or, exprs: [UnaryOperation { op: Not, expr: UnaryOperation { op: IsNull, expr: Column("metaData.id") } }, UnaryOperation { op: Not, expr: UnaryOperation { op: IsNull, expr: Column("protocol.min_reader_version") } }] })

[OussamaSaoudi-db]

Code coverage is not happy with the debug print lines. This is likely because debug! is conditionally compiled depending on the RUST_LOG env variable.

[nicklan]

just a couple of small nits

[zachschuermann]

lgtm!

[zachschuermann]

@OussamaSaoudi-db can you make a follow-up issue to actually implement the predicate pushdown and to test this more thoroughly (maybe by metrics or some other way)

[nicklan]

lgtm! thanks!