You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yaooqinn (via GitHub)" <gi...@apache.org> on 2025/02/25 03:11:59 UTC
[PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
yaooqinn opened a new pull request, #50074:
URL: https://github.com/apache/spark/pull/50074
### What changes were proposed in this pull request?
This PR uses CatalogUtils.URIToString instead of URI.toString to decode the location URI.
### Why are the changes needed?
For example, for partition specs like test1=X'16', test3=timestamp'2018-11-17 13:33:33', the stored path will include them as `test1=%16/test3=2018-11-17 13%3A33%3A33` because the special characters are escaped. Furthermore, while resolving the whole path string to a URI object, this path fragment becomes `test1=%2516/test3=2018-11-17 13%253A33%253A33`, so we need to decode `%25` -> `%` before displaying to users
### Does this PR introduce _any_ user-facing change?
yes, DESC TABLE will not show 2x-encoded paths.
### How was this patch tested?
new tests
### Was this patch authored or co-authored using generative AI tooling?
no
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1973633764
##########
sql/core/src/test/resources/sql-tests/results/describe.sql.out:
##########
@@ -890,6 +890,48 @@ a string CONCAT('a\n b\n ', 'c\n d')
b int 42
+-- !query
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESC FORMATTED f PARTITION (B='SPARK', C=TIMESTAMP'2018-11-17 13:33:33')
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+A string
+B binary
+C timestamp
+# Partition Information
+# col_name data_type comment
+B binary
+C timestamp
+
+# Detailed Partition Information
+Database default
+Table f
+Partition Values [B=SPARK, C=2018-11-17 13:33:33]
+Location [not included in comparison]/{warehouse_dir}/f/B=SPARK/C=2018-11-17 13%3A33%3A33
+Partition Parameters [numFiles=1, totalSize=486, transient_lastDdlTime=[not included in comparison]]
Review Comment:
Maybe we can test with `... AS JSON` to avoid seeing these unrelated fields.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #50074:
URL: https://github.com/apache/spark/pull/50074#issuecomment-2683710833
cc @cloud-fan @dongjoon-hyun @HyukjinKwon, thank you
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #50074:
URL: https://github.com/apache/spark/pull/50074#issuecomment-2699607782
Thank you @dongjoon-hyun
I have cherrypicked this to 4.0 and will send a backport PR for 3.5.6
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1978578972
##########
sql/core/src/test/resources/sql-tests/inputs/describe.sql:
##########
@@ -122,6 +122,12 @@ DESC TABLE EXTENDED e;
DESC FORMATTED e;
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C;
Review Comment:
I think it's still better to make it explicit.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1974946328
##########
sql/core/src/test/resources/sql-tests/results/describe.sql.out:
##########
@@ -890,6 +890,48 @@ a string CONCAT('a\n b\n ', 'c\n d')
b int 42
+-- !query
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESC FORMATTED f PARTITION (B='SPARK', C=TIMESTAMP'2018-11-17 13:33:33')
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+A string
+B binary
+C timestamp
+# Partition Information
+# col_name data_type comment
+B binary
+C timestamp
+
+# Detailed Partition Information
+Database default
+Table f
+Partition Values [B=SPARK, C=2018-11-17 13:33:33]
+Location [not included in comparison]/{warehouse_dir}/f/B=SPARK/C=2018-11-17 13%3A33%3A33
+Partition Parameters [numFiles=1, totalSize=486, transient_lastDdlTime=[not included in comparison]]
Review Comment:
This PR also added the masking for transient_lastDdlTime, do we have any concerns?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1974620691
##########
sql/core/src/test/resources/sql-tests/inputs/describe.sql:
##########
@@ -122,6 +122,12 @@ DESC TABLE EXTENDED e;
DESC FORMATTED e;
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C;
Review Comment:
Not necessary, the default behavior w/o `USING` keyword defaults to parquet
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1974624507
##########
sql/core/src/test/resources/sql-tests/results/describe.sql.out:
##########
@@ -890,6 +890,48 @@ a string CONCAT('a\n b\n ', 'c\n d')
b int 42
+-- !query
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESC FORMATTED f PARTITION (B='SPARK', C=TIMESTAMP'2018-11-17 13:33:33')
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+A string
+B binary
+C timestamp
+# Partition Information
+# col_name data_type comment
+B binary
+C timestamp
+
+# Detailed Partition Information
+Database default
+Table f
+Partition Values [B=SPARK, C=2018-11-17 13:33:33]
+Location [not included in comparison]/{warehouse_dir}/f/B=SPARK/C=2018-11-17 13%3A33%3A33
+Partition Parameters [numFiles=1, totalSize=486, transient_lastDdlTime=[not included in comparison]]
Review Comment:
> we have transient_lastDdlTime because it's a hive table?
This is not from table parameters, but partition stats, I'm not aware of the history of filtering out some of the hive properties. However, it's not related to this PR.
> Maybe we can test with ... AS JSON to avoid seeing these unrelated fields.
We already have an `AS JSON` test variant, unfortunately transient_lastDdlTime is still there
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #50074:
URL: https://github.com/apache/spark/pull/50074#issuecomment-2698538770
Merged to master.
Since this is reported as a `Bug`, could you make backporting PRs, @yaooqinn ?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #50074: [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display
URL: https://github.com/apache/spark/pull/50074
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1973627334
##########
sql/core/src/test/resources/sql-tests/inputs/describe.sql:
##########
@@ -122,6 +122,12 @@ DESC TABLE EXTENDED e;
DESC FORMATTED e;
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C;
Review Comment:
does it create a hive table, shall we put `USING json` to explicitly use spark natvie data source?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1973628519
##########
sql/core/src/test/resources/sql-tests/results/describe.sql.out:
##########
@@ -890,6 +890,48 @@ a string CONCAT('a\n b\n ', 'c\n d')
b int 42
+-- !query
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESC FORMATTED f PARTITION (B='SPARK', C=TIMESTAMP'2018-11-17 13:33:33')
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+A string
+B binary
+C timestamp
+# Partition Information
+# col_name data_type comment
+B binary
+C timestamp
+
+# Detailed Partition Information
+Database default
+Table f
+Partition Values [B=SPARK, C=2018-11-17 13:33:33]
+Location [not included in comparison]/{warehouse_dir}/f/B=SPARK/C=2018-11-17 13%3A33%3A33
+Partition Parameters [numFiles=1, totalSize=486, transient_lastDdlTime=[not included in comparison]]
Review Comment:
we have `transient_lastDdlTime` because it's a hive table?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1974717022
##########
sql/core/src/test/resources/sql-tests/results/describe.sql.out:
##########
@@ -890,6 +890,48 @@ a string CONCAT('a\n b\n ', 'c\n d')
b int 42
+-- !query
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C
+-- !query schema
+struct<>
+-- !query output
+
+
+
+-- !query
+DESC FORMATTED f PARTITION (B='SPARK', C=TIMESTAMP'2018-11-17 13:33:33')
+-- !query schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query output
+A string
+B binary
+C timestamp
+# Partition Information
+# col_name data_type comment
+B binary
+C timestamp
+
+# Detailed Partition Information
+Database default
+Table f
+Partition Values [B=SPARK, C=2018-11-17 13:33:33]
+Location [not included in comparison]/{warehouse_dir}/f/B=SPARK/C=2018-11-17 13%3A33%3A33
+Partition Parameters [numFiles=1, totalSize=486, transient_lastDdlTime=[not included in comparison]]
Review Comment:
well, at least the masking logic is already implemented for AS JSON and we can save some changes from this PR :)
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51307][SQL] locationUri in CatalogStorageFormat shall be decoded for display [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50074:
URL: https://github.com/apache/spark/pull/50074#discussion_r1978652866
##########
sql/core/src/test/resources/sql-tests/inputs/describe.sql:
##########
@@ -122,6 +122,12 @@ DESC TABLE EXTENDED e;
DESC FORMATTED e;
+CREATE TABLE f PARTITIONED BY (B, C) AS SELECT 'APACHE' A, CAST('SPARK' AS BINARY) B, TIMESTAMP'2018-11-17 13:33:33' C;
Review Comment:
addressed
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org