[SPARK-49611][SQL] Introduce TVF `collations()` & remove the `SHOW COLLATIONS` command #48087

panbingkun · 2024-09-12T11:43:31Z

What changes were proposed in this pull request?

The pr aims to

introduce TVF collations().
remove the SHOW COLLATIONS command.

Why are the changes needed?

Based on @cloud-fan's suggestion: #47364 (comment)
I believe that after this, we can do many things based on it, such as filtering and querying based on LANGUAGE or COUNTRY, etc. eg:

SELECT * FROM collations() WHERE LANGUAGE like '%Chinese%';

Does this PR introduce any user-facing change?

Yes, provide a new TVF collations() for end-users.

How was this patch tested?

Add new UT.
Pass GA.

Was this patch authored or co-authored using generative AI tooling?

No.

panbingkun · 2024-09-12T11:48:38Z

cc @cloud-fan @MaxGekk @uros-db @mihailom-db

uros-db

perhaps we should get @srielau's opinion here (incl. link to original PR: #47364)

mihailom-db

LGTM, @cloud-fan could you review/merge

cloud-fan · 2024-09-13T00:59:20Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

@@ -1158,6 +1158,7 @@ object TableFunctionRegistry {
    generator[PosExplode]("posexplode"),
    generator[PosExplode]("posexplode_outer", outer = true),
    generator[Stack]("stack"),
+    generator[AllCollations]("all_collations"),


given we have sql_keywords, shall we call it string_collations?

nit: To me this sounds weird. We suggest that there might exist collations for some other type, for keywords it is more likely to have other keywords (python, scala ...) @cloud-fan

oh, not only string type? then all_collations is good

Okay, Let's restore to all_collations.

all_collation is fine although I'm not sure why all is necessary. But no strong feelings.

It's all_collation, not all_collations, right?

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala

cloud-fan · 2024-09-13T01:09:10Z

can we also remove the SHOW COLLATIONS command in this PR? #47364 (comment) I'd like to avoid confusion as Spark has a very special LIKE semantic in SHOW commands (different from Spark's own LIKE operator for string matching)

panbingkun · 2024-09-13T01:14:33Z

can we also remove the SHOW COLLATIONS command in this PR? #47364 (comment) I'd like to avoid confusion as Spark has a very special LIKE semantic in SHOW commands (different from Spark's own LIKE operator for string matching)

Okay, Let me handle it together.

…essions/generators.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>

…o SPARK-49611

mihailom-db · 2024-09-13T06:23:01Z

sql/core/src/test/scala/org/apache/spark/sql/CollationSuite.scala

      Row("SYSTEM", "BUILTIN", "UTF8_BINARY", null, null,
        "ACCENT_SENSITIVE", "CASE_SENSITIVE", "NO_PAD", null))

-    checkAnswer(sql("SHOW COLLATIONS '*zh_Hant_HKG*'"),
+    checkAnswer(sql("SELECT * FROM string_collations() WHERE COLLATION_NAME LIKE '%zh_Hant_HKG%'"),


Could you please do some test where we have for example WHERE country collate UTF8_LCASE like '%china%' so that we know we can actually use search on other fields, which is one of the main reasons we added this functionality over SHOW COLLATIONS.

mihailom-db

LGTM apart from small suggestions

mihailom-db

LGTM

MaxGekk · 2024-09-13T09:00:41Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala

+       SYSTEM  BUILTIN  UTF8_BINARY NULL  NULL  ACCENT_SENSITIVE  CASE_SENSITIVE  NO_PAD  NULL
+       SYSTEM  BUILTIN  UTF8_LCASE  NULL  NULL  ACCENT_SENSITIVE  CASE_INSENSITIVE  NO_PAD  NULL


Is this output deterministic?

Yes, UTF8_BINARY and UTF8_LCASE have always been at the forefront

It should be, listCollationMeta enforces UTF8_* collations first.

I would replace this LIMIT by WHERE which always returns one row, to don't depend on order and how LIMIT behaves.

uros-db · 2024-09-13T09:40:57Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala

@@ -18,6 +18,7 @@
 package org.apache.spark.sql.catalyst.expressions

 import scala.collection.mutable
+import scala.jdk.CollectionConverters.CollectionHasAsScala


why this import?

Because it is necessary to use from java(List) to scala(Iterable), as follows:

uros-db

lgtm

MaxGekk · 2024-09-13T10:04:56Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala

+        UTF8String.fromString(m.catalog),
+        UTF8String.fromString(m.schema),
+        UTF8String.fromString(m.collationName),
+        if (m.language != null) UTF8String.fromString(m.language) else null,


fromString() does null-checking, not need to check this additionally, see:

public static UTF8String fromString(String str) { return str == null ? null : fromBytes(str.getBytes(StandardCharsets.UTF_8)); }

cloud-fan · 2024-09-13T14:22:07Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala

+  override def elementSchema: StructType = new StructType()
+    .add("COLLATION_CATALOG", StringType, nullable = false)
+    .add("COLLATION_SCHEMA", StringType, nullable = false)
+    .add("COLLATION_NAME", StringType, nullable = false)


do we really need the COLLATION prefix? Can it just be CATALOG, SCHEMA and NAME?

I agree with it because sql_keywords doesn't seem to have a prefix either.

Also, I noticed an issue. Do we need to register it as a SQL built-in function? I noticed that some functions in FunctionRegistry are both TVF and SQL built-in function, eg:

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

Lines 370 to 371 in 9fc58aa

expression[Inline]("inline"),

expressionGeneratorOuter[Inline]("inline_outer"),

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

Lines 1155 to 1156 in 9fc58aa

generator[Inline]("inline"),

generator[Inline]("inline_outer", outer = true),

so we can use it as follows:

SELECT all_collations();

or

SELECT * FROM all_collations();

Using generator functions in SELECT is not recommended. We only keep them for backward compatibility.

Thank you, I see.

srielau · 2024-09-13T19:54:09Z

An we discuss the name? Are there any other collations? Is there precedence? Can we lean into what we did for keywords function? Sent from my iPhone On Sep 13, 2024, at 3:02 AM, Maxim Gekk ***@***.***> wrote: @MaxGekk commented on this pull request.

________________________________ In sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala<#48087 (comment)>:

+ SYSTEM BUILTIN UTF8_BINARY NULL NULL ACCENT_SENSITIVE CASE_SENSITIVE NO_PAD NULL

+ SYSTEM BUILTIN UTF8_LCASE NULL NULL ACCENT_SENSITIVE CASE_INSENSITIVE NO_PAD NULL I would replace this LIMIT by WHERE which always returns one row, to don't depend on order and how LIMIT behaves. — Reply to this email directly, view it on GitHub<#48087 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA22CFHCAGKZHSXT3QNJPHDZWKZ3FAVCNFSM6AAAAABODCERQ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMBSG4YDGNJTGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

cloud-fan · 2024-09-14T04:12:34Z

@srielau @mihailom-db What are your thoughts about the function name? string_collations is also fine to me since collation only makes sense for string.

mihailom-db · 2024-09-14T05:01:32Z

I do not have strong preferences, but to me maybe something like collation(s)_info seems like the most appropriate. As this TVF is not generation only collation names but actually providing some more information on what the given collations are. @cloud-fan @srielau

panbingkun · 2024-09-14T06:02:42Z

Or call it collations() ?

uros-db

+1 for collations() - seems most appropriate to me, no unnecessary complications

MaxGekk · 2024-09-14T08:26:54Z

@panbingkun Which function name do other systems use?

panbingkun · 2024-09-14T11:01:39Z

@panbingkun Which function name do other systems use?

It seems to have no reference value, most of them are SQL command, only MS SQL Serve seems to be a TVF.

Database	Support	Name	Link
mysql	SHOW COLLATION	COLLATION	https://dev.mysql.com/doc/refman/8.4/en/show-collation.html
MariaDB	SHOW COLLATION LIKE 'latin2%';	COLLATION	https://mariadb.com/kb/en/show-collation/
Oracle	select value from v$nls_valid_values where parameter = 'SORT';		https://stackoverflow.com/questions/74795796/how-to-get-all-supported-collate-in-oracle-database https://docs.oracle.com/en/database/oracle/oracle-database/19/nlspg/appendix-A-locale-data.html#GUID-D2FCFD55-EDC3-473F-9832-AAB564457830 https://docs.oracle.com/en/database/oracle/oracle-database/19/refrn/V-NLS_VALID_VALUES.html
PingCap/TiDB	SHOW COLLATION	COLLATION	https://docs.pingcap.com/tidb/stable/sql-statement-show-collation
Firebird	SHOW COLLATIONS	COLLATIONS	https://www.firebirdsql.org/file/documentation/html/en/firebirddocs/isql/firebird-isql.html#isql-show-collations
MS SQL Server	SELECT name, description FROM sys.fn_helpcollations();	...collations()	https://learn.microsoft.com/en-us/sql/relational-databases/collations/view-collation-information?view=sql-server-ver16
PostgreSQL	SELECT * FROM pg_collation;		https://www.postgresql.org/docs/current/catalog-pg-collation.html

srielau · 2024-09-14T13:24:38Z

Oracle has SHOW COLLATION?? Do you have link? Also we should have a wider spread… 5 flavors of MySQL re still MySQL. Collations() works for me. Happens to line up with the ANSI information schema

…

Sent from my iPhone On Sep 14, 2024, at 4:02 AM, panbingkun ***@***.***> wrote: @panbingkun<https://github.com/panbingkun> Which function name do other systems use? Database Support Name mysql SHOW COLLATION COLLATION MariaDB SHOW COLLATION LIKE 'latin2%'; COLLATION oracle SHOW COLLATION COLLATION TiDB SHOW COLLATION COLLATION PingCap SHOW COLLATION COLLATION Firebird SHOW COLLATIONS COLLATIONS MS SQL Server SELECT name, description FROM sys.fn_helpcollations(); ...collations() — Reply to this email directly, view it on GitHub<#48087 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA22CFGODHWK2YYFZUVKQ6LZWQJSVAVCNFSM6AAAAABODCERQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJQHE2TENBTGA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

panbingkun · 2024-09-14T14:14:43Z

Oracle has SHOW COLLATION?? Do you have link?

It was my bad, I misread Oracle.
Additionally, I have updated links and screenshots for all databases.

cloud-fan · 2024-09-15T05:16:25Z

let's go with collations() then.

panbingkun · 2024-09-15T12:15:35Z

let's go with collations() then.

Updated, thanks!

mihailom-db

LGTM, collations() seems like the most appropriate choice

cloud-fan · 2024-09-16T08:33:19Z

thanks, merging to master!

panbingkun · 2024-09-16T12:41:24Z

Thanks all, @mihailom-db @uros-db @MaxGekk @cloud-fan @srielau

…LLATIONS` command ### What changes were proposed in this pull request? The pr aims to - introduce `TVF` `collations()`. - remove the `SHOW COLLATIONS` command. ### Why are the changes needed? Based on cloud-fan's suggestion: apache#47364 (comment) I believe that after this, we can do many things based on it, such as `filtering` and `querying` based on `LANGUAGE` or `COUNTRY`, etc. eg: ```sql SELECT * FROM collations() WHERE LANGUAGE like '%Chinese%'; ``` ### Does this PR introduce _any_ user-facing change? Yes, provide a new TVF `collations()` for end-users. ### How was this patch tested? - Add new UT. - Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48087 from panbingkun/SPARK-49611. Lead-authored-by: panbingkun <panbingkun@baidu.com> Co-authored-by: panbingkun <pbk1982@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

[SPARK-49611][SQL] Introduce TVF all_collations()

48c614e

github-actions bot added the SQL label Sep 12, 2024

panbingkun mentioned this pull request Sep 12, 2024

[SPARK-48906][SQL] Introduce SHOW COLLATIONS LIKE ... syntax to show all collations #47364

Closed

panbingkun marked this pull request as ready for review September 12, 2024 11:48

uros-db reviewed Sep 12, 2024

View reviewed changes

fix tab

089c860

mihailom-db approved these changes Sep 12, 2024

View reviewed changes

cloud-fan reviewed Sep 13, 2024

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Sep 13, 2024

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/generators.scala Show resolved Hide resolved

panbingkun and others added 4 commits September 13, 2024 09:27

Update sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expr…

98d41a2

…essions/generators.scala Co-authored-by: Wenchen Fan <cloud0fan@gmail.com>

Merge branch 'master' into SPARK-49611

5ba5cfe

Merge branch 'SPARK-49611' of https://github.com/panbingkun/spark int…

317f105

…o SPARK-49611

remove show collations command

5009a4f

github-actions bot added the DOCS label Sep 13, 2024

panbingkun changed the title ~~[SPARK-49611][SQL] Introduce TVF all_collations()~~ [SPARK-49611][SQL] Introduce TVF string_collations() and remove the SHOW COLLATIONS command Sep 13, 2024

panbingkun changed the title ~~[SPARK-49611][SQL] Introduce TVF string_collations() and remove the SHOW COLLATIONS command~~ [SPARK-49611][SQL] Introduce TVF string_collations() and remove the SHOW COLLATIONS command Sep 13, 2024

mihailom-db reviewed Sep 13, 2024

View reviewed changes

mihailom-db approved these changes Sep 13, 2024

View reviewed changes

panbingkun added 2 commits September 13, 2024 14:38

Merge branch 'master' into SPARK-49611

52d7709

add some UT

cbe8e08

mihailom-db approved these changes Sep 13, 2024

View reviewed changes

panbingkun changed the title ~~[SPARK-49611][SQL] Introduce TVF string_collations() and remove the SHOW COLLATIONS command~~ [SPARK-49611][SQL] Introduce TVF all_collations() and remove the SHOW COLLATIONS command Sep 13, 2024

MaxGekk reviewed Sep 13, 2024

View reviewed changes

uros-db reviewed Sep 13, 2024

View reviewed changes

uros-db approved these changes Sep 13, 2024

View reviewed changes

MaxGekk reviewed Sep 13, 2024

View reviewed changes

panbingkun added 2 commits September 13, 2024 18:45

Merge branch 'master' into SPARK-49611

1141c15

improve

fbb1d84

cloud-fan reviewed Sep 13, 2024

View reviewed changes

panbingkun added 2 commits September 13, 2024 23:01

update

abd58c0

update

72ff1b9

panbingkun changed the title ~~[SPARK-49611][SQL] Introduce TVF all_collations() and remove the SHOW COLLATIONS command~~ [SPARK-49611][SQL] Introduce TVF all_collations() & remove the SHOW COLLATIONS command Sep 13, 2024

uros-db reviewed Sep 14, 2024

View reviewed changes

rename to collations

ec6c084

mihailom-db approved these changes Sep 15, 2024

View reviewed changes

panbingkun changed the title ~~[SPARK-49611][SQL] Introduce TVF all_collations() & remove the SHOW COLLATIONS command~~ [SPARK-49611][SQL] Introduce TVF collations() & remove the SHOW COLLATIONS command Sep 15, 2024

cloud-fan approved these changes Sep 16, 2024

View reviewed changes

cloud-fan closed this in 2113f10 Sep 16, 2024

		SYSTEM BUILTIN UTF8_BINARY NULL NULL ACCENT_SENSITIVE CASE_SENSITIVE NO_PAD NULL
		SYSTEM BUILTIN UTF8_LCASE NULL NULL ACCENT_SENSITIVE CASE_INSENSITIVE NO_PAD NULL

	expression[Inline]("inline"),
	expressionGeneratorOuter[Inline]("inline_outer"),

	generator[Inline]("inline"),
	generator[Inline]("inline_outer", outer = true),

[SPARK-49611][SQL] Introduce TVF collations() & remove the SHOW COLLATIONS command #48087

[SPARK-49611][SQL] Introduce TVF collations() & remove the SHOW COLLATIONS command #48087

Conversation

panbingkun commented Sep 12, 2024 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

panbingkun commented Sep 12, 2024

uros-db left a comment

Choose a reason for hiding this comment

mihailom-db left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Sep 13, 2024

panbingkun commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihailom-db left a comment

Choose a reason for hiding this comment

mihailom-db left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uros-db left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

panbingkun Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srielau commented Sep 13, 2024 via email

cloud-fan commented Sep 14, 2024

mihailom-db commented Sep 14, 2024

panbingkun commented Sep 14, 2024

uros-db left a comment

Choose a reason for hiding this comment

MaxGekk commented Sep 14, 2024

panbingkun commented Sep 14, 2024 • edited Loading

srielau commented Sep 14, 2024 via email

panbingkun commented Sep 14, 2024 • edited Loading

cloud-fan commented Sep 15, 2024

panbingkun commented Sep 15, 2024

mihailom-db left a comment

Choose a reason for hiding this comment

cloud-fan commented Sep 16, 2024

panbingkun commented Sep 16, 2024

[SPARK-49611][SQL] Introduce TVF `collations()` & remove the `SHOW COLLATIONS` command #48087

[SPARK-49611][SQL] Introduce TVF `collations()` & remove the `SHOW COLLATIONS` command #48087

panbingkun commented Sep 12, 2024 •

edited

Loading

panbingkun Sep 13, 2024 •

edited

Loading

panbingkun commented Sep 14, 2024 •

edited

Loading

panbingkun commented Sep 14, 2024 •

edited

Loading