Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8569 )
Change subject: IMPALA-5310: Part 2: Add SAMPLED_NDV() function. ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8569/3/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/8569/3/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@524 PS3, Line 524: children_.set(1, > Done. I see. It seems dangerous to rely on knowing the implementation for LiteralExpr since (a) it could change, and (b) that's not the general contract (other Expr's don't work that way) and so kinda violates the abstraction. So, thanks for adding the explicit set. I see we have other places in the code that we rely on knowing how uncheckedCastTo() works at the caller that we may want to clean up later. -- To view, visit http://gerrit.cloudera.org:8080/8569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia51d56ee67ec6073e92f90bebb4005484138b820 Gerrit-Change-Number: 8569 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Comment-Date: Wed, 06 Dec 2017 17:08:44 +0000 Gerrit-HasComments: Yes
