[GitHub] [spark] rdblue commented on issue #25077: [SPARK-28301][SQL] fix the behavior of table name resolution with multi-catalog

[GitBox <gi...@apache.org>]

rdblue commented on issue #25077: [SPARK-28301][SQL] fix the behavior of table name resolution with multi-catalog
URL: https://github.com/apache/spark/pull/25077#issuecomment-509712414
 
 
   > The fix in #24768 leads to different behaviors than I expect: a) temp view should have higher priority. b) table provider should not affect table name resolution. The implementation also needs to be different for the proposed behaviors, that's one reason I open this PR.
   
   Usually, I would expect you to point out problems with a PR in a review instead of opening a separate PR that doesn't mention the original. In the future, just coordinate about how to get the work done.
   
   > 1. fix the table name resolution. 2. support v2 provider with Hive catalog
   
   That PR solves these two at the same time because the default catalog was introduced so that v2 providers could be used. If the default catalog behavior changes, then the v2 session catalog needs to be introduced to handle those cases. Similarly, if we add the v2 session catalog, then we need to update the table resolution rules.
   
   It is reasonable to fix those two at the same time. Fixing just table resolution in this PR breaks v2 providers, which is unnecessary.
   
   > The lookupRelation should belong to analyzer instead of catalog, since catalog doesn't know the relation plans Spark uses
   
   I'm not sure what you mean. The `ResolveTable` rule already does this in the analyzer. Moving this to `SessionCatalog` is a different approach entirely. Like I said, I'm happy to discuss the merits of this, but please don't mix these changes together.
   
   > the main point of this PR is to trigger discussion and reach a consensus about the behavior of table name resolution
   
   This doesn't make sense to me because we agree about what the behavior should be. This doesn't propose substantial changes to the behavior in the other PR, just minor fixes to temporary table handling that we all agree on. In practice, this PR only introduces an alternative implementation that breaks existing conventions and approaches.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org