[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions #37113

yikf · 2022-07-07T06:57:33Z

What changes were proposed in this pull request?

Currently, Spark don't support url encode/decode as built-in functions, the user might use reflect instead, It's a bit of a hassle, And often these functions are useful.

This pr aims to two points as follow:

add url encode/decode as built-in function support.
tidy up url-related functions to one scala file

Why are the changes needed?

url encode/decode functions are useful

Does this PR introduce any user-facing change?

yes, add new function as built-in function

How was this patch tested?

add new tests

yikf · 2022-07-07T06:59:33Z

cc: @cloud-fan , I'm not sure why we don't support url encode\decode function as built-in function, but if we intentionally don't, i will close this title.

LuciferYang · 2022-07-07T12:29:48Z

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

+import org.apache.spark.sql.types.{AbstractDataType, DataType, StringType}
+import org.apache.spark.unsafe.types.UTF8String
+
+trait UrlBase extends UnaryExpression with CodegenFallback with ImplicitCastInputTypes {


I think the file name should be urlExpressions

LuciferYang · 2022-07-07T12:38:25Z

@yikf Please run SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/testOnly *ExpressionsSchemaSuite" to regenerating the result file and update in this pr.

yikf · 2022-07-07T12:54:14Z

@yikf Please run SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/testOnly *ExpressionsSchemaSuite" to regenerating the result file and update in this pr.

thanks~ i will address those comment if this pr is necessary

cloud-fan · 2022-07-08T04:47:04Z

do other databases provide these functions?

yikf · 2022-07-08T07:35:06Z

do other databases provide these functions?

Both Trino & Presto have URL-related URL functions, including url_decode/url_encode, reference doc: https://trino.io/docs/current/functions/url.html, However, I can't find a function like url_decode in Hive or mysql, etc. maybe we have to use reflect function instead in hive, It's a bit of a hassle, And often these functions are useful.

cloud-fan · 2022-07-08T07:47:07Z

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

+  since = "3.4.0",
+  group = "url_funcs")
+// scalastyle:on line.size.limit
+case class UrlEncode(child: Expression) extends UrlBase {


We can make them extend RuntimeReplaceable, and use StaticInvoke as the real expression.

okay, please take a look when you have a time, thanks

AmplabJenkins · 2022-07-09T06:16:06Z

Can one of the admins verify this patch?

LuciferYang · 2022-07-11T08:00:02Z

@yikf Could you update the pr description because I found this pr also includes some refactoring work

yikf · 2022-07-11T09:17:01Z

@yikf Could you update the pr description because I found this pr also includes some refactoring work

done

cloud-fan · 2022-07-11T15:40:09Z

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

+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = """
+    _FUNC_(str) - Translates a string into {@code application/x-www-form-urlencoded} format using a specific encoding scheme.


Does {@code application/x-www-form-urlencoded} really work? Can you run DESC FUNCTION url_encode and give a screenshot?

Looks it work normally,

This means it does not work... The SQL doc can't interpret {@code ...}. Can we just use plain string?

.... into 'application/x-www-form-urlencoded' format ...

Sorry i misread this meaning, updated

cloud-fan

LGTM except for a minor comment

LuciferYang · 2022-07-12T09:33:52Z

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

+
+object UrlCodec {
+  def encode(src: UTF8String, enc: UTF8String): UTF8String = {
+    UTF8String.fromString(URLEncoder.encode(src.toString, enc.toString))


We use java.net.URLEncoder now.
I wonder if org.apache.commons.codec.net.URLCodec and java.net.URLEncoder have the same semantics?
Will org.apache.commons.codec.net.URLCodec have better performance?

I write a micro bench as follows:

def testEncode(url: String, valuesPerIteration: Int): Unit = { import org.apache.commons.codec.net.URLCodec val benchmark = new Benchmark("Test encode", valuesPerIteration, output = output) benchmark.addCase("Use java.net.URLEncoder") { _: Int => for (_ <- 0L until valuesPerIteration) { java.net.URLEncoder.encode(url, "UTF-8") } } val codec = new URLCodec() benchmark.addCase("Use org.apache.commons.codec.net.URLCodec") { _: Int => for (_ <- 0L until valuesPerIteration) { codec.encode(url) } } benchmark.run() } def testDecode(value: String, valuesPerIteration: Int): Unit = { import org.apache.commons.codec.net.URLCodec val benchmark = new Benchmark("Test decode", valuesPerIteration, output = output) benchmark.addCase("Use java.net.URLEncoder") { _: Int => for (_ <- 0L until valuesPerIteration) { java.net.URLDecoder.decode(value, "UTF-8") } } val codec = new URLCodec() benchmark.addCase("Use org.apache.commons.codec.net.URLCodec") { _: Int => for (_ <- 0L until valuesPerIteration) { codec.decode(value) } } benchmark.run() } override def runBenchmarkSuite(mainArgs: Array[String]): Unit = { val valuesPerIteration = 100000 // Test Contains testEncode("https://spark.apache.org", valuesPerIteration) testDecode("https%3A%2F%2Fspark.apache.org", valuesPerIteration) }

and run it use GA, the result as follows:

~~Java 8~~

OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 32 33 1 3.1 324.3 1.0X Use org.apache.commons.codec.net.URLCodec 29 29 0 3.4 291.4 1.1X OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 53 54 1 1.9 530.0 1.0X Use org.apache.commons.codec.net.URLCodec 37 37 0 2.7 365.5 1.5X

Java 11

OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 25 28 2 4.0 251.6 1.0X Use org.apache.commons.codec.net.URLCodec 22 25 1 4.6 219.5 1.1X OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 31 31 1 3.3 306.2 1.0X Use org.apache.commons.codec.net.URLCodec 26 29 2 3.8 260.4 1.2X

Java 17

OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 31 32 1 3.2 308.3 1.0X Use org.apache.commons.codec.net.URLCodec 26 29 2 3.8 262.6 1.2X OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 40 44 3 2.5 402.0 1.0X Use org.apache.commons.codec.net.URLCodec 31 33 1 3.2 311.0 1.3X ``` ~~ `org.apache.commons.codec.net.URLCodec` seems a little quick than `java.net.URLEncoder` from benchmark result, so should we change to use `org.apache.commons.codec.net.URLCodec` in this pr? @cloud-fan @Yikf

Sorry, the above bench test encode twice, need re-run, please ignore it

Update result:

Java 8

OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 40 41 1 2.5 404.9 1.0X Use org.apache.commons.codec.net.URLCodec 36 38 1 2.8 358.5 1.1X OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 38 39 1 2.6 379.0 1.0X Use org.apache.commons.codec.net.URLCodec 31 32 2 3.2 312.8 1.2X

Java 11

OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 26 26 0 3.9 256.5 1.0X Use org.apache.commons.codec.net.URLCodec 24 24 1 4.2 240.5 1.1X OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 22 22 0 4.6 217.5 1.0X Use org.apache.commons.codec.net.URLCodec 18 18 0 5.6 179.0 1.2X

Java 17

OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 29 33 2 3.4 290.1 1.0X Use org.apache.commons.codec.net.URLCodec 31 35 2 3.3 305.3 1.0X OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 22 26 2 4.5 223.1 1.0X Use org.apache.commons.codec.net.URLCodec 25 29 2 4.0 252.3 0.9X

Seems like java.net.URLEncoder is the future (Java 17)? Assuming that Spark will get to Java 17 eventually, I'd vote for java.net.URLEncoder to avoid any breaking change when we want to switch to java.net.URLEncoder in the future.

Thanks @LuciferYang for benchmark test result, Seems like the benchmark did not diff much and for the reason that Spark will bump JDK and avoid any breaking change, java.net.URLEncoder is a better fit, what do you think @LuciferYang , @cloud-fan

agree with @cloud-fan

LuciferYang

LGTM +1

yikf · 2022-07-14T15:44:44Z

@LuciferYang @cloud-fan Any other comments please?

LuciferYang · 2022-07-15T02:29:27Z

friendly ping @cloud-fan

cloud-fan · 2022-07-15T04:31:21Z

thanks, merging to master!

yikf · 2022-07-15T04:37:53Z

thank you all

github-actions bot added the SQL label Jul 7, 2022

LuciferYang reviewed Jul 7, 2022

View reviewed changes

yikf closed this Jul 7, 2022

yikf reopened this Jul 7, 2022

cloud-fan reviewed Jul 8, 2022

View reviewed changes

Support url encode/decode function

2050ca6

yikf changed the title ~~[WIP] Supports url encode/decode function~~ [SPARK-39741][SQL] Support url encode/decode as built-in function Jul 11, 2022

yikf marked this pull request as ready for review July 11, 2022 07:50

yikf changed the title ~~[SPARK-39741][SQL] Support url encode/decode as built-in function~~ [SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions Jul 11, 2022

cloud-fan reviewed Jul 11, 2022

View reviewed changes

cloud-fan approved these changes Jul 11, 2022

View reviewed changes

fix comment

61344df

LuciferYang reviewed Jul 12, 2022

View reviewed changes

LuciferYang approved these changes Jul 12, 2022

View reviewed changes

cloud-fan closed this in e5c1b82 Jul 15, 2022

bjornjorgensen mentioned this pull request Aug 4, 2023

[SPARK-44155] Adding a dev utility to improve error messages based on LLM #41711

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions #37113

[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions #37113

yikf commented Jul 7, 2022 •

edited

Loading

yikf commented Jul 7, 2022

LuciferYang Jul 7, 2022

LuciferYang commented Jul 7, 2022

yikf commented Jul 7, 2022 •

edited

Loading

cloud-fan commented Jul 8, 2022 •

edited

Loading

yikf commented Jul 8, 2022 •

edited

Loading

cloud-fan Jul 8, 2022

yikf Jul 11, 2022

AmplabJenkins commented Jul 9, 2022

LuciferYang commented Jul 11, 2022

yikf commented Jul 11, 2022

cloud-fan Jul 11, 2022

yikf Jul 11, 2022

cloud-fan Jul 12, 2022

yikf Jul 12, 2022

cloud-fan left a comment

LuciferYang Jul 12, 2022

LuciferYang Jul 12, 2022 •

edited

Loading

LuciferYang Jul 12, 2022

LuciferYang Jul 12, 2022

cloud-fan Jul 12, 2022

yikf Jul 12, 2022

LuciferYang Jul 12, 2022

LuciferYang left a comment

yikf commented Jul 14, 2022

LuciferYang commented Jul 15, 2022

cloud-fan commented Jul 15, 2022

yikf commented Jul 15, 2022

[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions #37113

[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions #37113

Conversation

yikf commented Jul 7, 2022 • 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?

yikf commented Jul 7, 2022

Choose a reason for hiding this comment

LuciferYang commented Jul 7, 2022

yikf commented Jul 7, 2022 • edited Loading

cloud-fan commented Jul 8, 2022 • edited Loading

yikf commented Jul 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Jul 9, 2022

LuciferYang commented Jul 11, 2022

yikf commented Jul 11, 2022

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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuciferYang Jul 12, 2022 • edited Loading

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

LuciferYang left a comment

Choose a reason for hiding this comment

yikf commented Jul 14, 2022

LuciferYang commented Jul 15, 2022

cloud-fan commented Jul 15, 2022

yikf commented Jul 15, 2022

yikf commented Jul 7, 2022 •

edited

Loading

yikf commented Jul 7, 2022 •

edited

Loading

cloud-fan commented Jul 8, 2022 •

edited

Loading

yikf commented Jul 8, 2022 •

edited

Loading

LuciferYang Jul 12, 2022 •

edited

Loading