hudi-agent commented on code in PR #19317: URL: https://github.com/apache/hudi/pull/19317#discussion_r3715349920
########## hudi-common/src/main/java/org/apache/hudi/metadata/VectorPostingPrefixRawKey.java: ########## @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.metadata; + +import lombok.Value; + +/** + * Raw key prefix for vector posting scans. + */ +@Value +public class VectorPostingPrefixRawKey implements RawKey { + + int generationId; + int clusterId; + Integer shardId; + + @Override + public String encode() { + return shardId == null + ? VectorIndexMetadataKey.postingPrefix(generationId, clusterId, 0).substring(0, 9) Review Comment: 🤖 nit: the bare `9` here (and at `HoodieTableMetadataUtil.getVectorIndexPostingPrefix(int, int)`) is easy to miss if the binary key layout ever changes — could you expose a named constant like `POSTING_CLUSTER_PREFIX_BYTES` from `VectorIndexMetadataKey`, or add a `postingClusterPrefix(generation, clusterId)` helper to keep the magic in one place? <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java: ########## @@ -253,6 +300,484 @@ protected HoodieMetadataPayload(String key, HoodieSecondaryIndexInfo secondaryIn this(key, MetadataPartitionType.SECONDARY_INDEX.getRecordType(), null, null, null, null, secondaryIndexMetadata, secondaryIndexMetadata.getIsDeleted()); } + protected HoodieMetadataPayload(String key, Object vectorIndexInfo) { + this.key = key; + this.type = MetadataPartitionType.VECTOR_INDEX.getRecordType(); + this.vectorIndexMetadata = vectorIndexInfo; + this.isDeletedRecord = vectorIndexInfo instanceof HoodieVectorIndexTombstone; Review Comment: 🤖 Setting `isDeletedRecord = true` for any `HoodieVectorIndexTombstone` means `getInsertValue()` returns `Option.empty()` for tombstones, so the tombstone payload built in `createVectorIndexPostingDeleteRecord` (with its `deleteInstant`/`deleteReason`) is never serialized — deletes degrade to plain delete markers and that provenance is dropped. Is that intended, given the schema models `HoodieVectorIndexTombstone` as a distinct union member with those fields? If tombstones are meant to be persisted and read back, this would need `isDeletedRecord = false` plus tombstone-aware handling in `constructMetadataPayload`/`combineMetadataPayloads`. <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java: ########## @@ -210,6 +256,7 @@ public class HoodieMetadataPayload implements HoodieRecordPayload<HoodieMetadata protected HoodieMetadataColumnStats columnStatMetadata = null; protected HoodieRecordIndexInfo recordIndexMetadata; protected HoodieSecondaryIndexInfo secondaryIndexMetadata; + protected Object vectorIndexMetadata; Review Comment: 🤖 nit: the other metadata fields (`HoodieMetadataBloomFilter`, `HoodieSecondaryIndexInfo`, etc.) are all strongly typed — have you considered a common marker interface for the vector payloads so `vectorIndexMetadata` doesn't have to be `Object`? That would save callers from unchecked casts. <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java: ########## @@ -253,6 +300,484 @@ protected HoodieMetadataPayload(String key, HoodieSecondaryIndexInfo secondaryIn this(key, MetadataPartitionType.SECONDARY_INDEX.getRecordType(), null, null, null, null, secondaryIndexMetadata, secondaryIndexMetadata.getIsDeleted()); } + protected HoodieMetadataPayload(String key, Object vectorIndexInfo) { + this.key = key; + this.type = MetadataPartitionType.VECTOR_INDEX.getRecordType(); + this.vectorIndexMetadata = vectorIndexInfo; + this.isDeletedRecord = vectorIndexInfo instanceof HoodieVectorIndexTombstone; + } + + /** + * Create the singleton reader-visible generation pointer. + */ + public static HoodieRecord<HoodieMetadataPayload> createVectorIndexActiveManifestRecord( + Integer activeGeneration, String metadataPartitionPath) { + String recordKey = VectorIndexMetadataKey.activeManifest(); + HoodieVectorIndexActiveManifest manifest = new HoodieVectorIndexActiveManifest(1, activeGeneration); + return new HoodieAvroRecord<>( + new HoodieKey(recordKey, metadataPartitionPath), + new HoodieMetadataPayload(recordKey, manifest)); + } + + /** + * Create the generation-one centroid record for the given index partition. + */ + public static HoodieRecord<HoodieMetadataPayload> createVectorIndexCentroidsRecord( + ByteBuffer centroidBytes, String partitionPath) { + HoodieVectorIndexCentroids centroids = new HoodieVectorIndexCentroids( + ByteBuffer.allocate(0), + centroidBytes, + ByteBuffer.allocate(0)); + String recordKey = VectorIndexMetadataKey.centroids(1, 0); + HoodieMetadataPayload payload = new HoodieMetadataPayload(recordKey, centroids); + HoodieKey key = new HoodieKey(recordKey, partitionPath); + return new HoodieAvroRecord<>(key, payload); + } + + public static HoodieRecord<HoodieMetadataPayload> createVectorIndexCentroidsRecord( + int generation, + int chunk, + ByteBuffer clusterIds, + ByteBuffer centroidBytes, + ByteBuffer clusterRadii, + String partitionPath) { + String recordKey = VectorIndexMetadataKey.centroids(generation, chunk); + HoodieVectorIndexCentroids centroids = new HoodieVectorIndexCentroids( + clusterIds, centroidBytes, clusterRadii); + return new HoodieAvroRecord<>( + new HoodieKey(recordKey, partitionPath), + new HoodieMetadataPayload(recordKey, centroids)); + } + + /** + * Create the generation-one quantizer metadata record for the given index partition. + */ + public static HoodieRecord<HoodieMetadataPayload> createVectorIndexQuantizerMetadataRecord( + String quantizerType, + int quantizedCodeBytes, + long randomSeed, + boolean assumeNormalized, + String partitionPath) { + return createVectorIndexQuantizerMetadataRecord( + quantizerType, + quantizedCodeBytes, + 1, + randomSeed, + assumeNormalized, + partitionPath); + } + + public static HoodieRecord<HoodieMetadataPayload> createVectorIndexQuantizerMetadataRecord( + String quantizerType, + int quantizedCodeBytes, + int rabitqBits, + long randomSeed, Review Comment: 🤖 nit: could you either pass `quantizedCodeBytes`, `rabitqBits`, and `assumeNormalized` through to the delegate, or remove them from this overload's signature? Right now the overload accepts those params and then silently drops all three, which will confuse any caller who passes non-default values. <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> -- 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]
