yyanyy commented on code in PR #57585:
URL: https://github.com/apache/spark/pull/57585#discussion_r3856989922
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/RelationCatalog.java:
##########
@@ -134,6 +139,34 @@ public interface RelationCatalog extends TableCatalog,
ViewCatalog {
*/
Relation loadRelation(Identifier ident) throws NoSuchTableException;
+ /**
+ * Load the relation for an identifier that may resolve to either a table or
a view, forwarding
+ * the user-specified options that may affect table state.
+ * <p>
+ * Behaves like {@link #loadRelation(Identifier)} but also receives
table-state options. The
Review Comment:
Had offline discussion:
1. `loadRelation(Identifier, RelationContext)` puts options inside the
context, unlike existing load APIs that always have the consistent
`(Identifier, Context, options)` arguments. We may eventually need separate
Table and View state options (for not polluting table options from views and
enable correct table pinning based on table state), but the view state option
mechanism has not been fully designed yet.
2. `loadChangelog(Identifier, ChangelogContext, options)` is an example that
shows the compatibility risk of defining an options contract too early, that we
need to revisit now that we require `options` to be responsible for table state
pinning. We therefore may prefer the conservative approach of requiring users
to implement the state-aware loadTable path with loadView fallback for now, and
defer the RelationCatalog API until View state is better understood.
--
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]
