-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HUDI-6872] Simplify Out Of Box Schema Evolution Functionality #9743
[HUDI-6872] Simplify Out Of Box Schema Evolution Functionality #9743
Conversation
Hello @jonvex, a little curious as to why we are maintaining so 2 implementations of schema evolution.
I believe it's a little confusing for users (especially new users) |
@voonhous there is also |
Can we discuss this as a community in email thread? Let people share ideas and needs around schema evolution and enforcement? |
…a and back, to standardize the ordering of null going first in unions. I tried to use a method so that ordering didn't matter, but the MDT uses a trick to get around union only allowing 2 values that was causing my first approach to fail
...lient/hudi-client-common/src/main/java/org/apache/hudi/common/table/log/CachingIterator.java
Outdated
Show resolved
Hide resolved
...lient/hudi-client-common/src/main/java/org/apache/hudi/common/table/log/CachingIterator.java
Outdated
Show resolved
Hide resolved
...hudi-client-common/src/main/java/org/apache/hudi/common/table/log/HoodieFileSliceReader.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/avro/AvroCastingGenericRecord.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/avro/AvroCastingGenericRecord.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/avro/AvroCastingGenericRecord.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/avro/AvroCastingGenericRecord.java
Outdated
Show resolved
Hide resolved
@hudi-bot run azure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on the patch man.
btw, can you do the following as well.
- Check for meta sync flows to see how schema change is deduced. We have optimization where in we trigger remote sync only incase of partitions add/remove and during schema changes. So, lets ensure we are good on that part.
- Also, did we test all the new behavior w/ diff meta syncs (like hive, presto, trino etc). I am sure we would have tested w/ spark. but did you get a chance to test our other query engines.
- Also, lets ensure we do a round of testing w/ spark-sql. Most common schema evolutions come from this writer.
- After evolving the schema, if we do savepoint and restore to a older commit, I assume the table will still be intact. i.e it might go back to older schema. and is readable.
- I am half way through reviewing the patch. But tell me something. if a commit which is trying to evolve the schema failed just before creating the completed commit metadata in the timeline, we are still intact right. there is complete isolation and a new writer is not impacted by it. i.e. it should not see the evolved schema at all at any point in time. For eg, incase its a MOR table, all log blocks rely on table scheme and not on previous log file schema. just calling out an example.
- Can you write a mini RFC may be on this patch. Even if it covers the current state of things, and flow of schemas and evolution for developers to understand the end to end flows, it would be great. Or you can publish it as a hudi blog as well. whatever works. For eg, where the schema reconciliation comes into play, what is reconcile, what happens in write handles, how partial file groups are updated when schema evols or how read happens w/ diff file groups in diff schemas etc.
- Can you call out how does time travel and incremental queries behave w/ schema evolution.
for eg:
C1: 5 col schema (V1)
C2: 6 col schema (V2)
C3: 6 col schema (V2) //no change in schema
C4: 7 col schema (V3)
for a time travel query with commit time C2, what does the schema look like? Is it V2 or V3 (assuming c4 is complete) when the query is issued.
and for an incremental query
for a. until C1: is it V1?
b. begin C1, end C2: is it V2?
c. being C1, end C4: is it V3?
hudi-common/src/main/java/org/apache/hudi/avro/AvroSchemaUtils.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieCommonConfig.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieActiveTimeline.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/internal/schema/utils/AvroSchemaEvolutionUtils.java
Show resolved
Hide resolved
...scala/org/apache/spark/sql/execution/datasources/parquet/HoodieParquetFileFormatHelper.scala
Show resolved
Hide resolved
hudi-utilities/src/main/java/org/apache/hudi/utilities/schema/LazyCastingIterator.java
Show resolved
Hide resolved
...scala/org/apache/spark/sql/execution/datasources/parquet/HoodieParquetFileFormatHelper.scala
Show resolved
Hide resolved
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java
Outdated
Show resolved
Hide resolved
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java
Show resolved
Hide resolved
hudi-utilities/src/main/java/org/apache/hudi/utilities/streamer/StreamSync.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieAvroDataBlock.java
Show resolved
Hide resolved
if (schema.getType() == Schema.Type.NULL) { | ||
return schema; | ||
} | ||
return convert(convert(schema), schema.getFullName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious to know why do we need two convertions. can't we directly fix or create AvorSchema by fixing the null ordering
public static Schema reconcileNullability(Schema sourceSchema, Schema targetSchema, Map<String, String> opts) { | ||
if (sourceSchema.getFields().isEmpty() || targetSchema.getFields().isEmpty()) { | ||
public static Schema reconcileSchemaRequirements(Schema sourceSchema, Schema targetSchema, Map<String, String> opts) { | ||
if (sourceSchema.getType() == Schema.Type.NULL || sourceSchema.getFields().isEmpty() || targetSchema.getFields().isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if source schema fields are empty, shouldn't we be returning targetSchema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonvex : do you have any pointers here
...spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala
Show resolved
Hide resolved
@@ -545,33 +552,37 @@ class HoodieSparkSqlWriterInternal { | |||
latestTableSchemaOpt: Option[Schema], | |||
internalSchemaOpt: Option[InternalSchema], | |||
opts: Map[String, String]): Schema = { | |||
val addNullForDeletedColumns = opts.getOrDefault(DataSourceWriteOptions.ADD_NULL_FOR_DELETED_COLUMNS.key(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we definitely need good docs for this method. lets enhance docs in L545. even having illustrative examples is totally fine. We have been soft/low key on schema evolution in general. lets button up and ensure we get it right this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the schema handling is grown up to be sizable now. Lets move it to a separate static class instead of adding everything to HoodieSparkSqlWrtier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am moving deduceWriterSchema() to HoodieSchemaUtils
...spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala
Show resolved
Hide resolved
...spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/internal/schema/utils/AvroSchemaEvolutionUtils.java
Show resolved
Hide resolved
|Table's schema ${latestTableSchema.toString(true)} | ||
|""".stripMargin) | ||
throw new SchemaCompatibilityException("Incoming batch schema is not compatible with the table's one") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, can you throw light on why we don't call AvroSchemaEvolutionUtils.reconcileSchema(canonicalizedSourceSchema, latestTableSchema)
in else block in L 658.
ie. when reconcile schema is set to false, and AVRO_SCHEMA_VALIDATE_ENABLE is set to true, looks like we don't ever call AvroSchemaEvolutionUtils.reconcileSchema(canonicalizedSourceSchema, latestTableSchema) .
also, curious to know whats the diff b/w AvroSchemaEvolutionUtils.reconcileSchema(canonicalizedSourceSchema, latestTableSchema) and HoodieSparkSqlWriter.canonicalizeSchema()
bcoz, both takes in both source schema and table schema as args
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonvex ping
00361ee
to
16128cd
Compare
@hudi-bot run azure |
…e#9743) Change how out of the box schema evolution works so it is easier to understand both by users and Hudi developers. Things you can't do: - Reorder columns - add new meta columns to nested structs Support being added OOB: - New fields can be added to the end of the schema or to the end of nested structs. Those fields will be in the schema of any future write. - Fields in the latest table schema that are missing from the incoming schema will be added to the incoming data with null values. - Type Promotion - Promotions work on complex types such as arrays or maps as well Promotions: int is promotable to long, float, double, or string long is promotable to float, double, or string float is promotable to double or string string is promotable to bytes bytes is promotable to string Rules: - If the incoming schema has a column that is promoted from the table schema's column type, the field will be the promoted type in the tables schema from now on - If the incoming schema has a column that is demoted from the table schema's column type, the incoming batch will have it's data promoted to the incoming schema
Apologies for necro-ing this PR. I was revisiting this PR today and noticed that using Hudi's comprehensive schema evolution using that is managed via Hudi's i.e. dataframe.write.format("hudi").options(...).mode(...).save(basePath) Where the options require these 2 confguration:
This means that we can still make use of Hudi comprehensive schema evolution to perform schema evolution. This behaviour is consistent with what we used to have in the past, which is good! However, this means of using schema evolution is not really documented and I am wondering if the community has any plans to ensure that the end-to-end flow for this use-case is error-free. For now, there are 2 entrypoints for Spark in which Hudi comprehensive schema evolution can be done. One, via Spark-SQL, and the other via Dataframe API as i described above just now. Considering the case where tables are sync-ed to a hive-catalogue, which is one of the more common use cases. Spark-SQL current does this via However, when Dataframe API, especially Deltrastreamer, hive-sync is done using Referring to the code below, if one does a Hudi comprehensive schema evolution outside of the scope defined here, hive-sync will fail, although UPSERT succeeds. hudi/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java Lines 424 to 439 in f0356de
Questions
CC @TengHuo |
Sorry for necro-ing this MR again. Complement about this question,
This issue mainly impact the pipelines of And add a little bit more about the current As we understand, Thus, in the method For solving this issue, we are thinking about utilising the information from |
Change Logs
Change how out of the box schema evolution works so it is easier to understand both by users and Hudi developers.
Things you can't do:
Added and Dropped Columns
Type Promotion
Promotions work on complex types such as arrays or maps as well
Promotions:
Rules:
Impact
Change how out of the box schema evolution works so it is easier to understand both by users and Hudi developers.
Risk level (write none, low medium or high below)
High
lots of testing has been done, including performance testing on tpcds 1tb to ensure MOR avro log block reading has not been degraded
Documentation Update
jira
pr
Contributor's checklist