fivetran-kostaszoumpatianos commented on PR #1772:
URL: https://github.com/apache/polaris/pull/1772#issuecomment-2932254387

   Thanks @adutra,
   
   Both`HiveCatalog` and `EcsCatalog` provide their own implementations of 
`tableExists`. `GlueCatalog` on the other hand doesn't but still it doesn't 
break when `history` (for example) is used as a new table name. This, however, 
is just a client,  same as `RESTCatalog`.
   What I want to stress is that it looks like common practice for inheritors 
of this class to provide their own `tableExists` implementations and since the 
part that breaks is within Polaris itself and not in the client, I was thinking 
that it probably made more sense to override that in `IcebergCatalog` in 
Polaris. It is also faster in terms of release cycles.
   
   That said, we could have fixed that at the `BaseMetastoreCatalog` level by 
overriding `tableExists` there as:
   ```
     @Override
     public boolean tableExists(TableIdentifier identifier) {
       if (isValidIdentifier(identifier)) {
         return newTableOps(identifier).current() != null;
       }
       return false;
     }
   ```
   Is this what you had in mind? I am wondering if that would create more 
issues downstream since inheritor implementations might rely on this "broken 
feature". 
   
   wdyt?
   


-- 
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: issues-unsubscr...@polaris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to