Github user clockfly commented on the issue:

    https://github.com/apache/spark/pull/15099
  
    My comments:
    
    # 1. There are too many hack about TableIdentifier. 
    
    ## Problem:
    Currently, our current use of TableIdentifier is kind of ambiguous.
    ```
    case class TableIdentifier(table: String, database: Option[String])
    ```
     For example, if database is not specified, sometimes we interpret the 
table identifier as a table in default database, sometimes, we interpret it as 
a temporary view. It is not good! I think if we want to call it as an Id, then 
it need to point to a exact table/view with no ambiguity.
    
    ## Suggestion:
    Suggest to resolve TableIdentifier to a full qualified table identifier in 
Analyzer stage. For a TableIdentifier from SQL parser which doesn't have a 
database, we should:
    1. If it is a temporary view, then we should assign it a built-in database 
representing temporary view. 
    2. If it is a table, then we should add the database prefix.
    
    After analysis, there should be no TableIdentifier which doesn't have 
database specified.
    
    #  The interface change in SessionCatalog is not consistent with other 
methods name conversion.
    
    ## Problem
    In class SessionCatalog, this PR adds two method dropTempView, getTempView, 
but at the same time all other methods in SessionCatalog use name conversion 
"table" instead of "view". For example, the following methods can operate  on 
temporary view:
    
    ```
    isTemporaryTable, 
    tableExists, 
    refreshTable, 
    clearTempTables
    ```
    
    I think the change will make class SessionCatalog harder to understand.
    
    ## Suggestion
    We still use the name "table", but use the TableIdentifier to differentiate 
whether it is a temporary view or table.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to