From d0910a77b78c183a1ce30935f2945cf5ee3defa7 Mon Sep 17 00:00:00 2001 From: Heng Qin Date: Wed, 5 Jun 2024 22:06:25 +0800 Subject: [PATCH] check schema --- .../com/datastrato/gravitino/GravitinoEnv.java | 10 ++++++++++ .../catalog/SchemaOperationDispatcher.java | 11 +++-------- .../catalog/TableOperationDispatcher.java | 17 +++++++++-------- .../catalog/TopicOperationDispatcher.java | 14 ++++++++------ .../catalog/TestTableOperationDispatcher.java | 1 + .../catalog/TestTopicOperationDispatcher.java | 1 + 6 files changed, 32 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java index c48e0fcf02c..672dc437fed 100644 --- a/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java +++ b/core/src/main/java/com/datastrato/gravitino/GravitinoEnv.java @@ -132,6 +132,16 @@ public void setCatalogDispatcher(CatalogDispatcher catalogDispatcher) { this.catalogDispatcher = catalogDispatcher; } + /** + * This method is used for testing purposes only to set the schema dispatcher for test. + * + * @param schemaDispatcher The schema dispatcher to be set. + */ + @VisibleForTesting + public void setSchemaDispatcher(SchemaDispatcher schemaDispatcher) { + this.schemaDispatcher = schemaDispatcher; + } + /** * This method is used for testing purposes only to set the entity store for test in package * `com.datastrato.gravitino.authorization`. diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java index 178b0f921fc..cbf5e279975 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/SchemaOperationDispatcher.java @@ -214,8 +214,7 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(true); + alteredSchema.properties())); } StringIdentifier stringId = getStringIdFromProperties(alteredSchema.properties()); @@ -226,8 +225,7 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(isEntityExist(ident)); + alteredSchema.properties())); } SchemaEntity updatedSchemaEntity = @@ -255,15 +253,12 @@ public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) "UPDATE", stringId.id()); - boolean imported = updatedSchemaEntity != null; - return EntityCombinedSchema.of(alteredSchema, updatedSchemaEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::schemaPropertiesMetadata, - alteredSchema.properties())) - .withImported(imported); + alteredSchema.properties())); } /** diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java index 52650303f33..836c3aa7237 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/TableOperationDispatcher.java @@ -10,6 +10,7 @@ import static com.datastrato.gravitino.rel.expressions.transforms.Transforms.EMPTY_TRANSFORM; import com.datastrato.gravitino.EntityStore; +import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.StringIdentifier; @@ -88,8 +89,12 @@ public Table loadTable(NameIdentifier ident) throws NoSuchTableException { return table; } - TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTable(ident)); + if (GravitinoEnv.getInstance() + .schemaDispatcher() + .schemaExists(NameIdentifier.of(ident.namespace().levels()))) { + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTable(ident)); + } return table; } @@ -231,8 +236,7 @@ public Table alterTable(NameIdentifier ident, TableChange... changes) getHiddenPropertyNames( getCatalogIdentifier(ident), HasPropertyMetadata::tablePropertiesMetadata, - alteredTable.properties())) - .withImported(isEntityExist(ident)); + alteredTable.properties())); } TableEntity updatedTableEntity = @@ -267,15 +271,12 @@ public Table alterTable(NameIdentifier ident, TableChange... changes) "UPDATE", stringId.id()); - boolean imported = updatedTableEntity != null; - return EntityCombinedTable.of(alteredTable, updatedTableEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( getCatalogIdentifier(ident), HasPropertyMetadata::tablePropertiesMetadata, - alteredTable.properties())) - .withImported(imported); + alteredTable.properties())); } /** diff --git a/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java b/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java index c0a43f73fe0..1bc69760633 100644 --- a/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java +++ b/core/src/main/java/com/datastrato/gravitino/catalog/TopicOperationDispatcher.java @@ -9,6 +9,7 @@ import static com.datastrato.gravitino.catalog.PropertiesMetadataHelpers.validatePropertyForCreate; import com.datastrato.gravitino.EntityStore; +import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.StringIdentifier; @@ -79,8 +80,12 @@ public Topic loadTopic(NameIdentifier ident) throws NoSuchTopicException { return topic; } - TreeLockUtils.doWithTreeLock( - NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTopic(ident)); + if (GravitinoEnv.getInstance() + .schemaDispatcher() + .schemaExists(NameIdentifier.of(ident.namespace().levels()))) { + TreeLockUtils.doWithTreeLock( + NameIdentifier.of(ident.namespace().levels()), LockType.WRITE, () -> importTopic(ident)); + } return topic; } @@ -219,15 +224,12 @@ public Topic alterTopic(NameIdentifier ident, TopicChange... changes) "UPDATE", getStringIdFromProperties(alteredTopic.properties()).id()); - boolean imported = updatedTopicEntity != null; - return EntityCombinedTopic.of(alteredTopic, updatedTopicEntity) .withHiddenPropertiesSet( getHiddenPropertyNames( catalogIdent, HasPropertyMetadata::topicPropertiesMetadata, - alteredTopic.properties())) - .withImported(imported); + alteredTopic.properties())); } /** diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java index f3c1324b671..8f2b555f9e9 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTableOperationDispatcher.java @@ -59,6 +59,7 @@ public static void initialize() throws IOException { doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + GravitinoEnv.getInstance().setSchemaDispatcher(schemaOperationDispatcher); } @Test diff --git a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java index ccf55f1a19d..2e34a024b81 100644 --- a/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java +++ b/core/src/test/java/com/datastrato/gravitino/catalog/TestTopicOperationDispatcher.java @@ -52,6 +52,7 @@ public static void initialize() throws IOException { doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); GravitinoEnv.getInstance().setLockManager(new LockManager(config)); + GravitinoEnv.getInstance().setSchemaDispatcher(schemaOperationDispatcher); } @Test