Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201607300723.u6U7NqMi022161@ip-10-146-233-104.ec2.internal>
Date: Sat, 30 Jul 2016 07:23:52 +0000
From: "Internal Jenkins (Code Review)" <gerrit@cloudera.org>
To: Alex Behm <alex.behm@cloudera.com>, impala-cr@cloudera.com,
        dev@impala.incubator.apache.org
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-3861=3A_Replace_BetweenPredicates_with_their_equivalent_CompoundPredicate=2E=0A?=
In-Reply-To: <gerrit.1469693631000.I0838b30444ed9704ce6a058d30718a24caa7444a@gerrit.cloudera.org>
References: <gerrit.1469693631000.I0838b30444ed9704ce6a058d30718a24caa7444a@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: Sat, 30 Jul 2016 07:23:57 -0000

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3861: Replace BetweenPredicates with their equivalent CompoundPredicate.
......................................................................


IMPALA-3861: Replace BetweenPredicates with their equivalent CompoundPredicate.

The bug: Our BetweenPredicate has a complex object structure that is unlike
most other Exprs because we generate an equivalent CompoundPredicate during
analysis and replace the original children. Keeping the various members in
sync and preserving the object structure during clone() and substitute() is
very difficult and error prone. In particular, subquery rewriting is
difficult because we extract and replace correlated BinaryPredicates.
Substituting BinaryPredicates in a BetweenPredicate's children is not
equivalent to a substitution on the BetweenPredicat's original children,
so keeping the various redundant members in sync is quite difficult.

The fix is to replace BetweenPredicates with their equivalent CompoundPredicates
before performing subquery rewrites.

We ultimately still want to fix clone() and substitute() for BetweenPredicates,
but an elegant solution is likely to more involved.

Change-Id: I0838b30444ed9704ce6a058d30718a24caa7444a
Reviewed-on: http://gerrit.cloudera.org:8080/3804
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
---
M fe/src/main/java/com/cloudera/impala/analysis/StmtRewriter.java
M fe/src/test/java/com/cloudera/impala/analysis/AnalyzeSubqueriesTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
3 files changed, 53 insertions(+), 10 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Alex Behm: Looks good to me, approved


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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0838b30444ed9704ce6a058d30718a24caa7444a
Gerrit-PatchSet: 7
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: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Internal Jenkins