JingsongLi commented on PR #10850: URL: https://github.com/apache/paimon/pull/10850#issuecomment-6594574588
I agree with the current direction: keep one table-loading path, allow metadata access with runtime-invalid options, and validate the effective configuration when the store is initialized. Preserving eager validation in Core `getTable()`/`copy()` is not a requirement from my side. My remaining concern is the scope of the production-code changes. Could we reduce the implementation to the minimum needed for that behavior? This concern excludes tests; please retain the regression coverage. For example, renaming the subclass `newRead()`/`newScan()` implementations to `createRead()`/`createScan()` and adding superclass wrappers appears to serve only to force `store()` initialization earlier. The normal scan/read execution paths already reach `store()` later, where the centralized validation can run. Unless there is a concrete path that would otherwise miss required validation, I would keep the existing methods and avoid introducing these extra hooks just to move the same check earlier. More generally, please keep only the production changes necessary to separate metadata access from store initialization and to validate effective options before constructing the store. Any additional restructuring should have a concrete need within this fix or be left for a follow-up. -- 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]
