Skip to content

[FLINK-38508][python] Allow string type specifications in Python UDF … #27106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Oct 16, 2025
Merged

[FLINK-38508][python] Allow string type specifications in Python UDF … #27106

merged 3 commits into from
Oct 16, 2025

Conversation

fsk119
Copy link
Member

@fsk119 fsk119 commented Oct 13, 2025

…decorator for SQL

What is the purpose of the change

Allow to use pyflink function with decorator in sql.

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

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

Documentation

  • Does this pull request introduce a new feature? (yes / no)

@flinkbot
Copy link
Collaborator

flinkbot commented Oct 13, 2025 •
edited
Loading

CI report:

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

davidradl
davidradl reviewed Oct 13, 2025
View reviewed changes
...nk/table/planner/plan/rules/physical/stream/StreamPhysicalConstantTableFunctionScanRule.java Outdated
final FlinkLogicalTableFunctionScan scan = call.rel(0);
return !RexUtil.containsInputRef(scan.getCall());
return !RexUtil.containsInputRef(scan.getCall())
&& PythonUtil.isNonPythonCall(scan.getCall());
Copy link
Contributor

@davidradl davidradl Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am curious why this if has been extended? why is isNonPythonCall important here? Does this have a migration impact?

Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a unit test for both cases please

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently Flink supports the following sql because of the StreamPhysicalConstantTableFunctionRule.

SELECT * FROM LATERAL TABLE(func(1))

If we don't modify this, flink uses the java operator to run the python function. It causes misleading exception here.

I think it's better we use another pr to fix this problem.

davidradl
davidradl reviewed Oct 13, 2025
View reviewed changes
flink-python/pyflink/table/tests/test_udf.py
'time_param is wrong value %s !' % time_param
return time_param

@udf(result_type='TIMESTAMP(3)')
Copy link
Contributor

Choose a reason for hiding this comment

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

should we do flink timestamp_ltz as well?
Do you think there is value in testing other TIMESTAMP precisions?

Copy link
Member Author

Choose a reason for hiding this comment

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

emmm I just move codes from the PyFlinkEmbeddedThreadTests to common test base..

Copy link
Member Author

Choose a reason for hiding this comment

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

I test timestamp_ltz and I find the results is not correct. I will open another issue to track this.

davidradl
davidradl reviewed Oct 13, 2025
View reviewed changes
flink-python/pyflink/table/tests/test_udf.py
'array_param is wrong value %s !' % array_param
return array_param[0]

@udf(result_type='MAP<BIGINT, STRING>')
Copy link
Contributor

Choose a reason for hiding this comment

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

nested object and array permutations would be worth adding tests for to confirm they are working as expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

Copy link
Member Author

Choose a reason for hiding this comment

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

I find the result is not corret in pemja. Fix this in FLINK-38526

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Oct 13, 2025
@fsk119 fsk119 merged commit cc01169 into apache:master Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dianfu dianfu dianfu approved these changes

+1 more reviewer

@davidradl davidradl davidradl left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fsk119 @flinkbot @dianfu @davidradl