cloud-fan edited a comment on issue #25747: [SPARK-29039][SQL] centralize the catalog and table lookup logic URL: https://github.com/apache/spark/pull/25747#issuecomment-536152638 Hi @rdblue , let me make the problem more clear. So what you proposed is kind of ``` catalog = lookup catalog if (catalog is not session catalog) { convert to v2 command with UnresolvedRelation } else { table = lookup table from the catalog if (table is not v1 table) { convert to v2 command with DataSourceV2Relation } else { convert to v1 command } } ``` This is definitely corrected, but we duplicate the code of converting to v2 commands. Since we are going to add more V2 commands (REFRESH TABLE, ANALYZE TABLE, etc.), this may slow down our development because we need to duplicate the code. This PR picks a simpler approach: ``` catalog = lookup catalog table = lookup table from the catalog if (table is not v1 table) { convert to v2 command with DataSourceV2Relation } else { convert to v1 command } ``` Note that `V1Table` is private, so this approach is effectively the same as your proposal. If you worry about that people may abuse `V1Table`, we can update the condition to convert to v2 commands: `if (catalog is not session catalog || table is not v1 table)` I think it's OK to do catalog and table resolution together. Do you have some concerns about what issues this can bring?
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
