Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single part checkpoint write api #779

Draft
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

sebastiantia
Copy link
Collaborator

What changes are proposed in this pull request?

How was this change tested?

@github-actions github-actions bot added the breaking-change Change that will require a version bump label Mar 29, 2025
Copy link

codecov bot commented Mar 30, 2025

Codecov Report

Attention: Patch coverage is 86.71024% with 183 lines in your changes missing coverage. Please review.

Project coverage is 84.77%. Comparing base (9290930) to head (c8bcc2e).

Files with missing lines Patch % Lines
kernel/src/checkpoints/mod.rs 89.21% 30 Missing and 10 partials ⚠️
kernel/src/engine/parquet_row_group_skipping.rs 0.00% 36 Missing ⚠️
kernel/src/actions/visitors.rs 94.28% 4 Missing and 18 partials ⚠️
kernel/src/checkpoints/tests.rs 84.80% 2 Missing and 17 partials ⚠️
kernel/src/path.rs 77.41% 6 Missing and 8 partials ⚠️
kernel/src/expressions/scalars.rs 37.50% 10 Missing ⚠️
kernel/src/log_replay.rs 90.19% 2 Missing and 8 partials ⚠️
ffi/src/expressions/kernel.rs 0.00% 9 Missing ⚠️
kernel/src/checkpoints/log_replay.rs 95.45% 4 Missing and 2 partials ⚠️
kernel/src/scan/log_replay.rs 93.67% 0 Missing and 5 partials ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #779      +/-   ##
==========================================
+ Coverage   84.64%   84.77%   +0.12%     
==========================================
  Files          82       86       +4     
  Lines       19735    20979    +1244     
  Branches    19735    20979    +1244     
==========================================
+ Hits        16705    17784    +1079     
- Misses       2214     2320     +106     
- Partials      816      875      +59     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sebastiantia added a commit that referenced this pull request Apr 2, 2025
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
2. Run `cargo t --all-features --all-targets` to get started testing,
and run `cargo fmt`.
  3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  5. Be sure to keep the PR description updated to reflect all changes.
-->

<!--
PR title formatting:
This project uses conventional commits:
https://www.conventionalcommits.org/

Each PR corresponds to a commit on the `main` branch, with the title of
the PR (typically) being
used for the commit message on main. In order to ensure proper
formatting in the CHANGELOG please
ensure your PR title adheres to the conventional commit specification.

Examples:
- new feature PR: "feat: new API for snapshot.update()"
- bugfix PR: "fix: correctly apply DV in read-table example"
-->

## What changes are proposed in this pull request?
<!--
Please clarify what changes you are proposing and why the changes are
needed.
The purpose of this section is to outline the changes, why they are
needed, and how this PR fixes the issue.
If the reason for the change is already explained clearly in an issue,
then it does not need to be restated here.
1. If you propose a new API or feature, clarify the use case for a new
API or feature.
  2. If you fix a bug, you can clarify why it is a bug.
-->

This PR introduces the `CheckpointMetadata` action.

This action is only allowed in checkpoints following [V2
spec](https://github.com/delta-io/delta/blob/master/PROTOCOL.md#v2-spec).
For more information: [[link to
protocol]](https://github.com/delta-io/delta/blob/master/PROTOCOL.md#checkpoint-metadata)

This PR is part of the on-going effort to implement single-file
checkpoint write support. For reference, [[link to write API
proposal]](#779)

There already exists a `CheckpointMetadata` named struct
[[link]](https://github.com/delta-io/delta-kernel-rs/blob/9290930bbeb1100e7af98c228dbd339eea38143a/kernel/src/snapshot.rs#L149)
which represents the `_last_checkpoint` file. This PR renames this
struct to `LastCheckpointHint`
<!--
Uncomment this section if there are any changes affecting public APIs:
### This PR affects the following public APIs

If there are breaking changes, please ensure the `breaking-changes`
label gets added by CI, and describe why the changes are needed.

Note that _new_ public APIs are not considered breaking.
-->




## How was this change tested?
<!--
Please make sure to add test cases that check the changes thoroughly
including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please
clarify how you tested, ideally via a reproducible test documented in
the PR description.
-->

- `test_checkpoint_metadata_schema`: tests schema projection
sebastiantia added a commit that referenced this pull request Apr 7, 2025
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
2. Run `cargo t --all-features --all-targets` to get started testing,
and run `cargo fmt`.
  3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  5. Be sure to keep the PR description updated to reflect all changes.
-->

<!--
PR title formatting:
This project uses conventional commits:
https://www.conventionalcommits.org/

Each PR corresponds to a commit on the `main` branch, with the title of
the PR (typically) being
used for the commit message on main. In order to ensure proper
formatting in the CHANGELOG please
ensure your PR title adheres to the conventional commit specification.

Examples:
- new feature PR: "feat: new API for snapshot.update()"
- bugfix PR: "fix: correctly apply DV in read-table example"
-->

## What changes are proposed in this pull request?
<!--
Please clarify what changes you are proposing and why the changes are
needed.
The purpose of this section is to outline the changes, why they are
needed, and how this PR fixes the issue.
If the reason for the change is already explained clearly in an issue,
then it does not need to be restated here.
1. If you propose a new API or feature, clarify the use case for a new
API or feature.
  2. If you fix a bug, you can clarify why it is a bug.
-->

This PR introduces the helper methods:
- `new_uuid_parquet_checkpoint` which creates a new
`ParsedCheckpointPath<Url>` for a uuid-named parquet checkpoint file at
the specified version. The UUID-naming scheme looks like:
`n.checkpoint.u.parquet`, where u is a UUID and n is the snapshot
version that this checkpoint represents.
- `new_classic_parquet_checkpoint` which creates a new
`ParsedCheckpointPath<Url>` for a classic-named parquet checkpoint file
at the specified version. The classic-naming scheme looks like:
`n.checkpoint.parquet`, where n is the snapshot version that this
checkpoint represents.
- **Updates the `uuid` dependency to always include `v4` and `fast-rng`
features:**
     - This ensures that `uuid::new_v4()` is always available.
- The `fast-rng` feature improves performance when generating UUIDs.




For more information on the two checkpoint naming-schemes:

https://github.com/delta-io/delta/blob/master/PROTOCOL.md#uuid-named-checkpoint

https://github.com/delta-io/delta/blob/master/PROTOCOL.md#classic-checkpoint

This PR is part of the on-going effort to implement single-file
checkpoint write support. For reference, [[link to write API
proposal]](#779)

<!--
Uncomment this section if there are any changes affecting public APIs:
### This PR affects the following public APIs

If there are breaking changes, please ensure the `breaking-changes`
label gets added by CI, and describe why the changes are needed.

Note that _new_ public APIs are not considered breaking.
-->


## How was this change tested?
<!--
Please make sure to add test cases that check the changes thoroughly
including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please
clarify how you tested, ideally via a reproducible test documented in
the PR description.
-->

- `test_new_uuid_parquet_checkpoint` - verifies UUID-named Parquet
checkpoint creation with proper attributes.
- `test_new_classic_parquet_checkpoint` - verifies classic-named Parquet
checkpoint creation with proper attributes.
zachschuermann pushed a commit to hntd187/delta-kernel-rs that referenced this pull request Apr 8, 2025
…io#782)

<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
2. Run `cargo t --all-features --all-targets` to get started testing,
and run `cargo fmt`.
  3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  5. Be sure to keep the PR description updated to reflect all changes.
-->

<!--
PR title formatting:
This project uses conventional commits:
https://www.conventionalcommits.org/

Each PR corresponds to a commit on the `main` branch, with the title of
the PR (typically) being
used for the commit message on main. In order to ensure proper
formatting in the CHANGELOG please
ensure your PR title adheres to the conventional commit specification.

Examples:
- new feature PR: "feat: new API for snapshot.update()"
- bugfix PR: "fix: correctly apply DV in read-table example"
-->

## What changes are proposed in this pull request?
<!--
Please clarify what changes you are proposing and why the changes are
needed.
The purpose of this section is to outline the changes, why they are
needed, and how this PR fixes the issue.
If the reason for the change is already explained clearly in an issue,
then it does not need to be restated here.
1. If you propose a new API or feature, clarify the use case for a new
API or feature.
  2. If you fix a bug, you can clarify why it is a bug.
-->

This PR introduces the helper methods:
- `new_uuid_parquet_checkpoint` which creates a new
`ParsedCheckpointPath<Url>` for a uuid-named parquet checkpoint file at
the specified version. The UUID-naming scheme looks like:
`n.checkpoint.u.parquet`, where u is a UUID and n is the snapshot
version that this checkpoint represents.
- `new_classic_parquet_checkpoint` which creates a new
`ParsedCheckpointPath<Url>` for a classic-named parquet checkpoint file
at the specified version. The classic-naming scheme looks like:
`n.checkpoint.parquet`, where n is the snapshot version that this
checkpoint represents.
- **Updates the `uuid` dependency to always include `v4` and `fast-rng`
features:**
     - This ensures that `uuid::new_v4()` is always available.
- The `fast-rng` feature improves performance when generating UUIDs.




For more information on the two checkpoint naming-schemes:

https://github.com/delta-io/delta/blob/master/PROTOCOL.md#uuid-named-checkpoint

https://github.com/delta-io/delta/blob/master/PROTOCOL.md#classic-checkpoint

This PR is part of the on-going effort to implement single-file
checkpoint write support. For reference, [[link to write API
proposal]](delta-io#779)

<!--
Uncomment this section if there are any changes affecting public APIs:
### This PR affects the following public APIs

If there are breaking changes, please ensure the `breaking-changes`
label gets added by CI, and describe why the changes are needed.

Note that _new_ public APIs are not considered breaking.
-->


## How was this change tested?
<!--
Please make sure to add test cases that check the changes thoroughly
including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please
clarify how you tested, ideally via a reproducible test documented in
the PR description.
-->

- `test_new_uuid_parquet_checkpoint` - verifies UUID-named Parquet
checkpoint creation with proper attributes.
- `test_new_classic_parquet_checkpoint` - verifies classic-named Parquet
checkpoint creation with proper attributes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Change that will require a version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant