IGNITE-19082: Catalog. Cleanup dead code #3669
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lowka
force-pushed
the
ignite-19082
branch
5 times, most recently
from
600d20a
to
f965bd9
Compare
await for catalog initialization i n system view manager.
update catalog tests.
update catalog tests.
await catalog activation.
await catalog activation.
await catalog activation.
await catalog activation.
await catalog activation.
await catalog activation.
fix some tests.
check style.
check style.
remove unsed SchemaRegistryImpl.
Use schemaId.
Use schemaId. Fix javadoc.
Fix check style error.
I have addressed this issue. |
AMashenkov
approved these changes
May 8, 2024
code style.
Return completed futures.
korlov42
reviewed
May 9, 2024
| @@ -49,18 +49,19 @@ public class NewColumnsEntry implements UpdateEntry, Fireable { | |||
|
|
|||
| private final int tableId; | |||
| private final List<CatalogTableColumnDescriptor> descriptors; | |||
| private final String schemaName; | |||
| private final int schemaId; | |||
There was a problem hiding this comment.
I would delete schema at all because it creates ambiguity. It's better to derive schema from table (table descriptor has schemaId).
The same for rest of the updateEntries
fix serde. removed schema id from new columns entry.
korlov42
reviewed
May 9, 2024
| new CatalogTableDescriptor[0], | ||
| new CatalogIndexDescriptor[0], | ||
| new CatalogSystemViewDescriptor[0], | ||
| 0 |
There was a problem hiding this comment.
Suggested change
| 0 | |
| INITIAL_CAUSALITY_TOKEN |
Add tests.
Add tests.
Update tests.
Remove schema refs.
Remove schema refs.
Remove schema refs.
assertions.
korlov42
reviewed
May 10, 2024
| @@ -33,7 +33,7 @@ | |||
| * the {@link CatalogIndexStatus#STOPPING} state. | |||
| */ | |||
| public class DropIndexEntry extends AbstractChangeIndexStatusEntry implements Fireable { | |||
| public static final DropIndexEntrySerializer SERIALIZER = new DropIndexEntrySerializer(); | |||
| public static final CatalogObjectSerializer<DropIndexEntry> SERIALIZER = new DropIndexEntrySerializer(); | |||
|
|
|||
| private final int tableId; | |||
There was a problem hiding this comment.
looks like tableId is not used anywhere but tests. Let's remove it as well
| @@ -44,17 +44,17 @@ public class NewIndexEntry implements UpdateEntry, Fireable { | |||
|
|
|||
| private final CatalogIndexDescriptor descriptor; | |||
|
|
|||
| private final String schemaName; | |||
| private final int schemaId; | |||
There was a problem hiding this comment.
why do we need schemaId? It can be derived from table, I believe
remove remaining schema id.
use initial token.
# Conflicts: # modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java # modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java
AMashenkov
approved these changes
May 10, 2024
Remove tableId.
# Conflicts: # modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
check style.
check style.
korlov42
approved these changes
May 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/IGNITE-19082
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
The Review Checklist
- There is a single JIRA ticket related to the pull request.
- The web-link to the pull request is attached to the JIRA ticket.
- The JIRA ticket has the Patch Available state.
- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
Notes