yihua commented on code in PR #17599:
URL: https://github.com/apache/hudi/pull/17599#discussion_r2651904639
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/bootstrap/HoodieBootstrapSchemaProvider.java:
##########
@@ -45,11 +44,11 @@ public HoodieBootstrapSchemaProvider(HoodieWriteConfig
writeConfig) {
* @param partitions List of partitions with files within them
* @return Avro Schema
*/
- public final Schema getBootstrapSchema(HoodieEngineContext context,
List<Pair<String, List<HoodieFileStatus>>> partitions) {
+ public final HoodieSchema getBootstrapSchema(HoodieEngineContext context,
List<Pair<String, List<HoodieFileStatus>>> partitions) {
if (writeConfig.getSchema() != null) {
// Use schema specified by user if set
- Schema userSchema = new Schema.Parser().parse(writeConfig.getSchema());
- if (!HoodieAvroUtils.getNullSchema().equals(userSchema)) {
+ HoodieSchema userSchema = HoodieSchema.parse(writeConfig.getSchema());
+ if (!HoodieSchema.create(HoodieSchemaType.NULL).equals(userSchema)) {
Review Comment:
nit: reuse `HoodieSchema.NULL_SCHEMA`
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/index/TestHoodieIndexUtils.java:
##########
@@ -544,18 +520,17 @@ public void testIsEligibleForExpressionIndex() {
*/
@Test
public void testIsEligibleForExpressionIndexWithNullableFields() {
+ // An int with default 0 must have the int type defined first.
+ // If null is defined first, which HoodieSchema#createNullable does, an
error will be thrown
+ HoodieSchema nullableIntWithDefault =
HoodieSchema.createUnion(HoodieSchema.create(HoodieSchemaType.INT),
HoodieSchema.create(HoodieSchemaType.NULL));
Review Comment:
Does `HoodieSchema.createNullable` work in this case, instead of calling
`HoodieSchema.createUnion`?
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/index/TestHoodieIndexUtils.java:
##########
@@ -227,32 +218,31 @@ public void testValidateDataTypeForSecondaryIndex() {
@Test
public void testValidateDataTypeForSecondaryIndexWithLogicalTypes() {
// Supported logical types
- Schema timestampMillis =
LogicalTypes.timestampMillis().addToSchema(Schema.create(Schema.Type.LONG));
- Schema timestampMicros =
LogicalTypes.timestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
- Schema date =
LogicalTypes.date().addToSchema(Schema.create(Schema.Type.INT));
- Schema timeMillis =
LogicalTypes.timeMillis().addToSchema(Schema.create(Schema.Type.INT));
- Schema timeMicros =
LogicalTypes.timeMicros().addToSchema(Schema.create(Schema.Type.LONG));
-
+ HoodieSchema timestampMillis = HoodieSchema.createTimestampMillis();
+ HoodieSchema timestampMicros = HoodieSchema.createTimestampMicros();
+ HoodieSchema date = HoodieSchema.createDate();
+ HoodieSchema timeMillis = HoodieSchema.createTimeMillis();
+ HoodieSchema timeMicros = HoodieSchema.createTimeMicros();
+
// Unsupported logical types
- Schema decimal = LogicalTypes.decimal(10,
2).addToSchema(Schema.create(Schema.Type.BYTES));
- Schema uuid =
LogicalTypes.uuid().addToSchema(Schema.create(Schema.Type.STRING));
- Schema localTimestampMillis =
LogicalTypes.localTimestampMillis().addToSchema(Schema.create(Schema.Type.LONG));
- Schema localTimestampMicros =
LogicalTypes.localTimestampMicros().addToSchema(Schema.create(Schema.Type.LONG));
-
- Schema schemaWithLogicalTypes = SchemaBuilder.record("TestRecord")
- .fields()
+ HoodieSchema decimal = HoodieSchema.createDecimal(10, 2);
+ HoodieSchema uuid = HoodieSchema.createUUID();
+ HoodieSchema localTimestampMillis =
HoodieSchema.createLocalTimestampMillis();
+ HoodieSchema localTimestampMicros =
HoodieSchema.createLocalTimestampMicros();
+
+ HoodieSchema schemaWithLogicalTypes =
HoodieSchema.createRecord("TestRecord", null, null, Arrays.asList(
// Supported logical types
- .name("timestampMillisField").type(timestampMillis).noDefault()
- .name("timestampMicrosField").type(timestampMicros).noDefault()
- .name("dateField").type(date).noDefault()
- .name("timeMillisField").type(timeMillis).noDefault()
- .name("timeMicrosField").type(timeMicros).noDefault()
+ HoodieSchemaField.of("timestampMillisField", timestampMillis),
+ HoodieSchemaField.of("timestampMicrosField", timestampMicros),
+ HoodieSchemaField.of("dateField", date),
+ HoodieSchemaField.of("timeMillisField", timeMillis),
+ HoodieSchemaField.of("timeMicrosField", timeMicros),
// Unsupported logical types
- .name("decimalField").type(decimal).noDefault()
- .name("uuidField").type(uuid).noDefault()
-
.name("localTimestampMillisField").type(localTimestampMillis).noDefault()
-
.name("localTimestampMicrosField").type(localTimestampMicros).noDefault()
- .endRecord();
+ HoodieSchemaField.of("decimalField", decimal),
Review Comment:
Should we test both `FIXED` and `BYTES` for decimal?
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/index/TestHoodieIndexUtils.java:
##########
@@ -544,18 +520,17 @@ public void testIsEligibleForExpressionIndex() {
*/
@Test
public void testIsEligibleForExpressionIndexWithNullableFields() {
+ // An int with default 0 must have the int type defined first.
Review Comment:
Is this a restriction of Avro?
--
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]
