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

[#4305] improvement(core): Improved the way of fill parentEntityId in POBuilder #6114

Merged
merged 5 commits into from
Jan 7, 2025
Merged

[#4305] improvement(core): Improved the way of fill parentEntityId in POBuilder #6114

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6632784
[CORE]Improved the way of fill parentEntityId in POBuilder
Jan 6, 2025
c5fc23a
extract method getParentEntityIdsByNamespace into CommonMetaService
Jan 6, 2025
e232f6d
fix spotless run failed
Jan 6, 2025
f31d537
change initial array length to namespace's length
Jan 6, 2025
f404e42
Merge branch 'main' into main
luoshipeng Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/CommonMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,29 @@ public Long getParentEntityIdByNamespace(Namespace namespace) {
"Parent entity id should not be null and should be greater than 0.");
return parentEntityId;
}

public Long[] getParentEntityIdsByNamespace(Namespace namespace) {
Preconditions.checkArgument(
Copy link
Contributor

Choose a reason for hiding this comment

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

The length of schema won't be 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so this is the same problem as the last one?

Copy link
Contributor

Choose a reason for hiding this comment

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

If namespace belongs to a schema, the length of namespace will be 2 instead of 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. so if i update the array length to namespace's length, it will solve this problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to replace the similar code of ColumnMetaService?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked the code of ColumnMetaService, and not found the similar code. Did i miss that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ColumnPO get parentEntityId from TablePO, so i think there's no need to replace it

Copy link
Contributor

Choose a reason for hiding this comment

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

ColumnPO get parentEntityId from TablePO, so i think there's no need to replace it

OK. I got it.

!namespace.isEmpty() && namespace.levels().length <= 3,
"Namespace should not be empty and length should be less than or equal to 3.");
Long[] parentEntityIds = new Long[3];
Copy link
Contributor

Choose a reason for hiding this comment

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

The length should be the same as namespace's.

if (namespace.levels().length >= 1) {
parentEntityIds[0] =
MetalakeMetaService.getInstance().getMetalakeIdByName(namespace.level(0));
}

if (namespace.levels().length >= 2) {
parentEntityIds[1] =
CatalogMetaService.getInstance()
.getCatalogIdByMetalakeIdAndName(parentEntityIds[0], namespace.level(1));
}

if (namespace.levels().length >= 3) {
parentEntityIds[2] =
SchemaMetaService.getInstance()
.getSchemaIdByCatalogIdAndName(parentEntityIds[1], namespace.level(2));
}

return parentEntityIds;
}
}
26 changes: 5 additions & 21 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,26 +314,10 @@ public int deleteFilesetVersionsByRetentionCount(Long versionRetentionCount, int

private void fillFilesetPOBuilderParentEntityId(FilesetPO.Builder builder, Namespace namespace) {
NamespaceUtil.checkFileset(namespace);
Long parentEntityId = null;
for (int level = 0; level < namespace.levels().length; level++) {
String name = namespace.level(level);
switch (level) {
case 0:
parentEntityId = MetalakeMetaService.getInstance().getMetalakeIdByName(name);
builder.withMetalakeId(parentEntityId);
continue;
case 1:
parentEntityId =
CatalogMetaService.getInstance()
.getCatalogIdByMetalakeIdAndName(parentEntityId, name);
builder.withCatalogId(parentEntityId);
continue;
case 2:
parentEntityId =
SchemaMetaService.getInstance().getSchemaIdByCatalogIdAndName(parentEntityId, name);
builder.withSchemaId(parentEntityId);
break;
}
}
Long[] parentEntityIds =
CommonMetaService.getInstance().getParentEntityIdsByNamespace(namespace);
builder.withMetalakeId(parentEntityIds[0]);
builder.withCatalogId(parentEntityIds[1]);
builder.withSchemaId(parentEntityIds[2]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can change to this style for here and below.

    builder.withMetalakeId(parentEntityIds[0])
          .withCatalogId(parentEntityIds[1]);
          .withSchemaId(parentEntityIds[2]);

}
}
18 changes: 4 additions & 14 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,10 @@ ModelPO getModelPOById(Long modelId) {

private void fillModelPOBuilderParentEntityId(ModelPO.Builder builder, Namespace ns) {
NamespaceUtil.checkModel(ns);
String metalake = ns.level(0);
String catalog = ns.level(1);
String schema = ns.level(2);

Long metalakeId = MetalakeMetaService.getInstance().getMetalakeIdByName(metalake);
builder.withMetalakeId(metalakeId);

Long catalogId =
CatalogMetaService.getInstance().getCatalogIdByMetalakeIdAndName(metalakeId, catalog);
builder.withCatalogId(catalogId);

Long schemaId =
SchemaMetaService.getInstance().getSchemaIdByCatalogIdAndName(catalogId, schema);
builder.withSchemaId(schemaId);
Long[] parentEntityIds = CommonMetaService.getInstance().getParentEntityIdsByNamespace(ns);
builder.withMetalakeId(parentEntityIds[0]);
builder.withCatalogId(parentEntityIds[1]);
builder.withSchemaId(parentEntityIds[2]);
}

ModelPO getModelPOByIdentifier(NameIdentifier ident) {
Expand Down
20 changes: 4 additions & 16 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,21 +316,9 @@ public int deleteSchemaMetasByLegacyTimeline(Long legacyTimeline, int limit) {

private void fillSchemaPOBuilderParentEntityId(SchemaPO.Builder builder, Namespace namespace) {
NamespaceUtil.checkSchema(namespace);
Long parentEntityId = null;
for (int level = 0; level < namespace.levels().length; level++) {
String name = namespace.level(level);
switch (level) {
case 0:
parentEntityId = MetalakeMetaService.getInstance().getMetalakeIdByName(name);
builder.withMetalakeId(parentEntityId);
continue;
case 1:
parentEntityId =
CatalogMetaService.getInstance()
.getCatalogIdByMetalakeIdAndName(parentEntityId, name);
builder.withCatalogId(parentEntityId);
break;
}
}
Long[] parentEntityIds =
CommonMetaService.getInstance().getParentEntityIdsByNamespace(namespace);
builder.withMetalakeId(parentEntityIds[0]);
builder.withCatalogId(parentEntityIds[1]);
}
}
26 changes: 5 additions & 21 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,27 +253,11 @@ public int deleteTableMetasByLegacyTimeline(Long legacyTimeline, int limit) {

private void fillTablePOBuilderParentEntityId(TablePO.Builder builder, Namespace namespace) {
NamespaceUtil.checkTable(namespace);
Long parentEntityId = null;
for (int level = 0; level < namespace.levels().length; level++) {
String name = namespace.level(level);
switch (level) {
case 0:
parentEntityId = MetalakeMetaService.getInstance().getMetalakeIdByName(name);
builder.withMetalakeId(parentEntityId);
continue;
case 1:
parentEntityId =
CatalogMetaService.getInstance()
.getCatalogIdByMetalakeIdAndName(parentEntityId, name);
builder.withCatalogId(parentEntityId);
continue;
case 2:
parentEntityId =
SchemaMetaService.getInstance().getSchemaIdByCatalogIdAndName(parentEntityId, name);
builder.withSchemaId(parentEntityId);
break;
}
}
Long[] parentEntityIds =
CommonMetaService.getInstance().getParentEntityIdsByNamespace(namespace);
builder.withMetalakeId(parentEntityIds[0]);
builder.withCatalogId(parentEntityIds[1]);
builder.withSchemaId(parentEntityIds[2]);
}

private TablePO getTablePOBySchemaIdAndName(Long schemaId, String tableName) {
Expand Down
26 changes: 5 additions & 21 deletions core/src/main/java/org/apache/gravitino/storage/relational/service/TopicMetaService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,27 +154,11 @@ public TopicPO getTopicPOById(Long topicId) {

private void fillTopicPOBuilderParentEntityId(TopicPO.Builder builder, Namespace namespace) {
NamespaceUtil.checkTopic(namespace);
Long parentEntityId = null;
for (int level = 0; level < namespace.levels().length; level++) {
String name = namespace.level(level);
switch (level) {
case 0:
parentEntityId = MetalakeMetaService.getInstance().getMetalakeIdByName(name);
builder.withMetalakeId(parentEntityId);
continue;
case 1:
parentEntityId =
CatalogMetaService.getInstance()
.getCatalogIdByMetalakeIdAndName(parentEntityId, name);
builder.withCatalogId(parentEntityId);
continue;
case 2:
parentEntityId =
SchemaMetaService.getInstance().getSchemaIdByCatalogIdAndName(parentEntityId, name);
builder.withSchemaId(parentEntityId);
break;
}
}
Long[] parentEntityIds =
CommonMetaService.getInstance().getParentEntityIdsByNamespace(namespace);
builder.withMetalakeId(parentEntityIds[0]);
builder.withCatalogId(parentEntityIds[1]);
builder.withSchemaId(parentEntityIds[2]);
}

public TopicEntity getTopicByIdentifier(NameIdentifier identifier) {
Expand Down
Loading