Skip to content

Conversation

@panbingkun
Copy link
Contributor

What changes were proposed in this pull request?

The pr aims to add Codegen Support for parse_url.

Why are the changes needed?

  • improve codegen coverage.
  • simplified code.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA & Existed UT (eg: UrlFunctionsSuite#*parse_url*)

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

No.

@panbingkun panbingkun marked this pull request as ready for review November 11, 2024 14:49
@panbingkun
Copy link
Contributor Author

cc @MaxGekk @cloud-fan

case class ParseUrlEvaluator(
cachedUrl: () => URI,
cachedExtractPartFunc: URI => String,
cachedPattern: () => Pattern,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to not put lambda functions here as ParseUrlEvaluator will be serialized and sent to the executor side.

Can we pass the necessary basic information so that we can generate these lambda functions within ParseUrlEvaluator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!
(PS: This is really a good suggestion. For myself, the code looks much better now, thanks! ❤️)

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 00bff28 Nov 12, 2024
@panbingkun
Copy link
Contributor Author

thanks, merging to master!

Thanks! ❤️

@dongjoon-hyun
Copy link
Member

Thank you, @panbingkun and @cloud-fan .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants