feat(spark): implement Spark `try_parse_url` function #17485

rafafrdz · 2025-09-09T09:17:22Z

Which issue does this PR close?

part of #15914

Rationale for this change

Migrate Spark functions from https://github.com/lakehq/sail/ to DataFusion engine to unify codebase

What changes are included in this PR?

Implement Spark UDF try_parse_url https://spark.apache.org/docs/latest/api/python/reference/pyspark.sql/api/pyspark.sql.functions.try_parse_url.html
Signature of the parse_url function

Fix

PATH part: if there is no path pat it returns an empty string (not "/")
AUTHORITY part: the authority section must include the port if the URL specifies one. However, at the moment, if the authority contains a well-known port (e.g., 80, 23, etc.), it is omitted and only shown when the port is a non-standard (“custom”) one.

Are these changes tested?

unit-tests and sqllogictests added

Are there any user-facing changes?

Now can be called in queries

Jefffrey

Looks like some CI failures to address

datafusion/spark/src/function/url/parse_url.rs

Jefffrey · 2025-09-11T02:21:06Z

datafusion/spark/src/function/url/parse_url.rs

-            signature: Signature::one_of(
-                vec![
-                    TypeSignature::Uniform(
-                        1,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                    TypeSignature::Uniform(
-                        2,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                    TypeSignature::Uniform(
-                        3,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                ],
-                Volatility::Immutable,
-            ),
+            signature: Signature::user_defined(Volatility::Immutable),


Does this change interfere with dictionary types now? I think this works on main but doesn't work on this PR:

query T SELECT parse_url(arrow_cast('http://spark.apache.org/path?query=1', 'Dictionary(Int32, Utf8)'), 'HOST'::string); ---- spark.apache.org

Sorry, I don’t quite follow… Why should dictionary types be used as arguments for parse_url or try_parse_url? As far as I understand, the spec only expects string types parse_url doc ref

Dictionary type is another way to represent strings, similar to how view type is a different way to represent strings.

Can see here in the doc there's a reference to handling dictionary types:

datafusion/datafusion/expr-common/src/signature.rs

Lines 229 to 239 in 351675d

/// One or arguments of all the same string types.

///

/// The precedence of type from high to low is Utf8View, LargeUtf8 and Utf8.

/// Null is considered as `Utf8` by default

/// Dictionary with string value type is also handled.

///

/// For example, if a function is called with (utf8, large_utf8), all

/// arguments will be coerced to `LargeUtf8`

///

/// For functions that take no arguments (e.g. `random()` use [`TypeSignature::Nullary`]).

String(usize),

I see, let me check it out then 😃

After rereading this several times, my understanding is that when you pass a Dictionary with string values, DataFusion attempts to match it against the String signature. However, parse_url is defined to accept only plain string arguments ref. It does not expect any dictionary inputs.

We mark the UDF’s signature as user_defined to enable coercion across string types (Utf8, Utf8View, LargeUtf8), but a dictionary array is still not a string type, so it isn’t coerced, and the call won’t match.

In short, even if the String signature seems to "capture" Dictionary with string values, parse_url will still reject them because the underlying physical type is a dictionary, not a string

That's correct, but my point was previously (on main), parse_url seems to be capable of accepting string dictionary types, but with these changes that is no longer possible; is this something to be concerned about?

Is it possible to simplify the type signature to just be:

signature: Signature::one_of( vec![ TypeSignature::String(1), TypeSignature::String(2), TypeSignature::String(3), ], Volatility::Immutable, ),

So they get cast to the same string type? This would avoid the need for the large match statement in spark_handled_parse_url as well.

See related comment on other PR: #17195 (comment)

datafusion/sqllogictest/test_files/spark/url/try_parse_url.slt

Jefffrey

Looks like some fixes for PATH and AUTHORITY are now included in this PR, so would be good to call this out in the PR body.

Jefffrey · 2025-09-12T01:24:49Z

datafusion/spark/src/function/url/parse_url.rs

-            signature: Signature::one_of(
-                vec![
-                    TypeSignature::Uniform(
-                        1,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                    TypeSignature::Uniform(
-                        2,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                    TypeSignature::Uniform(
-                        3,
-                        vec![DataType::Utf8View, DataType::Utf8, DataType::LargeUtf8],
-                    ),
-                ],
-                Volatility::Immutable,
-            ),
+            signature: Signature::user_defined(Volatility::Immutable),


That's correct, but my point was previously (on main), parse_url seems to be capable of accepting string dictionary types, but with these changes that is no longer possible; is this something to be concerned about?

datafusion/sqllogictest/test_files/spark/url/try_parse_url.slt

datafusion/spark/src/function/url/mod.rs

Jefffrey · 2025-09-12T01:47:30Z

datafusion/spark/src/function/url/parse_url.rs

+        Arc::new(LargeStringArray::from(vals.to_vec())) as ArrayRef
+    }
+
+    #[test]


I feel quite a few of these tests could be done as slt tests; @alamb do we have a preference on where tests should be done? Should we prefer slt over rust tests, and fallback only to rust if it is something that slt can't handle?

Took a look at https://datafusion.apache.org/contributor-guide/testing.html but it doesn't mention if we have a specific preference, other than slt's being easier to maintain.

I personally prefer slt tests. But i agree we don't have clear guidance

Jefffrey · 2025-09-14T06:05:45Z

I recommend running cargo clippy & test locally first so we don't need to find out via the CI checks

rafafrdz · 2025-09-15T14:57:20Z

I recommend running cargo clippy & test locally first so we don't need to find out via the CI checks

done

Jefffrey

One minor suggestion, other than that this should be good to go once the CI failures are resolved 👍

datafusion/spark/src/function/url/parse_url.rs

Jefffrey · 2025-09-29T08:40:47Z

Looks like some test failures:

1. query result mismatch:
[SQL] SELECT parse_url('http://[email protected]/path?query=1#Ref'::string, 'AUTHORITY'::string);
[Diff] (-expected|+actual)
-   userinfo@spark.apache.org
+   userinfo@spark.apache.org:80
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/spark/url/parse_url.slt:63


2. query failed: DataFusion error: Execution error: RelativeUrlWithoutBase
[SQL] SELECT parse_url('www.example.com/path?x=1', 'HOST');
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/spark/url/parse_url.slt:83


3. query failed: DataFusion error: Execution error: RelativeUrlWithoutBase
[SQL] SELECT parse_url('www.example.com/path?x=1', 'host');
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/spark/url/parse_url.slt:88


4. query failed: DataFusion error: Execution error: RelativeUrlWithoutBase
[SQL] SELECT parse_url('notaurl', 'HOST');
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/spark/url/parse_url.slt:133


5. query failed: DataFusion error: Execution error: RelativeUrlWithoutBase
[SQL] SELECT parse_url('notaurl', 'host');
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/spark/url/parse_url.slt:138

For reference I checked expected outputs against Spark 4.0.0:

spark-sql (default)> SELECT parse_url('http://[email protected]/path?query=1#Ref'::string, 'AUTHORITY'::string);
userinfo@spark.apache.org
Time taken: 0.779 seconds, Fetched 1 row(s)
spark-sql (default)> SELECT parse_url('www.example.com/path?x=1', 'HOST');
NULL
Time taken: 0.054 seconds, Fetched 1 row(s)
spark-sql (default)> SELECT parse_url('www.example.com/path?x=1', 'host');
NULL
Time taken: 0.041 seconds, Fetched 1 row(s)
spark-sql (default)> SELECT parse_url('notaurl', 'HOST');
NULL
Time taken: 0.022 seconds, Fetched 1 row(s)
spark-sql (default)> SELECT parse_url('notaurl', 'host');
NULL
Time taken: 0.021 seconds, Fetched 1 row(s)
spark-sql (default)>

Also I noticed we don't have a test that shows the difference between parse_url and try_parse_url; we could use this one from the Spark docs: https://spark.apache.org/docs/latest/api/sql/index.html#try_parse_url

SELECT try_parse_url('inva lid://spark.apache.org/path?query=1', 'QUERY');

Where for parse_url it errors as expected but try_parse_url returns null

alamb · 2025-10-04T10:47:39Z

@rafafrdz -- do you think you'll be able to help finish this PR any time soon?

rafafrdz · 2025-10-04T11:56:25Z

@alamb @Jefffrey sorry, I've been too bussy these last weeks. I'll try to finish it by this weekend

alamb

Looks good to me -- thank you @rafafrdz and @Jefffrey

alamb · 2025-10-06T14:26:46Z

(this is a really nice addition)

github-actions bot added sqllogictest SQL Logic Tests (.slt) spark documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation labels Sep 9, 2025

rafafrdz marked this pull request as draft September 9, 2025 12:39

rafafrdz added 2 commits September 9, 2025 16:38

impl try_parse_url spark function

e995629

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

8869878

rafafrdz force-pushed the feat/try_parse_url branch from fc60bd0 to 8869878 Compare September 9, 2025 14:38

rafafrdz marked this pull request as ready for review September 9, 2025 14:40

Jefffrey reviewed Sep 11, 2025

View reviewed changes

rafafrdz marked this pull request as draft September 11, 2025 11:04

rafafrdz added 3 commits September 11, 2025 13:20

suggestions

54e0125

fix parse_url

944d204

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

bf39835

rafafrdz force-pushed the feat/try_parse_url branch from f0090f8 to bf39835 Compare September 11, 2025 12:17

fix parse_url

87405e2

rafafrdz marked this pull request as ready for review September 11, 2025 13:59

fix parse_url

532bd38

Jefffrey reviewed Sep 12, 2025

View reviewed changes

rafafrdz added 3 commits September 12, 2025 18:27

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

82f5a9b

suggestions

37dc796

suggestions

36ead8b

rafafrdz requested a review from Jefffrey September 12, 2025 17:04

rafafrdz added 4 commits September 12, 2025 19:07

suggestions

a07f7eb

suggestions

80e7259

edit

b58b4c1

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

3cc0cbe

rafafrdz added 2 commits September 15, 2025 16:54

tests and clippy

47d9a21

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

1e40033

Jefffrey reviewed Sep 16, 2025

View reviewed changes

datafusion/spark/src/function/url/parse_url.rs Outdated Show resolved Hide resolved

rafafrdz and others added 3 commits September 16, 2025 13:01

suggestions and tests

2e0cc6a

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

ae48283

Merge branch 'main' into feat/try_parse_url

6904c7d

rafafrdz and others added 4 commits October 4, 2025 13:56

Merge branch 'main' into feat/try_parse_url

60488a6

fixing parse_url

8626fdb

Merge remote-tracking branch 'upstream/main' into feat/try_parse_url

3c97788

fixing parse_url

80e2dbc

alamb changed the title ~~feat(spark): implement Spark try_parse_url function~~ feat(spark): implement Spark parse_url and try_parse_url function Oct 6, 2025

alamb changed the title ~~feat(spark): implement Spark parse_url and try_parse_url function~~ feat(spark): implement Spark try_parse_url function Oct 6, 2025

alamb approved these changes Oct 6, 2025

View reviewed changes

Merge branch 'main' into feat/try_parse_url

e70efa3

alamb added this pull request to the merge queue Oct 7, 2025

Merged via the queue into apache:main with commit 58ddf0d Oct 7, 2025
1 check passed

	/// One or arguments of all the same string types.
	///
	/// The precedence of type from high to low is Utf8View, LargeUtf8 and Utf8.
	/// Null is considered as `Utf8` by default
	/// Dictionary with string value type is also handled.
	///
	/// For example, if a function is called with (utf8, large_utf8), all
	/// arguments will be coerced to `LargeUtf8`
	///
	/// For functions that take no arguments (e.g. `random()` use [`TypeSignature::Nullary`]).
	String(usize),

feat(spark): implement Spark try_parse_url function #17485

feat(spark): implement Spark try_parse_url function #17485

Uh oh!

Conversation

rafafrdz commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafafrdz Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jefffrey commented Sep 14, 2025

Uh oh!

rafafrdz commented Sep 15, 2025

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jefffrey commented Sep 29, 2025

Uh oh!

alamb commented Oct 4, 2025

Uh oh!

rafafrdz commented Oct 4, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb commented Oct 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(spark): implement Spark `try_parse_url` function #17485

feat(spark): implement Spark `try_parse_url` function #17485

rafafrdz commented Sep 9, 2025 •

edited

Loading

rafafrdz Sep 11, 2025 •

edited

Loading