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/04/16 20:11:28 UTC

[GitHub] [incubator-hudi] afilipchik opened a new pull request #1524: Adding a way to post process schema after it is fetched

afilipchik opened a new pull request #1524: Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524
 
 
   ## What is the purpose of the pull request
   
   This PR adds extension point to SchemaProvider. Sometimes it is needed to postprocess schemas after they are fetched from the external sources. Some examples of postprocessing:
   - make sure all the defaults are set correctly.
   - insert marker columns into records with no fields (no writable as parquest)
   - ...
   
   ## Brief change log
     - Added SchemaPostProcessor
     - refactored SchemaProvider to use SchemaPostProcessor and load schemas when constructed. 
     - Updated current schemas providers to use 
   
   ## Verify this pull request
   
   This change added tests and can be verified as follows:
     - Added TestSchemaPostProcessor
   
   ## 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] [hudi] pratyakshsharma commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on pull request #1524:
URL: https://github.com/apache/hudi/pull/1524#issuecomment-690099859


   @afilipchik still working on this? 


----------------------------------------------------------------
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] pratyakshsharma commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r410439724
 
 

 ##########
 File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java
 ##########
 @@ -18,6 +18,30 @@
 
 package org.apache.hudi.utilities;
 
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.StringReader;
+import java.nio.ByteBuffer;
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
 
 Review comment:
   can we fix the import order here? 

----------------------------------------------------------------
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 a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r410440366
 
 

 ##########
 File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/schema/NullTargetSchemaRegistryProvider.java
 ##########
 @@ -18,14 +18,14 @@
 
 package org.apache.hudi.utilities.schema;
 
-import org.apache.hudi.common.config.TypedProperties;
-
 import org.apache.avro.Schema;
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.util.collection.Pair;
 
 Review comment:
   ditto.

----------------------------------------------------------------
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 a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r410440760
 
 

 ##########
 File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/schema/RowBasedSchemaProvider.java
 ##########
 @@ -22,29 +22,30 @@
 import org.apache.hudi.common.config.TypedProperties;
 
 import org.apache.avro.Schema;
+import org.apache.hudi.common.util.collection.Pair;
 
 Review comment:
   please reorder imports according to our checkstyle rules everywhere. 

----------------------------------------------------------------
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 a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r410439985
 
 

 ##########
 File path: hudi-utilities/src/main/java/org/apache/hudi/utilities/schema/FilebasedSchemaProvider.java
 ##########
 @@ -18,24 +18,27 @@
 
 package org.apache.hudi.utilities.schema;
 
+import java.io.IOException;
+import java.util.Collections;
+import org.apache.avro.Schema;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
 
 Review comment:
   ditto

----------------------------------------------------------------
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] [hudi] bvaradar merged pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar merged pull request #1524:
URL: https://github.com/apache/hudi/pull/1524


   


----------------------------------------------------------------
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 issue #1524: Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
vinothchandar commented on issue #1524: Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#issuecomment-614931414
 
 
   Please create a JIRA for this work, tagged against 0.6.0 release 

----------------------------------------------------------------
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 a change in pull request #1524: Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1524: Adding a way to post process schema after it is fetched
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r410180045
 
 

 ##########
 File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestSchemaPostProcessor.java
 ##########
 @@ -0,0 +1,61 @@
+package org.apache.hudi.utilities;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.IOException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.SchemaBuilder;
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.utilities.schema.SchemaPostProcessor;
+import org.apache.hudi.utilities.schema.SchemaProvider;
+import org.apache.hudi.utilities.schema.SchemaProvider.Config;
+import org.apache.spark.api.java.JavaSparkContext;
+import org.junit.Test;
+
+public class TestSchemaPostProcessor {
+
+  private TypedProperties properties = new TypedProperties();
+
+  @Test
+  public void testPostProcessor() throws IOException {
+    properties.put(Config.SCHEMA_POST_PROCESSOR_PROP, DummySchemaPostProcessor.class.getName());
+
+    JavaSparkContext jsc =
+        UtilHelpers.buildSparkContext(this.getClass().getName() + "-hoodie", "local[2]");
+    SchemaProvider provider =
+        UtilHelpers.createSchemaProvider(DummySchemaProvider.class.getName(), properties, jsc);
+
+    Schema schema = provider.getSourceSchema();
+    assertEquals(schema.getType(), Type.RECORD);
+    assertEquals(schema.getName(), "test");
+    assertNotNull(schema.getField("testString"));
+  }
+
+  public static class DummySchemaProvider extends SchemaProvider {
 
 Review comment:
   Can we try to use the one from TestHoodieDeltaStreamer? That would help avoid code duplication. 

----------------------------------------------------------------
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] [hudi] bvaradar commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1524:
URL: https://github.com/apache/hudi/pull/1524#issuecomment-690554395


   @pratyakshsharma : I will help getting this landed.


----------------------------------------------------------------
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] [hudi] bvaradar commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1524:
URL: https://github.com/apache/hudi/pull/1524#issuecomment-695340529


   @afilipchik @pratyakshsharma : I have rebased and fixed the test. All comments are resolved. I will go ahead and merge this change.


----------------------------------------------------------------
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] afilipchik commented on issue #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
afilipchik commented on issue #1524:
URL: https://github.com/apache/incubator-hudi/pull/1524#issuecomment-617474888


   Wow, rebased on top of master and now it is million changes :)


----------------------------------------------------------------
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] bvaradar commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1524:
URL: https://github.com/apache/incubator-hudi/pull/1524#issuecomment-620846353


   @afilipchik : Doesn't look like the rebase worked fine as I see other folk's commits in the PR. Can you rebase again ? 


----------------------------------------------------------------
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] bvaradar commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1524:
URL: https://github.com/apache/incubator-hudi/pull/1524#issuecomment-629547495


   @afilipchik : Please take a look.


----------------------------------------------------------------
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] afilipchik commented on a change in pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
afilipchik commented on a change in pull request #1524:
URL: https://github.com/apache/incubator-hudi/pull/1524#discussion_r411546036



##########
File path: hudi-utilities/src/test/java/org/apache/hudi/utilities/TestSchemaPostProcessor.java
##########
@@ -0,0 +1,61 @@
+package org.apache.hudi.utilities;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.IOException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.SchemaBuilder;
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.utilities.schema.SchemaPostProcessor;
+import org.apache.hudi.utilities.schema.SchemaProvider;
+import org.apache.hudi.utilities.schema.SchemaProvider.Config;
+import org.apache.spark.api.java.JavaSparkContext;
+import org.junit.Test;
+
+public class TestSchemaPostProcessor {
+
+  private TypedProperties properties = new TypedProperties();
+
+  @Test
+  public void testPostProcessor() throws IOException {
+    properties.put(Config.SCHEMA_POST_PROCESSOR_PROP, DummySchemaPostProcessor.class.getName());
+
+    JavaSparkContext jsc =
+        UtilHelpers.buildSparkContext(this.getClass().getName() + "-hoodie", "local[2]");
+    SchemaProvider provider =
+        UtilHelpers.createSchemaProvider(DummySchemaProvider.class.getName(), properties, jsc);
+
+    Schema schema = provider.getSourceSchema();
+    assertEquals(schema.getType(), Type.RECORD);
+    assertEquals(schema.getName(), "test");
+    assertNotNull(schema.getField("testString"));
+  }
+
+  public static class DummySchemaProvider extends SchemaProvider {

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



[GitHub] [incubator-hudi] bvaradar commented on pull request #1524: [HUDI-801] Adding a way to post process schema after it is fetched

Posted by GitBox <gi...@apache.org>.
bvaradar commented on pull request #1524:
URL: https://github.com/apache/incubator-hudi/pull/1524#issuecomment-629454559


   Rebased to get the correct view of the 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