Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8830 )

Change subject: KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)
......................................................................


Patch Set 28:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8830/28/src/kudu/client/predicate-test.cc
File src/kudu/client/predicate-test.cc:

http://gerrit.cloudera.org:8080/#/c/8830/28/src/kudu/client/predicate-test.cc@978
PS28, Line 978: TEST_F(PredicateTest, TestDecimalPredicates) {
Sorry to keep harping on this, but I still feel we may be missing some 
important predicate coverage.  IIUC this is only covering Decimal32, right?  
I'd like to at least see Decimal128 coverage as well, since that's a completely 
new physical type.

In the follow up patch that does decimal primary keys we should make sure we 
have predicate coverage over decimal PKs as well, since the predicate 
optimization over PKs is extra special.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b06142f43c66973f36376bd2c88ca6f8d9f7632
Gerrit-Change-Number: 8830
Gerrit-PatchSet: 28
Gerrit-Owner: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 17:42:51 +0000
Gerrit-HasComments: Yes

Reply via email to