Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14197 )
Change subject: IMPALA-5092 Add support for VARCHAR in Kudu tables ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@a2217 PS10, Line 2217: Should this be move to AnalyzesOk just below the decimal one above? http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14197/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@344 PS10, Line 344: "valvarchar varchar(10) default null) " + nit: Maybe call it `valvc` to follow the pattern of the other columns. It might also be a bit easier to read. http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test File testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test: http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_delete.test@7 PS10, Line 7: valdec16 decimal(38, 0) null, valvarchar varchar(500)) Any reason you use 500 here and 10 in the insert test? http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test File testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test: http://gerrit.cloudera.org:8080/#/c/14197/10/testdata/workloads/functional-query/queries/QueryTest/kudu_insert.test@7 PS10, Line 7: decimal16 decimal(38, 0) null, valvarchar varchar(10) null) Do any of these tests validate truncation behavior? -- To view, visit http://gerrit.cloudera.org:8080/14197 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0d4959410fdd882bfa980cb55e8a7837c7823da8 Gerrit-Change-Number: 14197 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Comment-Date: Tue, 18 Feb 2020 16:33:25 +0000 Gerrit-HasComments: Yes
