impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4252: Min-max runtime filters for Kudu
Date Tue, 07 Nov 2017 15:06:55 GMT
Thomas Tauber-Marshall has posted comments on this change. (

Change subject: IMPALA-4252: Min-max runtime filters for Kudu

Patch Set 11:

Commit Message:
PS9, Line 27: 
> Thanks for explaining.
File be/src/exec/filter-context.h:
PS11, Line 118: Materialize filter values.
> what does that mean?
File be/src/runtime/runtime-filter-bank.h:
PS11, Line 78:   /// may both be NULL, representing a filter that allows all rows to pass.
> is it the case that at most one of bloom_filter and min_max_filter should b
File be/src/util/min-max-filter.h:
PS11, Line 62: Materialize filter values
> what does that mean?
File common/thrift/PlanNodes.thrift:
PS9, Line 103:   12: optional string kudu_col_name
> case sensitive?
File fe/src/main/java/org/apache/impala/planner/
PS11, Line 359:     public Operator getJoinOp() { return exprCmpOp_; }
> getExprCmpOp()
PS11, Line 602:         if (!(targetExpr instanceof SlotRef)
> I think only explicit casts are problematic. Implicit casts should be ok, o
Right, we should be able to support integer implicit casts. 

The complication is that if, say, the calculated max is outside of the range for the type
of the targeted column, we can't just pass that value into Kudu as it will complain.

In that case, we would need to convert the max we send to be the max for the type of the targeted
column. I've added some code to the BE to do this conversion.
File testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test:
PS11, Line 144:     on a.month = cast(b.month + 10000 as int);
> Was this your change? Why the change?
This was necessary if we didn't support implicit casts on the target. Removed now.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02bad890f5b5f78388a3041bf38f89369b5e2f1c
Gerrit-Change-Number: 7793
Gerrit-PatchSet: 11
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Anonymous Coward #345
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Mostafa Mokhtar <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-Comment-Date: Tue, 07 Nov 2017 15:06:55 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message