Skip to content
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.

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

[FLINK-37222] Do not reuse views across TableEnvironments in SQL client #26093

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

Zakelly
Copy link
Contributor

@Zakelly Zakelly commented Jan 30, 2025

What is the purpose of the change

After FLINK-37098, there is a bug when submitting jobs via sql client. This PR fixes this.

Brief change log

  • A test verifying the sql client with complex sql job.
  • A fix that not reuse views across TableEnvironments in SQL client

Verifying this change

This change has added test named testExecuteNexmark

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 applicable

@Zakelly
Copy link
Contributor Author

Zakelly commented Jan 30, 2025

cc @dawidwys

@flinkbot
Copy link
Collaborator

flinkbot commented Jan 30, 2025

CI report:

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

@dawidwys
Copy link
Contributor

dawidwys commented Feb 3, 2025

I'll have a look

@dawidwys
Copy link
Contributor

dawidwys commented Feb 3, 2025

As I thought, the SQL client goes against TableEnvironment design. I added a hotfix. I was not sure how to change your test to make it pass. It fails now with some missing properties. Do you mind helping to make the test run properly?

@Zakelly
Copy link
Contributor Author

Zakelly commented Feb 4, 2025

@flinkbot run azure

@Zakelly
Copy link
Contributor Author

Zakelly commented Feb 4, 2025

I've tested with the latest commit locally, and it seems the issue is resolved. The CI fails due to a missing license. I added one, and let's see if the CI passes now.

@Zakelly Zakelly changed the title [FLINK-37222][Test] Introduce simple test submitting nexmark sql via sql client [FLINK-37222] Do not reuse views across TableEnvironments in SQL client Feb 4, 2025
@Zakelly
Copy link
Contributor Author

Zakelly commented Feb 5, 2025

CI passed. And I also created a backport PR #26104. cc @dawidwys

@Zakelly
Copy link
Contributor Author

Zakelly commented Feb 5, 2025

Thanks. Merging...

@Zakelly Zakelly merged commit ac4f595 into apache:master Feb 5, 2025
@Zakelly Zakelly deleted the f37222 branch February 5, 2025 11:39
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