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.

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

[FLINK-37093][table] Fix catalog that failed validation due to no type still exists in catalogStoreHolder #25946

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[FLINK-37093][table] Fix catalog that failed validation due to no type still exists in catalogStoreHolder #25946

wants to merge 1 commit into from
+14 −3

Conversation

jiefei30
Copy link
Contributor

What is the purpose of the change

This pull request fix a bug which a catalog that failed validation due to no type still exists in catalogStoreHolder.
And this catalog you cannot USE, ALTER.

Brief change log

  • fix a bug which a catalog that failed validation due to no type still exists in catalogStoreHolder.

Verifying this change

This change is already covered by existing tests.

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 Jan 10, 2025 •
edited
Loading

CI report:

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

@davidradl
Copy link
Contributor

Reviewed by Chi on 16/01/2025 Approve - looking for committer to merge

@jiefei30 jiefei30 changed the title [FLINK-37093][table] store catalog in catalogStoreHolder should after initCatalog and open when createCatalog [FLINK-37093][table] Fix catalog that failed validation due to no type still exists in catalogStoreHolder Jan 17, 2025
hackergin
hackergin reviewed Jan 17, 2025
View reviewed changes
Copy link
Contributor

@hackergin hackergin left a comment

Choose a reason for hiding this comment

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

@jiefei30 Thanks for your contribution, only one minor comments.

...le/flink-table-api-java/src/test/java/org/apache/flink/table/catalog/CatalogManagerTest.java Outdated
@@ -589,6 +600,7 @@ void testCatalogStore() throws Exception {
assertTrue(catalogManager.listCatalogs().contains("cat2"));
assertTrue(catalogManager.listCatalogs().contains("cat3"));
assertTrue(catalogManager.listCatalogs().contains("cat_comment"));
assertFalse(catalogManager.listCatalogs().contains("cat_no_type"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to put this assertion together with the above assertThatThrownBy?

@jiefei30
Copy link
Contributor Author

@hackergin done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hackergin hackergin hackergin left review comments

@davidradl davidradl davidradl approved these changes

Assignees
No one assigned
Labels
component=TableSQL/API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jiefei30 @flinkbot @davidradl @hackergin