[No Merge] Changes with BaseMetadata #10
Conversation
nk1506
force-pushed
the
hive_table_ops_refactor_with_issue_9514
branch
from
052ff12 to
9e440b4
Compare
| protected void doCommit(TableMetadata base, TableMetadata metadata) { | ||
| protected void doCommit(BaseMetadata baseMetadata, BaseMetadata newMetadata) { | ||
| TableMetadata metadata = (TableMetadata) newMetadata; | ||
| TableMetadata base = (TableMetadata) baseMetadata; |
There was a problem hiding this comment.
If we cast to table metadata I think we can't extract the common code for view,
I am expecting NessieBaseMetastoreTableOperations extends BaseMetastoreTableOperations
to handle common code and call one interface doCommitImpl() which will be implemented by NessieTableOperations and NessieViewOperations.
Similarly for other catalogs.
There was a problem hiding this comment.
I think this should be doable. I am waiting for feedback on parent PR.
| protected abstract void deleteRemovedMetadataFiles( | ||
| BaseMetadata baseMetadata, BaseMetadata newMetadata);; | ||
|
|
||
| protected void doCommit(BaseMetadata baseMetadata, BaseMetadata newMetadata) { |
There was a problem hiding this comment.
We can have a new API with BaseMetadata. Although I am seeing challenge on Backward compatibility.
Couple of questions I have :
- We can't change signature of doCommit on tableOperation or ViewOperation for backward compatible.
- If we deprecate the original one and create
doCommit(BaseMetadata baseMetadata, BaseMetadata newMetadata)with Base class it can cause issue on runtime. - Should we rename doCommit on base class? and gradually deprecate ?
|
This is just a draft patch to understand the cleanup with BaseMetadata interface. I think this is making code more cleaner for tableOperations and viewOperations. Only challenge I am seeing for compatibility. |
No description provided.