You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/01/17 10:18:16 UTC
[GitHub] [spark] Peng-Lei opened a new pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Peng-Lei opened a new pull request #35227:
URL: https://github.com/apache/spark/pull/35227
### What changes were proposed in this pull request?
Quote the column name just needed instead of anyway.
### Why are the changes needed?
[#comments](https://github.com/apache/spark/pull/35204#discussion_r785725545)
### Does this PR introduce _any_ user-facing change?
Yes， when end-user get the table metadata， if the columns does not need quote. it will be a normal string.
### How was this patch tested?
existed testcase.
--
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
[GitHub] [spark] cloud-fan closed pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #35227:
URL: https://github.com/apache/spark/pull/35227
--
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
[GitHub] [spark] gatorsmile commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if needed
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1032804383
This is a user-facing breaking change. The third party libraries (esp. SQL clients) might rely on the current format. We should avoid making such a change.
--
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
[GitHub] [spark] cloud-fan edited a comment on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1015375493
> Yes， when end-user get the table metadata， if the columns does not need quote. it will be a normal string.
Let's be specific here. This only affects schema DDL string AFAIK, while "get the table metadata" is low-level operation and it's scarying to change 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-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
[GitHub] [spark] Peng-Lei commented on a change in pull request #35227: [SPARK-37931][SQL] Quote the column name if needed
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on a change in pull request #35227:
URL: https://github.com/apache/spark/pull/35227#discussion_r794154056
##########
File path: sql/core/src/test/resources/sql-tests/results/charvarchar.sql.out
##########
@@ -162,8 +162,8 @@ show create table char_tbl3
struct<createtab_stmt:string>
-- !query output
CREATE TABLE default.char_tbl3 (
- `c` CHAR(5),
- `v` VARCHAR(6))
+ c CHAR(5),
+ v VARCHAR(6))
Review comment:
Sorry, I left this out. I will do it by a follow up.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1017330147
Yeah, I think this is fine.
--
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
[GitHub] [spark] cloud-fan commented on a change in pull request #35227: [SPARK-37931][SQL] Quote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #35227:
URL: https://github.com/apache/spark/pull/35227#discussion_r793683152
##########
File path: sql/core/src/test/resources/sql-tests/results/charvarchar.sql.out
##########
@@ -162,8 +162,8 @@ show create table char_tbl3
struct<createtab_stmt:string>
-- !query output
CREATE TABLE default.char_tbl3 (
- `c` CHAR(5),
- `v` VARCHAR(6))
+ c CHAR(5),
+ v VARCHAR(6))
Review comment:
I noticed that the view test in this file still has quotes. `ShowCreateTableCommand` has a branch for view, and we need to update the quoting logic in that branch as well. @Peng-Lei
--
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
[GitHub] [spark] cloud-fan commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1033293391
I think we should clearly document the display-like commands. Their output is not structured and we only expect humans to read it, not programs.
--
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
[GitHub] [spark] cloud-fan commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1015377198
looks fine and I can't think of any breaking cases. cc @HyukjinKwon @viirya as well
--
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
[GitHub] [spark] Peng-Lei commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1015254586
@cloud-fan Could you take a look ? Thank you very much.
--
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
[GitHub] [spark] Peng-Lei commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
Peng-Lei commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1015584161
> Let's be specific here. This only affects schema DDL string AFAIK, while "get the table metadata" is low-level operation and it's scarying to change it.
I found that the `toDDL` first introduce by [#21803](https://github.com/apache/spark/pull/21803). and also for `show create table` at the beginning.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if needed
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1033205101
Hm, I think it's less likely that this breaks something because it removes the backquotes when they are unnecessary, meaning that it's still able to read and parse it back with lower versions of Spark.
--
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
[GitHub] [spark] cloud-fan commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1015375493
> Yes， when end-user get the table metadata， if the columns does not need quote. it will be a normal string.
Let's be specific here. This only affects `SHOW CREATE TABLE` AFAIK, while "get the table metadata" is low-level operation and it's scarying to change 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-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
[GitHub] [spark] cloud-fan commented on pull request #35227: [SPARK-37931][SQL] Quote the column name if neededQuote the column name if needed
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35227:
URL: https://github.com/apache/spark/pull/35227#issuecomment-1017226396
thanks, merging to master!
--
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