[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759937743 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala ## @@ -186,7 +185,10 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat RefreshTableExec(r.catalog, r.identifier, recacheTable(r)) :: Nil case ReplaceTable(catalog, ident, schema, parts, props, orCreate) => - val propsWithOwner = CatalogV2Util.withDefaultOwnership(props) + val newProps = props.get(TableCatalog.PROP_LOCATION).map { loc => +props + (TableCatalog.PROP_LOCATION -> makeQualifiedDBObjectPath(loc)) + }.getOrElse(props) Review comment: As [#PR](https://github.com/apache/spark/pull/34764) not merged -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759926219 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala ## @@ -271,6 +271,14 @@ object CatalogUtils { } } + def makeQualifiedLocationPath( Review comment: ok. -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759926219 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala ## @@ -271,6 +271,14 @@ object CatalogUtils { } } + def makeQualifiedLocationPath( Review comment: en. -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759922889 ## File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala ## @@ -1180,7 +1182,7 @@ class DataSourceV2SQLSuite val tableCatalog = catalog("testcat").asTableCatalog val identifier = Identifier.of(Array(), "reservedTest") assert(tableCatalog.loadTable(identifier).properties() - .get(TableCatalog.PROP_LOCATION) == "foo", + .get(TableCatalog.PROP_LOCATION) == makeQualifiedLocationPath("foo"), Review comment: OK. -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759885112 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/CreateTableExec.scala ## @@ -42,7 +44,12 @@ case class CreateTableExec( tableSpec.properties, tableSpec.options, tableSpec.serde, tableSpec.location, tableSpec.comment, tableSpec.provider, tableSpec.external) -CatalogV2Util.withDefaultOwnership(props) +val newProps = props.get(TableCatalog.PROP_LOCATION).map { loc => + props + (TableCatalog.PROP_LOCATION -> Review comment: en. I do it. -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759884718 ## File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala ## @@ -1180,7 +1182,7 @@ class DataSourceV2SQLSuite val tableCatalog = catalog("testcat").asTableCatalog val identifier = Identifier.of(Array(), "reservedTest") assert(tableCatalog.loadTable(identifier).properties() - .get(TableCatalog.PROP_LOCATION) == "foo", + .get(TableCatalog.PROP_LOCATION) == makeQualifiedLocationPath("foo"), Review comment: "file:" + WAREHOUSE_PATH + "/foo". eg: `file:/D:/code/bigdata/spark/spark-warehouse/org.apache.spark.sql.connector.DataSourceV2SQLSuite/foo` -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759884718 ## File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala ## @@ -1180,7 +1182,7 @@ class DataSourceV2SQLSuite val tableCatalog = catalog("testcat").asTableCatalog val identifier = Identifier.of(Array(), "reservedTest") assert(tableCatalog.loadTable(identifier).properties() - .get(TableCatalog.PROP_LOCATION) == "foo", + .get(TableCatalog.PROP_LOCATION) == makeQualifiedLocationPath("foo"), Review comment: "file:" + WAREHOUSE_PATH + "/foo". eg: `file:/D:/code/bigdata/spark/spark-warehouse/org.apache.spark.sql.connector.DataSourceV2SQLSuite/foo` -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] Peng-Lei commented on a change in pull request #34758: [SPARK-37501][SQL] CREATE/REPLACE TABLE should qualify location for v2 command
Peng-Lei commented on a change in pull request #34758: URL: https://github.com/apache/spark/pull/34758#discussion_r759830709 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala ## @@ -95,10 +95,13 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat } private def makeQualifiedNamespacePath(location: String): String = { -val warehousePath = session.sharedState.conf.get(WAREHOUSE_PATH) -val nsPath = CatalogUtils.makeQualifiedNamespacePath( - CatalogUtils.stringToURI(location), warehousePath, session.sharedState.hadoopConf) -CatalogUtils.URIToString(nsPath) + CatalogUtils.makeQualifiedLocationPath(session.sharedState.conf.get(WAREHOUSE_PATH), Review comment: Thank you for your advice. It would be better if merge them to one. How about `makeQualifiedDBObjectPath`? -- 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. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org