date:20171208

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8758 )

Change subject: IMPALA-6190/6246: Add instances tab and event sequence
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/coordinator-backend-state.h@129
PS3, Line 129: InstancesToJson
Is InstanceStatsToJson() a more appropriate name ?


http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:

http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/fragment-instance-state.h@172
PS3, Line 172: PREPARE_START,
 : CODEGEN_START,
 : CODEGEN_END,
 : OPEN_START,
 : WAITING_FOR_BATCH,
 : BATCH_RECEIVED,
 : BATCH_SENT,
 : END_OF_STREAM,
 : EXEC_END
It'd be nice to add comments to indicate what each of this state means.


http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/fragment-instance-state.cc@241
PS3, Line 241: UpdateState(StateEvent::CODEGEN_END);
Doesn't this overlap with the codegen time shown in the RuntimeProfile ? Also, 
doesn't moving to the next state imply the end of codegen ?


http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/runtime/fragment-instance-state.cc@302
PS3, Line 302:   UpdateState(StateEvent::WAITING_FOR_BATCH);
 :   SCOPED_TIMER(plan_exec_timer);
 :   RETURN_IF_ERROR(
 :   exec_tree_->GetNext(runtime_state_, row_batch_.get(), 
_tree_complete));
 :   UpdateState(StateEvent::BATCH_RECEIVED);
 : }
 : if (VLOG_ROW_IS_ON) 
row_batch_->VLogRows("FragmentInstanceState::ExecInternal()");
 : COUNTER_ADD(rows_produced_counter_, row_batch_->num_rows());
 : RETURN_IF_ERROR(sink_->Send(runtime_state_, 
row_batch_.get()));
 : UpdateState(StateEvent::BATCH_SENT);
In terms of observability, how much benefit is there for updating the state 
once for every row batch produced vs just updating the state once before 
entering the loop to indicate that we are producing row ?

I guess the finer granularity is helpful if a fragment instance is stuck but 
then we can also infer that from various timers in the runtime profile too. On 
the other hand, that's kind of the point of this patch to make it easier to 
find out at which state of execution is a fragment instance stuck. I am a bit 
worried about the cost of updating the state as UpdateState() looks a bit heavy 
weight too.

I wonder if just recording the state of a fragment instance:  PREPARE, OPEN, 
EXEC, and EOS is sufficient for initial pass or is it not enough details ?


http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/service/impala-http-handler.h
File be/src/service/impala-http-handler.h:

http://gerrit.cloudera.org:8080/#/c/8758/3/be/src/service/impala-http-handler.h@99
PS3, Line 99: backend states
fragment instances ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I626456b6afa9101ffd5cda10c4096d63d7f9
Gerrit-Change-Number: 8758
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Sat, 09 Dec 2017 02:48:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 5
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 09 Dec 2017 01:42:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs
..

IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs

For some time Impala in a production environment has been able
to access data stored in Amazon S3 buckets using credentials specified
in a number of ways:
- storing Amazon access keys in environment variables or
in core-site.xml.
- using proprietary management tools to store Amazon access keys
securely
- using Amazon IAM roles bound to VMs running in EC2.

The development minicluster environment used the first approach,
which risked leaking these keys.

This change enables Impala builds to use IAM
roles to access S3 buckets when running on an Amazon EC2 virtual
machine. The changes mainly ensure that environment variables carrying
the traditional AWS credentials do not conflict with credentials supplied
by the IAM role attached to the VM instance.

IAM role based credentials are accessible through the EC2
instance-property mechanism; for further details see Amazon's docs at
http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials

The change also removes the remaining references to the s3n: provider.
In the FE tests all URIs referring to s3n: are replaced with their
s3a: equivalents, except for a single negative test in
AnalyzeStmtsTest.java, which is removed.

In addition to the code changes, the s3n: and s3a: credential properties
are also removed from core-site.xml.tmpl. The s3a: provider can pick up
AWS S3 credentials from environment variables or IAM properties bound
to the VM instance, which is a more flexible approach.

As environment variables have precedence over IAM roles, care must be
taken when managing the canonical environment variables carrying
AWS credentials. There are two requirements to be reconciled:
1. The FE tests have code that examines s3a: URIs; this code needs
existing, but not necessarily valid AWS credentials.
2. When the Impala test suite is executed on an EC2 VM, AWS credentials
can be supplied via IAM roles. These credentials can be used only
if the AWS_* environment variables are unset (do not exist).

The tradeoff is managed following these rules:
1. When AWS_* environment variables are set before invoking the
Impala configuration scripts, their value is preserved and
the config scripts ensure that the variables are exported.
2. If the AWS_* variables are missing or empty, they will be unset
to ensure that credentials supplied by Amazon's IAM roles can be
accessed,
3. except if the scripts are running outside of EC2 (so there can be
no IAM roles) and TARGET_FILESYSTEM is not set "s3". This combination
is most often the case on a developer's local workstation.
In this case the AWS_* credential variables are forcibly set to
dummy values to allow the FE tests to succeed.
The removal of S3 credential parameters from core-site.xml[.tmpl]
also allows users to set up their own credentials there,
the config scripts will not change those settings.

Environment variables carrying AWS security credentials will be set
up according to the following table:

Instance: Running outside EC2 || Running in EC2 |
++++++
TARGET_FILESYSTEM | S3 | not S3 || S3 | not S3 |
++++++
||||||
empty | unset | dummy || unset | unset |
AWS_* ||||||
env --++++++
var ||||||
not empty | export | export || export | export |
||||||
++++++

Legend: unset: the variable is unset
export: the variable is exported with its current value
dummy: the variable is set to a preset dummy value and
exported

Running on an EC2 VM is indicated by setting RUNNING_IN_EC2 to "true" and
exporting it before impala_config.sh is invoked.

The change also moves the logic performing the S3 access checks into a separate
script file: bin/check-s3-access.sh. This file now contains all the S3-specific
logic and network access to check if the requested S3 bucket can be accessed.

Testing:

Performed local builds for HDFS as well as automated builds against
HDFS and S3, using both IAM roles and explicit AWS_* credentials for
authentication.
Verified that FE tests that parse s3a: URLs are still successful in
all these combinations (when they are run).

Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Reviewed-on: http://gerrit.cloudera.org:8080/8294

[Impala-ASF-CR] IMPALA-5754: Improve randomness of rand()/random()

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8355 )

Change subject: IMPALA-5754: Improve randomness of rand()/random()
..


Patch Set 16: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1600/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
Gerrit-Change-Number: 8355
Gerrit-PatchSet: 16
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Sat, 09 Dec 2017 01:15:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8034 )

Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
..


Patch Set 18:

@pranay - looks like compilation failed 
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/775/console


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299
Gerrit-Change-Number: 8034
Gerrit-PatchSet: 18
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:24:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6242: Reduce flakiness in TimerCounterTest

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8670 )

Change subject: IMPALA-6242: Reduce flakiness in TimerCounterTest
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1603/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc038908857060ccbabfe30c46e72fd93907f412
Gerrit-Change-Number: 8670
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:22:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6242: Reduce flakiness in TimerCounterTest

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8670 )

Change subject: IMPALA-6242: Reduce flakiness in TimerCounterTest
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc038908857060ccbabfe30c46e72fd93907f412
Gerrit-Change-Number: 8670
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:22:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8775 )

Change subject: IMPALA-4993: extend dictionary filtering to collections
..


Patch Set 1:

(19 comments)

The approach looks good to me, I mainly have some clarifying questions.

http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:

http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-parquet-scanner.h@452
PS1, Line 452: Only scalar columns can be
 :   /// dictionary filtered
Can you mention in the comment why?


http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:

http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scan-node-base.h@167
PS1, Line 167: return thrift_dict_filter_conjuncts_;
You could make the calling code cleaner by returning an empty vector here. 
You'd need have a static empty vector though, so I'm not sure it's worth it.


http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scan-node-base.h@382
PS1, Line 382:   /// Dictionary filtering eligible conjuncts for each slot.
Can you add to the comment that this can be nullptr?


http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scanner.cc@90
PS1, Line 90: auto
We usually only use auto for iterators and const& in for loops. I think this 
would be more readable with the type spelled out, especially since it's not 
clear from its name what 'second' is. It seems to be 
std::vector, right?

I also cannot figure out why you use a pointer here instead of a const ref to 
the vector.


http://gerrit.cloudera.org:8080/#/c/8775/1/be/src/exec/hdfs-scanner.cc@97
PS1, Line 97: entry.tuple_id
you could just use 'tuple_id' here (from L86).


http://gerrit.cloudera.org:8080/#/c/8775/1/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/8775/1/common/thrift/PlanNodes.thrift@202
PS1, Line 202: between a TupleId, SlotId pair to a
nit: I think it should be "mapping between ... and ..." or "mapping from ... to 
..."? I may be wrong though.


http://gerrit.cloudera.org:8080/#/c/8775/1/common/thrift/PlanNodes.thrift@209
PS1, Line 209: optional
Shouldn't they all be required?


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@187
PS1, Line 187:   // Map from pairs of TupleDescriptor, SlotIds to indices in 
PlanNodes.conjuncts_ and
I think this comment could benefit from a description of what makes a 
slot/conjunct eligible (depending on a single slot). Overall I think it would 
help to have a paragraph that describes how dictionary filtering works but I'm 
not yet sure where it would fit best.


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@188
PS1, Line 188: collectionConjuncts
nit: missing _


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@188
PS1, Line 188:   // collectionConjuncts that are eligible for dictionary 
filtering. The TupleDescriptor
nit: I'd phrase it as "slots in the TupleDescriptor of this scan node map to 
indices...".


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@624
PS1, Line 624: Preconditions.checkState(tupleIds.size() == 1);
It doesn't seem obvious to me why this is true. Can you add a comment?


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@656
PS1, Line 656:* Walks through conjuncts and collectionConjuncts and 
populates
nit: trailing _s


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@668
PS1, Line 668: notEmptyCollections_.contains(entry.getKey())
This could be factored into its own method together with the comment above. I 
suspect the comment is hard to understand if you don't already know what it's 
about. Maybe add a reference to SelectStmt#registerIsNotEmptyPredicates as an 
additional breadcrumb?


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1010
PS1, Line 1010: for (Map.Entry, 
List> entry : dictionaryFilterConjuncts_.entrySet()) {
long line


http://gerrit.cloudera.org:8080/#/c/8775/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1075
PS1, Line 1075:   if (!dictionaryFilterConjuncts_.isEmpty()) {
This check looks like it's not needed.

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1602/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:11:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> I think the default whitelist should work with all existing CpuInfo::IsSupp
I don't think we should include that kind of refactoring in this patch. I'll 
file a follow-on.

Made the flag hidden.


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@115
PS2, Line 115: additional flags
> nit: "enable additional LLVM CPU attribute flags", to disambiguate from dif
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:10:51 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Hello Michael Ho, Jim Apple, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8802

to look at the new patch set (#5).

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..

IMPALA-6291: disable AVX512 codegen in LLVM

Adds a whitelist of LLVM CPU attributes that I know that we routinely
test Impala with. This excludes the problematic AVX512 attributes as
well as some other flags we don't test with - e.g. AMD-only
instructions, NVM-related instructions, etc. We're unlikely to get
significant benefit from these instruction set extensions without
explicitly using them via instrinsics.

Testing:
Ran core tests on a system with AVX512 support with a prototype patch
that disabled only the AVX512 flags. Added a backend test to make sure
that the whitelisting is working as expected.

Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
---
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
3 files changed, 71 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8802/5
--
To view, visit http://gerrit.cloudera.org:8080/8802
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 5: Code-Review+2

carry


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 09 Dec 2017 00:11:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:59:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> Yes, I like that idea of using cpu_attrs_ in IRBuilder.
I think the default whitelist should work with all existing 
CpuInfo::IsSupported() uses so I am fine with merging the patch to work around 
the issue.

I prefer to have this flag hidden by default so that users would not 
accidentally change it.

We may need a follow-on patch to address the CpuInfo mask or IRBuilder to make 
sure they are consistent with the cpu_attrs_.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:59:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8791 )

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..

IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

When a sort is inserted into a plan for an INSERT due to either the
target table being a Kudu table or the use of the 'clustered' hint,
and a TupleIsNullPredicate is present in the output of the sort, the
TupleIsNullPredicate may reference an incorrect tuple (i.e. not the
materialized sort tuple), leading to errors.

The solution is to materialize the TupleIsNullPredicate into the sort
tuple and then perform the appropriate expr substitutions, as is
already done for the case of analytic sorts.

Testing:
- Added an e2e test with a query that would previously fail.

Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Reviewed-on: http://gerrit.cloudera.org:8080/8791
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test
4 files changed, 60 insertions(+), 15 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> We'd have to maintain a mapping from LLVM's names to our CPU info flags, wh
Yes, I like that idea of using cpu_attrs_ in IRBuilder.

Do you want to put that in a follow-on patch or in this one?

Michael, how do you feel about the state of this patch?


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@115
PS2, Line 115: additional flags
nit: "enable additional LLVM CPU attribute flags", to disambiguate from 
different impalad startup flags.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:52:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6245: Tolerate column indenting from Hive

2017-12-08 Thread Zach Amsden (Code Review)

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8719 )

Change subject: IMPALA-6245: Tolerate column indenting from Hive
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
File testdata/workloads/functional-query/queries/QueryTest/views-ddl.test:

http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@281
PS4, Line 281:  QUERY
> Move this and the query validation into Python as well. We don't want to ha
Done


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@289
PS4, Line 289: # Test that the plan respects the plan hints for shuffle and 
broadcast
> I suggest moving this into Python as well. Might want to use explain_level=
Done


http://gerrit.cloudera.org:8080/#/c/8719/4/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@327
PS4, Line 327: # Test querying the hinted view.
> Move into Python as well.
Done


http://gerrit.cloudera.org:8080/#/c/8719/1/tests/common/test_result_verifier.py
File tests/common/test_result_verifier.py:

http://gerrit.cloudera.org:8080/#/c/8719/1/tests/common/test_result_verifier.py@100
PS1, Line 100:   'Number of columns returned > the number of column 
types: %s' % column_types
Garbage, will post another diff



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I49e53b1230520ca6e850af28078526e6627d69de
Gerrit-Change-Number: 8719
Gerrit-PatchSet: 4
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:47:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6114: Require type equality of NumericLiteral::localEquals().

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8448 )

Change subject: IMPALA-6114: Require type equality of 
NumericLiteral::localEquals().
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8448/2/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
File fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java:

http://gerrit.cloudera.org:8080/#/c/8448/2/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java@141
PS2, Line 141: if ((isAnalyzed() && tmp.isAnalyzed()) && (type_ != 
tmp.getType())) return false;
> 1. Done.
2. NullLiterals may be cast to any type. Casts are applied in-place and will 
directly change the type of literals (including NullLiteral).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
Gerrit-Change-Number: 8448
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:30:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 23:11:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..

IMPALA-6286: Remove invalid runtime filter targets.

If the target expression of a runtime filter evaluates to a
non-NULL value for outer-join non-matches, then assigning
the filter below the nullable side of an outer join may
lead to incorrect query results.
See IMPALA-6286 for an example and explanation.

This patch adds a conservative check that prevents the
creation of runtime filters that could potentially
have such incorrect targets. Some safe opportunities
are deliberately missed to keep the code simple.
See RuntimeFilterGenerator#getTargetSlots().

Testing:
- added planner tests which passed locally

Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Reviewed-on: http://gerrit.cloudera.org:8080/8783
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
4 files changed, 168 insertions(+), 43 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 9
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-5191: Behavior of column aliases should be more standard conforming

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Behavior of column aliases should be more standard 
conforming
..


Patch Set 1:

Please see my comments on the JIRA


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:56:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:42:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..

IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

Previously, CodegenAnyVal used an LLVM function for floating point
comparison that considered 'nan' = 'nan' to be true. This is
inconsistent with the way we handle 'nan' in the non-codegen path,
where we consider 'nan' = 'nan' to be false, leading to inconsisent
results.

This patch fixes CodegenAnyVal to use an LLVM function for floating
point comparison that considers 'nan' = 'nan' to be false.

Testing:
- Added e2e tests for the two scenarios affected by this: CASE and
  joins.

Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Reviewed-on: http://gerrit.cloudera.org:8080/8790
Reviewed-by: Tim Armstrong 
Reviewed-by: Michael Ho 
Tested-by: Impala Public Jenkins
---
M be/src/codegen/codegen-anyval.cc
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M testdata/workloads/functional-query/queries/QueryTest/joins.test
3 files changed, 36 insertions(+), 2 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, but someone else must approve
  Michael Ho: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 3:

Right, I'm sure it calls getHostCpuFeatures() like we do, which is this lovely 
function: http://llvm.org/doxygen/Host_8cpp_source.html#l01129

Michael's suggestion, if I understood it correctly, was to alter our CpuInfo 
based on this flag, which would require translating llvm's names into our own 
CpuInfo flags.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:37:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6225: Part 2: Query profile date-time strings should have ns precision.

2017-12-08 Thread Philip Zeyliger (Code Review)

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8784 )

Change subject: IMPALA-6225: Part 2: Query profile date-time strings should 
have ns precision.
..


Patch Set 3:

(3 comments)

Thanks for the iteration. I think this is almost done. Just a few more comments.

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py
File tests/common/impala_service.py:

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py@64
PS3, Line 64:   def get_thrift_profile(self, query_id, timeout=10, interval=1):
please add pydoc. This will get used again, I'm sure.


http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py@73
PS3, Line 73:   LOG.info("Thrift profile for query %s not yet available.")
Is there a more specific exception you can use? Alternately, put the try/except 
around line 68. I worry that zlib.decompress, and base64, and deserialize can 
all throw interesting exceptions that I wouldn't want to swallow.

Also: should this be returning None? or re-throwing? Are you intentionally 
returning an empty thrift thingy?


http://gerrit.cloudera.org:8080/#/c/8784/3/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/query_test/test_observability.py@125
PS3, Line 125: service = ImpalaCluster().get_first_impalad().service
I think this maybe should be

service = self.cluster.impalads[0].service

i.e., you shouldn't be creating a new ImpalaCluster(), but rather using the 
onet hat your test has provided you.

"git grep self.cluster.get" is more prominent than "git grep ImpalaCluster\(", 
though the latter certainly identifies several classes of this bug.

09:03:15 [130]pannier Impala [maven ?*] ~/src/Impala
$git grep self.cluster.get
bin/remote_data_load.py:services = dict((s.type, s) for s in 
self.cluster.get_all_services())
tests/common/failure_injector.py:
self.cluster.get_impala_service().set_process_auto_restart_config(value=True)
tests/common/failure_injector.py:# 
self.cluster.get_impala_service().restart()
tests/common/failure_injector.py:num_impalad_procs = 
len(self.cluster.get_impala_service().get_all_impalad_processes())
tests/common/failure_injector.py:   
self.cluster.get_impala_service().get_all_impalad_processes())
tests/common/failure_injector.py:state_store = 
self.cluster.get_impala_service().get_state_store_process()
tests/comparison/cluster.py:endpoint = 
self.cluster.get_hadoop_config("dfs.namenode.http-address",
tests/comparison/cluster.py:  port = 
self.cluster.get_hadoop_config("dfs.https.port", 20102)
tests/comparison/cluster.py:
self.cluster.get_hadoop_config("hive.metastore.warehouse.dir")).path
tests/custom_cluster/test_insert_behaviour.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_insert_behaviour.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_query_concurrency.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_query_expiration.py:impalad = 
self.cluster.get_first_impalad()
tests/custom_cluster/test_query_expiration.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_query_expiration.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_s3a_access.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_seq_file_filtering.py:impalad = 
self.cluster.get_any_impalad()
tests/custom_cluster/test_session_expiration.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:worker_impalad = 
self.cluster.get_different_impalad(impalad)
tests/experiments/test_process_failures.py:impalad = 
self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:impalad = 
self.cluster.get_any_impalad()



--
To view, visit

[Impala-ASF-CR] IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1601/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 5
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:11:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs

2017-12-08 Thread Zach Amsden (Code Review)

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8294 )

Change subject: IMPALA-6067: Enable S3 access via IAM roles for EC2 VMs
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 5
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:10:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 3:

> (1 comment)

Clang must have a way of doing a mapping, since you can call it with 
"-march=native". Maybe we could re-use it?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:07:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] Add .pep8rc for Impala's Python style

Lars Volker has abandoned this change. ( http://gerrit.cloudera.org:8080/5829 )

Change subject: Add .pep8rc for Impala's Python style
..


Abandoned

Not finding time to work on this one.
--
To view, visit http://gerrit.cloudera.org:8080/5829
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I65a99fc6e364eb1e49e21d0ba97546bc25f65a27
Gerrit-Change-Number: 5829
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> Oh I see. Yeah I think that could cause us to hit LLVM asserts. Definitely
This flag is definitely useful to workaround issues and we don't usually expect 
users to muck with it. I agree using a flag provides more flexibility for 
tweaking.

I wonder how hard it is to also update CpuInfo mask based on the whitelist ? 
Will it be too error-prone ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 21:38:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> Oh I see. Yeah I think that could cause us to hit LLVM asserts. Definitely
The only reason I made it a flag instead of hardcoding it was to allow 
developers to tweak it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 21:34:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5754: Improve randomness of rand()/random()

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8355 )

Change subject: IMPALA-5754: Improve randomness of rand()/random()
..


Patch Set 16: Code-Review+2

Thanks for updating the patch to move to  pcg32. LGTM.

Not sure if clang-tidy may flag styling related stuff in the newly added 
thirdparty library but we'll see.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
Gerrit-Change-Number: 8355
Gerrit-PatchSet: 16
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Fri, 08 Dec 2017 21:34:26 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 21:12:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6114: Require type equality of NumericLiteral::localEquals().

2017-12-08 Thread Zoram Thanga (Code Review)

Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8448 )

Change subject: IMPALA-6114: Require type equality of 
NumericLiteral::localEquals().
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8448/2/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
File fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java:

http://gerrit.cloudera.org:8080/#/c/8448/2/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java@141
PS2, Line 141: if ((isAnalyzed() && tmp.isAnalyzed()) && 
(!getType().equals(tmp.getType( return false;
> 1. Use equals() for the type
1. Done.
2. Not clear about how to add this for NullLiteral, because its type is always 
NULL.

Test case has been added.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
Gerrit-Change-Number: 8448
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 08 Dec 2017 20:35:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6114: Require type equality of NumericLiteral::localEquals().

2017-12-08 Thread Zoram Thanga (Code Review)

Zoram Thanga has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8448 )

Change subject: IMPALA-6114: Require type equality of 
NumericLiteral::localEquals().
..

IMPALA-6114: Require type equality of NumericLiteral::localEquals().

This patch fixes a regression introduced as part of IMPALA-1788, where
an expression like 'CAST(0 AS DECIMAL(14))' is rewritten as a
NumericLiteral expression of type DECIMAL(14,0). The query had
another NumericLiteral of type TINYINT. While analyzing the DISTINCT
aggregation clause of the SELECT query, AggregateInfo::create() removes
duplicate expressions from groupingExprs.

NumericLiteral::localEquals() is used to check for equality. Now
since the method does not consider expression types, a TINYINT literal
is considered to be duplicate of a DECIMAL literal. This results in a
query like the following to fail:

SELECT DISTINCT CAST(0 AS DECIMAL(14), 0 FROM functional.alltypes

We propose to fix the issue by accounting for types as well
when comparing analyzed numeric literals. A test case has been added
to AnalyzeExprsTest.

Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
---
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 13 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/8448/3
--
To view, visit http://gerrit.cloudera.org:8080/8448
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
Gerrit-Change-Number: 8448
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga

[Impala-ASF-CR] IMPALA-6114: Require type equality of NumericLiteral::equals().

2017-12-08 Thread Zoram Thanga (Code Review)

Zoram Thanga has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8448


Change subject: IMPALA-6114: Require type equality of NumericLiteral::equals().
..

IMPALA-6114: Require type equality of NumericLiteral::equals().

This patch fixes a regression introduced as part of IMPALA-1788, where
an expression like 'CAST(0 AS DECIMAL(14))' is rewritten as a
NumericLiteral expression of type DECIMAL(14,0). The query had
another NumericLiteral of type TINYINT. While analyzing the DISTINCT
aggregation clause of the SELECT query, AggregateInfo::create() removes
duplicate expressions from groupingExprs.

NumericLiteral::equals() is used to check for equality. Now
since the method does not consider expression types, a TINYINT literal
is considered to be duplicate of a DECIMAL literal. This results in a
query like the following to fail:

SELECT DISTINCT CAST(0 AS DECIMAL(14) as foo, 0 AS bar where ...

where foo and bar are columns of type DECIMAL and INT respectively.

We propose to fix the issue by accounting for types as well for
analyzed numeric literals.

Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
TODO: Add test case.
---
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
1 file changed, 7 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/8448/2
--
To view, visit http://gerrit.cloudera.org:8080/8448
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia88d54088dfd128b103759dc01103b6c35bf6257
Gerrit-Change-Number: 8448
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8791 )

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1599/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 08 Dec 2017 20:23:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> Isn't that just something we need to catch in code review and testing? If w
May be my question wasn't quite clear.

What if a user removes avx2 from the whitelist above ?

When FilterContext::CodegenInsert() runs, CpuInfo::IsSupported(CpuInfo::AVX2) 
can still return true if the system CPU supports this AVX2 and the branch to 
emit IRFunction::BLOOM_FILTER_INSERT_AVX2 will still be taken.

My question is whether the mismatch between the target attribute and the 
instruction emitted an issue or not ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 20:23:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> How do we ensure that codegen functions don't accidentally emit instruction
Isn't that just something we need to catch in code review and testing? If we're 
branching on CpuInfo::IsSupported() we should be testing both branches in some 
fashion.


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@155
PS2, Line 155: target_features_attr_
> My understanding is that even if we blacklisted some instructions not on th
Right. We also already handle the case where CPU attrs on the cross-compiled 
function are a subset of the system CPU attrs.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 20:07:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

2017-12-08 Thread Thomas Tauber-Marshall (Code Review)

Hello Alex Behm,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8791

to look at the new patch set (#3).

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..

IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

When a sort is inserted into a plan for an INSERT due to either the
target table being a Kudu table or the use of the 'clustered' hint,
and a TupleIsNullPredicate is present in the output of the sort, the
TupleIsNullPredicate may reference an incorrect tuple (i.e. not the
materialized sort tuple), leading to errors.

The solution is to materialize the TupleIsNullPredicate into the sort
tuple and then perform the appropriate expr substitutions, as is
already done for the case of analytic sorts.

Testing:
- Added an e2e test with a query that would previously fail.

Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
---
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test
4 files changed, 60 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/8791/3
--
To view, visit http://gerrit.cloudera.org:8080/8791
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8791 )

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 08 Dec 2017 20:21:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8791 )

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8791/2/testdata/workloads/functional-query/queries/QueryTest/insert.test
File testdata/workloads/functional-query/queries/QueryTest/insert.test:

http://gerrit.cloudera.org:8080/#/c/8791/2/testdata/workloads/functional-query/queries/QueryTest/insert.test@949
PS2, Line 949: # IMPALA-6280: clustered with outer join, inline view, and 
TupleisNullPredicate
Are you sure this test is executed even without a RESULTS section?


http://gerrit.cloudera.org:8080/#/c/8791/2/testdata/workloads/functional-query/queries/QueryTest/insert.test@954
PS2, Line 954: right outer join functional.alltypes t1 on t1.id = v.id
let's use alltypestiny for t1



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:38:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Hello Michael Ho, Jim Apple, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8802

to look at the new patch set (#3).

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..

IMPALA-6291: disable AVX512 codegen in LLVM

Adds a whitelist of LLVM CPU attributes that I know that we routinely
test Impala with. This excludes the problematic AVX512 attributes as
well as some other flags we don't test with - e.g. AMD-only
instructions, NVM-related instructions, etc. We're unlikely to get
significant benefit from these instruction set extensions without
explicitly using them via instrinsics.

Testing:
Ran core tests on a system with AVX512 support with a prototype patch
that disabled only the AVX512 flags. Added a backend test to make sure
that the whitelisting is working as expected.

Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
---
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
3 files changed, 70 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8802/3
--
To view, visit http://gerrit.cloudera.org:8080/8802
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
> And as the recommended LLVM version increases
I think we should be doing that manually when we are confident that we'll be 
routinely testing on systems with those features. It'll be a bit annoying to 
have to do this but it avoids surprises.


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@111
PS2, Line 111: 
"mmx,rdseed,hle,xsave,invpcid,avx,rtm,fma,bmi,rdrnd,sse4.1,sse4.2,avx2,sse,lzcnt,"
I sorted these lists to make them more readable.


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@1663
PS2, Line 1663:   result.emplace_back(attr);
> nit: I think this is the same as push_back, due to reference collapsing. Do
Switched to push_back() everywhere since I thing it's the same -- all the args 
are either const string& or string&&. The move() doesn't make sense for the 
const string& argument here.


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@1666
PS2, Line 1666: string attr_name = attr.substr(1);
> nit: const string
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:37:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1598/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:35:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:34:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Hello Dimitris Tsirogiannis, Vuk Ercegovac, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8783

to look at the new patch set (#8).

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..

IMPALA-6286: Remove invalid runtime filter targets.

If the target expression of a runtime filter evaluates to a
non-NULL value for outer-join non-matches, then assigning
the filter below the nullable side of an outer join may
lead to incorrect query results.
See IMPALA-6286 for an example and explanation.

This patch adds a conservative check that prevents the
creation of runtime filters that could potentially
have such incorrect targets. Some safe opportunities
are deliberately missed to keep the code simple.
See RuntimeFilterGenerator#getTargetSlots().

Testing:
- added planner tests which passed locally

Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
4 files changed, 168 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/8783/8
--
To view, visit http://gerrit.cloudera.org:8080/8783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 8
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1491
PS6, Line 1491: d
> nit: predicates
Done


http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1533
PS6, Line 1533: isTrueWithNullSlots
> ok, that's what I was looking for. for runtime filter and dictionary prunin
I adjusted the function comment to make these point explicit.


http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1923
PS6, Line 1923: return FeSupport.EvalPredicate(nullTuplePred, 
getQueryCtx());
> makes sense-- we should then see the BE failure for TupleIsNullPredicate in
exactly



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:33:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5993: Fix the file offset in value parsing error

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8747 )

Change subject: IMPALA-5993: Fix the file offset in value parsing error
..


Patch Set 4:

Can you adjust the logging details according to the discussion in the Jira?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a4ac3199ffe12fcce0bf792b3e6ce529b9af61f
Gerrit-Change-Number: 8747
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:29:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1597/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:07:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8790/2/testdata/workloads/functional-query/queries/QueryTest/joins.test
File testdata/workloads/functional-query/queries/QueryTest/joins.test:

http://gerrit.cloudera.org:8080/#/c/8790/2/testdata/workloads/functional-query/queries/QueryTest/joins.test@774
PS2, Line 774:  QUERY
> disable_codegen_rows_threshold=0 is already set, as a default from ImpalaTe
Thanks for confirming.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 19:03:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

2017-12-08 Thread Thomas Tauber-Marshall (Code Review)

Hello Alex Behm,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8791

to look at the new patch set (#2).

Change subject: IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts
..

IMPALA-6280: Materialize TupleIsNullPredicate for insert sorts

When a sort is inserted into a plan for an INSERT due to either the
target table being a Kudu table or the use of the 'clustered' hint,
and a TupleIsNullPredicate is present in the output of the sort, the
TupleIsNullPredicate may reference an incorrect tuple (i.e. not the
materialized sort tuple), leading to errors.

The solution is to materialize the TupleIsNullPredicate into the sort
tuple and then perform the appropriate expr substitutions, as is
already done for the case of analytic sorts.

Testing:
- Added an e2e test with a query that would previously fail.

Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
---
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test
4 files changed, 57 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/8791/2
--
To view, visit http://gerrit.cloudera.org:8080/8791
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c0ca717aa4321a5cc84edd1d5857912f8c85583
Gerrit-Change-Number: 8791
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Alex Behm

[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8621 )

Change subject: IMPALA-3703: Store query context in thread-local variables
..


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8621/5/be/src/common/thread-debug-info.h
File be/src/common/thread-debug-info.h:

http://gerrit.cloudera.org:8080/#/c/8621/5/be/src/common/thread-debug-info.h@81
PS5, Line 81:   /// it cuts off the ending of the string
Would it make sense to keep the last few bytes of the string and truncate it in 
the middle? I feel that often long thread names would be generated and might 
have some counter at the end.


http://gerrit.cloudera.org:8080/#/c/8621/4/be/src/common/thread-info.h
File be/src/common/thread-info.h:

http://gerrit.cloudera.org:8080/#/c/8621/4/be/src/common/thread-info.h@56
PS4, Line 56:
> The idea was to store some description of the thread that is easy to read f
I feel that having separate members may make it easier to use this in debugging 
tools, e.g. by enumerating all the threads and extracting particular values for 
each thread. Maybe Tim had some other usage in mind that requires us to have a 
general-purpose text field?


http://gerrit.cloudera.org:8080/#/c/8621/4/be/src/common/thread-info.h@103
PS4, Line 103:
> I wanted to choose a size that will be large enough for the purpose, so I c
See my comment above about special purpose members.


http://gerrit.cloudera.org:8080/#/c/8621/4/be/src/common/thread-info.h@104
PS4, Line 104:
> I wanted it to be power of two, and also thought that it might be getting b
I agree that it's helpful to keep these as a string. Is there a benefit from 
having the size be a power of two?


http://gerrit.cloudera.org:8080/#/c/8621/4/be/src/common/thread-info.h@112
PS4, Line 112:
> If you think of the name_copy variable, note that it is a std::string. It o
Thanks for the clarification, that makes sense.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Gerrit-Change-Number: 8621
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 08 Dec 2017 18:41:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@110
PS2, Line 110: DEFINE_string(llvm_cpu_attr_whitelist, 
"sse2,cx16,prfchw,bmi2,fsgsbase,popcnt,aes,smap,"
What is the plan for keeping this up-to-date as Intel and AMD CPUs get more and 
more instruction sets?


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@1663
PS2, Line 1663:   result.emplace_back(attr);
nit: I think this is the same as push_back, due to reference collapsing. Do you 
want

result.emplace_back(std::move(attr))

Same for the call on line 1670, but not the on on 1673, which I think is an 
rvalue reference already


http://gerrit.cloudera.org:8080/#/c/8802/2/be/src/codegen/llvm-codegen.cc@1666
PS2, Line 1666: string attr_name = attr.substr(1);
nit: const string



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Fri, 08 Dec 2017 18:25:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5929: Remove redundant explicit casts to string

2017-12-08 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8660 )

Change subject: IMPALA-5929: Remove redundant explicit casts to string
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8660/1/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java
File fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java:

http://gerrit.cloudera.org:8080/#/c/8660/1/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@55
PS1, Line 55: oolean isPotentiallyRedundantCast
> I intentionally did this for improving code readability to avoid putting hu
Not a big deal either way, but I don't think it really improves readabililty



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I91b7c6452d0693115f9b9ed9ba09f3ffe0f36b2b
Gerrit-Change-Number: 8660
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 18:24:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

2017-12-08 Thread Thomas Tauber-Marshall (Code Review)

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8790/2/testdata/workloads/functional-query/queries/QueryTest/joins.test
File testdata/workloads/functional-query/queries/QueryTest/joins.test:

http://gerrit.cloudera.org:8080/#/c/8790/2/testdata/workloads/functional-query/queries/QueryTest/joins.test@774
PS2, Line 774:  QUERY
> Are these queries effective in testing 'nan' != 'nan' only when codegen is 
disable_codegen_rows_threshold=0 is already set, as a default from 
ImpalaTestSuite::__create_exec_option_dimension

I also verified that the test doesn't pass without the fix



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:58:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

2017-12-08 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..

Patch Set 7: Code-Review+1

(3 comments)

thanks for the explanation... looks fine to me.

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1491
PS6, Line 1491: d
nit: predicates

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1533
PS6, Line 1533: isTrueWithNullSlots
> The predicates generated in this function are for optimization purposes and
ok, that's what I was looking for. for runtime filter and dictionary pruning,
they're clearly for optimization purposes so skipping is easy to reason about.
its unclear for this method (getBoundPredicates)-- that a caller can expect
some predicates to not be bound due to env/runtime issues. In particular, the
previous behavior would have aborted this query, right? In which case, I'd
expect additional queries to run with this change.

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1923
PS6, Line 1923: }
> Btw, I plan to add new targeted tests for these expr evaluation failure cas
makes sense-- we should then see the BE failure for TupleIsNullPredicate
instead of the assertion failure... didn't see the TupleIsNullPredicate
example, since this change did not require changes at that callsite.
for, getBoundPredicate-- we'd expect to run some queries that previously would
fail the assertion.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 7
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:56:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'

Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8790 )

Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan'
..


Patch Set 2:

The change looks good. Please address the question about the test and I can +2 
it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Gerrit-Change-Number: 8790
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:51:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1596/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:40:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1595/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:40:00 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Hello Michael Ho, Jim Apple, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8802

to look at the new patch set (#2).

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..

IMPALA-6291: disable AVX512 codegen in LLVM

Adds a whitelist of LLVM CPU attributes that I know that we routinely
test Impala with. This excludes the problematic AVX512 attributes as
well as some other flags we don't test with - e.g. AMD-only
instructions, NVM-related instructions, etc. We're unlikely to get
significant benefit from these instruction set extensions without
explicitly using them via instrinsics.

Testing:
Ran core tests on a system with AVX512 support with a prototype patch
that disabled only the AVX512 flags. Added a backend test to make sure
that the whitelisting is working as expected.

Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
---
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
3 files changed, 70 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8802/2
--
To view, visit http://gerrit.cloudera.org:8080/8802
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

2017-12-08 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8802 )

Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1595/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:38:14 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6291: disable AVX512 codegen in LLVM

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8802


Change subject: IMPALA-6291: disable AVX512 codegen in LLVM
..

IMPALA-6291: disable AVX512 codegen in LLVM

Adds a whitelist of LLVM CPU attributes that I know that we routinely
test Impala with. This excludes the problematic AVX512 attributes as
well as some other flags we don't test with - e.g. AMD-only
instructions, NVM-related instructions, etc. We're unlikely to get
significant benefit from these instruction set extensions without
explicitly using them via instrinsics.

Testing:
Ran core tests on a system with AVX512 support with a prototype patch
that disabled only the AVX512 flags. Added a backend test to make sure
that the whitelisting is working as expected.

Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
---
M be/src/codegen/llvm-codegen-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
3 files changed, 72 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/8802/1
--
To view, visit http://gerrit.cloudera.org:8080/8802
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Gerrit-Change-Number: 8802
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1923
PS6, Line 1923: return FeSupport.EvalPredicate(nullTuplePred, 
getQueryCtx());
> Correct.
Btw, I plan to add new targeted tests for these expr evaluation failure cases 
(essential vs. non-essential). I'd prefer to not include them in this patch 
because some PlannerTest infra changes are needed and I'd like to limit the 
scope of this correctness bug fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:11:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Hello Dimitris Tsirogiannis, Vuk Ercegovac, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8783

to look at the new patch set (#7).

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..

IMPALA-6286: Remove invalid runtime filter targets.

If the target expression of a runtime filter evaluates to a
non-NULL value for outer-join non-matches, then assigning
the filter below the nullable side of an outer join may
lead to incorrect query results.
See IMPALA-6286 for an example and explanation.

This patch adds a conservative check that prevents the
creation of runtime filters that could potentially
have such incorrect targets. Some safe opportunities
are deliberately missed to keep the code simple.
See RuntimeFilterGenerator#getTargetSlots().

Testing:
- added planner tests which passed locally

Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
4 files changed, 160 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/8783/7
--
To view, visit http://gerrit.cloudera.org:8080/8783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 7
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6286: Remove invalid runtime filter targets.

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8783 )

Change subject: IMPALA-6286: Remove invalid runtime filter targets.
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1533
PS6, Line 1533: isTrueWithNullSlots
> prior, this would have tripped an assertion on backend error. why is it ok
The predicates generated in this function are for optimization purposes and not 
required for query correctness. We should be able to tolerate the failure of 
such non-essential expr evaluations.


http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1909
PS6, Line 1909: boolean
> might be clearer to have an enum here: true, false, unknown.
If we switch to an enum then the exception cause it truly swallowed. I prefer 
to throw to preserve the root cause.

The caller is free to not eat the exception or log. Some callers of this 
function do not swallow the exception


http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1923
PS6, Line 1923: return FeSupport.EvalPredicate(nullTuplePred, 
getQueryCtx());
> just noting that prior, we explicitly asserted the exception case should no
Correct.
* The previous Preconditions check was triggered due to a low mem limit.
* Most callers of this function are trying to perform an optimization that is 
not required for query correctness. In those cases we might still be able to 
run the query if we ignore the exception and the optimization. I don't think 
the mem-limit test should be changed because we should be able to tolerate 
failures of such non-essential expr evaluations  without failing the query.
* One notable caller is TupleIsNullPredicate.requiresNullWrapping() which 
requires the evaluation to succeed for query correctness. That caller forwards 
the exception and will fail the query.
* I added LOG.warn() at the non-essential call sites.


http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/8783/6/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@640
PS6, Line 640: continue
> would it be useful to know that pruning could not be done due to a backend 
Added LOG.warn() here and in other non-essential callers.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88153eea9f4b5117df60366fad2bd91776b95298
Gerrit-Change-Number: 8783
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 08 Dec 2017 17:03:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6242: Reduce flakiness in TimerCounterTest