Skip to content

[FLINK-40390][table] Optimised JSON_LENGTH codegen - #28978

Open
VasShabu wants to merge 2 commits into
apache:masterfrom
VasShabu:jsonLengthPathHandling
Open

[FLINK-40390][table] Optimised JSON_LENGTH codegen#28978
VasShabu wants to merge 2 commits into
apache:masterfrom
VasShabu:jsonLengthPathHandling

Conversation

@VasShabu

@VasShabu VasShabu commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What is the purpose of the change

This pull request alters JSON_LENGTH's codeGen to be inline with JSON_TYPE, as these 2 functions are very similar in nature its smart to ensure that the planning stage works the same way.

(For example: This pull request makes task deployment go through the blob server, rather than through RPC. That way we avoid re-transferring them on each deployment (during recovery).)

Brief change log

(for example:)

  • Alters JSON_LENGTH's prefix regex so that it doesnt miss any cases
  • isPathDefinite function got moved to the Codegen stage in order to call a validation error over a runtime error
  • JSON_LENGTH test cases got changed to accomodate for the new validation error
  • reuse getOrCreateParsedJson(jsonType) for jsonLength

Verifying this change

Please make sure both new and modified tests in this PR follow the conventions for tests defined in our code quality guide.

(Please pick either of the following options)

This change added tests and can be verified as follows:
./mvnw -o -pl flink-table/flink-table-planner -Dtest='JsonFunctionsITCase' -Dsurefire.failIfNoSpecifiedTests=false -Dcheckstyle.skip=true -Dspotless.check.skip=true -Drat.skip=true -e -Denforcer.skip=true test

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

  • Dependencies (does it add or upgrade a dependency): (no)
  • 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): (yes)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (don't know)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (yno)
  • If yes, how is the feature documented? (not documented) (just optimisation)

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

Generated-by: Claude Opus 5

@flinkbot

flinkbot commented Aug 14, 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

@snuyanzin snuyanzin left a comment

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.

Please do not ignore contributor's form and fill it properly

@VasShabu

VasShabu commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Please do not ignore contributor's form and fill it properly

I did do the form, for some reason its not showing up on github.
image

@snuyanzin

Copy link
Copy Markdown
Contributor

your problem is that now everything is commented since you have NOT closed markdown comment started with <!--

@VasShabu

Copy link
Copy Markdown
Contributor Author

your problem is that now everything is commented since you have NOT closed markdown comment started with <!--

There we go, thanks Sergey.

@gustavodemorais gustavodemorais left a comment

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.

Hey @VasShabu, thanks for following up on this. JSON_LENGTH shares JSON_TYPE's codegen and validation strategy now, but not its tests - the lax/bare-field/$["strict value"] cases added to jsonTypeSpec were never mirrored into jsonLengthSpec. Could you look into extending the tests?

@VasShabu

Copy link
Copy Markdown
Contributor Author

Hey @VasShabu, thanks for following up on this. JSON_LENGTH shares JSON_TYPE's codegen and validation strategy now, but not its tests - the lax/bare-field/$["strict value"] cases added to jsonTypeSpec were never mirrored into jsonLengthSpec. Could you look into extending the tests?

What do you mean specifically by bare-field, could you please clarify, I had added the other 2 cases however.

At the moment i think airlock is not allowing me to push my changes, will try to resolve this

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants