yuqi1129 commented on code in PR #6752:
URL: https://github.com/apache/gravitino/pull/6752#discussion_r2019780983
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/TableColumnMapper.java:
##########
@@ -71,4 +71,7 @@ Long selectColumnIdByTableIdAndName(
@SelectProvider(type = TableColumnSQLProviderFactory.class, method =
"selectColumnPOById")
ColumnPO selectColumnPOById(@Param("columnId") Long columnId);
+
+ @SelectProvider(type = TableColumnSQLProviderFactory.class, method =
"listColumnPOsByColumnIds")
+ List<ColumnPO> listColumnPOsByTableIds(@Param("columnIds") List<Long>
columnIds);
Review Comment:
It's columnIds and I will modify it.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetadataObjectService.java:
##########
@@ -389,6 +261,51 @@ public static Map<Long, String>
getTableObjectsFullName(List<Long> tableIds) {
return tableIdAndNameMap;
}
+ /**
+ * Retrieves a map of column object IDs to their full names.
+ *
+ * @param columnsIds A list of column object IDs to fetch names for.
+ * @return A Map where the key is the column ID and the value is the column
full name. The map may
+ * contain null values for the names if its parent object is deleted.
Returns an empty map if
+ * no column objects are found for the given IDs. {@code @example} value
of table full name:
+ * "catalog1.schema1.table1.column1"
+ */
+ public static Map<Long, String> getColumnObjectsFullName(List<Long>
columnsIds) {
+ List<ColumnPO> columnPOs =
+ SessionUtils.getWithoutCommit(
+ TableColumnMapper.class, mapper ->
mapper.listColumnPOsByTableIds(columnsIds));
+
+ if (columnPOs == null || columnPOs.isEmpty()) {
+ return new HashMap<>();
+ }
+
+ List<Long> tableIds =
columnPOs.stream().map(ColumnPO::getTableId).collect(Collectors.toList());
+ Map<Long, String> tableIdAndNameMap = getTableObjectsFullName(tableIds);
+
+ HashMap<Long, String> columnIdAndNameMap = new HashMap<>();
+
+ columnPOs.forEach(
+ columnPO -> {
+ // since the table can be deleted, we need to check the null value,
+ // and when the table is deleted, we will set fullName of column to
+ // null
+ String tableName =
tableIdAndNameMap.getOrDefault(columnPO.getTableId(), null);
+ if (tableName == null) {
+ LOG.warn(
+ "The table '{}' of column '{}' may be deleted",
+ columnPO.getTableId(),
+ columnPO.getColumnId());
+ columnIdAndNameMap.put(columnPO.getColumnId(), null);
+ return;
Review Comment:
According to the comment and code logic, the table can be deleted when when
query the columns, we should ingore it. please see:
https://github.com/apache/gravitino/blob/2668aaf2bd01120318fb80f8dd010364cd330d0d/core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java#L238-L242
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java:
##########
@@ -132,4 +132,37 @@ public String selectColumnPOById(@Param("columnId") Long
columnId) {
+ " WHERE column_id = #{columnId} AND deleted_at = 0"
+ " ORDER BY table_version DESC LIMIT 1";
}
+
+ public String listColumnPOsByTableIds(@Param("columnIds") List<Long>
columnIds) {
+ return "<script>"
+ + " SELECT c.column_id AS columnId, c.column_name AS columnName,"
+ + " c.column_position AS columnPosition, c.metalake_id AS metalakeId,
c.catalog_id AS catalogId,"
+ + " c.schema_id AS schemaId, c.table_id AS tableId,"
+ + " c.table_version AS tableVersion, c.column_type AS columnType,"
+ + " c.column_comment AS columnComment, c.column_nullable AS nullable,"
+ + " c.column_auto_increment AS autoIncrement,"
+ + " c.column_default_value AS defaultValue, c.column_op_type AS
columnOpType,"
+ + " c.deleted_at AS deletedAt, c.audit_info AS auditInfo"
+ + " FROM "
+ + TableColumnMapper.COLUMN_TABLE_NAME
+ + " c "
+ + "JOIN ("
+ + " SELECT column_id, MAX(table_version) AS max_version"
+ + " FROM "
+ + TableColumnMapper.COLUMN_TABLE_NAME
+ + " WHERE column_id IN ("
+ + " <foreach collection='columnIds' item='columnId'
separator=','>"
+ + " #{columnId}"
+ + " </foreach>"
+ + " ) AND deleted_at = 0 GROUP BY column_id"
+ + ") latest "
+ + "ON c.column_id = latest.column_id AND c.table_version =
latest.max_version"
+ + " WHERE c.column_id IN ("
+ + "<foreach collection='columnIds' item='columnId' separator=','>"
+ + "#{columnId}"
+ + "</foreach>"
+ + ") "
+ + " AND c.deleted_at = 0"
+ + "</script>";
+ }
Review Comment:
`TagIT` has tests `tag.associatedObjects()` to cover this changes and It
should be work for H2 and PG as the IT has been run with backend `H2` and `PG`.
<img width="1274" alt="image"
src="https://github.com/user-attachments/assets/b73440d0-69dc-48ae-b0b4-9fc4248ba618";
/>
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/TagMetaService.java:
##########
@@ -230,20 +245,32 @@ public List<MetadataObject>
listAssociatedMetadataObjectsForTag(NameIdentifier t
mapper.listTagMetadataObjectRelsByMetalakeAndTagName(metalakeName, tagName));
List<MetadataObject> metadataObjects = Lists.newArrayList();
- for (TagMetadataObjectRelPO po : tagMetadataObjectRelPOs) {
- String fullName =
- MetadataObjectService.getMetadataObjectFullName(
- po.getMetadataObjectType(), po.getMetadataObjectId());
-
- // Metadata object may be deleted asynchronously when we query the
name, so it will return
- // null. We should skip this metadata object.
- if (fullName == null) {
- continue;
- }
-
- MetadataObject.Type type =
MetadataObject.Type.valueOf(po.getMetadataObjectType());
- metadataObjects.add(MetadataObjects.parse(fullName, type));
- }
+ tagMetadataObjectRelPOs.stream()
+ .collect(
+ Collectors.groupingBy(t ->
MetadataObject.Type.valueOf(t.getMetadataObjectType())))
+ .forEach(
+ (type, rels) -> {
+ List<Long> metadataObjectIds =
+ rels.stream()
+ .map(TagMetadataObjectRelPO::getMetadataObjectId)
+ .collect(Collectors.toList());
+ Map<Long, String> metadataObjectNames =
+ Optional.of(TYPE_FUNCTION_MAP.get(type))
+ .map(f -> f.apply(metadataObjectIds))
+ .orElseThrow(
+ () ->
+ new IllegalArgumentException(
+ "Unexpected to reach here for metadata
object type: " + type));
+
+ metadataObjectNames.forEach(
+ (id, fullName) -> {
+ // Metadata object may be deleted asynchronously when we
query the name, so it
+ // will return null, we should skip this metadata object.
+ if (fullName != null) {
+ metadataObjects.add(MetadataObjects.parse(fullName,
type));
+ }
+ });
+ });
Review Comment:
There exists UT that can covert it and I have checked several problems
during developing process through it, please see
TestTagMetaService#testListAssociatedMetadataObjectsForTag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
