Skip to content

Conversation

@dylanhz
Copy link
Contributor

@dylanhz dylanhz commented Dec 26, 2025

What is the purpose of the change

FamilyArgumentTypeStrategy is designed to request an argument of a specific LogicalTypeFamily and nullability.
When expectedNullability is explicitly set, the input argument should be implicitly cast to match that nullability during type inference. However, the current implementation ignores this requirement and incorrectly yields the original input type as the inferred type.

Consider a built-in function with an argument declared as logical(EXACT_NUMERIC, true) (i.e., nullable). If the actual input is SMALLINT NOT NULL, the expected inferred type should be SMALLINT NULL. However, under the current implementation, it remains SMALLINT NOT NULL. Furthermore, If the function's return type inference further depends on the input types (for instance, nullableIfAllArgs()), the error propagates.

Brief change log

  • Enforce expected nullability in FamilyArgumentStrategy.
  • Fix incorrect error message.

Verifying this change

  • InputTypeStrategiesTest
  • No built-in functions currently use FamilyArgumentStrategy with explicit nullability requirements, so this change has no impact on the existing public API.

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): (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? (not documented)

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 26, 2025

CI report:

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


// type is part of the family
if (actualType.getTypeRoot().getFamilies().contains(expectedFamily)) {
if (Objects.equals(expectedNullability, Boolean.TRUE) && !actualType.isNullable()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful for the issue text / PR text to give SQL examples of the reported issue and the expected behaviour - currently descriptions are very low level and require table planner knowledge. This would help users find and identify this issue at the SQL level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion! I have updated the PR description. Please let me know if you have any further feedback.

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Dec 27, 2025
@dylanhz dylanhz requested a review from davidradl December 31, 2025 09:11
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