flyingImer commented on PR #4052: URL: https://github.com/apache/polaris/pull/4052#issuecomment-4172776720
I do think this is directionally better now. Pulling S3 Tables into a more dedicated path makes sense to me, and I think this is in a better place than the earlier version where the logic was drifting further into the generic S3 vending path. That said, I still think there are two issues that block this for me: 1. `StorageTypeFileIO` is no longer a stable discriminator once both `S3` and `S3_TABLES` point at the same `S3FileIO`, but the validation path still reverse-maps from FileIO impl back to storage type. 2. Once we know the catalog is `S3_TABLES`, the no-`tableId` path still warns and continues instead of failing closed. So IIUC, the create-time/unresolved-scope gap is still not really closed yet. So overall, I'm aligned with the structural direction here, but I still don't think the contract is explicit enough yet for merge. -- 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]
