bvaradar commented on code in PR #14265:
URL: https://github.com/apache/hudi/pull/14265#discussion_r2543270952
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchemaField.java:
##########
@@ -245,36 +244,17 @@ public void testFieldToString() {
assertTrue(fieldString.contains("true")); // hasDefault
}
- @Test
- public void testFieldOrderHandling() {
- Schema stringSchema = Schema.create(Schema.Type.STRING);
-
- // Test different field orders
- Schema.Field ascendingField = new Schema.Field("asc", stringSchema, null,
null, Schema.Field.Order.ASCENDING);
- Schema.Field descendingField = new Schema.Field("desc", stringSchema,
null, null, Schema.Field.Order.DESCENDING);
- Schema.Field ignoreField = new Schema.Field("ignore", stringSchema, null,
null, Schema.Field.Order.IGNORE);
-
- HoodieSchemaField hoodieAsc = new HoodieSchemaField(ascendingField);
- HoodieSchemaField hoodieDesc = new HoodieSchemaField(descendingField);
- HoodieSchemaField hoodieIgnore = new HoodieSchemaField(ignoreField);
-
- assertEquals(HoodieFieldOrder.ASCENDING, hoodieAsc.order());
- assertEquals(HoodieFieldOrder.DESCENDING, hoodieDesc.order());
- assertEquals(HoodieFieldOrder.IGNORE, hoodieIgnore.order());
- }
-
@Test
public void testFieldCreationWithOrder() {
HoodieSchema stringSchema = HoodieSchema.create(HoodieSchemaType.STRING);
// Test creation with explicit order
Review Comment:
Same here.
##########
hudi-common/src/main/java/org/apache/hudi/common/schema/HoodieSchemaField.java:
##########
@@ -88,28 +88,13 @@ public static HoodieSchemaField fromAvroField(Schema.Field
avroField) {
* @return new HoodieSchemaField instance
*/
public static HoodieSchemaField of(String name, HoodieSchema schema, String
doc, Object defaultVal) {
- return of(name, schema, doc, defaultVal, HoodieFieldOrder.ASCENDING);
- }
-
- /**
- * Creates a new HoodieSchemaField with the specified properties, including
field order.
- *
- * @param name the name of the field
- * @param schema the schema of the field
- * @param doc the documentation string, can be null
- * @param defaultVal the default value, can be null
- * @param order the field order for sorting
- * @return new HoodieSchemaField instance
- */
- public static HoodieSchemaField of(String name, HoodieSchema schema, String
doc, Object defaultVal, HoodieFieldOrder order) {
ValidationUtils.checkArgument(name != null && !name.isEmpty(), "Field name
cannot be null or empty");
ValidationUtils.checkArgument(schema != null, "Field schema cannot be
null");
- ValidationUtils.checkArgument(order != null, "Field order cannot be null");
Schema avroSchema = schema.getAvroSchema();
ValidationUtils.checkState(avroSchema != null, "Schema's Avro schema
cannot be null");
- Schema.Field avroField = new Schema.Field(name, avroSchema, doc,
defaultVal, order.toAvroOrder());
+ Schema.Field avroField = new Schema.Field(name, avroSchema, doc,
defaultVal);
Review Comment:
Not passing the order here means there is delta between what the current
code and new code. Lets not do that. Lets keep the order as-is.
##########
hudi-common/src/test/java/org/apache/hudi/common/schema/TestHoodieSchema.java:
##########
@@ -638,11 +638,6 @@ public void testCreateRecordWithInvalidParameters() {
assertThrows(IllegalArgumentException.class, () -> {
HoodieSchema.createRecord("TestRecord", null, null, null);
}, "Should throw exception for null fields");
-
- // Test empty fields
Review Comment:
@the-other-tim-brown : If the empty list is a valid test, case, then can you
change it so that it is not throwing the exception instead of removing it.
--
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]
