Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9306 )
Change subject: IMPALA-5752: Add support for DECIMAL on Kudu tables ...................................................................... Patch Set 3: (4 comments) > For example, you could add tests for insert/update/delete with decimal values > that are outside the range for the precision/scale of the column, creating a > table with a decimal primary key, etc. Do these tests exist for the decimal type in general? For other formats like text, parquet, and Avro? I looked in the decimal.test and decimal_exprs.test workloads but didn't see anything. I suspect that sort of "validation" occurs before calling down to the specific format. If not, reusing the existing tests would be great. http://gerrit.cloudera.org:8080/#/c/9306/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9306/2//COMMIT_MSG@10 PS2, Line 10: > We generally include a "Testing" section here, eg. what tests were added, w Done http://gerrit.cloudera.org:8080/#/c/9306/1/be/src/exec/kudu-util.h File be/src/exec/kudu-util.h: http://gerrit.cloudera.org:8080/#/c/9306/1/be/src/exec/kudu-util.h@a69 PS1, Line 69: > Well its useful for debugging I filed KUDU-2303 to add a ToString method to KuduSchema itself. http://gerrit.cloudera.org:8080/#/c/9306/2/be/src/exec/kudu-util.cc File be/src/exec/kudu-util.cc: http://gerrit.cloudera.org:8080/#/c/9306/2/be/src/exec/kudu-util.cc@214 PS2, Line 214: > I imagine the formatter told you to do this. We don't follow it 100%, so yo Yeah, I just took what the formatter suggested. Will update. http://gerrit.cloudera.org:8080/#/c/9306/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java File fe/src/main/java/org/apache/impala/util/KuduUtil.java: http://gerrit.cloudera.org:8080/#/c/9306/2/fe/src/main/java/org/apache/impala/util/KuduUtil.java@400 PS2, Line 400: Fall through below */ > Another place you can ignore the formatter. Done -- To view, visit http://gerrit.cloudera.org:8080/9306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f927fce25dc4fa9529a4e0b688825699f5c0ea6 Gerrit-Change-Number: 9306 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Fri, 16 Feb 2018 16:21:58 +0000 Gerrit-HasComments: Yes
