Skip to content

Conversation

@murali-db
Copy link
Contributor

@murali-db murali-db commented Jun 10, 2025

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

This PR is refactoring only, move all the validation checks from IcebergWriterCompatV1MetadataValidatorAndUpdater.java to a new base class IcebergWriterCompatMetadataValidatorAndUpdater.java so that later newly added IcebergWriterCompatV3MetadataValidatorAndUpdater.java can use them.

How was this patch tested?

Existing unit tests since it is only doing refactoring.

Does this PR introduce any user-facing changes?

@murali-db murali-db changed the title Refactor WriterCompat metadata validator and updater [Kernel][#1] Refactor WriterCompat metadata validator and updater Jun 10, 2025
@murali-db murali-db changed the title [Kernel][#1] Refactor WriterCompat metadata validator and updater [Kernel][#2] Refactor WriterCompat metadata validator and updater Jun 10, 2025
format

style issues

# Conflicts:
#	kernel/kernel-api/src/main/java/io/delta/kernel/internal/icebergcompat/IcebergWriterCompatV1MetadataValidatorAndUpdater.java
@murali-db murali-db force-pushed the refactor-writer-metadata-validator branch from f1808ff to d5fd1f5 Compare June 11, 2025 16:36
protected static IcebergCompatCheck createUnsupportedTypesCheck(String compatFeatureName) {
return (inputContext) -> {
List<Tuple2<List<String>, StructField>> matches =
SchemaUtils.filterRecursively(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @emkornfield is removing this API. Please use the SchemaIterable interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, #4742 adds a stream interface and #4752 for how it is used for validators.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both PRs are waiting a on second reviewer before merging

@murali-db murali-db force-pushed the refactor-writer-metadata-validator branch from 58e7723 to 8ab6280 Compare June 13, 2025 19:30
@murali-db murali-db force-pushed the refactor-writer-metadata-validator branch from 8ab6280 to 07af1a9 Compare June 13, 2025 19:31
@vkorukanti vkorukanti merged commit d90b060 into delta-io:master Jun 16, 2025
22 checks passed
vkorukanti pushed a commit that referenced this pull request Jun 18, 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-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [x] Kernel
- [ ] Other (fill in here)

## Description

<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->
Follow up to #4734 and removes a missed function definition during
refactoring.

## How was this patch tested?

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->
Unit tests.

## Does this PR introduce _any_ user-facing changes?

<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
No
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.

4 participants