Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14705 )
Change subject: [KUDU-2632] Added support of Kudu DATE type to Impala ...................................................................... Patch Set 8: (6 comments) Looks like the version of Kudu the Impala builds against will need to be bumped before this can be submitted. Do you know how to do that/have a plan for it? If not, feel free to ping me about it http://gerrit.cloudera.org:8080/#/c/14705/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14705/8//COMMIT_MSG@7 PS8, Line 7: [KUDU-2632] Added support of Kudu DATE type to Impala Please reference an Impala JIRA here. Rather than filing a new one, its probably fine to take over IMPALA-8800 for this work. Also, please follow our contribution guidelines for formatting: https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala eg. the commit message should start with the JIRA number followed by a colon http://gerrit.cloudera.org:8080/#/c/14705/8//COMMIT_MSG@8 PS8, Line 8: Its probably worth including a little more info about what this patch does here, eg. it supports both reading and writing DATE values, it doesn't add min-max filter runtime support but there's a followup JIRA for that, various e2e tests were updated to test this functionality, reference the corresponding Kudu JIRA, etc. Fine if its pretty brief. http://gerrit.cloudera.org:8080/#/c/14705/8/be/src/exec/kudu-util.cc File be/src/exec/kudu-util.cc: http://gerrit.cloudera.org:8080/#/c/14705/8/be/src/exec/kudu-util.cc@135 PS8, Line 135: return Status(TErrorCode::RUNTIME_ERROR, : "Invalid DateValue"); nit: single line http://gerrit.cloudera.org:8080/#/c/14705/8/be/src/exec/kudu-util.cc@195 PS8, Line 195: KUDU_RETURN_IF_ERROR( : row->SetDate(col, days), "Could not set Kudu row value."); nit: single line http://gerrit.cloudera.org:8080/#/c/14705/8/be/src/exec/kudu-util.cc@290 PS8, Line 290: RETURN_IF_ERROR(ConvertDateValue( : reinterpret_cast<const DateValue*>(value), &days)); nit: single line http://gerrit.cloudera.org:8080/#/c/14705/8/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test: http://gerrit.cloudera.org:8080/#/c/14705/8/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test@155 PS8, Line 155: valdate date default null) Is there a test anywhere for trying to create a table with an invalid DATE default value? -- To view, visit http://gerrit.cloudera.org:8080/14705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91656749a58ac769b54c2a63bdd4f85c89520b32 Gerrit-Change-Number: 14705 Gerrit-PatchSet: 8 Gerrit-Owner: Volodymyr Verovkin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Tue, 14 Jan 2020 19:47:26 +0000 Gerrit-HasComments: Yes
