peter-toth commented on code in PR #57585:
URL: https://github.com/apache/spark/pull/57585#discussion_r3743283049
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/RelationCatalog.java:
##########
@@ -178,6 +206,28 @@ default Table loadTable(Identifier ident) throws
NoSuchTableException {
throw new NoSuchTableException(ident);
Review Comment:
**Finding 11.** With the `loadTable(Identifier, TableContext,
CaseInsensitiveStringMap)` default added below, `loadTable(Identifier)` is off
the read path entirely for a `RelationCatalog`: analysis calls
`loadRelation(ident, options)`
(`sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala:269`),
and refresh / recache / the `SupportsCatalogOptions` `load()` path all arrive
at `:225`, which derives from the same overload. What still lands here is
write-privilege loads (`TableCatalog`'s `loadTable(ident, writePrivileges)`
default falls through, `TableCatalog.java:238`), DDL and misc commands
(`ShowTablesExtendedExec.scala:52`, `RenameTableExec.scala:53`,
`TableCatalog#listTableSummaries` at `TableCatalog.java:135`) and the
`V2TableReference` reload (`RelationResolution.scala:510`).
So the advice at `:198` — "Override only if a tables-only path is materially
cheaper than the unified one" — now points at an optimization that never fires
for the case it reads like it is for. The description states the consequence;
the javadoc is where a connector author will look for it. Rough shape:
```java
* The default implementation derives from {@link #loadRelation}: a {@link
View} is rejected as
* not-a-table; a {@link Table} is returned. Override only if a
tables-only path is materially
* cheaper than the unified one -- note that reads do not reach this
method: they go to
* {@link #loadRelation(Identifier, CaseInsensitiveStringMap)}, directly
from the resolver or
* via {@link #loadTable(Identifier, TableContext,
CaseInsensitiveStringMap)}. What remains
* here is write-privilege loads, DDL and misc lookups.
```
--
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]
