Anonymous Coward #27 has posted comments on this change. Change subject: IMPALA-3167: Fix assignment of WHERE-clause predicate through grouping agg + outer join. ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4960/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: PS1, Line 1426: hasOjTest Where is this function used? http://gerrit.cloudera.org:8080/#/c/4960/1/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: PS1, Line 851: public static <C extends Expr> void removeDuplicates(List<C> l) Suggest to refactor this function a bit like below (if possible): public static <C extends Expr> List<C> removeDuplicates(List<C> l) { if (l == null) { return l; } else { return Lists.newArrayList(Sets.newHashSet(l)); } } This makes this function thread-safe. The passed in list won't be modified, and thus can be safely shared among threads. PS1, Line 853: List<C> origList = Lists.newArrayList(l); : l.clear(); : for (C expr: origList) if (!l.contains(expr)) l.add(expr); Would it be better to use a Set instead of List? Set<C> s = Sets.newHashSet(l); l.clear(); l.addAll(s); -- To view, visit http://gerrit.cloudera.org:8080/4960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I774d13a13ad1e8fe82512df98dc29983bdd232eb Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Anonymous Coward #27 Gerrit-HasComments: Yes
