Skip to content

Conversation

luoyuxia
Copy link
Contributor

What is the purpose of the change

To fix sql client can't show result for DELETE statement when delete is pushed down

Brief change log

Check the ModifyOperation is DeleteFromFilterOperation or not in OperationExecutor#callModifyOperations,
if it's DeleteFromFilterOperation, return the result directly without getting the JobClient.

Verifying this change

Added test in flink-table/flink-sql-client/src/test/resources/sql/delete.q.

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)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@luoyuxia
Copy link
Contributor Author

cc @fsk119

@flinkbot
Copy link
Collaborator

flinkbot commented Apr 26, 2023

CI report:

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

@luoyuxia
Copy link
Contributor Author

@flinkbot run azure

2 similar comments
@luoyuxia
Copy link
Contributor Author

@flinkbot run azure

@luoyuxia
Copy link
Contributor Author

@flinkbot run azure

@luoyuxia luoyuxia force-pushed the FLINK-31882 branch 2 times, most recently from 194855f to be3e690 Compare May 12, 2023 01:39
Copy link
Member

@fsk119 fsk119 left a comment

Choose a reason for hiding this comment

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

Thanks for your fix. LGTM

Copy link
Member

@fsk119 fsk119 left a comment

Choose a reason for hiding this comment

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

Thanks for your fix. LGTM

@luoyuxia luoyuxia merged commit 886dda2 into apache:master May 15, 2023
luoyuxia added a commit to luoyuxia/flink that referenced this pull request May 15, 2023
luoyuxia added a commit that referenced this pull request May 15, 2023
RocMarshal pushed a commit to RocMarshal/flink that referenced this pull request May 9, 2024
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.

3 participants