shaoyu-li commented on PR #13058:
URL: https://github.com/apache/gravitino/pull/13058#issuecomment-5629339587
Pushed as three commits on top of the reviewed one, so your threads stay
anchored:
- `29a4ea9` — the review changes: external tables skipped on unprovision,
the cascade resolving its
schema once, the format check and cache lookup hoisted above provisioning,
a name-to-class index
in the factory, member ordering, the inlined location check.
- `be4c82f` — a bug that surfaced while implementing the external skip: the
context read `external`
more permissively than the table formats do, so `external=yes` had the
catalog and the format
disagree about the same table. Details in the thread on the drop path.
- `3ae14da` — the catalog docs still described discovery as it behaved
before the index, and a
trailing-slash assertion an earlier revision had and the rewrite lost.
88 tests in the module, 0 failures, 0 skipped; `javadoc` and `spotlessCheck`
clean. The description
is updated too -- it listed three test names that do not exist, from an
earlier draft.
Two things I pushed back on rather than changed, both argued in their
threads: `requiresUnprovision()`
(defaulting to false reintroduces the accidental-default failure we agreed
to avoid on unprovision),
and the third of your three reads (`LanceTableOperations` calling
`super.loadTable`), which is below
this SPI and would need `TableOperations.dropTable` widened. Happy to do the
second one if you want
it here.
--
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]
