mccheah commented on a change in pull request #24410: [SPARK-24252][SQL] Move Table and TableCapabilities to catalyst module URL: https://github.com/apache/spark/pull/24410#discussion_r276875317
########## File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/Table.java ########## @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.spark.sql.sources.v2; +package org.apache.spark.sql.catalog.v2; Review comment: While I agree we want to get this in sooner, I think it's cumbersome to have two package move PRs if we don't need to. I.e. we should do as few move-only PRs as possible. What I'd like to avoid is a case in the future where we decide that we should move packages again before we merge another feature. I.e. we end up repeating the situation we're in now, again in some future feature. So I think it's at least worth considering if there's any reason to think we want the package hierarchy to be different from what we have proposed here. But I wouldn't belabor the point too long - we can always still change our mind later if necessary. That being said I don't feel strongly either way. @rxin and @cloud-fan, what's the specific rationale for having a separate `table` package? ---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
