[SPARK-44685][SQL] Remove deprecated Catalog#createExternalTable #42356
Conversation
|
This needs an update to the release notes and migration guide. This should also be in the JIRA - release-notes and text of release note |
Hi @srowen , thanks for review. I already add it into migration guide. But I have some problem about update to the release notes and JIRA, I don't know how to do this, is only need change affect version to 4.0.0? Can you tell me more? Thanks! |
|
Tag the JIRA with |
Done. Thanks for your guidance! |
| "cacheTable", | ||
| "clearCache", | ||
| "createDataFrame", | ||
| "createExternalTable", |
There was a problem hiding this comment.
Hmm .. I wouldn't remove it for now. We have a bunch of deprecated API, and we don't have an explicit plan to how to handle them, and we're not even sure if we're actually going for 4.0.0.
There was a problem hiding this comment.
For example, if we apply the same argument, we should also remove SQLContext, etc., etc that would be largely a breaking change, which I believe can be controversial.
There was a problem hiding this comment.
Certainly, there may be an argument to not remove some methods. But these can only be removed at major versions; removing one does not imply all deprecated methods must be removed, either. What do you mean by 'going for 4.0.0' and how does this relate to SQLContext? there is indeed not one single plan for all deprecated methods, but what is the argument against this particular one?
There was a problem hiding this comment.
Oh I apologize that I forgot we already discussed that the next Spark version would likely be 4.0.0 - my bad.
My concern is mainly about "Considerations when breaking APIs" at https://spark.apache.org/versioning-policy.html. We discussed about removing some of deprecated API including SQLContext at Spark 3.0.0 but we did not remove in the end because it might be too breaking changes. At that time, we had to remove and revert some of those PRs (see also #22843 and #27815) - this includes createExternalTable.
I personally would like to have a bit of discussion in the mailing list (probably after 3.5.0 release) before we remove some, and find out that some of them are too much breaking changes.
There was a problem hiding this comment.
Yeah I think it's fair to hold for a more joined up discussion of what should be removed
|
We should only remove an API if there is clear evidence showing that no one is using it. |
Thanks, I will open a discussion in the mailing list after 3.5.0 released. |
|
Set this pr to draft to avoid accidental merging. |
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
@Hisoka-X Will you still initiate this discussion? |
I think @srowen already open a discussion in https://lists.apache.org/thread/yf2wvypvkqk7rqtv4s641ygbk4gr3tc2 |
OK, I see, thanks |
5 participants
What changes were proposed in this pull request?
Since we deprecated
Catalog#createExternalTablein 2.2.0, I think it's a good time to remove it on 4.0.0Why are the changes needed?
Remove deprecated code.
Does this PR introduce any user-facing change?
Yes, the deprecated method can't be used.
How was this patch tested?
exist test.