ayushtkn commented on code in PR #4292:
URL: https://github.com/apache/hive/pull/4292#discussion_r1215132378
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java:
##########
@@ -550,14 +550,19 @@ default boolean isTimeTravelAllowed() {
return false;
}
- default boolean isMetadataTableSupported() {
+ default boolean isTableMetaRefSupported() {
return false;
}
Review Comment:
We don't just have it in Iceberg code:
https://github.com/apache/hive/blob/rel/release-4.0.0-alpha-2/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java#L464-L465
We added in the parent interface itself and it is indeed released in the
previous release also.
https://github.com/apache/hive/blob/rel/release-4.0.0-alpha-2/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveStorageHandler.java#L464-L465
So, technically if some client code does getStorageHandler() and calls this
it is gonna fail, or if someone had a custom/internal storage handler where he
implemented it, he is gonna suffer as well.
Mostly nobody would have, but we can't gurantee based on your sentiments
:-), But it is still an incompatible change. "Changing a public method
name/signature", I think this method name change ain't serving any functional
benefits, we can keep it as is
If people still want it: The correct way is deprecate the original, add a
new one, call the new one from the deprecated method. But I don't think we want
it that hard. I would be against breaking compat just for look & feel :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
