You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "learningchess2003 (via GitHub)" <gi...@apache.org> on 2023/05/25 23:05:53 UTC
[GitHub] [spark] learningchess2003 opened a new pull request, #41319: [SC-131010] Add to_varchar alias for to_char
learningchess2003 opened a new pull request, #41319:
URL: https://github.com/apache/spark/pull/41319
### What changes were proposed in this pull request?
Add the ```to_varchar``` alias for ```to_char``` to support built-in Snowflake SQL functions.
### Why are the changes needed?
To minimize code changes user needs for adapting formerly a Snowflake SQL application to a Databricks one.
### Does this PR introduce _any_ user-facing change?
Yes. The user can now invoke ```to_varchar``` instead of ```to_char``` in their SQL applications.
### How was this patch tested?
```to_varchar``` was added in ```numeric.sql``` and a new Golden file was generated to check that the query output was correct.
--
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] dongjoon-hyun commented on a diff in pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1208061394
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,10 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
Review Comment:
These three lines look repeating. Could you simply the above three lines into a single liner? Maybe, like the following?
```
-- to_varchar is an alias for to_char
```
--
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] learningchess2003 commented on pull request #41319: [WIP][SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1564761047
Jenkins Trigger: CodeSync-RegenGoldenFiles { "test_suite": "SQL", "test_case": "numeric.sql"}
--
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] learningchess2003 commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207345799
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,26 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+create table num_data (id int, val decimal(38,10)) using parquet;
+
+insert into num_data values (0, 0);
+insert into num_data values (1, 0);
+insert into num_data values (2, -34338492.215397047);
+insert into num_data values (3, 4.31);
+insert into num_data values (4, 7799461.4119);
+insert into num_data values (5, 16397.038491);
+insert into num_data values (6, 93901.57763026);
+insert into num_data values (7, -83028485);
+insert into num_data values (8, 74881);
+insert into num_data values (9, -24926804.045047420);
+
+select * from num_data;
+
+select '' as to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
+from num_data;
+
+drop table num_data;
Review Comment:
Should I just stick with the original then and make it shorter?
--
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] learningchess2003 commented on pull request #41319: [SC-131010][WIP] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1563616664
@dtenedor Once I am finished implementing the other functions, you can review. Of course, if there is something which you noticed I should fix earlier on, let me know!
--
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] MaxGekk commented on pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1568900587
+1, LGTM. Merging to master.
Thank you, @learningchess2003 and @dongjoon-hyun @dtenedor for review.
--
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] learningchess2003 commented on pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1564870274
@dtenedor @MaxGekk Addressed the comments. Let me know what you think!
--
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] MaxGekk commented on pull request #41319: [WIP] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1564619248
@learningchess2003 Allow GitHub action in your fork, please: https://github.com/apache/spark/pull/41319/checks?check_run_id=13791306580
--
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] MaxGekk commented on a diff in pull request #41319: [WIP] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1206393787
##########
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/numeric.sql:
##########
@@ -884,6 +884,12 @@ SELECT '' AS to_char_10, to_char(val, 'S0999999999999999.999999999999999'), val
-- SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999');
-- SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999');
+-- [SC-131010] Add support for Snowflake built-in functions
Review Comment:
Please, don't use some internal ids like SC-131010
--
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] MaxGekk commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207298536
##########
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/numeric.sql:
##########
@@ -884,6 +884,12 @@ SELECT '' AS to_char_10, to_char(val, 'S0999999999999999.999999999999999'), val
-- SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999');
-- SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999');
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+SELECT '' AS to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
Review Comment:
Could you move this test out from the `postgreSQL` folder, for instance to `sql-tests/inputs/charvarchar.sql`.
The tests like this were borrowed from PostgreSQL when we tried to achieve feature parity to the DBMS:
https://issues.apache.org/jira/browse/SPARK-27763
It is better to don't add new checks 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
[GitHub] [spark] learningchess2003 commented on a diff in pull request #41319: [WIP] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1206958763
##########
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/numeric.sql:
##########
@@ -884,6 +884,12 @@ SELECT '' AS to_char_10, to_char(val, 'S0999999999999999.999999999999999'), val
-- SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999');
-- SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999');
+-- [SC-131010] Add support for Snowflake built-in functions
Review Comment:
Alright, will fix that.
--
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] dtenedor commented on a diff in pull request #41319: [WIP] Add to_varchar alias for to_char
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207085742
##########
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/numeric.sql:
##########
@@ -884,6 +884,12 @@ SELECT '' AS to_char_10, to_char(val, 'S0999999999999999.999999999999999'), val
-- SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999');
-- SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999');
+-- [SPARK-XXX] Add support for additional SQL functions
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+SELECT '' AS to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
Review Comment:
I think you will also need to get analyzer golden file results 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] MaxGekk closed pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
URL: https://github.com/apache/spark/pull/41319
--
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] learningchess2003 commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207339425
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,26 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+create table num_data (id int, val decimal(38,10)) using parquet;
+
+insert into num_data values (0, 0);
+insert into num_data values (1, 0);
+insert into num_data values (2, -34338492.215397047);
+insert into num_data values (3, 4.31);
+insert into num_data values (4, 7799461.4119);
+insert into num_data values (5, 16397.038491);
+insert into num_data values (6, 93901.57763026);
+insert into num_data values (7, -83028485);
+insert into num_data values (8, 74881);
+insert into num_data values (9, -24926804.045047420);
+
+select * from num_data;
+
+select '' as to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
+from num_data;
+
+drop table num_data;
Review Comment:
@MaxGekk I don't think to_char/to_varchar accepts a timestamp literal yet. At the very least, when running this command, I got the following error:
```
-- !query
select to_varchar(date'26-May-2023', 'yyyy.mm.dd')
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException
{
"errorClass" : "INVALID_TYPED_LITERAL",
"sqlState" : "42604",
"messageParameters" : {
"value" : "'26-May-2023'",
"valueType" : "\"DATE\""
},
"queryContext" : [ {
"objectType" : "",
"objectName" : "",
"startIndex" : 19,
"stopIndex" : 35,
"fragment" : "date'26-May-2023'"
} ]
}
```
--
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] learningchess2003 commented on a diff in pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1210511764
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,10 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
Review Comment:
Alright, will do so.
--
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] learningchess2003 commented on pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1568725048
@MaxGekk @dongjoon-hyun Changed accordingly!
--
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] dongjoon-hyun commented on pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1568726440
Thank you for updating, @learningchess2003 .
--
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] MaxGekk commented on a diff in pull request #41319: [SPARK-43815][WIP] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207164442
##########
sql/core/src/test/resources/sql-tests/inputs/postgreSQL/numeric.sql:
##########
@@ -884,6 +884,12 @@ SELECT '' AS to_char_10, to_char(val, 'S0999999999999999.999999999999999'), val
-- SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999');
-- SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999');
+-- [SPARK-43815] Add support for additional SQL functions
Review Comment:
Please, change the ticket title. Now it is too much generic. The PR's title looks nice for that.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -531,6 +531,7 @@ object FunctionRegistry {
expression[ToNumber]("to_number"),
expression[TryToNumber]("try_to_number"),
expression[ToCharacter]("to_char"),
+ expression[ToCharacter]("to_varchar", setAlias = true, Some("3.4.0")),
Review Comment:
Change the version to 3.5.0. We are not going to backport this alias back to already released 3.4.0.
--
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] MaxGekk commented on pull request #41319: [WIP] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1563982657
@learningchess2003 Do you have an account at JIRA? If not, please, submit a request:
<img width="1134" alt="Screenshot 2023-05-26 at 11 06 12" src="https://github.com/apache/spark/assets/1580697/fec86460-68bb-49fb-a324-e78254f9e32a">
--
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] learningchess2003 commented on pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1564932498
@MaxGekk Comments 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
[GitHub] [spark] MaxGekk commented on pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1568913633
@learningchess2003 Congratulations with your first contribution to Apache 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] dongjoon-hyun commented on a diff in pull request #41319: [SPARK-43815][SQL] Add `to_varchar` alias for `to_char`
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1208061394
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,10 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
Review Comment:
Thank you for making a PR, @learningchess2003 .
These three lines look repeating. Could you simply the above three lines into a single liner? Maybe, like the following?
```
-- to_varchar is an alias for to_char
```
--
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] MaxGekk commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207334093
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,26 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+create table num_data (id int, val decimal(38,10)) using parquet;
+
+insert into num_data values (0, 0);
+insert into num_data values (1, 0);
+insert into num_data values (2, -34338492.215397047);
+insert into num_data values (3, 4.31);
+insert into num_data values (4, 7799461.4119);
+insert into num_data values (5, 16397.038491);
+insert into num_data values (6, 93901.57763026);
+insert into num_data values (7, -83028485);
+insert into num_data values (8, 74881);
+insert into num_data values (9, -24926804.045047420);
+
+select * from num_data;
+
+select '' as to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
+from num_data;
+
+drop table num_data;
Review Comment:
Let's replace this by simpler check:
```sql
select to_varchar(123, '9999999999999999.999999999999999PR');
```
This should be enough to check that the alias exists.
--
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] MaxGekk commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207334093
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,26 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+create table num_data (id int, val decimal(38,10)) using parquet;
+
+insert into num_data values (0, 0);
+insert into num_data values (1, 0);
+insert into num_data values (2, -34338492.215397047);
+insert into num_data values (3, 4.31);
+insert into num_data values (4, 7799461.4119);
+insert into num_data values (5, 16397.038491);
+insert into num_data values (6, 93901.57763026);
+insert into num_data values (7, -83028485);
+insert into num_data values (8, 74881);
+insert into num_data values (9, -24926804.045047420);
+
+select * from num_data;
+
+select '' as to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
+from num_data;
+
+drop table num_data;
Review Comment:
Let's replace this by simpler check:
```sql
select to_varchar(date'26-May-2023', 'yyyy.mm.dd');
```
--
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] MaxGekk commented on a diff in pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41319:
URL: https://github.com/apache/spark/pull/41319#discussion_r1207341701
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql:
##########
@@ -117,3 +117,26 @@ drop table char_tbl4;
-- ascii value for Latin-1 Supplement characters
select ascii('§'), ascii('÷'), ascii('×10');
select chr(167), chr(247), chr(215);
+
+-- [SPARK-43815] Add to_varchar alias for to_char
+-- TO_VARCHAR()
+-- Alias of TO_CHAR()
+create table num_data (id int, val decimal(38,10)) using parquet;
+
+insert into num_data values (0, 0);
+insert into num_data values (1, 0);
+insert into num_data values (2, -34338492.215397047);
+insert into num_data values (3, 4.31);
+insert into num_data values (4, 7799461.4119);
+insert into num_data values (5, 16397.038491);
+insert into num_data values (6, 93901.57763026);
+insert into num_data values (7, -83028485);
+insert into num_data values (8, 74881);
+insert into num_data values (9, -24926804.045047420);
+
+select * from num_data;
+
+select '' as to_varchar_3, to_varchar(val, '9999999999999999.999999999999999PR'), val
+from num_data;
+
+drop table num_data;
Review Comment:
Yep, let's test what it supports.
--
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] learningchess2003 commented on pull request #41319: [SPARK-43815] Add to_varchar alias for to_char
Posted by "learningchess2003 (via GitHub)" <gi...@apache.org>.
learningchess2003 commented on PR #41319:
URL: https://github.com/apache/spark/pull/41319#issuecomment-1564955114
@MaxGekk Alright, shortened it and just had it call some standalone examples.
--
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