[GitHub] [spark] cloud-fan edited a comment on issue #26847: [SPARK-30214][SQL] Support COMMENT ON syntax

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

cloud-fan edited a comment on issue #26847: [SPARK-30214][SQL] Support COMMENT ON syntax
URL: https://github.com/apache/spark/pull/26847#issuecomment-564510061
 
 
   This is a new command (no legacy v1 command), and is a good chance to discuss how commands should be resolved and executed ideally.
   
   Since we have a v2 adapter for v1 catalog (`V2SessionCatalog`), all the table/namespace commands can be implemented via v2 APIs.
   
   Usually a command needs to know which catalog it needs to operate, but different commands have different requirements about what to resolve. A few examples:
   1. DROP NAMESPACE: only need to know the name of the namespace.
   2. DESC NAMESPACE: need to lookup the namespace and get metadata, but is done during execution
   3. DROP TABLE: need to do lookup and make sure it's a table not (temp) view.
   4. DESC TABLE: need to lookup the table and get metadata.
   
   For namespaces, analyzer only needs to find the catalog and the namespace name. The command can do lookup during execution if needed. For tables, mostly commands need analyzer to do lookup.
   
   Note that, table and namespace have a difference: `DESC NAMESPACE testcat` works and describes the root namespace under `testcat`, while `DESC TABLE testcat` fails if there is no table `testcat` under the current catalog. It's because namespace can be named `[]`, but table can't. The commands should explicitly specify it needs to operate on namespace or table.
   
   Here is my proposal: introduce `UnresolvedNamespace` and `UnresolvedV2Relation`, which can be resolved to `ResolvedNamespace(catalog, nameParts)` and `ResolvedV2Relation(catalog, v2Relation)`. The parser creates command containing either `UnresolvedNamespace` or `UnresolvedV2Relation`, and planner converts commands with `ResolvedNamespace` or `ResolvedV2Relation` to physical plans.
   
   Note: there is already a `UnresolvedV2Relation`. We should rename it to something else.
   
   What do you think? @brkyvz @rdblue @imback82 
   
   
   

----------------------------------------------------------------
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