diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java index 7b88a0ab979b..6266d965fd4b 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java @@ -146,21 +146,24 @@ public static String[] extractRecordKeysByFields(String recordKey, List public static String getRecordKey(GenericRecord record, List recordKeyFields, boolean consistentLogicalTimestampEnabled) { boolean keyIsNullEmpty = true; StringBuilder recordKey = new StringBuilder(); - for (String recordKeyField : recordKeyFields) { + for (int i = 0; i < recordKeyFields.size(); i++) { + String recordKeyField = recordKeyFields.get(i); String recordKeyValue = HoodieAvroUtils.getNestedFieldValAsString(record, recordKeyField, true, consistentLogicalTimestampEnabled); if (recordKeyValue == null) { - recordKey.append(recordKeyField + DEFAULT_COMPOSITE_KEY_FILED_VALUE + NULL_RECORDKEY_PLACEHOLDER + DEFAULT_RECORD_KEY_PARTS_SEPARATOR); + recordKey.append(recordKeyField).append(DEFAULT_COMPOSITE_KEY_FILED_VALUE).append(NULL_RECORDKEY_PLACEHOLDER); } else if (recordKeyValue.isEmpty()) { - recordKey.append(recordKeyField + DEFAULT_COMPOSITE_KEY_FILED_VALUE + EMPTY_RECORDKEY_PLACEHOLDER + DEFAULT_RECORD_KEY_PARTS_SEPARATOR); + recordKey.append(recordKeyField).append(DEFAULT_COMPOSITE_KEY_FILED_VALUE).append(EMPTY_RECORDKEY_PLACEHOLDER); } else { - recordKey.append(recordKeyField + DEFAULT_COMPOSITE_KEY_FILED_VALUE + recordKeyValue + DEFAULT_RECORD_KEY_PARTS_SEPARATOR); + recordKey.append(recordKeyField).append(DEFAULT_COMPOSITE_KEY_FILED_VALUE).append(recordKeyValue); keyIsNullEmpty = false; } + if (i != recordKeyFields.size() - 1) { + recordKey.append(DEFAULT_RECORD_KEY_PARTS_SEPARATOR); + } } - recordKey.deleteCharAt(recordKey.length() - 1); if (keyIsNullEmpty) { throw new HoodieKeyException("recordKey values: \"" + recordKey + "\" for fields: " - + recordKeyFields.toString() + " cannot be entirely null or empty."); + + recordKeyFields + " cannot be entirely null or empty."); } return recordKey.toString(); } @@ -172,20 +175,27 @@ public static String getRecordPartitionPath(GenericRecord record, List p } StringBuilder partitionPath = new StringBuilder(); - for (String partitionPathField : partitionPathFields) { + for (int i = 0; i < partitionPathFields.size(); i++) { + String partitionPathField = partitionPathFields.get(i); String fieldVal = HoodieAvroUtils.getNestedFieldValAsString(record, partitionPathField, true, consistentLogicalTimestampEnabled); if (fieldVal == null || fieldVal.isEmpty()) { - partitionPath.append(hiveStylePartitioning ? partitionPathField + "=" + HUDI_DEFAULT_PARTITION_PATH - : HUDI_DEFAULT_PARTITION_PATH); + if (hiveStylePartitioning) { + partitionPath.append(partitionPathField).append("="); + } + partitionPath.append(HUDI_DEFAULT_PARTITION_PATH); } else { if (encodePartitionPath) { fieldVal = PartitionPathEncodeUtils.escapePathName(fieldVal); } - partitionPath.append(hiveStylePartitioning ? partitionPathField + "=" + fieldVal : fieldVal); + if (hiveStylePartitioning) { + partitionPath.append(partitionPathField).append("="); + } + partitionPath.append(fieldVal); + } + if (i != partitionPathFields.size() - 1) { + partitionPath.append(DEFAULT_PARTITION_PATH_SEPARATOR); } - partitionPath.append(DEFAULT_PARTITION_PATH_SEPARATOR); } - partitionPath.deleteCharAt(partitionPath.length() - 1); return partitionPath.toString(); } diff --git a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestComplexKeyGenerator.java b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestComplexKeyGenerator.java index 296cf3d6e0db..2fa09861d25c 100644 --- a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestComplexKeyGenerator.java +++ b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestComplexKeyGenerator.java @@ -78,7 +78,7 @@ public void testNullPartitionPathFields() { @Test public void testNullRecordKeyFields() { GenericRecord record = getRecord(); - Assertions.assertThrows(StringIndexOutOfBoundsException.class, () -> { + Assertions.assertThrows(HoodieKeyException.class, () -> { ComplexKeyGenerator keyGenerator = new ComplexKeyGenerator(getPropertiesWithoutRecordKeyProp()); keyGenerator.getRecordKey(record); }); diff --git a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestGlobalDeleteRecordGenerator.java b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestGlobalDeleteRecordGenerator.java index df69279cc89f..4c9fc1c9ddaa 100644 --- a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestGlobalDeleteRecordGenerator.java +++ b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestGlobalDeleteRecordGenerator.java @@ -62,7 +62,7 @@ private TypedProperties getProps() { @Test public void testNullRecordKeyFields() { GenericRecord record = getRecord(); - Assertions.assertThrows(StringIndexOutOfBoundsException.class, () -> { + Assertions.assertThrows(HoodieKeyException.class, () -> { BaseKeyGenerator keyGenerator = new GlobalDeleteKeyGenerator(getPropertiesWithoutRecordKeyProp()); keyGenerator.getRecordKey(record); }); diff --git a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestNonpartitionedKeyGenerator.java b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestNonpartitionedKeyGenerator.java index fb740d00e2a5..187f96197b1d 100644 --- a/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestNonpartitionedKeyGenerator.java +++ b/hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/keygen/TestNonpartitionedKeyGenerator.java @@ -69,7 +69,7 @@ private TypedProperties getWrongRecordKeyFieldProps() { @Test public void testNullRecordKeyFields() { GenericRecord record = getRecord(); - Assertions.assertThrows(StringIndexOutOfBoundsException.class, () -> { + Assertions.assertThrows(HoodieKeyException.class, () -> { BaseKeyGenerator keyGenerator = new NonpartitionedKeyGenerator(getPropertiesWithoutRecordKeyProp()); keyGenerator.getRecordKey(record); }); diff --git a/hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestDataSourceDefaults.scala b/hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestDataSourceDefaults.scala index a2598c766b19..784ddd6c883b 100644 --- a/hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestDataSourceDefaults.scala +++ b/hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/TestDataSourceDefaults.scala @@ -262,7 +262,7 @@ class TestDataSourceDefaults extends ScalaAssertionSupport { } // Record's key field not specified - assertThrows(classOf[StringIndexOutOfBoundsException]) { + assertThrows(classOf[HoodieKeyException]) { val props = new TypedProperties() props.setProperty(DataSourceWriteOptions.PARTITIONPATH_FIELD.key, "partitionField") val keyGen = new ComplexKeyGenerator(props) @@ -494,7 +494,7 @@ class TestDataSourceDefaults extends ScalaAssertionSupport { val props = new TypedProperties() props.setProperty(DataSourceWriteOptions.PARTITIONPATH_FIELD.key, "partitionField") - assertThrows(classOf[StringIndexOutOfBoundsException]) { + assertThrows(classOf[HoodieKeyException]) { new GlobalDeleteKeyGenerator(props).getRecordKey(baseRecord) } }