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

Remove withPartitionPath from the public API #507

Closed
rdblue opened this issue Sep 30, 2019 · 8 comments
Closed

Remove withPartitionPath from the public API #507

rdblue opened this issue Sep 30, 2019 · 8 comments
Labels

Comments

@rdblue
Copy link
Contributor

rdblue commented Sep 30, 2019

Several issues (#428, #505) have been opened where people are using DataFiles.Builder#withPartitionPath to parse partition key strings. This is an anti-pattern and Iceberg should not support serialization to and from string. Conversion to a partition key is a one-way conversion.

Having the withPartitionPath method in the public API encourages using this anti-pattern. I think we should remove it.

@rdblue
Copy link
Contributor Author

rdblue commented Sep 30, 2019

@aokolnychyi, @prodeezy, what do you think about this issue and possibly targeting this for the release?

@aokolnychyi
Copy link
Contributor

aokolnychyi commented Sep 30, 2019

We used that only for importing existing tables to Iceberg. We have tests but I haven't seen a real use case that required it.

@rdblue
Copy link
Contributor Author

rdblue commented Sep 30, 2019

Yeah, for importing, I'm wondering if we shouldn't require going through an import API call, like the one in SparkTableUtil. That would make imports still work, but avoid a misleading call in the API.

@aokolnychyi
Copy link
Contributor

That would work for us. We developed our version of migration before we had it in SparkTableUtil.

@prodeezy
Copy link
Contributor

prodeezy commented Oct 8, 2019

We unfortunately depend on this currently but we will work on taking this dependency out.
cc @fbocse @rominparekh

@rdblue
Copy link
Contributor Author

rdblue commented Oct 8, 2019

@prodeezy, we'll leave it in for now. Just let us know when you've moved away from it.

Copy link

This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.

@github-actions github-actions bot added the stale label Jan 23, 2024
Copy link

github-actions bot commented Feb 7, 2024

This issue has been closed because it has not received any activity in the last 14 days since being marked as 'stale'

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants