Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15176 )

Change subject: KUDU-3049: [spark] Automatic handling of schema drift
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java:

http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@540
PS2, Line 540:     try {
             :       client.alterTable(tableName,
             :               new AlterTableOptions().addNullableColumn("11", 
Type.INT32));
             :       fail();
             :     } catch (NonRecoverableException e) {
             :       assertEquals("number of columns 11 is greater than the 
permitted maximum 10",
             :               e.getMessage());
             :     }
Do the fancy JUnit exception test?


http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:

http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1320
PS2, Line 1320: while(true)
while (true)


http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:

http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@349
PS2, Line 349:         syncClient.alterTable(tableName, alter)
Should we take any care to avoid failing if there are multiple concurrent Spark 
applications all with handleSchemaDrift=true? One will successfully add the 
columns while the others will fail in alter, right?


http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduWriteOptions.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduWriteOptions.scala:

http://gerrit.cloudera.org:8080/#/c/15176/2/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduWriteOptions.scala@35
PS2, Line 35: when new fields are encountered the
            :  *                          Kudu table will be altered to include 
new columns for
            :  *                          those fields
May want to clarify that our test for "new" is done by column/field name.



--
To view, visit http://gerrit.cloudera.org:8080/15176
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1edebb293d6ae79c26a0ecb9ce7755308f667f4
Gerrit-Change-Number: 15176
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 11 Feb 2020 00:23:31 +0000
Gerrit-HasComments: Yes

Reply via email to