Use parse_url kernel for HOST parsing #9845

thirtiseven · 2023-11-23T07:53:58Z

Contributes to #8963

This PR add plugin support for HOST parsing in parse_url.

Depends on JNI PR: NVIDIA/spark-rapids-jni#1569

Depends on: #9481

Signed-off-by: Haoyang Li <[email protected]>

…park-rapids into thirtiseven-parse_url_protocol

Signed-off-by: Haoyang Li <[email protected]>

integration_tests/src/main/python/url_test.py

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/urlFunctions.scala

integration_tests/src/main/python/url_test.py

sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/urlFunctions.scala

winningsix · 2023-11-25T01:44:17Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/urlFunctions.scala

+          parts match {
+            case partScalar: GpuScalar =>
+              GpuColumnVector.from(doColumnar(urls, partScalar), dataType)
+            case _ =>


Just curious in which case we will hit this exception.

columnarEvalAny will return Any, so it is possible that there is something other than GpuScalar in the batch.

An example from GpuExpressions.scala:

spark-rapids/sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuExpressions.scala

Lines 309 to 328 in c65c5b8

override def columnarEval(batch: ColumnarBatch): GpuColumnVector = {

withResourceIfAllowed(left.columnarEvalAny(batch)) { lhs =>

withResourceIfAllowed(right.columnarEvalAny(batch)) { rhs =>

(lhs, rhs) match {

case (l: GpuColumnVector, r: GpuColumnVector) =>

GpuColumnVector.from(doColumnar(l, r), dataType)

case (l: GpuScalar, r: GpuColumnVector) =>

GpuColumnVector.from(doColumnar(l, r), dataType)

case (l: GpuColumnVector, r: GpuScalar) =>

GpuColumnVector.from(doColumnar(l, r), dataType)

case (l: GpuScalar, r: GpuScalar) =>

GpuColumnVector.from(doColumnar(batch.numRows(), l, r), dataType)

case (l, r) =>

throw new UnsupportedOperationException(s"Unsupported data '($l: " +

s"${l.getClass}, $r: ${r.getClass})' for GPU binary expression.")

}

}

}

}

}

In the real world, I think this case won't be hit if we do the right thing in GpuOverrides, the exception here is more for defensive purpose.

winningsix · 2023-11-25T01:44:23Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/urlFunctions.scala

+                GpuColumnVector.from(doColumnar(urlCv, partScalar, keyScalar), dataType)
+              case _ =>
+                throw new 
+                    UnsupportedOperationException(s"Cannot columnar evaluate expression: $this")


winningsix · 2023-11-25T01:45:01Z

tests/src/test/scala/com/nvidia/spark/rapids/UrlFunctionsSuite.scala

+      "http://www.example.com/wpstyle/?p=364",
+      "https://www.example.com/foo/?bar=baz&inga=42&quux",
+      "http://✪df.ws/123",
+      // "http://userid:[email protected]:8080",


Remove these if we don't want.

They are failing tests now, will uncomment them after the kernel fixes these cases.

Should be working now.

winningsix · 2023-11-25T01:45:44Z

tests/src/test/scala/com/nvidia/spark/rapids/UrlFunctionsSuite.scala

@@ -0,0 +1,198 @@
+/*


Seems duplicated to url_test.py. We can just keep url_test.py which can be run in parallel.

Yes, it is just make it easy to test for now. Will remove it before it's ready for review.

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven · 2023-12-07T09:39:59Z

build

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven · 2023-12-08T02:24:31Z

build

integration_tests/src/main/python/url_test.py

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/GpuParseUrl.scala

ttnghia · 2023-12-08T21:58:38Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/GpuParseUrl.scala

+      // return a null columnvector
+      return ColumnVector.fromStrings(null, null)
+    }
+    throw new UnsupportedOperationException(s"$this only supports partToExtract = PROTOCOL")


The message seems to be vague here. We say we are supporting PROTOCOL and HOST so here needs more explanation.

Thanks, done.

ttnghia · 2023-12-08T22:02:15Z

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/GpuParseUrl.scala

+      }
+    } else {
+      // 3-arg, i.e. QUERY with key
+      assert(children.size == 3)


How about checking and throwing IllegalArgumentException on object constructor (object GpuParseUrl#apply())?

I believe the assert here is unlikely to fail because there is an argument checking for it in GpuOverrides.

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven · 2023-12-13T02:45:43Z

dependent PROTOCOL PR is merged, so this one is ready to merge too.

revans2

Looks good

revans2 · 2023-12-13T19:37:57Z

build

thirtiseven and others added 30 commits July 20, 2023 13:11

WIP: Support parse_url

8235c95

Signed-off-by: Haoyang Li <[email protected]>

Merge branch 'NVIDIA:branch-23.08' into prase_url

9f17539

fix build failures

729fe35

regex refactor

85c3284

Merge branch 'NVIDIA:branch-23.08' into prase_url

6819214

Separate regexes and UTF-8 special characters support

4166362

hostname validation

43acceb

hostname validation

64d8373

ipv4 validation

e6a45d3

verify

8c4dc7a

wip ipv6 and SPARK-44500

fee5a3d

optional protocol and ref validation

e81d8a3

IPV6 VALIDATION

93a9342

clean up

1ad665f

Fix ipv6 validation, it is still wip

3edb929

Fix ipv6 validation and some clean up

daa61ea

Merge branch 'prase_url' into parse_url_protocol

70a5d88

Use parse_url kernel for PROTOCOL parsing

b3abaf6

Signed-off-by: Haoyang Li <[email protected]>

verify

592c642

Signed-off-by: Haoyang Li <[email protected]>

edit compatibility and update IT

9db1b2a

Signed-off-by: Haoyang Li <[email protected]>

update integration tests

d09f06d

Signed-off-by: Haoyang Li <[email protected]>

address comments

3b71c4d

Signed-off-by: Haoyang Li <[email protected]>

remove unnecessary error handling

46527f3

Signed-off-by: Haoyang Li <[email protected]>

clean up

6161fa4

Signed-off-by: Haoyang Li <[email protected]>

Merge branch 'parse_url_protocol' of https://github.com/thirtiseven/s…

e16fe1e

…park-rapids into thirtiseven-parse_url_protocol

Merge branch 'thirtiseven-parse_url_protocol' into parse_url_protocol

8e7ed44

Merge branch 'NVIDIA:branch-23.12' into parse_url_protocol

f93b944

Revert scala tests temporarily for easier testing

8f4990c

Signed-off-by: Haoyang Li <[email protected]>

Fix two nits

3376376

Signed-off-by: Haoyang Li <[email protected]>

Updated results

4e98888

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven added 2 commits November 23, 2023 14:31

Merge branch 'branch-24.02' into parse_url_host

e2cbf2f

Use parse_url kernel for HOST parsing

2ba1a53

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven self-assigned this Nov 23, 2023

winningsix reviewed Nov 25, 2023

View reviewed changes

sameerz added the feature request New feature or request label Nov 28, 2023

thirtiseven and others added 7 commits November 28, 2023 14:15

rename urlFunctions to GpuParseUrl

1b36090

Signed-off-by: Haoyang Li <[email protected]>

Merge branch 'parse_url_protocol' into parse_url_host

12ea091

Merge branch 'branch-24.02' into parse_url_host

6b3feb6

address comments

451c201

Signed-off-by: Haoyang Li <[email protected]>

Merge branch 'NVIDIA:branch-24.02' into parse_url_host

5f43687

update test names

a62c00c

Signed-off-by: Haoyang Li <[email protected]>

remove scala test

2704487

Signed-off-by: Haoyang Li <[email protected]>

thirtiseven marked this pull request as ready for review December 6, 2023 08:08

thirtiseven and others added 2 commits December 8, 2023 10:15

Merge branch 'NVIDIA:branch-24.02' into parse_url_host

11ef578

verify

e56173b

Signed-off-by: Haoyang Li <[email protected]>

ttnghia reviewed Dec 8, 2023

View reviewed changes

integration_tests/src/main/python/url_test.py Outdated Show resolved Hide resolved

ttnghia reviewed Dec 8, 2023

View reviewed changes

sql-plugin/src/main/scala/org/apache/spark/sql/rapids/GpuParseUrl.scala Outdated Show resolved Hide resolved

ttnghia reviewed Dec 8, 2023

View reviewed changes

thirtiseven added 2 commits December 11, 2023 14:03

format

c50b3df

Signed-off-by: Haoyang Li <[email protected]>

Merge branch 'branch-24.02' into parse_url_host

1732ece

thirtiseven requested a review from winningsix December 13, 2023 02:45

revans2 approved these changes Dec 13, 2023

View reviewed changes

thirtiseven merged commit 8d590eb into NVIDIA:branch-24.02 Dec 13, 2023
37 of 38 checks passed

thirtiseven deleted the parse_url_host branch December 19, 2023 08:44

hyperbolic2346 mentioned this pull request May 23, 2024

[FEA] Support parse_url #6969

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use parse_url kernel for HOST parsing #9845

Use parse_url kernel for HOST parsing #9845

thirtiseven commented Nov 23, 2023

winningsix Nov 25, 2023

thirtiseven Nov 28, 2023

winningsix Nov 25, 2023

winningsix Nov 25, 2023

thirtiseven Nov 27, 2023

hyperbolic2346 Dec 6, 2023

winningsix Nov 25, 2023

thirtiseven Nov 27, 2023

thirtiseven Dec 6, 2023

thirtiseven commented Dec 7, 2023

thirtiseven commented Dec 8, 2023

ttnghia Dec 8, 2023

thirtiseven Dec 11, 2023

ttnghia Dec 8, 2023 •

edited

Loading

thirtiseven Dec 11, 2023

thirtiseven commented Dec 13, 2023

revans2 left a comment

revans2 commented Dec 13, 2023

	override def columnarEval(batch: ColumnarBatch): GpuColumnVector = {
	withResourceIfAllowed(left.columnarEvalAny(batch)) { lhs =>
	withResourceIfAllowed(right.columnarEvalAny(batch)) { rhs =>
	(lhs, rhs) match {
	case (l: GpuColumnVector, r: GpuColumnVector) =>
	GpuColumnVector.from(doColumnar(l, r), dataType)
	case (l: GpuScalar, r: GpuColumnVector) =>
	GpuColumnVector.from(doColumnar(l, r), dataType)
	case (l: GpuColumnVector, r: GpuScalar) =>
	GpuColumnVector.from(doColumnar(l, r), dataType)
	case (l: GpuScalar, r: GpuScalar) =>
	GpuColumnVector.from(doColumnar(batch.numRows(), l, r), dataType)
	case (l, r) =>
	throw new UnsupportedOperationException(s"Unsupported data '($l: " +
	s"${l.getClass}, $r: ${r.getClass})' for GPU binary expression.")
	}
	}
	}
	}
	}

Use parse_url kernel for HOST parsing #9845

Use parse_url kernel for HOST parsing #9845

Conversation

thirtiseven commented Nov 23, 2023

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

Choose a reason for hiding this comment

thirtiseven commented Dec 7, 2023

thirtiseven commented Dec 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttnghia Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thirtiseven commented Dec 13, 2023

revans2 left a comment

Choose a reason for hiding this comment

revans2 commented Dec 13, 2023

ttnghia Dec 8, 2023 •

edited

Loading