jonvex commented on code in PR #13741:
URL: https://github.com/apache/hudi/pull/13741#discussion_r2298296150
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/HoodieSparkIndexClient.java:
##########
@@ -113,9 +115,13 @@ private void createRecordIndex(HoodieTableMetaClient
metaClient, String userInde
throw new HoodieMetadataIndexException("Index already exists: " +
userIndexName);
}
+ Map<String, String> overrideOpts = new HashMap<>();
Review Comment:
Use singletonlist and emptylist instead
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestRecordLevelIndex.scala:
##########
Review Comment:
Also add some validations to `TestPartitionedRecordLevelIndex`
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/TestHoodieTableMetaClient.java:
##########
@@ -276,7 +276,7 @@ void testDeleteDefinition() throws IOException {
assertTrue(metaClient.getIndexForMetadataPartition(indexName).isPresent());
assertTrue(metaClient.getStorage().exists(new
StoragePath(metaClient.getIndexDefinitionPath())));
metaClient.deleteIndexDefinition(indexName);
- assertTrue(metaClient.getIndexMetadata().isEmpty());
Review Comment:
assertTrue(metaClient.getIndexMetadata().isPresent());
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:
##########
@@ -656,16 +656,7 @@ public static <R> HoodieRecord<R>
createNewTaggedHoodieRecord(HoodieRecord<R> ol
public static void register(HoodieTableMetaClient metaClient,
HoodieIndexDefinition indexDefinition) {
LOG.info("Registering index {} of using {}",
indexDefinition.getIndexName(), indexDefinition.getIndexType());
// build HoodieIndexMetadata and then add to index definition file
- boolean indexDefnUpdated =
metaClient.buildIndexDefinition(indexDefinition);
- if (indexDefnUpdated) {
- String indexMetaPath = metaClient.getIndexDefinitionPath();
- // update table config if necessary
- if
(!metaClient.getTableConfig().getProps().containsKey(HoodieTableConfig.RELATIVE_INDEX_DEFINITION_PATH.key())
- ||
!metaClient.getTableConfig().getRelativeIndexDefinitionPath().isPresent()) {
-
metaClient.getTableConfig().setValue(HoodieTableConfig.RELATIVE_INDEX_DEFINITION_PATH,
FSUtils.getRelativePartitionPath(metaClient.getBasePath(), new
StoragePath(indexMetaPath)));
- HoodieTableConfig.update(metaClient.getStorage(),
metaClient.getMetaPath(), metaClient.getTableConfig().getProps());
- }
- }
+ metaClient.buildIndexDefinition(indexDefinition);
Review Comment:
If we get rid of everything here, then we should just get rid of the static
method entirely because all it is doing is invoking this on the meta client
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/HoodieSparkIndexClient.java:
##########
@@ -194,13 +200,13 @@ public void drop(HoodieTableMetaClient metaClient, String
indexName, boolean ign
Option<HoodieIndexDefinition> indexDefinitionOpt =
metaClient.getIndexMetadata()
.map(HoodieIndexMetadata::getIndexDefinitions)
.map(definition -> definition.get(indexName));
- try (SparkRDDWriteClient writeClient = getWriteClient(metaClient,
indexDefinitionOpt, Option.empty())) {
+ try (SparkRDDWriteClient writeClient = getWriteClient(metaClient,
indexDefinitionOpt, Option.empty(), Collections.emptyMap())) {
writeClient.dropIndex(Collections.singletonList(indexName));
}
}
private SparkRDDWriteClient getWriteClient(HoodieTableMetaClient metaClient,
Option<HoodieIndexDefinition> indexDefinitionOpt,
- Option<String> indexTypeOpt) {
+ Option<String> indexTypeOpt,
Map<String, String> overrideConfigs) {
Review Comment:
no need for "override" in the name
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/RecordLevelIndexTestBase.scala:
##########
Review Comment:
revert changes to this file
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -259,6 +260,13 @@ public boolean buildIndexDefinition(HoodieIndexDefinition
indexDefinition) {
}
if (updateIndexDefn) {
writeIndexMetadataToStorage();
+ String indexMetaPath = getIndexDefinitionPath();
+ // update table config if necessary
Review Comment:
why wasn't this needed before?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -476,7 +478,9 @@ private boolean initializeFromFilesystem(String
dataTableInstantTime, List<Metad
initializeFilegroupsAndCommit(partitionType,
COLUMN_STATS.getPartitionPath(), fileGroupCountAndRecordsPair,
instantTimeForPartition, colStatsColumnsAndRecord.getKey());
break;
case RECORD_INDEX:
-
initializeFilegroupsAndCommitToRecordIndexPartition(instantTimeForPartition,
lazyLatestMergedPartitionFileSliceList);
+ boolean isPartitioned =
dataWriteConfig.isPartitionedRecordIndexEnabled();
+ createRecordIndexDefinition(dataMetaClient,
Collections.singletonMap(HoodieRecordIndex.IS_PARTITIONED_OPTION,
String.valueOf(isPartitioned)));
Review Comment:
can we pull this into `initializeFilegroupsAndCommitToRecordIndexPartition`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
