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

feat: Enable simple schema evolution for Change Data Feed #625

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

OussamaSaoudi-db
Copy link
Collaborator

@OussamaSaoudi-db OussamaSaoudi-db commented Jan 7, 2025

What changes are proposed in this pull request?

This PR enabled simple forms of schema evolution to change data feed. The supported schema changes are:

  • Converting of a column from non-nullable to nullable
  • Adding a new nullable column

This PR also removes several unnecessary uses of allow(unused).

How was this change tested?

Updated existing tests to check that performing schema evolution check fails in these cases:

  • schema with extra column cannot be read with schema not containing that row
  • Column with wider type cannot be read with narrower type in schema
  • Type widening is not supported (as of this PR)
  • incompatible data types fail
  • schema with non-nullable column can be read with schema containing the nullable column
  • can read schema with an extra nullable column

The updates to the tests also control more variables so that we test only one thing at a time.

@OussamaSaoudi-db OussamaSaoudi-db changed the title [WIP] feat: Enable simple schema evolution for Change Data Feed [ \[WIP\] feat: Enable simple schema evolution for Change Data Feed](https://github.com/delta-io/delta-kernel-rs/pull/625/files/596aa049376d8e6c133f77bb7ae38c7d60c8cd7c..a6eb6cfc2ed9da6e8b1ada567a4449f120322997) Jan 7, 2025
@OussamaSaoudi-db OussamaSaoudi-db changed the title [ \[WIP\] feat: Enable simple schema evolution for Change Data Feed](https://github.com/delta-io/delta-kernel-rs/pull/625/files/596aa049376d8e6c133f77bb7ae38c7d60c8cd7c..a6eb6cfc2ed9da6e8b1ada567a4449f120322997) [[WIP] feat: Enable simple schema evolution for Change Data Feed Jan 7, 2025
@OussamaSaoudi-db OussamaSaoudi-db changed the title [[WIP] feat: Enable simple schema evolution for Change Data Feed [WIP] feat: Enable simple schema evolution for Change Data Feed Jan 7, 2025
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.07%. Comparing base (6751838) to head (e2a624c).

Files with missing lines Patch % Lines
kernel/src/table_changes/log_replay/tests.rs 97.14% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
- Coverage   84.08%   84.07%   -0.02%     
==========================================
  Files          76       76              
  Lines       17526    17540      +14     
  Branches    17526    17540      +14     
==========================================
+ Hits        14736    14746      +10     
- Misses       2077     2079       +2     
- Partials      713      715       +2     

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

@OussamaSaoudi-db OussamaSaoudi-db changed the title [WIP] feat: Enable simple schema evolution for Change Data Feed feat: Enable simple schema evolution for Change Data Feed Jan 7, 2025
@OussamaSaoudi-db OussamaSaoudi-db marked this pull request as ready for review January 7, 2025 23:34
@OussamaSaoudi OussamaSaoudi added the merge hold Don't allow the PR to merge label Jan 21, 2025
Add schema compatibility tests

Add error-based schema compatibility check

Remove old schema compat

change naming, remove redundant test

Improve comments, add check that field names aren't case-insensitive duplicates

Change ordering, remove unnecessary comment

Add comments to can_read_as methods

Add schema compatibility check to CDF, update cdf tests

remove todo comments

Add clarifying comments
@github-actions github-actions bot added the breaking-change Change that will require a version bump label Jan 23, 2025
@OussamaSaoudi OussamaSaoudi removed merge hold Don't allow the PR to merge breaking-change Change that will require a version bump labels Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants