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

Spec: deprecate distinct_counts in data_file #8395

Conversation

gtrettenero
Copy link

Deprecating 111 distinct_counts, which was removed a long time ago

Copy link
Member

@ajantha-bhat ajantha-bhat left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for finding this out and fixing it.

PR title can be
"Spec: deprecate distint_counts in data_file"

@gtrettenero gtrettenero changed the title Deprecate 111 distint_counts Spec: deprecate distinct_counts in data_file Aug 29, 2023
@gtrettenero
Copy link
Author

Thanks for the LGTM, I cannot merge since I don't have write access, would you merge it please?

@ajantha-bhat
Copy link
Member

cc: @Fokko, @nastra

@nastra
Copy link
Contributor

nastra commented Aug 30, 2023

I think we can just remove it from the spec, no need to deprecate it, because deprecating would mean that there's still code that reads this field

@ajantha-bhat
Copy link
Member

Long back while removing some fields from code, we just deprecated in the spec.
#914

So, I suggested a similar approach for this instead of removing.

@gtrettenero gtrettenero closed this Sep 5, 2023
@gtrettenero gtrettenero reopened this Sep 5, 2023
Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 14, 2024
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants