Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10898 )
Change subject: PREVIEW: IMPALA-7211: Fix the between predicate for decimals ...................................................................... Patch Set 1: (3 comments) change looks fine. main comments are about how obvious the change is for future reviewers. http://gerrit.cloudera.org:8080/#/c/10898/1/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java File fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java: http://gerrit.cloudera.org:8080/#/c/10898/1/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java@60 PS1, Line 60: ( remove or should something be added here? http://gerrit.cloudera.org:8080/#/c/10898/1/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java@62 PS1, Line 62: noFloats worth it to add a comment about why floats are specially handled? http://gerrit.cloudera.org:8080/#/c/10898/1/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java@72 PS1, Line 72: for(int i = 0; i < children_.size(); ++i) { if I read this correctly, for children with types decimal, int, int, we'll cast the ints to a min resolution decimal, which is independent of the decimal type. that may differ from current casting. for this not to matter, there must be some assumption with comparisons and decimal (of differing precisions)-- where is this described? worth it to elaborate here? -- To view, visit http://gerrit.cloudera.org:8080/10898 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312 Gerrit-Change-Number: 10898 Gerrit-PatchSet: 1 Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 10 Jul 2018 21:05:51 +0000 Gerrit-HasComments: Yes
