Skip to content

[SPARK-48441][SQL] Fix StringTrim behaviour for non-UTF8_BINARY collations #46762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
uros-db wants to merge 21 commits into from
Closed

[SPARK-48441][SQL] Fix StringTrim behaviour for non-UTF8_BINARY collations #46762

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0209fc4
Initial commit
uros-db May 27, 2024
1f6881d
Small fixes
uros-db May 28, 2024
204d26e
Merge branch 'apache:master' into alter-trim
uros-db May 28, 2024
5039ddf
Fix trim logic
uros-db May 30, 2024
361c7b1
Lint fixes
uros-db May 30, 2024
aaa46ff
Merge branch 'master' into alter-trim
uros-db Jun 10, 2024
1af57bf
Update CollationSupportSuite.java
uros-db Jun 10, 2024
544548c
Merge branch 'master' into alter-trim
uros-db Jul 4, 2024
e559e83
Merge branch 'apache:master' into alter-trim
uros-db Jul 5, 2024
292acf8
Fix test
uros-db Jul 7, 2024
097a846
Refactor trim
uros-db Jul 8, 2024
176b148
Add tests
uros-db Jul 8, 2024
9192b55
Merge branch 'apache:master' into alter-trim
uros-db Jul 8, 2024
598f522
Remove unused code
uros-db Jul 8, 2024
12cc084
Fix java lint
uros-db Jul 8, 2024
469c325
Fix
uros-db Jul 9, 2024
c2133d7
Merge branch 'apache:master' into alter-trim
uros-db Jul 9, 2024
06575fc
Fix comments
uros-db Jul 12, 2024
4cf90d7
Merge branch 'master' into alter-trim
uros-db Jul 12, 2024
8c5787d
Fixes
uros-db Jul 14, 2024
d15d92a
Fix tests
uros-db Jul 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString);
} else {
return execLowercase(srcString); // TODO: ICU implementation
}
}
public static UTF8String exec(
Expand All @@ -545,8 +547,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString, trimString);
} else {
return execLowercase(srcString, trimString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -556,8 +560,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrim.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s)", srcString);
} {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s)", srcString);
} else {
return String.format(expr + "Lowercase(%s)", srcString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -568,8 +574,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrim.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s, %s)", srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString);
} else {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString); // TODO: ICU implementation
}
}
public static UTF8String execBinary(
Expand Down Expand Up @@ -599,8 +607,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString);
} else {
return execLowercase(srcString); // TODO: ICU implementation
}
}
public static UTF8String exec(
Expand All @@ -610,8 +620,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString, trimString);
} else {
return execLowercase(srcString, trimString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -621,8 +633,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrimLeft.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s)", srcString);
} else {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s)", srcString);
} else {
return String.format(expr + "Lowercase(%s)", srcString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -633,8 +647,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrimLeft.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s, %s)", srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString);
} else {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString); // TODO: ICU implementation
}
}
public static UTF8String execBinary(
Expand Down Expand Up @@ -664,8 +680,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString);
} else {
return execLowercase(srcString); // TODO: ICU implementation
}
}
public static UTF8String exec(
Expand All @@ -675,8 +693,10 @@ public static UTF8String exec(
CollationFactory.Collation collation = CollationFactory.fetchCollation(collationId);
if (collation.supportsBinaryEquality) {
return execBinary(srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return execLowercase(srcString, trimString);
} else {
return execLowercase(srcString, trimString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -686,8 +706,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrimRight.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s)", srcString);
} else {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s)", srcString);
} else {
return String.format(expr + "Lowercase(%s)", srcString); // TODO: ICU implementation
}
}
public static String genCode(
Expand All @@ -698,8 +720,10 @@ public static String genCode(
String expr = "CollationSupport.StringTrimRight.exec";
if (collation.supportsBinaryEquality) {
return String.format(expr + "Binary(%s, %s)", srcString, trimString);
} else {
} else if (collation.supportsLowercaseEquality) {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString);
} else {
return String.format(expr + "Lowercase(%s, %s)", srcString, trimString); // TODO: ICU implementation
}
}
public static UTF8String execBinary(
Expand Down
4 changes: 2 additions & 2 deletions ...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.apache.spark.sql.catalyst.trees.TreePattern.{TreePattern, UPPER_OR_LO
import org.apache.spark.sql.catalyst.util.{ArrayData, CollationFactory, CollationSupport, GenericArrayData, TypeUtils}
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.internal.types.{AbstractArrayType, StringTypeAnyCollation, StringTypeBinaryLcase}
import org.apache.spark.sql.internal.types.{AbstractArrayType, StringTypeAnyCollation}
import org.apache.spark.sql.types._
import org.apache.spark.unsafe.UTF8StringBuilder
import org.apache.spark.unsafe.array.ByteArrayMethods
Expand Down Expand Up @@ -1021,7 +1021,7 @@ trait String2TrimExpression extends Expression with ImplicitCastInputTypes {

override def children: Seq[Expression] = srcStr +: trimStr.toSeq
override def dataType: DataType = srcStr.dataType
override def inputTypes: Seq[AbstractDataType] = Seq.fill(children.size)(StringTypeBinaryLcase)
override def inputTypes: Seq[AbstractDataType] = Seq.fill(children.size)(StringTypeAnyCollation)

final lazy val collationId: Int = srcStr.dataType.asInstanceOf[StringType].collationId

Expand Down