the-other-tim-brown commented on code in PR #14382:
URL: https://github.com/apache/hudi/pull/14382#discussion_r2593093624
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/schema/SchemaProvider.java:
##########
@@ -29,9 +29,9 @@ public abstract class SchemaProvider implements Serializable {
private static final long serialVersionUID = 1L;
- public abstract Schema getSourceSchema();
+ public abstract HoodieSchema getSourceSchema();
- public Schema getTargetSchema() {
+ public HoodieSchema getTargetSchema() {
Review Comment:
Since the user can provide there own implementation of SchemaProvider, we
will want to change all the callers to operate on the HoodieSchema while
allowing the user provided code to still work as expected. You can follow the
approach we used for the spark based utilities:
1. Rename these methods to `getHoodieSourceSchema` and
`getHoodieTargetSchema` using the IDE's refactoring tools.
2. Add back the original `getSourceSchema` and `getTargetSchema` that return
avro schema. Mark them as deprecated
3. Update this base class to delegate to the older methods.
See this class for reference:
https://github.com/apache/hudi/blob/master/hudi-utilities/src/main/java/org/apache/hudi/utilities/schema/SchemaProvider.java
##########
hudi-hadoop-mr/src/test/java/org/apache/hudi/hadoop/realtime/TestHoodieRealtimeRecordReader.java:
##########
@@ -194,7 +196,7 @@ private void
testReaderInternal(ExternalSpillableMap.DiskMapType diskMapType,
boolean isCompressionEnabled,
boolean partitioned,
HoodieLogBlock.HoodieLogBlockType logBlockType) throws Exception {
// initial commit
- Schema schema =
HoodieAvroUtils.addMetadataFields(SchemaTestUtil.getEvolvedSchema());
+ HoodieSchema schema =
HoodieSchemaUtils.addMetadataFields(SchemaTestUtil.getEvolvedSchema(), false);
Review Comment:
I see a lot of places we add `false`, can you add a helper that defaults to
`false` like the HoodieAvroUtils does today?
##########
hudi-hadoop-mr/src/test/java/org/apache/hudi/hadoop/hive/TestHoodieCombineHiveInputFormat.java:
##########
@@ -132,7 +134,7 @@ public void testInternalSchemaCacheForMR() throws Exception
{
HoodieCommitMetadata commitMetadataOne =
CommitUtils.buildMetadata(Collections.emptyList(), Collections.emptyMap(),
Option.empty(), WriteOperationType.UPSERT,
schema.toString(), HoodieTimeline.COMMIT_ACTION);
// mock the latest schema to the commit metadata
- InternalSchema internalSchema =
InternalSchemaConverter.convert(HoodieSchema.fromAvroSchema(schema));
+ InternalSchema internalSchema =
InternalSchemaConverter.convert(HoodieSchema.fromAvroSchema(schema.toAvroSchema()));
Review Comment:
```suggestion
InternalSchema internalSchema = InternalSchemaConverter.convert(schema));
```
--
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]
