Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19949 )

Change subject: KUDU-3483 Fix flushing data in batch when table schema changed
......................................................................


Patch Set 20:

(23 comments)

http://gerrit.cloudera.org:8080/#/c/19949/20//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19949/20//COMMIT_MSG@11
PS20, Line 11: data
> nit: rows
Done


http://gerrit.cloudera.org:8080/#/c/19949/20//COMMIT_MSG@12
PS20, Line 12: these data
> nit: the data
Done


http://gerrit.cloudera.org:8080/#/c/19949/20//COMMIT_MSG@18
PS20, Line 18: outbound
> nit: out of bound
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/TestSchema.java
File java/kudu-client/src/test/java/org/TestSchema.java:

http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/TestSchema.java@97
PS20, Line 97: name of columns
> nit: column names
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/TestSchema.java@110
PS20, Line 110: type of columns.
> nit: column types
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/TestSchema.java@121
PS20, Line 121:   }
> Could you also add a sub-scenario to test for the equality of two column sc
Thanks for your advice.


http://gerrit.cloudera.org:8080/#/c/19949/20/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/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@138
PS20, Line 138: countRowCount
> nit: rename into countRows() ?
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@149
PS20, Line 149:   public void testInsertDataWithChangedSchema() throws 
Exception {
> Could you add a comment for this new test, mentioning KUDU-3483 and clarify
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@154
PS20, Line 154: AUTO_FLUSH_BACKGROUND
> Does it make sense to run this scenario with the MANUAL_FLUSH mode as well?
Yes.


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@191
PS20, Line 191: table
> nit: the table
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@195
PS20, Line 195: new
> nit: the new
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@195
PS20, Line 195: data
> nit: row
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@214
PS20, Line 214: rename
> nit: 'renaming' or 'renamed'
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@217
PS20, Line 217: data
> nit: row
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@229
PS20, Line 229: table
> nit: the table
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@233
PS20, Line 233: new
> nit: the new
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@233
PS20, Line 233: data
> nit: row
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@249
PS20, Line 249: failed
> nit: failed to insert
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@255
PS20, Line 255: data
> nit: row
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@267
PS20, Line 267: table
> nit: the table
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@271
PS20, Line 271: new
> nit: the new
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@271
PS20, Line 271: data
> nit: row
Done


http://gerrit.cloudera.org:8080/#/c/19949/20/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@286
PS20, Line 286: failed
> nit: failed to insert
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6501962b32814d121f180b2942999c402d927db
Gerrit-Change-Number: 19949
Gerrit-PatchSet: 20
Gerrit-Owner: Wang Xixu <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 23 Oct 2023 06:49:43 +0000
Gerrit-HasComments: Yes

Reply via email to