Skip to content

[FLINK-36126] Qualification of current catalog and current database should happen during execution, not parsing#25230

Merged
snuyanzin merged 4 commits into
apache:masterfrom
snuyanzin:flink36126
Aug 21, 2024
Merged

[FLINK-36126] Qualification of current catalog and current database should happen during execution, not parsing#25230
snuyanzin merged 4 commits into
apache:masterfrom
snuyanzin:flink36126

Conversation

@snuyanzin

Copy link
Copy Markdown
Contributor

What is the purpose of the change

The behavior was changed in FLINK-36085 while refactoring
this PR makes the behavior as before and adds some tests for it

Also adds some missing javadocs and getters

Verifying this change

SqlShowToOperationConverterTest.java

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)

@flinkbot

flinkbot commented Aug 21, 2024

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

import java.util.List;
import java.util.Objects;

/** Abstract class for SHOW sql call. */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

looks like was missed in prev PR and somehow checkstyle didn't detect it...

Comment on lines 74 to +76
T sqlShowCall,
@Nullable String catalogName,
@Nullable String databaseName,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just making the order same as for getOperation

@twalthr twalthr 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.

Thanks for quick update!

@snuyanzin

Copy link
Copy Markdown
Contributor Author

@flinkbot run azure

@snuyanzin snuyanzin merged commit e217fa1 into apache:master Aug 21, 2024
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