delta-io / delta-kernel-rs

A native Delta implementation for integration with any query engine
Apache License 2.0
144 stars 41 forks source link

parse table metadata.configuration as `TableProperties` #453

Open zachschuermann opened 1 week ago

zachschuermann commented 1 week ago

What changes are proposed in this pull request?

New TableProperties struct parses the Metadata action's configuration into a strongly-typed configuration struct. Then, throughout the kernel we can leverage the configuration to appropriately handle DVs, column mapping, etc.

Pragmatically, the changes are:

  1. New TableProperties struct along with new Snapshot::get_table_properties() API
  2. table_properties module including (1) and a custom serde deserializer to map the HashMap<String, String> into serde's internal data format, and Deserialize implementations derived for TableProperties and its fields

TODOs

This PR affects the following public APIs

How was this change tested?

more testing TODO!

Credit: @roeap for the original PR which this is based on #222

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 94.67085% with 17 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@266e9b4). Learn more about missing BASE report. Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
kernel/src/table_properties.rs 91.58% 7 Missing and 2 partials :warning:
kernel/src/table_properties/deserialize.rs 97.01% 1 Missing and 5 partials :warning:
kernel/src/expressions/column_names.rs 0.00% 1 Missing :warning:
kernel/src/snapshot.rs 75.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #453 +/- ## ======================================= Coverage ? 78.51% ======================================= Files ? 57 Lines ? 11914 Branches ? 11914 ======================================= Hits ? 9354 Misses ? 2053 Partials ? 507 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zachschuermann commented 1 week ago

need to make all properties Options. why? for propagating unchanged in write. e.g. if appendOnly is omitted we want to omit in the write (not write its default)