Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201610192041.u9JKftA9018350@ip-10-146-233-104.ec2.internal>
Date: Wed, 19 Oct 2016 20:41:55 +0000
From: "Matthew Jacobs (Code Review)" <gerrit@cloudera.org>
To: Alex Behm <alex.behm@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Marcel Kornacker <marcel@cloudera.com>
Reply-To: mj@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4301=3A_Fix_IGNORE_NULLS_with_subquery_rewriting=2E=0A?=
In-Reply-To: <gerrit.1476593024000.I708de7925fe6aeef582fd7510da93d24c71229d9@gerrit.cloudera.org>
References: <gerrit.1476593024000.I708de7925fe6aeef582fd7510da93d24c71229d9@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Wed, 19 Oct 2016 20:42:02 -0000

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4301: Fix IGNORE NULLS with subquery rewriting.
......................................................................


Patch Set 4:

(2 comments)

I think this makes sense. I mention an alternative inline, I'm OK either way.

http://gerrit.cloudera.org:8080/#/c/4732/4/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java:

Line 771: 
An alternative to having to check *_IGNORE_NULLS here and in analyze() is to create the rewritten FunctionCallExpr with params that have IgnoreNulls set false. Then we can check isIgnoreNulls() and know it applies to the same original functions only.


PS4, Line 772:       fnCall_.setIsAnalyticFnCall(true);
             :       fnCall_.setIsInternalFnCall(true);
             :       fnCall_.analyzeNoThrow(analyzer);
             :       analyticFnName = getFnCall().getFnName();
             :       Preconditions.checkState(type_.equals(fnCall_.getType()));
not a real problem, but we'll re-execute this. that won't happen if we unset the ignore nulls flag in params.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I708de7925fe6aeef582fd7510da93d24c71229d9
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes