Gabriel39 commented on PR #66581: URL: https://github.com/apache/doris/pull/66581#issuecomment-5215563518
I found one blocking compatibility issue with the `tableExist` change. `LanceExternalCatalog.tableExist()` now calls `namespace.tableExists()`, which uses `POST /v1/table/:id/exists` for a REST namespace. However, the existing Doris Lance REST fixture (`docker/thirdparties/docker-compose/iceberg/scripts/lance_rest_server.py`) only implements ListNamespaces, ListTables, and DescribeTable; every other POST request returns 404. The unit-test mock in `LanceRestCatalogTest` has the same limitation. Consequently, a table that can be listed and described successfully by the currently supported REST fixture can be reported as non-existent (or fail with an exception, depending on the SDK error mapping) whenever a caller uses `tableExist()`. Please update the REST fixture and unit-test mock to implement the table-exists endpoint, and add coverage for: - an existing table; - a missing table/namespace; - authentication or service errors, which must not be converted to `false`. If Doris intends to remain compatible with namespace implementations that support List/Describe but not TableExists, please also fall back to `describeTable()` when TableExists is unsupported. Also, the PR checklist currently says that this is a pure refactor with no behavior change and no tests needed. That is inaccurate: the previous implementation converted every `RuntimeException` to `false`, while the new implementation only converts explicit not-found errors and propagates other failures. This is a desirable behavior change, but it should be documented in the PR description and covered by tests. -- 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]
