You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/03/27 18:44:28 UTC
[GitHub] [incubator-hudi] prashantwason opened a new pull request #1457:
[HUDI-741] Added checks to validate Hoodie's schema evolution.
prashantwason opened a new pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457
## What is the purpose of the pull request
HUDI specific validation of schema evolution should ensure that a newer schema can be used for the dataset by checking that the data written using the old schema can be read using the new schema.
Please see [HUDI-741 ](https://issues.apache.org/jira/browse/HUDI-741) for my detailed analysis of why this is required and how this is implemented.
## Brief change log
Code changes:
1. Added a new config in HoodieWriteConfig to enable schema validation check (disabled by default)
2. Moved code that reads schema from base/log files into hudi-common from hudi-hive-sync
3. Added a class org.apache.hudi.common.util.SchemaCompatibility which implements checks for schema evolution. This is based on org.apache.avro.SchemaCompatibility but performs HUDI specific checks.
Testing changes:
4. Extended HoodieTestDataGenerator to generate records using a custom Schema
5. Extended TestHoodieClientBase to add insertBatch API which allows inserting a new batch of unique records into a HUDI table
6. Added a unit test to verify schema evolution for both COW and MOR tables.
## Verify this pull request
This change added tests and can be verified as follows:
- Added unit test TestTableSchemaEvolution
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-613754532
Just to level set, we have relied always to an external system validating the schema backwards compatibility outside of Hudi. bringing this inside, seems like a good idea based on what we have learnt in production over the past year seems like.
I am supportive of that..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r409104802
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * NOTE: This code is copied from org.apache.avro.SchemaCompatibility and changed for HUDI use case.
Review comment:
Not required any longer as I have removed the copied file. Please see the latest diff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-605508358
@pratyakshsharma @umehrot2 could you folks help review this, given your interests in this area
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r405969176
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -487,6 +492,58 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
return updateIndexAndCommitIfNeeded(writeStatusRDD, hoodieTable, instantTime);
}
+ /**
+ * Ensure that the current writerSchema is compatible with the latest schema of this dataset.
+ *
+ * When inserting/updating data, we read records using the schema saved in the data/log files
+ * and convert them to the GenericRecords with writerSchema. Hence, we need to ensure that
+ * this conversion can take place without errors.
+ *
+ * @param hoodieTable The Hoodie Table
+ * @param isUpsert If this is a check during upserts
+ * @throws HoodieUpsertException If schema check fails during upserts
+ * @throws HoodieInsertException If schema check fails during inserts
+ */
+ private void validateSchema(HoodieTable<T> hoodieTable, final boolean isUpsert)
Review comment:
there is a pending PR to move this code into the new `table.action` package.. FYI.. this may need to move there as well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] afilipchik commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
afilipchik commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-613613150
I found a way to get out compaction situation. Had to modify parquet-avro to allow avro style compatibility. Currently if avro doesn't have all the fields that are in parquet -> it is exception. I made it so if fields are missing in avro, parquet-avro reader will just skip them. Was able to resolve incompatible change on our dev env fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400284466
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
Review comment:
lets have this as a single ERROR statement?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400286409
##########
File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -52,6 +52,8 @@
private static final String TIMELINE_LAYOUT_VERSION = "hoodie.timeline.layout.version";
private static final String BASE_PATH_PROP = "hoodie.base.path";
private static final String AVRO_SCHEMA = "hoodie.avro.schema";
+ private static final String SCHEMA_CHECK = "hoodie.schema.check";
Review comment:
rename to `hoodie.avro.schema.validate` ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400285226
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
+ LOG.warn("WriterSchema: " + writerSchema);
+ LOG.warn("Table latest schema: " + savedSchema);
+ throw new HoodieUpsertException(msg);
+ }
+ } catch (Exception e) {
+ // If this is the first insert into the table then schema will not be present
+ if (hoodieTable.getActiveTimeline().getCommitsTimeline().filterCompletedInstants().countInstants() > 0) {
Review comment:
can we handle this in code nicely above before try, instead of the catch block as an error case?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r406973747
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
+ HoodieWriteConfig hoodieDevolvedWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ client = getHoodieWriteClient(hoodieDevolvedWriteConfig, false);
+ final List<HoodieRecord> failedRecords = dataGenDevolved.generateInserts("004", numRecords);
+ try {
+ // We cannot use insertBatch directly here because we want to insert records
+ // with a devolved schema and insertBatch inserts records using the TRIP_EXMPLE_SCHEMA.
Review comment:
I mean devolved here as updates with a devolved schema (with missing field) should not be allowed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-612258453
> Structure looks much better now. thanks @prashantwason ..
>
> I raised an issue on the need to copy the avro compatibility code into the project.. Would like to understand why we cannot re-use as is.. I don't know if we can maintain this and keep in sync over time..
>
> Nonetheless. this change also needs to update NOTICE/LICENSE appropriately as well, if we need to reuse that code
Please see the details on [HUDI-741](https://issues.apache.org/jira/browse/HUDI-741?focusedCommentId=17081025&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17081025) for the limitation with the original code.
This is just one way to compare two schemas. If there is a better way for HUDI, then I will be happy to integrate that instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527600
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaUtil.java
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Utilities to read Schema from data files and log files.
+ */
+public class SchemaUtil {
Review comment:
Done. Unit test TBD.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400291594
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
Review comment:
lets move this to `org.apache.hudi.common.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400290759
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * NOTE: This code is copied from org.apache.avro.SchemaCompatibility and changed for HUDI use case.
Review comment:
this needs to be attributed in LICENSE and NOTICE .. Can we document all the changes? I am wondering if we can just wrap or extend the `org.apache.avro.SchemaCompatibility` class instead of copying in full..
cc @bvaradar as well..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] n3nash commented on a change in pull request
#1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r401964812
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
Review comment:
nit : devolved/evolved
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] bvaradar commented on a change in pull request
#1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
bvaradar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r405979601
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/TableSchemaResolver.java
##########
@@ -0,0 +1,274 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.table;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.avro.SchemaCompatibility;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Helper class to read schema from data files and log files and to convert it between different formats.
+ */
+public class TableSchemaResolver {
+
+ private static final Logger LOG = LogManager.getLogger(TableSchemaResolver.class);
+ private HoodieTableMetaClient metaClient;
+
+ public TableSchemaResolver(HoodieTableMetaClient metaClient) {
+ this.metaClient = metaClient;
+ }
+
+ /**
+ * Gets the schema for a hoodie table. Depending on the type of table, read from any file written in the latest
+ * commit. We will assume that the schema has not changed within a single atomic write.
+ *
+ * @return Parquet schema for this table
+ * @throws Exception
+ */
+ public MessageType getDataSchema() throws Exception {
Review comment:
Since we log schema now in commit metadata, can't we just try to read it first. If it is not there, then we can try reading from parquet file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason edited a comment on issue #1457:
[HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason edited a comment on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-612258453
> Structure looks much better now. thanks @prashantwason ..
>
> I raised an issue on the need to copy the avro compatibility code into the project.. Would like to understand why we cannot re-use as is.. I don't know if we can maintain this and keep in sync over time..
>
> Nonetheless. this change also needs to update NOTICE/LICENSE appropriately as well, if we need to reuse that code
@vinothchandar
Please see the details on [HUDI-741](https://issues.apache.org/jira/browse/HUDI-741?focusedCommentId=17081025&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17081025) for the limitation with the original code.
This is just one way to compare two schemas. If there is a better way for HUDI, then I will be happy to integrate that instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527249
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400283983
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
Review comment:
can this and the previous line be combined into one
```
Schema tableSchema = schemaUtil.getLatestSchema()
```
I don't think any other code cares about `savedParquetSchema`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-613761823
Those are my pending comments.. otherwise @n3nash can review and merge.. But please ensure NOTICE and LICENSE are properly updated, this will cause large delays in releasing down the line.. @n3nash if you 'd rather have me double review lmk :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r406973660
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
Review comment:
I mean devolved here as insert with a devolved schema (with missing field) should not be allowed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r406978802
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/TableSchemaResolver.java
##########
@@ -0,0 +1,274 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.table;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.avro.SchemaCompatibility;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Helper class to read schema from data files and log files and to convert it between different formats.
+ */
+public class TableSchemaResolver {
+
+ private static final Logger LOG = LogManager.getLogger(TableSchemaResolver.class);
+ private HoodieTableMetaClient metaClient;
+
+ public TableSchemaResolver(HoodieTableMetaClient metaClient) {
+ this.metaClient = metaClient;
+ }
+
+ /**
+ * Gets the schema for a hoodie table. Depending on the type of table, read from any file written in the latest
+ * commit. We will assume that the schema has not changed within a single atomic write.
+ *
+ * @return Parquet schema for this table
+ * @throws Exception
+ */
+ public MessageType getDataSchema() throws Exception {
Review comment:
This simplifies the code. I have updated to use the schema from commit metadata.
For MOR tables, the compaction operation also leads to a commit (.commit extension) which saves HoodieCommitMetadata but without the SCHEMA. I guess this is a miss and not as per design. I have fixed this as I test both types of tables.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar edited a comment on issue #1457:
[HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar edited a comment on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-613761497
@prashantwason can you please
- reduce the size of the SchemaCompatibility file we copied from avro, reusing as much as we can.. this will also be a forcing function to revisit and pick up fixes here time and again if we update avro.
- please look at NOTICE/LICENSE file for how we have attributed other such code reuse and attribute them as well ....
I am not sure if the stricter checking we add here will affect the issue listed here https://github.com/apache/incubator-hudi/pull/1406#issuecomment-606240326 .. basically there was a difference in namespace (which IIUC matters when comparing schemas)... I am wondering if we can read 0.5.0 data using 0.5.2 for eg
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527107
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
Review comment:
Moved to HoodieAvroUtils
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] codecov-io commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-614333053
# [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=h1) Report
> Merging [#1457](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-hudi/commit/661b0b3bab85c8fd958a9e256dab99790ea92713&el=desc) will **increase** coverage by `0.09%`.
> The diff coverage is `71.60%`.
[](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1457 +/- ##
============================================
+ Coverage 72.20% 72.29% +0.09%
- Complexity 289 294 +5
============================================
Files 372 374 +2
Lines 16267 16369 +102
Branches 1637 1654 +17
============================================
+ Hits 11745 11834 +89
- Misses 3787 3799 +12
- Partials 735 736 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...in/java/org/apache/hudi/io/HoodieAppendHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllQXBwZW5kSGFuZGxlLmphdmE=) | `84.17% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...java/org/apache/hudi/hive/util/HiveSchemaUtil.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1oaXZlLXN5bmMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGl2ZS91dGlsL0hpdmVTY2hlbWFVdGlsLmphdmE=) | `68.10% <50.00%> (ø)` | `0.00 <0.00> (?)` | |
| [.../apache/hudi/common/table/TableSchemaResolver.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL1RhYmxlU2NoZW1hUmVzb2x2ZXIuamF2YQ==) | `63.86% <63.86%> (ø)` | `0.00 <0.00> (?)` | |
| [...in/java/org/apache/hudi/hive/HoodieHiveClient.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1oaXZlLXN5bmMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGl2ZS9Ib29kaWVIaXZlQ2xpZW50LmphdmE=) | `74.63% <75.00%> (+2.43%)` | `0.00 <0.00> (ø)` | |
| [...java/org/apache/hudi/client/HoodieWriteClient.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpZW50L0hvb2RpZVdyaXRlQ2xpZW50LmphdmE=) | `70.24% <96.55%> (+2.28%)` | `0.00 <0.00> (ø)` | |
| [...java/org/apache/hudi/config/HoodieWriteConfig.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29uZmlnL0hvb2RpZVdyaXRlQ29uZmlnLmphdmE=) | `84.64% <100.00%> (+0.27%)` | `0.00 <0.00> (ø)` | |
| [...ain/java/org/apache/hudi/io/HoodieWriteHandle.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vSG9vZGllV3JpdGVIYW5kbGUuamF2YQ==) | `74.46% <100.00%> (-0.54%)` | `0.00 <0.00> (ø)` | |
| [...ain/java/org/apache/hudi/avro/HoodieAvroUtils.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvYXZyby9Ib29kaWVBdnJvVXRpbHMuamF2YQ==) | `93.13% <100.00%> (+1.13%)` | `0.00 <0.00> (ø)` | |
| [...c/main/java/org/apache/hudi/hive/HiveSyncTool.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS1oaXZlLXN5bmMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGl2ZS9IaXZlU3luY1Rvb2wuamF2YQ==) | `74.15% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...lities/checkpointing/KafkaConnectHdfsProvider.java](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NoZWNrcG9pbnRpbmcvS2Fma2FDb25uZWN0SGRmc1Byb3ZpZGVyLmphdmE=) | `89.28% <0.00%> (-3.03%)` | `14.00% <0.00%> (+2.00%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/incubator-hudi/pull/1457/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=footer). Last update [661b0b3...4f1c184](https://codecov.io/gh/apache/incubator-hudi/pull/1457?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400281970
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
Review comment:
using the Handle classes at this level, breaks the layering.. Please refactor into a common helper.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] n3nash commented on a change in pull request
#1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r401966800
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/avro/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.avro;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * NOTE: This code is copied from org.apache.avro.SchemaCompatibility and changed for HUDI use case.
+ *
+ * HUDI requires a Schema to be specified in HoodieWriteConfig and is used by the HoodieWriteClient to
+ * create the records. The schema is also saved in the data files (parquet format) and log files (avro format).
+ * Since a schema is required each time new data is ingested into a HUDI dataset, schema can be evolved over time.
+ *
+ * HUDI specific validation of schema evolution should ensure that a newer schema can be used for the dataset by
+ * checking that the data written using the old schema can be read using the new schema.
+ *
+ * New Schema is compatible only if:
+ * 1. There is no change in schema
+ * 2. A field has been added and it has a default value specified
+ *
+ * New Schema is incompatible if:
+ * 1. A field has been deleted
+ * 2. A field has been renamed (treated as delete + add)
+ * 3. A field's type has changed to be incompatible with the older type
+ */
+public class SchemaCompatibility {
Review comment:
@prashantwason Can you please mark the lines/methods you have changed with may be **MOD** for future references ? Also, can you add 1-2 lines as to why that change is needed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] afilipchik commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
afilipchik commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-611819504
What will happen if there is incompatible message in Kafka? Will pipeline stall? What will be the way to fix it without purging whole kafka topic?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r406976510
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/avro/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.avro;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * NOTE: This code is copied from org.apache.avro.SchemaCompatibility and changed for HUDI use case.
+ *
+ * HUDI requires a Schema to be specified in HoodieWriteConfig and is used by the HoodieWriteClient to
+ * create the records. The schema is also saved in the data files (parquet format) and log files (avro format).
+ * Since a schema is required each time new data is ingested into a HUDI dataset, schema can be evolved over time.
+ *
+ * HUDI specific validation of schema evolution should ensure that a newer schema can be used for the dataset by
+ * checking that the data written using the old schema can be read using the new schema.
+ *
+ * New Schema is compatible only if:
+ * 1. There is no change in schema
+ * 2. A field has been added and it has a default value specified
+ *
+ * New Schema is incompatible if:
+ * 1. A field has been deleted
+ * 2. A field has been renamed (treated as delete + add)
+ * 3. A field's type has changed to be incompatible with the older type
+ */
+public class SchemaCompatibility {
Review comment:
Done. I have added the limitation with org.apache.avro.SchemaCompatibility in the file as well as in the [HUDI-741](https://issues.apache.org/jira/browse/HUDI-741)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-613761497
@prashantwason can you please
-reduce the size of the SchemaCompatibility file we copied from avro, reusing as much as we can.. this will also be a forcing function to revisit and pick up fixes here time and again if we update avro.
- please look at NOTICE/LICENSE file for how we have attributed other such code reuse and attribute them as well ....
I am not sure if the stricter checking we add here will affect the issue listed here https://github.com/apache/incubator-hudi/pull/1406#issuecomment-606240326 .. basically there was a difference in namespace (which IIUC matters when comparing schemas)... I am wondering if we can read 0.5.0 data using 0.5.2 for eg
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527316
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
Review comment:
The same msg is also used to raise the exception a few lines below. So I did not duplicate the string and created a local variable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] pratyakshsharma commented on issue #1457:
[HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-605612911
ack.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] n3nash commented on a change in pull request
#1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r401964863
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
+ HoodieWriteConfig hoodieDevolvedWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ client = getHoodieWriteClient(hoodieDevolvedWriteConfig, false);
+ final List<HoodieRecord> failedRecords = dataGenDevolved.generateInserts("004", numRecords);
+ try {
+ // We cannot use insertBatch directly here because we want to insert records
+ // with a devolved schema and insertBatch inserts records using the TRIP_EXMPLE_SCHEMA.
Review comment:
same
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason edited a comment on issue #1457:
[HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason edited a comment on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-612262645
> What will happen if there is incompatible message in Kafka? Will pipeline stall? What will be the way to fix it without purging whole kafka topic?
@afilipchik
The current state is that:
1. COW tables:
- Update to existing parquet file: Will raise as exception during commit as conversion of record to the writerSchema will fail.
- Insert to new parquet file: Will be ok.
2. MOR Table:
- Update and insert both will be successful. But will raise exception during compaction.
I am not very sure on the reader side. Either an exception or the record may be missing the fields.
So even today, the pipeline may stall (due to exception). I dont think HUDI has a way out of it yet. You may drop the offending record (before calling HoodieWriteClient::insert()).
This change only checks the schema. So if the writerSchema is same, then this code has no extra effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527391
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
+ LOG.warn("WriterSchema: " + writerSchema);
+ LOG.warn("Table latest schema: " + savedSchema);
+ throw new HoodieUpsertException(msg);
Review comment:
Refactored.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527532
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527481
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaCompatibility.java
##########
@@ -0,0 +1,566 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * NOTE: This code is copied from org.apache.avro.SchemaCompatibility and changed for HUDI use case.
Review comment:
The functions that are changed in org.apache.avro.SchemaCompatibility are all private.
I can reduce the size of this file by referring to classes from org.apache.avro.SchemaCompatibility but I think starting with a copy makes it easier to see a diff.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400325322
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaUtil.java
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Utilities to read Schema from data files and log files.
+ */
+public class SchemaUtil {
Review comment:
Edit: `TableSchemaResolver` is a better name.. ? since it does more complex than things than just reading
and btw let's add some unit tests to both these classes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] n3nash commented on a change in pull request
#1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r401965698
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
+ HoodieWriteConfig hoodieDevolvedWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ client = getHoodieWriteClient(hoodieDevolvedWriteConfig, false);
+ final List<HoodieRecord> failedRecords = dataGenDevolved.generateInserts("004", numRecords);
+ try {
+ // We cannot use insertBatch directly here because we want to insert records
+ // with a devolved schema and insertBatch inserts records using the TRIP_EXMPLE_SCHEMA.
+ writeBatch(client, "005", "004", Option.empty(), "003", numRecords,
+ (String s, Integer a) -> failedRecords, HoodieWriteClient::insert, false, 0, 0, 0);
+ fail("Insert with devolved scheme should fail");
+ } catch (HoodieInsertException ex) {
+ // no new commit
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+ client.rollback("005");
+ }
+
+ // Update with devolved schema is also not allowed
Review comment:
If you mean to say "older schema" or "shorter schema" or "retracted schema" - not sure if devolved is a work so I'm not sure are you evolving or retracting
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400285880
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
+ LOG.warn("WriterSchema: " + writerSchema);
+ LOG.warn("Table latest schema: " + savedSchema);
+ throw new HoodieUpsertException(msg);
Review comment:
so this is recaught below? IMO this method should just throw exception from one place..
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r409104956
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaUtil.java
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Utilities to read Schema from data files and log files.
+ */
+public class SchemaUtil {
Review comment:
Unit tests implemented.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527359
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
+ // Ensure that the current writerSchema is compatible with the latest schema of this
+ // dataset.
+ // When inserting/updating data, we read records using the schema saved in the
+ // data/log files and convert them to the GenericRecords with writerSchema.
+ // Hence, we need to ensure that this conversion can take place without errors.
+ try {
+ SchemaUtil schemaUtil = new SchemaUtil(hoodieTable.getMetaClient());
+ MessageType savedParquetSchema = schemaUtil.getDataSchema();
+ Schema savedSchema = schemaUtil.convertParquetSchemaToAvro(savedParquetSchema);
+ Schema writerSchema = HoodieWriteHandle.createHoodieWriteSchema(config.getSchema());
+ if (! schemaUtil.isSchemaCompatible(savedSchema, writerSchema)) {
+ String msg = "WriterSchema is not compatible with the schema present in the Table";
+ LOG.error(msg);
+ LOG.warn("WriterSchema: " + writerSchema);
+ LOG.warn("Table latest schema: " + savedSchema);
+ throw new HoodieUpsertException(msg);
+ }
+ } catch (Exception e) {
+ // If this is the first insert into the table then schema will not be present
+ if (hoodieTable.getActiveTimeline().getCommitsTimeline().filterCompletedInstants().countInstants() > 0) {
Review comment:
Reading the schema may fail due to IO / timeout errors too which will raise their exceptions.
Having this code here:
1. removes code duplication (as we need to throw HoodieUpsertException here)
2. Handles the rare case gracefully - when reading schema fails due to IO errors but there was no 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527428
##########
File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -52,6 +52,8 @@
private static final String TIMELINE_LAYOUT_VERSION = "hoodie.timeline.layout.version";
private static final String BASE_PATH_PROP = "hoodie.base.path";
private static final String AVRO_SCHEMA = "hoodie.avro.schema";
+ private static final String SCHEMA_CHECK = "hoodie.schema.check";
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-612262645
> What will happen if there is incompatible message in Kafka? Will pipeline stall? What will be the way to fix it without purging whole kafka topic?
The current state is that:
1. COW tables:
- Update to existing parquet file: Will raise as exception during commit as conversion of record to the writerSchema will fail.
- Insert to new parquet file: Will be ok.
2. MOR Table:
- Update and insert both will be successful. But will raise exception during compaction.
I am not very sure on the reader side. Either an exception or the record may be missing the fields.
So even today, the pipeline may stall (due to exception). I dont think HUDI has a way out of it yet. You may drop the offending record (before calling HoodieWriteClient::insert()).
This change only checks the schema. So if the writerSchema is same, then this code has no extra effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400527168
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
Review comment:
done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on issue #1457: [HUDI-741]
Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on issue #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-614253179
I have reworked the schema compatibility check code to remove copying the entire avro.SchemaCompatibility class. I took out the relevant portion and it worked. I think the checks are now simpler and clearly defined within TableSchemaResolver.isisSchemaCompatible(...).
No need to NOTICE and LICENSE updates as we are not copying the avro.SchemaCompatibility class.
All unit tests have been completed too. So this is now good for a final review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] prashantwason commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r406974059
##########
File path: hudi-client/src/test/java/org/apache/hudi/client/TestTableSchemaEvolution.java
##########
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.client;
+
+import org.apache.hudi.common.HoodieClientTestUtils;
+import org.apache.hudi.common.HoodieTestDataGenerator;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.exception.HoodieInsertException;
+import org.apache.hudi.exception.HoodieUpsertException;
+import org.apache.hudi.index.HoodieIndex.IndexType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hudi.common.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.apache.hudi.common.table.timeline.versioning.TimelineLayoutVersion.VERSION_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestTableSchemaEvolution extends TestHoodieClientBase {
+ private final String initCommitTime = "000";
+ private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ private HoodieTestDataGenerator dataGenDevolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ private HoodieTestDataGenerator dataGenEvolved = new HoodieTestDataGenerator(TRIP_EXAMPLE_SCHEMA_EVOLVED);
+
+ // TRIP_EXAMPLE_SCHEMA with a new_field added
+ public static final String TRIP_EXAMPLE_SCHEMA_EVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"}," + "{\"name\": \"driver\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"new_field\", \"type\": [\"null\", \"string\"], \"default\": null},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+ // TRIP_EXAMPLE_SCHEMA with driver field removed
+ public static final String TRIP_EXAMPLE_SCHEMA_DEVOLVED = "{\"type\": \"record\"," + "\"name\": \"triprec\"," + "\"fields\": [ "
+ + "{\"name\": \"timestamp\",\"type\": \"double\"}," + "{\"name\": \"_row_key\", \"type\": \"string\"},"
+ + "{\"name\": \"rider\", \"type\": \"string\"},"
+ + "{\"name\": \"begin_lat\", \"type\": \"double\"}," + "{\"name\": \"begin_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"end_lat\", \"type\": \"double\"}," + "{\"name\": \"end_lon\", \"type\": \"double\"},"
+ + "{\"name\": \"fare\",\"type\": {\"type\":\"record\", \"name\":\"fare\",\"fields\": ["
+ + "{\"name\": \"amount\",\"type\": \"double\"},{\"name\": \"currency\", \"type\": \"string\"}]}},"
+ + "{\"name\": \"_hoodie_is_deleted\", \"type\": \"boolean\", \"default\": false} ]}";
+
+ @Before
+ public void setUp() throws Exception {
+ initResources();
+ }
+
+ @After
+ public void tearDown() {
+ cleanupSparkContexts();
+ }
+
+ @Test
+ public void testMORTable() throws Exception {
+ tableType = HoodieTableType.MERGE_ON_READ;
+ initMetaClient();
+
+ // Create the table
+ HoodieTableMetaClient.initTableType(metaClient.getHadoopConf(), metaClient.getBasePath(),
+ HoodieTableType.MERGE_ON_READ, metaClient.getTableConfig().getTableName(),
+ metaClient.getArchivePath(), metaClient.getTableConfig().getPayloadClass(), VERSION_1);
+
+ HoodieWriteConfig hoodieWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA);
+ HoodieWriteClient client = getHoodieWriteClient(hoodieWriteConfig, false);
+
+ // Initial inserts with TRIP_EXAMPLE_SCHEMA
+ int numRecords = 10;
+ insertFirstBatch(hoodieWriteConfig, client, "001", initCommitTime,
+ numRecords, HoodieWriteClient::insert, false, false, numRecords);
+ checkLatestDeltaCommit("001");
+
+ // Compact once so we can incrementally read later
+ assertTrue(client.scheduleCompactionAtInstant("002", Option.empty()));
+ client.compact("002");
+
+ // Updates with same schema is allowed
+ final int numUpdateRecords = 5;
+ updateBatch(hoodieWriteConfig, client, "003", "002", Option.empty(),
+ initCommitTime, numUpdateRecords, HoodieWriteClient::upsert, false, false, 0, 0, 0);
+ checkLatestDeltaCommit("003");
+ checkReadRecords("000", numRecords);
+
+ // Delete with same schema is allowed
+ final int numDeleteRecords = 2;
+ numRecords -= numDeleteRecords;
+ deleteBatch(hoodieWriteConfig, client, "004", "003", initCommitTime, numDeleteRecords,
+ HoodieWriteClient::delete, false, false, 0, 0);
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+
+ // Insert with devolved schema is not allowed
+ HoodieWriteConfig hoodieDevolvedWriteConfig = getWriteConfig(TRIP_EXAMPLE_SCHEMA_DEVOLVED);
+ client = getHoodieWriteClient(hoodieDevolvedWriteConfig, false);
+ final List<HoodieRecord> failedRecords = dataGenDevolved.generateInserts("004", numRecords);
+ try {
+ // We cannot use insertBatch directly here because we want to insert records
+ // with a devolved schema and insertBatch inserts records using the TRIP_EXMPLE_SCHEMA.
+ writeBatch(client, "005", "004", Option.empty(), "003", numRecords,
+ (String s, Integer a) -> failedRecords, HoodieWriteClient::insert, false, 0, 0, 0);
+ fail("Insert with devolved scheme should fail");
+ } catch (HoodieInsertException ex) {
+ // no new commit
+ checkLatestDeltaCommit("004");
+ checkReadRecords("000", numRecords);
+ client.rollback("005");
+ }
+
+ // Update with devolved schema is also not allowed
Review comment:
I had the same confusion with what word to use here. Devolved plays well with evolved and [seems to be a valid word](https://www.merriam-webster.com/dictionary/devolve).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400282621
##########
File path: hudi-client/src/main/java/org/apache/hudi/client/HoodieWriteClient.java
##########
@@ -457,6 +461,37 @@ private void saveWorkloadProfileMetadataToInflight(WorkloadProfile profile, Hood
private JavaRDD<WriteStatus> upsertRecordsInternal(JavaRDD<HoodieRecord<T>> preppedRecords, String commitTime,
HoodieTable<T> hoodieTable, final boolean isUpsert) {
+ if (getConfig().getSchemaCheck()) {
Review comment:
this whole block can be pulled into a private method like `validateSchema()` and reused? without being inlined into `upsertRecordsInternal()`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on a change in pull
request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on a change in pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457#discussion_r400292884
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/SchemaUtil.java
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.util;
+
+import java.io.IOException;
+
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hudi.common.model.HoodieCommitMetadata;
+import org.apache.hudi.common.model.HoodieFileFormat;
+import org.apache.hudi.common.model.HoodieLogFile;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.log.HoodieLogFormat;
+import org.apache.hudi.common.table.log.HoodieLogFormat.Reader;
+import org.apache.hudi.common.table.log.block.HoodieAvroDataBlock;
+import org.apache.hudi.common.table.log.block.HoodieLogBlock;
+import org.apache.hudi.common.table.timeline.HoodieActiveTimeline;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.exception.HoodieException;
+import org.apache.hudi.exception.InvalidTableException;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.Logger;
+import org.apache.parquet.avro.AvroSchemaConverter;
+import org.apache.parquet.format.converter.ParquetMetadataConverter;
+import org.apache.parquet.hadoop.ParquetFileReader;
+import org.apache.parquet.hadoop.metadata.ParquetMetadata;
+import org.apache.parquet.schema.MessageType;
+
+/**
+ * Utilities to read Schema from data files and log files.
+ */
+public class SchemaUtil {
Review comment:
let's rename to `TableSchemaReader` and move to `org.apache.hudi.common.table` ? (I am trying to move us out of the habit of piling up more util classes)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] n3nash merged pull request #1457: [HUDI-741] Added
checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
n3nash merged pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
URL: https://github.com/apache/incubator-hudi/pull/1457
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] vinothchandar commented on pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1457:
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-619414521
@n3nash @prashantwason @bvaradar Noticed that the checks are not performed for bulk_insert.. Is n't this a problem? Why exclude bulk_insert?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hudi] prashantwason commented on pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on pull request #1457:
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-620125502
1548 looks good.
On Mon, Apr 27, 2020 at 9:54 AM vinoth chandar <no...@github.com>
wrote:
> @prashantwason
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_prashantwason&d=DwMCaQ&c=r2dcLCtU9q6n0vrtnDw9vg&r=c89AU9T1AVhM4r2Xi3ctZA&m=TTdYCvj60bIXAvnBAXKsIMQkIx6vb452NiyGQcRLi8c&s=jjuyzFOAZLdj0hvYoF4n-HnTC1Beh6EMFd17pecgNps&e=>
> no worries.. I already fixed it here #1548
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Dhudi_pull_1548&d=DwMCaQ&c=r2dcLCtU9q6n0vrtnDw9vg&r=c89AU9T1AVhM4r2Xi3ctZA&m=TTdYCvj60bIXAvnBAXKsIMQkIx6vb452NiyGQcRLi8c&s=EtugoOEb8FcNSYz7bo1D5aemF4YrI8YFrm8c_lvMLNo&e=>
> . Please take a quick skim to confirm
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_incubator-2Dhudi_pull_1457-23issuecomment-2D620107259&d=DwMCaQ&c=r2dcLCtU9q6n0vrtnDw9vg&r=c89AU9T1AVhM4r2Xi3ctZA&m=TTdYCvj60bIXAvnBAXKsIMQkIx6vb452NiyGQcRLi8c&s=Y0pBKlXubwj3__9yg111nY9VtVCQnhnzs4fYSRatvd0&e=>,
> or unsubscribe
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AN55SSYEOG2UKDE43Z3ZIEDROW2FBANCNFSM4LVG2GDQ&d=DwMCaQ&c=r2dcLCtU9q6n0vrtnDw9vg&r=c89AU9T1AVhM4r2Xi3ctZA&m=TTdYCvj60bIXAvnBAXKsIMQkIx6vb452NiyGQcRLi8c&s=kbtVscWq16pK1dyiOqrUh-z9bqX2-kRw-JUyRrS6T7g&e=>
> .
>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hudi] prashantwason commented on pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
prashantwason commented on pull request #1457:
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-620105240
I guess its an oversight on my part. I will file another PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-hudi] vinothchandar commented on pull request #1457: [HUDI-741] Added checks to validate Hoodie's schema evolution.
Posted by GitBox <gi...@apache.org>.
vinothchandar commented on pull request #1457:
URL: https://github.com/apache/incubator-hudi/pull/1457#issuecomment-620107259
@prashantwason no worries.. I already fixed it here https://github.com/apache/incubator-hudi/pull/1548 . Please take a quick skim to confirm
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org