hudi-agent commented on code in PR #19471:
URL: https://github.com/apache/hudi/pull/19471#discussion_r3712017687
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestPartitionBucketIndexSupport.scala:
##########
@@ -171,34 +169,84 @@ class TestPartitionBucketIndexSupport extends
TestBucketIndexSupport {
exprFilePathAnswerCheck(bucketIndexSupport, equalTo,
Set.apply(bucket4Id7FileName), allFileNames)
}
+ /**
+ * Every partition used by the other tests matches the expression, so all of
them carry the same
+ * bucket count and nothing there notices if a partition is hashed with the
wrong one. Pair a
+ * partition the expression matches, which gets [[EXPRESSION_BUCKET_NUMBER]]
buckets, with one it
+ * does not, which falls back to the table default, and check that a file is
only a candidate
+ * under the bucket count belonging to its own partition.
+ */
+ @Test
+ def testCandidateFilesUsePerPartitionBucketCount(): Unit = {
+ val configProperties = new TypedProperties()
+
configProperties.setProperty(HoodieIndexConfig.BUCKET_INDEX_HASH_FIELD.key, "A")
+ configProperties.setProperty(HoodieTableConfig.RECORDKEY_FIELDS.key, "A")
+ configProperties.setProperty(KeyGeneratorOptions.RECORDKEY_FIELD_NAME.key,
"A")
+
configProperties.setProperty(HoodieIndexConfig.BUCKET_INDEX_NUM_BUCKETS.key,
String.valueOf(DEFAULT_BUCKET_NUMBER))
+ metaClient.getTableConfig.setValue(HoodieTableConfig.CREATE_SCHEMA.key(),
avroSchemaStr)
+ val metadataConfig = HoodieMetadataConfig.newBuilder
+ .fromProperties(configProperties)
+ .enable(configProperties.getBoolean(HoodieMetadataConfig.ENABLE.key,
true)).build()
+ val bucketIndexSupport = new PartitionBucketIndexSupport(spark,
metadataConfig, metaClient)
+
+ val record = new GenericData.Record(schema.toAvroSchema)
+ record.put("A", "3")
+ val recordKey = new
NonpartitionedKeyGenerator(configProperties).getKey(record).getRecordKey
+ val bucketIdInExpressionPartition =
BucketIdentifier.getBucketId(recordKey, "A", EXPRESSION_BUCKET_NUMBER)
+ val bucketIdInDefaultPartition = BucketIdentifier.getBucketId(recordKey,
"A", DEFAULT_BUCKET_NUMBER)
+ // the two bucket counts have to disagree for this record, otherwise the
check below proves nothing
+ assert(bucketIdInExpressionPartition != bucketIdInDefaultPartition)
+
+ // the file id prefix carries a fresh uuid every call, so the same bucket
can be named twice
+ def fileNameForBucket(bucketId: Int): String =
FSUtils.makeBaseFileName("00000000000000000",
+ FSUtils.makeWriteToken(1, 0, 1),
BucketIdentifier.newBucketFileIdPrefix(bucketId) + "-0",
+ HoodieTableConfig.BASE_FILE_FORMAT.defaultValue.getFileExtension)
+
+ val expressionPartition = DEFAULT_PARTITION_PATH(0)
+ val defaultPartition = NON_MATCHING_PARTITION_PATH
+ val expectedFromExpressionPartition =
fileNameForBucket(bucketIdInExpressionPartition)
+ val expectedFromDefaultPartition =
fileNameForBucket(bucketIdInDefaultPartition)
+ // same bucket as the matching partition's candidate, but sitting in the
partition that hashes
+ // into the table default, so it must not be picked up
+ val decoyInDefaultPartition =
fileNameForBucket(bucketIdInExpressionPartition)
+
+ val input = Seq(
+ (Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(expressionPartition, Array())),
+ fileSlicesOf(expressionPartition,
Seq(expectedFromExpressionPartition))),
+ (Option.apply(new
BaseHoodieTableFileIndex.PartitionPath(defaultPartition, Array())),
+ fileSlicesOf(defaultPartition, Seq(expectedFromDefaultPartition,
decoyInDefaultPartition))))
+
+ val candidate = bucketIndexSupport.computeCandidateFileNames(fileIndex,
+ splitConjunctivePredicates(optimizeResolvedExpr("A = 3")), Seq(), input,
false)
+
+ assert(candidate.get.equals(Set.apply(expectedFromExpressionPartition,
expectedFromDefaultPartition)))
+ }
+
def exprFilePathAnswerCheck(bucketIndexSupport: PartitionBucketIndexSupport,
exprRaw: String, expectResult: Set[String],
allFileStatus: Set[String]): Unit = {
Review Comment:
🤖 nit: the comment attributes the non-determinism to UUIDs embedded in the
file names, but the actual issue is that `Set[String]` iteration order is
hash-dependent. Could you rephrase to something like `// Set iteration order is
non-deterministic; sort so the file→partition assignment is stable across
runs.`?
<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]
