Skip to content

[FLINK-39868][table-runtime] Remove per-row logging from parseUrl and subString#28336

Open
raminqaf wants to merge 3 commits into
apache:masterfrom
raminqaf:FLINK-39868
Open

[FLINK-39868][table-runtime] Remove per-row logging from parseUrl and subString#28336
raminqaf wants to merge 3 commits into
apache:masterfrom
raminqaf:FLINK-39868

Conversation

@raminqaf

@raminqaf raminqaf commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Removing hot path error log of the methods parseURL and subString.

Brief change log

Drop the per-row log calls in SqlFunctionUtils. Both functions already return null on the logged condition, so this is behavior-preserving.

  • parseUrl
  • subString

No new type strategy. No behavior change.

Verifying this change

  • Add test to StringFunctionsITCase

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): on
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? no

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

… subString

SqlFunctionUtils#parseUrl and #subString logged an ERROR on every row hitting an invalid input (malformed URL, negative substring length). Both already return null on that condition, so the logging only adds hot-path noise without changing behaviour. Remove it.

Add integration tests in StringFunctionsITCase covering the null-returning branches plus a sanity case for each, since neither function previously had IT-level coverage.
@flinkbot

flinkbot commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jun 6, 2026
… StringFunctionsITCase

Move the PARSE_URL, SUBSTRING and SUBSTR coverage from the legacy ScalarFunctionsTest (ExpressionTestBase) to StringFunctionsITCase (BuiltInFunctionTestBase), so these functions get end-to-end IT coverage instead of expression-only tests. This follows the ongoing migration to the new function-test framework.

The ported cases keep the existing happy paths and the null-returning branches. PARSE_URL specs use a small fluent builder so each extracted part reads as one line. The original scala methods are removed; unrelated substring uses in other tests stay.
Comment on lines 50 to 52
substrTestCases(),
translateTestCases(),
concatenateTestCases())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we have abc order for the whole list here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. It is now sorted

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

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants