date:20170213

Jim Apple has posted comments on this change.

Change subject: A blog post about IMPALA-4916
..


Patch Set 1:

I'm not planning on committing this until I talk to a few other contributors 
and see if they would be interested in blogging from time to time.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) A blog post about IMPALA-4916

Jim Apple has uploaded a new change for review.

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

Change subject: A blog post about IMPALA-4916
..

A blog post about IMPALA-4916

While I'm here, fix IMPALA-4928 and allow the install instructions to
work on Linux.

Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae
---
M blog/2017/index.html
M blog/archive.html
M blog/index.html
M blog/posts/impala-blog-coming-soon/index.html
M blog/sitemap.xml
M blog/sitemapindex.xml
A nikola_site_generator/posts/where-did-i-leave-my-keys.md
M nikola_site_generator/requirements.txt
8 files changed, 195 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifa90bf5621ef6466a4821f77a6e8a8b20c5512ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

2017-02-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


IMPALA-4897: AnalysisException: specified cache pool does not exist

A few tests were added with IMPALA-1670 that made use of HDFS caching.
This patch moves these tests to a new file and only executes them
when the default filesystem is HDFS.

There was also a bug where the tests used absolute locations instead
of locations relative to the table they were in which could easily
collide with locations of other tables if they raced. That has been
fixed too.

Also added a testcase for testing alter table ADD multiple PARTITIONS
for non-HDFS filesystems.

Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Reviewed-on: http://gerrit.cloudera.org:8080/5972
Reviewed-by: Sailesh Mukil 
Tested-by: Impala Public Jenkins
---
A 
testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M tests/metadata/test_ddl.py
3 files changed, 129 insertions(+), 66 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Sailesh Mukil: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

2017-02-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] Make sure impala doesn't pickup the system's boost cmake module

Alex Behm has posted comments on this change.

Change subject: Make sure impala doesn't pickup the system's boost cmake module
..


Patch Set 1:

(2 comments)

Looks good to me, but would be good to track the info in a JIRA so it's easier 
for other's to find.

http://gerrit.cloudera.org:8080/#/c/5994/1//COMMIT_MSG
Commit Message:

Line 7: Make sure impala doesn't pickup the system's boost cmake module
Can you create an upstream JIRA for this and paste the stacks etc, in there?


Line 29: systems boost.
system's


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I759e1a4f8f69727cc1224bf460326140fd2131a2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add a build flag for the undefined behavior sanitizer, aka "ubsan".

Jim Apple has posted comments on this change.

Change subject: Add a build flag for the undefined behavior sanitizer, aka 
"ubsan".
..


Patch Set 5:

> Jim, if you haven't done so already, I resolved the merge conflicts
 > here against master as of this morning, as I wanted to build with
 > this locally.  Let me know (preferably via google message) if you
 > want the updated diff.

Thank you. Rebasing old patches can sometimes make more work for reviewers in 
gerrit, so I plan to hold off on rebase until I get a +2. If you would hold on 
to the diffs, I would appreciate it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88c7234bd7c5eb7404490a0913d90470c10835e7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: No

[Impala-ASF-CR] Qualify min() in header

2017-02-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: Qualify min() in header
..


Patch Set 1:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/267/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I840869bc2b8ffebc34f4bf4bbefe89976d4e54f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR] Add a build flag for the undefined behavior sanitizer, aka "ubsan".

Zach Amsden has posted comments on this change.

Change subject: Add a build flag for the undefined behavior sanitizer, aka 
"ubsan".
..


Patch Set 5:

Jim, if you haven't done so already, I resolved the merge conflicts here 
against master as of this morning, as I wanted to build with this locally.  Let 
me know (preferably via google message) if you want the updated diff.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88c7234bd7c5eb7404490a0913d90470c10835e7
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4809: Enable support for DECIMAL V2 in decimal casting.py

Zach Amsden has posted comments on this change.

Change subject: IMPALA-4809: Enable support for DECIMAL_V2 in decimal_casting.py
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5989/1/tests/query_test/test_decimal_casting.py
File tests/query_test/test_decimal_casting.py:

Line 79: assert expected == actual, "Cast: {0}, Expected: {1}, Actual: 
{2}".format(cast,\
+1 internets


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 3:

(2 comments)

Addressed Laurel's comments.

http://gerrit.cloudera.org:8080/#/c/5957/3/docs/topics/impala_auditing.xml
File docs/topics/impala_auditing.xml:

PS3, Line 39: To monitor how Impala data is being used within your 
organization, ensure
:   that your Impala authorization and authentication policies 
are effective,
:   and detect attempts at intrusion or unauthorized access to 
Impala
:   data, you can use the auditing feature in Impala 1.2.1 and 
higher
> Really long, rambling sentence. Could be broken up to make it easier to par
Done


PS3, Line 60: s
> Should be "each log file" (singular)
Fixed typo.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Hello Laurel Hale, Jim Apple,

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

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

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

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..

IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

This set of edits removes references and links to Cloudera Navigator
and Cloudera Manager from the auditing and lineage topics. Those
were either marked as 'hidden' or replaced with a generic suggestion
to use cluster management software with a focus on governance.

Some paragraphs with overflowing lines were also fixed.

Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
---
M docs/topics/impala_auditing.xml
M docs/topics/impala_lineage.xml
2 files changed, 57 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/5957/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5957
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4:

> Uploaded patch set 5.

Laurel, could you try building locally again? There was a missing tag issue 
with the impala_kerberos topic. I don't see any more build errors and the  html 
and PDF I built have all the changes across all the topics.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Hello Laurel Hale,

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

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

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

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..

IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

This is part 2 of the work being done to genericize the Impala security
topics. All references to Cloudera have been either marked 'hidden'
or replaced with links to the relevant open-source docs.

Note:
-Links to the standalone Cloudera ODBC driver doc have not been
removed.
-External links to the MIT Kerberos docs and Hadoop security
docs were added to impala_keydefs.

Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
---
M docs/impala_keydefs.ditamap
M docs/shared/impala_common.xml
M docs/topics/impala_delegation.xml
M docs/topics/impala_kerberos.xml
M docs/topics/impala_ldap.xml
5 files changed, 66 insertions(+), 42 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] Make sure impala doesn't pickup the system's boost cmake module

2017-02-13 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Make sure impala doesn't pickup the system's boost cmake module
..

Make sure impala doesn't pickup the system's boost cmake module

In some systems with an old boost installed system-wide the impala build
would fail with something like:

CMake Error at /usr/lib64/boost/BoostConfig.cmake:64 (get_target_property):
  get_target_property() called with non-existent target
  "boost_thread-shared".
Call Stack (most recent call first):
  toolchain/cmake-3.2.3-p1/share/cmake-3.2/Modules/FindBoost.cmake:206 
(find_package)
  CMakeLists.txt:116 (find_package)

CMake Error at /usr/lib64/boost/BoostConfig.cmake:72 (get_target_property):
  get_target_property() called with non-existent target
  "boost_thread-shared-debug".
Call Stack (most recent call first):
  toolchain/cmake-3.2.3-p1/share/cmake-3.2/Modules/FindBoost.cmake:206 
(find_package)
  CMakeLists.txt:116 (find_package)

This because, if it exists, cmake's FindBoost.cmake will look for and
use that module, even though boost's cmake build hasn't been maintained
in years and the impala build is actually configured to not use the
systems boost.

This patch sets the cmake flag Boost_NO_BOOST_CMAKE to ON, making sure
the old cmake module is not picked up.

Change-Id: I759e1a4f8f69727cc1224bf460326140fd2131a2
---
M CMakeLists.txt
1 file changed, 4 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I759e1a4f8f69727cc1224bf460326140fd2131a2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Laurel Hale has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 3: Code-Review+1

(2 comments)

Just a couple of "writing" nits, but it built very cleanly, all your changes 
came through, and you did a nice job of writing around the Cloudera references.

http://gerrit.cloudera.org:8080/#/c/5957/3/docs/topics/impala_auditing.xml
File docs/topics/impala_auditing.xml:

PS3, Line 39: To monitor how Impala data is being used within your 
organization, ensure
:   that your Impala authorization and authentication policies 
are effective,
:   and detect attempts at intrusion or unauthorized access to 
Impala
:   data, you can use the auditing feature in Impala 1.2.1 and 
higher
Really long, rambling sentence. Could be broken up to make it easier to parse.


PS3, Line 60: s
Should be "each log file" (singular)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Qualify min() in header

Jim Apple has posted comments on this change.

Change subject: Qualify min() in header
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I840869bc2b8ffebc34f4bf4bbefe89976d4e54f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4809: Enable support for DECIMAL V2 in decimal casting.py

Michael Ho has posted comments on this change.

Change subject: IMPALA-4809: Enable support for DECIMAL_V2 in decimal_casting.py
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5989/1//COMMIT_MSG
Commit Message:

PS1, Line 9: This change enables decimal_v2 for casting to decimal except
   : for cases which involve rounding/truncation
> What about for DECIMAL to DECIMAL casting that does involve rounding? Is th
Yes. Please see test_underflow().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4809: Enable support for DECIMAL V2 in decimal casting.py

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4809: Enable support for DECIMAL_V2 in decimal_casting.py
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5989/1//COMMIT_MSG
Commit Message:

PS1, Line 9: This change enables decimal_v2 for casting to decimal except
   : for cases which involve rounding/truncation
What about for DECIMAL to DECIMAL casting that does involve rounding? Is that 
enabled now?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 13: Code-Review+1

(1 comment)

Carry Michael and Zach's +1.

http://gerrit.cloudera.org:8080/#/c/5952/12/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
File fe/src/main/java/org/apache/impala/analysis/TypesUtil.java:

Line 25: import com.google.common.base.Preconditions;
removed this now unnecessary import.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Hello Michael Ho, Zach Amsden,

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

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

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

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..

IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.

For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.

DECIMAL Version 1:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0   |
+---+

DECIMAL Version 2:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0.33|
+---+

DECIMAL Version 1:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| NULL  |
+---+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL

DECIMAL Version 2:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| 10.00 |
+---+

Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/testutil/impalad-query-executor.h
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M tests/query_test/test_decimal_queries.py
11 files changed, 388 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/13
-- 
To view, visit http://gerrit.cloudera.org:8080/5952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 12: Code-Review+1

(7 comments)

Carry Michael and Zach's +1.

http://gerrit.cloudera.org:8080/#/c/5952/11/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

PS11, Line 1293: V2 entries have
> the V2's entry will have
Done


Line 1298: // Either testing version 1, or the results for version 2 were 
not specified.
> May want to DCHECK(!v2 || (!expected[1].null && expected[1].precision == 0)
Isn't that trivially true, though, given it's just the inverse of the above 
if-stmt?


PS11, Line 1392:   { "cast(1 as decimal(38,0)) % cast(3 as decimal(38,0))",
   : {{ false, 1, 38, 0 }} },
> Can we also have some cases such as:
Done


http://gerrit.cloudera.org:8080/#/c/5952/11/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

PS11, Line 237: scale and
> scale
Done


Line 237:   // We need to scale x up by the result scale and then do an integer 
divide.
> Please fix this comment - "We need to scale x up by the result scale and th
Done


Line 238:   // This truncates the result to the output scale.
> ditto
Done


PS11, Line 239: implement rounding when decimal_v2=true.
> May help to rephrase this to state that we need to implement rounding for d
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Hello Michael Ho, Zach Amsden,

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

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

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

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..

IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.

For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.

DECIMAL Version 1:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0   |
+---+

DECIMAL Version 2:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0.33|
+---+

DECIMAL Version 1:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| NULL  |
+---+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL

DECIMAL Version 2:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| 10.00 |
+---+

Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/testutil/impalad-query-executor.h
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M tests/query_test/test_decimal_queries.py
11 files changed, 389 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/12
-- 
To view, visit http://gerrit.cloudera.org:8080/5952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-2020: Add rounding for decimal casts

Zach Amsden has uploaded a new patch set (#6).

Change subject: IMPALA-2020: Add rounding for decimal casts
..

IMPALA-2020: Add rounding for decimal casts

This change adds support for DECIMAL_V2 rounding behavior for both
DECIMAL to INT and DOUBLE to DECIMAL casts.  Still writing tests,
but this is ready for human eyes to look at.

Testing: Still in progrress.

[localhost:21000] > select cast(0.5 AS int);
+--+
| cast(0.5 as int) |
+--+
| 0|
+--+
Fetched 1 row(s) in 0.01s
[localhost:21000] > select cast(cast(0.5999 as float) as decimal(5,1));
+-+
| cast(cast(0.5999 as float) as decimal(5,1)) |
+-+
| 0.5 |
+-+
Fetched 1 row(s) in 0.01s
[localhost:21000] > set decimal_v2=1;
DECIMAL_V2 set to 1
[localhost:21000] > select cast(0.5 AS int);
+--+
| cast(0.5 as int) |
+--+
| 1|
+--+
Fetched 1 row(s) in 0.01s
[localhost:21000] > select cast(cast(0.5999 as float) as decimal(5,1));
+-+
| cast(cast(0.5999 as float) as decimal(5,1)) |
+-+
| 0.6 |
+-+
Fetched 1 row(s) in 0.01s

Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
---
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr.h
M be/src/exprs/literal.cc
M be/src/runtime/decimal-test.cc
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M be/src/udf/udf.h
7 files changed, 149 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/5951/6
-- 
To view, visit http://gerrit.cloudera.org:8080/5951
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Jim Apple has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4:

> @Jim--Did you check the output? I got the same list of
 > "recoverable" errors and it _did_ build, but it didn't seem to pick
 > up the changes made when I looked at the raw  XML.

Yes. I had hoped to imply that I checked the output by my statement "the output 
produced is missing kerberos".

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Laurel Hale has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4:

@Jim--Did you check the output? I got the same list of "recoverable" errors and 
it _did_ build, but it didn't seem to pick up the changes made when I looked at 
the raw  XML.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Jim Apple has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4: -Code-Review

> I did a build and got a lot of errors and it doesn't look like the
 > build picked up any of your changes. I'm not sure why this is
 > happening. Suggest you try to build before you do your next patch
 > set.

I ran a test build: http://jenkins.impala.io:8080/job/docs-build/169/console

It succeeded, in that dita returned status 0, which is unix code for OK return. 
OTOH, the output produced is missing kerberos. This is probably a dita bug - 
when it misses output, it should return non-0.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Laurel Hale has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4: Code-Review-1

I did a build and got a lot of errors and it doesn't look like the build picked 
up any of your changes. I'm not sure why this is happening. Suggest you try to 
build before you do your next patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Zach Amsden has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5952/8/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 236:   }
Please fix this comment - "We need to scale x up by the result scale and then 
do an integer divide."

The transposition of scale and precision is quite confusing here.


http://gerrit.cloudera.org:8080/#/c/5952/11/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 237:   // We need to scale x up by the result precision and then do an 
integer divide.
Please fix this comment - "We need to scale x up by the result scale and then 
do an integer divide."

The transposition of precision and scale is quite confusing here.


Line 238:   // This truncates the result to the output precision.
ditto


http://gerrit.cloudera.org:8080/#/c/5952/11/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
File fe/src/main/java/org/apache/impala/analysis/TypesUtil.java:

Line 253: resultPrecision = p1 - s1 + s2 + resultScale;
Looks correct


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Zach Amsden has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 11: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Michael Ho has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 11: Code-Review+1

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5952/11/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

PS11, Line 1293: we'll have both
the V2's entry will have


Line 1298: // Either testing version 1, or the results for version 2 were 
not specified.
May want to DCHECK(!v2 || (!expected[1].null && expected[1].precision == 0));


PS11, Line 1365: static_cast(10) * 100ll *
   :100ll * 100ll
Feel free to ignore but you can consider using 
DecimalUtil::GetScaleMultiplier() here.


PS11, Line 1392:   { "cast(1 as decimal(38,0)) % cast(3 as decimal(38,0))",
   : {{ false, 1, 38, 0 }} },
Can we also have some cases such as:

cast(x as DECIMAL(0,38)) % cast(y as DECIMAL(0,38))
cast(x as DECIMAL(0,38)) % cast(y as DECIMAL(38,0))
cast(x as DECIMAL(38,0)) % cast(y as DECIMAL(0,38))


http://gerrit.cloudera.org:8080/#/c/5952/11/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

PS11, Line 237: precision
scale


PS11, Line 239: confirm with standard that truncate is okay.
May help to rephrase this to state that we need to implement rounding for 
decimal_v2.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Hello Impala Public Jenkins, Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..

IMPALA-4897: AnalysisException: specified cache pool does not exist

A few tests were added with IMPALA-1670 that made use of HDFS caching.
This patch moves these tests to a new file and only executes them
when the default filesystem is HDFS.

There was also a bug where the tests used absolute locations instead
of locations relative to the table they were in which could easily
collide with locations of other tables if they raced. That has been
fixed too.

Also added a testcase for testing alter table ADD multiple PARTITIONS
for non-HDFS filesystems.

Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
---
A 
testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M tests/metadata/test_ddl.py
3 files changed, 129 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/5972/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5972
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

2017-02-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 4:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/266/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 4: Code-Review+2

(1 comment)

Carry +2.

http://gerrit.cloudera.org:8080/#/c/5972/3/testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test
File 
testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test:

PS3, Line 1: # This test file covers alter table statements that add multiple 
partitions and use hdfs caching
   : 
> Can you change the description to something like "This test file covers alt
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

2017-02-13 Thread Dimitris Tsirogiannis (Code Review)

Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5972/3/testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test
File 
testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test:

PS3, Line 1: # IMPALA-1670: Support adding multiple partitions in ALTER TABLE 
ADD PARTITION
   : # with caching
Can you change the description to something like "This test file covers alter 
table statements that add multiple partitions and use hdfs caching" or 
something like that?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Qualify min() in header

2017-02-13 Thread Henry Robinson (Code Review)

Henry Robinson has uploaded a new change for review.

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

Change subject: Qualify min() in header
..

Qualify min() in header

Change-Id: I840869bc2b8ffebc34f4bf4bbefe89976d4e54f2
---
M be/src/exprs/anyval-util.h
1 file changed, 3 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I840869bc2b8ffebc34f4bf4bbefe89976d4e54f2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 3:

There were a few instances in the codebase where absolute paths were used for 
partitions and they ended up getting stored in: [namenode]/[partition_name].

They could race if multiple tests end up using the same paths, causing more 
files to exist than the test actually expects.

I've fixed those to paths relative to the table. Ran a private build and it 
passed too.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Hello Impala Public Jenkins, Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..

IMPALA-4897: AnalysisException: specified cache pool does not exist

A few tests were added with IMPALA-1670 that made use of HDFS caching.
This patch moves these tests to a new file and only executes them
when the default filesystem is HDFS.

There was also a bug where the tests used absolute locations instead
of locations relative to the table they were in which could easily
collide with locations of other tables if they raced. That has been
fixed too.

Also added a testcase for testing alter table ADD multiple PARTITIONS
for non-HDFS filesystems.

Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
---
A 
testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M tests/metadata/test_ddl.py
3 files changed, 130 insertions(+), 66 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-3406: [DOCS] Remove stale and Cloudera-specific URLs

Laurel Hale has posted comments on this change.

Change subject: IMPALA-3406: [DOCS] Remove stale and Cloudera-specific URLs
..


Patch Set 1:

Also, there is a merge conflict that must be resolved, but you probably already 
realize that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I26a03cc7912dcb398b1ea246e938dc5eaf6df764
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3406: [DOCS] Remove stale and Cloudera-specific URLs

Laurel Hale has posted comments on this change.

Change subject: IMPALA-3406: [DOCS] Remove stale and Cloudera-specific URLs
..


Patch Set 1: Code-Review+1

I grepped "archive.cloudera.com" on all the impala xml files (impala*) and saw 
the archive mentioned in "impala_admission.xml," 
impala_security_guidelines.xml," impala_txtfile.xml," and in "impala_udf.xml" 
Would it make sense to clean them all up with this fix, which does seem to work?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I26a03cc7912dcb398b1ea246e938dc5eaf6df764
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2020: Add rounding for decimal casts

Dan Hecht has posted comments on this change.

Change subject: IMPALA-2020: Add rounding for decimal casts
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

PS5, Line 303: return to_type(dv.whole_part(scale)); 
> Not really - I thought the saner behavior for ToInt was rounding and wrote 
I don't feel too strongly about it, but think the symmetry in the interfaces is 
nice. 

If you leave the non-rounding case here, then line 303 should be formatted this 
way per our style:

} else {
   return ...
}

We only omit line breaks and braces if the entire if-stmt fits on one line (and 
maybe doesn't have an else clause?).


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/literal.cc
File be/src/exprs/literal.cc:

Line 185: value_.decimal4_val = Decimal4Value::FromDouble(type, v, 
&overflow, true);
> In what sense?
Will Impala 2.9 and Impala 2.8 give different answers when decmial_v2=false?


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/runtime/decimal-value.h
File be/src/runtime/decimal-value.h:

PS5, Line 54: bool* overflow,
:   bool round
> Done.  Means I can't give a default value to round unless I also give a def
Regarding default arguments, let's avoid adding them unless they really make 
things simpler. Often, they just lead to coding errors since new users of the 
interface don't bother to think about the right values to pass.

Regarding default value for overflow, I don't think that makes sense since as 
you concluded, all callers should care about overflow. If they don't, that 
probably indicates a bug.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4809: Enable support for DECIMAL V2 in decimal casting.py

Michael Ho has uploaded a new change for review.

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

Change subject: IMPALA-4809: Enable support for DECIMAL_V2 in decimal_casting.py
..

IMPALA-4809: Enable support for DECIMAL_V2 in decimal_casting.py

This change enables decimal_v2 for casting to decimal except
for cases which involve rounding/truncation. In which case,
we only enable decimal_v2 for casting from numbers to decimal.
Due to prior misunderstanding, some of the changes made in
commit 2088cf2 weren't necessary. By default, Impala interprets
floating numbers as decimal unless they exceed the maximum
decimal type supported. This change also removess a minor quirk:
previously, the test will revert to using string to decimal
casting for numbers larger than 2^63-1. This is unnecessary as
Impala is able to treat numbers larger than that bound as
decimal type.

Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
---
M tests/query_test/test_decimal_casting.py
1 file changed, 23 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho

[Impala-ASF-CR] IMPALA-2020: Add rounding for decimal casts

Zach Amsden has posted comments on this change.

Change subject: IMPALA-2020: Add rounding for decimal casts
..


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

PS5, Line 303: return to_type(dv.whole_part(scale)); 
> any reason not to have ToInt<>() handle this case too by passing in 'round'
Not really - I thought the saner behavior for ToInt was rounding and wrote this 
cast first, which didn't require a parameter, whereas FromDouble does as it is 
used in places where no context is available.  I can add this if you think it 
is useful.


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/literal.cc
File be/src/exprs/literal.cc:

Line 185: value_.decimal4_val = Decimal4Value::FromDouble(type, v, 
&overflow, true);
> where is this used? will it break compatibility?
In what sense?


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/runtime/decimal-value.h
File be/src/runtime/decimal-value.h:

PS5, Line 54: bool* overflow,
:   bool round
> here and below, let's keep input parameters first, then output params.
Done.  Means I can't give a default value to round unless I also give a default 
value to overflow, but lets do that anyway.

Is it okay if I change the ABI here to be

>From Double (const ColumnType& t, double d, bool round = true, bool& overflow 
>= of_ignored)

and then have a static bool DecimalValue::of_ignored which will get inserted 
when the caller doesn't bother to check overflow.  (Actually, Q: can reference 
arguments even have default values?)  The reason being to prevent a nullptr 
deref crash.  Worth having a debate about references vs. pointers here.  
Passing a pointer allows us to omit the overflow check from ever happening (ptr 
== nullptr should be known at compile time) but we currently don't do that or 
null checks in any of the places where we pass back this style of overflow.  I 
think we probably will want the overflow check in all runtime cases so maybe 
this doesn't matter.


Line 232:   inline typename RESULT_T::underlying_type_t ToInt(int scale, bool& 
overflow) const;
> add a function comment.
Done


PS5, Line 232:  bool& overflow
> use 'bool* overflow'. we generally use pointers for output params to make i
You have answered my last question.  Please ignore


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 50: if (abs(d) >= max_value) {
> is this correct? 'd' is already scaled here.  please be sure to test the up
Yeah that is bogus.  What we need is the maximum integer value that represents 
a base 10 number of length precision, so the -scale should go.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Michael Brown has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

build succeeded and here are the artifacts 
http://jenkins.impala.io:8080/job/ubuntu-14.04-from-scratch/867/artifact/Impala/logs_static/logs/custom_cluster_tests/results/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

Dan Hecht has posted comments on this change.

Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for 
"RYW""
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4925: Cancel finstance if query has finished

2017-02-13 Thread Henry Robinson (Code Review)

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-4925: Cancel finstance if query has finished
..

IMPALA-4925: Cancel finstance if query has finished

This patch is a partial fix for the issue where an finst would not
detect that it should cancel if the query limit had not been hit. It
changes the UpdateExecStatus() RPC to return a cancelled status to an
finst if the query has finished because it hit a limit.

For certain queries, this allows them to finish much more quickly than
they otherwise would. However, there's still a few-second delay for the
finst to pick up the cancellation signal, because there
UpdateExecStatus() RPC is only called every few seconds.

A complete fix would also call CancelInternal() when returned_all_results_
was set to true. That would be a much larger change. The improvement
here is to bound the delay between query completion and fragment
teardown to a few seconds.

Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
---
M be/src/runtime/coordinator.cc
1 file changed, 10 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I59f45e64978c9ab9914b5c33e86009960b4a88c4
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson

[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

2017-02-13 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has uploaded a new patch set (#2).

Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for 
"RYW""
..

Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

Reverting until we have a fix for KUDU-1869:
Scans do not work with hybrid time disabled and snapshot
reads enabled

This reverts commit 32ff959814646458a34278500bd01fc7741951ce.

Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
---
M be/src/exec/kudu-scanner.cc
1 file changed, 6 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs

[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

2017-02-13 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for 
"RYW""
..


Patch Set 1:

(1 comment)

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

Line 49: "result in using READ_LATEST.");
> Maybe just delete that sentence. A user shouldn't really rely on this anywa
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

Dan Hecht has posted comments on this change.

Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for 
"RYW""
..


Patch Set 1:

(1 comment)

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

Line 49: "result in using READ_LATEST.");
> That is meant to indicate that invalid values provided for this flag result
Maybe just delete that sentence. A user shouldn't really rely on this anyway - 
they should just specify a valid mode.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Alex Behm has submitted this change and it was merged.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

The bug: Our detection of partition compatibility for
grouping aggregations and analytic functions did not take into
account the effect of outer joins within the same fragment.
As a result, we used to incorrectly omit a required hash exchange.
For example, a hash exchange + merge phase is required if the
grouping expressions of an aggregation reference tuples
that are made nullable within the same fragment. The exchange is
needed to bring together NULLs produced by outer-join non-matches.

The fix: Check that the grouping/partition exprs do not reference
tuples that are made nullable within the same fragment.

Testing: Planner tests pass locally.

Change-Id: I121222179378e56836422a69451d840a012c9e54
Reviewed-on: http://gerrit.cloudera.org:8080/5774
Reviewed-by: Alex Behm 
Tested-by: Alex Behm 
---
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
4 files changed, 190 insertions(+), 24 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Alex Behm has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


Patch Set 2: Verified+1

Passed:
http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/265/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 11:

> Uploaded patch set 11.

Added the logic to expr-test to allow V2 expected results to be left unset when 
V1 and V2 results should match.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has uploaded a new patch set (#11).

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..

IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.

For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.

DECIMAL Version 1:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0   |
+---+

DECIMAL Version 2:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0.33|
+---+

DECIMAL Version 1:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| NULL  |
+---+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL

DECIMAL Version 2:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| 10.00 |
+---+

Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/testutil/impalad-query-executor.h
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M tests/query_test/test_decimal_queries.py
11 files changed, 376 insertions(+), 138 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/11
-- 
To view, visit http://gerrit.cloudera.org:8080/5952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""

2017-02-13 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for 
"RYW""
..


Patch Set 1:

(1 comment)

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

Line 49: "result in using READ_LATEST.");
> i'm not following. read_latest will return invalid results, and that's also
That is meant to indicate that invalid values provided for this flag result in 
using ReadMode READ_LATEST.

e.g. -kudu_read_mode=garbage


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 10:

Rebased and resolved conflicts with Michael's decimal_v2 codegen change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has uploaded a new patch set (#10).

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..

IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.

For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.

DECIMAL Version 1:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0   |
+---+

DECIMAL Version 2:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0.33|
+---+

DECIMAL Version 1:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| NULL  |
+---+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL

DECIMAL Version 2:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| 10.00 |
+---+

Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/testutil/impalad-query-executor.h
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M tests/query_test/test_decimal_queries.py
11 files changed, 383 insertions(+), 131 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/10
-- 
To view, visit http://gerrit.cloudera.org:8080/5952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Jim Apple has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


Patch Set 2:

http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/265/ finished 
successfully.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version
V2
..

Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5952/8/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

PS8, Line 228: other
> not your change but it may be clearer to call this divisor.
All the other operators use the terminology 'this' and 'other', so would like
to stay consistent, unless you feel strongly.

http://gerrit.cloudera.org:8080/#/c/5952/8/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
File fe/src/main/java/org/apache/impala/analysis/TypesUtil.java:

Line 100: ArithmeticExpr.Operator op, TQueryOptions queryOptions) throws
AnalysisException {
> instead of queryoptions, how about just passing a bool in here? narrower in
Done. I thought it was slightly nicer to have the decimal code itself make the
decision rather than make that decision in the generic arithmetic logic. But I
don't feel too strongly.

Line 152:* on whether DECIMAL version 1 or DECIMAL version 2 is enabled.
> since we're going to toss out the v1 behavior when we sunset cdh5, leave a
Sure, but it should be easy to find by tracing down DECIMAL_V2 uses. Filed
IMPALA-4924 and added a TODO here.

Line 233:*precision. But an algorithm of reducing scale to a minimum
reduction of 6 is
> "to a minimum of 6"
Done

http://gerrit.cloudera.org:8080/#/c/5952/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
File fe/src/main/java/org/apache/impala/catalog/ScalarType.java:

Line 138: public static ScalarType createDecimalTypeClipPrecScale(int
precision, int scale) {
> how about simply createClippedDecimalType()? or truncated instead of clippe
Didn't want to use "truncate" since we use that terminology in the backend to
describe what happens when you lose fractional digits, whereas here we are
losing whole number digits on the type. So went with
createClippedDecimalType().

Added the preconditions check (this is why I made createDecimalTypeWildCard()
but then forgot to do that. Also renamed that to createWildCardDecimalType()
for consistency.

http://gerrit.cloudera.org:8080/#/c/5952/8/tests/query_test/test_decimal_queries.py
File tests/query_test/test_decimal_queries.py:

PS8, Line 51: new_vector.get_value('exec_option')['decimal_v2'] =
vector.get_value('decimal_v2')
: new_vector.get_value('exec_option')['batch_size'] =
vector.get_value('batch_size')
> or simply do cls.ImpalaTestMatrix.add_dimension(create_exec_option_dimensio
Ah, didn't know about that. Done.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Dan Hecht has uploaded a new patch set (#9).

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..

IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.

For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.

DECIMAL Version 1:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0   |
+---+

DECIMAL Version 2:

+---+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-+
| 0.33|
+---+

DECIMAL Version 1:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| NULL  |
+---+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL

DECIMAL Version 2:

+---+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+---+
| 10.00 |
+---+

Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/testutil/impalad-query-executor.h
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M tests/query_test/test_decimal_queries.py
10 files changed, 328 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/9
-- 
To view, visit http://gerrit.cloudera.org:8080/5952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Jim Apple has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5978/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote 
chars
> Is our policy 72 chars for commit messages then?
It seems to be common practice.

$ git log | grep -oE '^([^ ].*)$' | cut -d ' ' -f 5- | awk '{print 
length;}' | sort -n | uniq -c


...
941 65
   1059 66
   1197 67
   1680 68
   1435 69
   1426 70
   1137 71
   1118 72
368 73
325 74
349 75
327 76
...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 2:

> > Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/264/
 > 
 > This test failed because while running the test, one of the
 > partitions ended up having 2 files in it while it should have only
 > 1.
 > 
 > This test passes locally always. I'll try to figure out why and
 > where this extra file came from.

Found the bug, running it through the entire test suite once more just to be 
sure. Will upload after that passes.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4839: Remove implicit 'localhost' for KUDU MASTER HOSTS

Michael Brown has posted comments on this change.

Change subject: IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS
..


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Ishaan Joshi 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Michael Brown has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5978/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote 
chars
> Gerrit puts a red line at 72 for me.
Is our policy 72 chars for commit messages then?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4810: Make DECIMAL expr-test cases table driven

Dan Hecht has posted comments on this change.

Change subject: IMPALA-4810: Make DECIMAL expr-test cases table driven
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5933/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 1288: {{ false, 2230, 5, 3 }, { false, 2230, 5, 3 }} },
> as part of this patch or a follow-on?
this patch was merged before your review started, so has to be a follow on.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb2573de46bba64d1b4e8caf15cc0238a84ea1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Jim Apple has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5978/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote 
chars
> This is 73 chars long. Are our commit messages subject to a different line 
Gerrit puts a red line at 72 for me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Michael Brown has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

(1 comment)

Thanks for the review.

http://gerrit.cloudera.org:8080/#/c/5978/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote 
chars
> long line
This is 73 chars long. Are our commit messages subject to a different line 
length than our code (90 chars)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Alex Behm has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


Patch Set 2: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Hello Marcel Kornacker,

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

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

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

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..

IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

The bug: Our detection of partition compatibility for
grouping aggregations and analytic functions did not take into
account the effect of outer joins within the same fragment.
As a result, we used to incorrectly omit a required hash exchange.
For example, a hash exchange + merge phase is required if the
grouping expressions of an aggregation reference tuples
that are made nullable within the same fragment. The exchange is
needed to bring together NULLs produced by outer-join non-matches.

The fix: Check that the grouping/partition exprs do not reference
tuples that are made nullable within the same fragment.

Testing: Planner tests pass locally.

Change-Id: I121222179378e56836422a69451d840a012c9e54
---
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
4 files changed, 190 insertions(+), 24 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Alex Behm has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


Patch Set 1:

(3 comments)

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

Line 799: if (!childFragment.refsNullableTupleId(partitionExprs)) {
> combine with preceding if
Done


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

Line 386:* Returns true if 'exprs' reference a tuple that is made nullable 
in this fragment.
> "in this fragment but not in any of its input fragments"
Done


Line 390: List groupingExprTids = Lists.newArrayList();
> remove the reference to grouping, it's overly specific.
Good catch, was not intentional. Done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Jim Apple has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3973: optional 2nd and 3rd arguments for instr().

2017-02-13 Thread John Russell (Code Review)

John Russell has posted comments on this change.

Change subject: IMPALA-3973: optional 2nd and 3rd arguments for instr().
..


Patch Set 2:

I'll come back to this one shortly. Just going to pursue +2s in a couple of 
more knotty gerrits first.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I17268bdb480230938f94559fe1eabe34ac2448b7
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Reviewer: zi+z...@cloudera.com
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Hello Jim Apple,

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

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

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

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..

IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

This is part 2 of the work being done to genericize the Impala security
topics. All references to Cloudera have been either marked 'hidden'
or replaced with links to the relevant open-source docs.

Note:
-Links to the standalone Cloudera ODBC driver doc have not been
removed.
-External links to the MIT Kerberos docs and Hadoop security
docs were added to impala_keydefs.

Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
---
M docs/impala_keydefs.ditamap
M docs/shared/impala_common.xml
M docs/topics/impala_delegation.xml
M docs/topics/impala_kerberos.xml
M docs/topics/impala_ldap.xml
5 files changed, 64 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/5962/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 4:

Fixed a typo in keydef tag.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4916: Fix maintenance of set of item sets in DisjointSet.

Alex Behm has uploaded a new change for review.

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

Change subject: IMPALA-4916: Fix maintenance of set of item sets in DisjointSet.
..

IMPALA-4916: Fix maintenance of set of item sets in DisjointSet.

The bug: The DisjointSet maintains a set of unique item sets
using a HashSet>. The problem is that we modified
the Set elements after inserting them into the HashSet.
This caused the removal of elements from the HashSet to fail.
Removal is required for maintaining a consistent DisjointSet.
The removal could even fail for the same Set instance because
the hashCode() changed from when the Set was originally
inserted to when the removal was attempted due to mutation
of the Set.
An inconsistent DisjointSet can lead to incorrect equivalence
classes, which can lead to missing, redundant and even
non-executable predicates. Incorrect results and crashes are
possible.

For most queries, an inconsistent DisjointSet does not alter
the equivalence classes, and even fewer queries have incorrect
plans.
In fact, many of our existing planner tests trigger this bug,
but only 3 of them lead to an incorrect value transfer graph,
and all 3 had correct plans.

The fix: Added a new ReferenceHashSet that stores a set of
unique references. It does not rely on the hashCode() and
equals() of the stored elements, so the same object can be
added and later removed, even when mutated in the meantime.
The DisjointSet maintains the set of item sets using this
new ReferenceHashSet.

Testing:
- Added a Preconditions check in DisjointSet that asserts
  correct removal of an item set. Many of our existing tests
  hit the check before this fix.
- Added a new unit test for DisjointSet which triggers
  the bug.
- Augmented DisjointSet.checkConsistency() to check for
  inconsistency in the set of item sets.
- Added validation of the value-transfer graph in
  single-node planner tests.
- A private core/hdfs run succeeded.

Change-Id: I609c8795c09becd78815605ea8e82e2f99e82212
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/common/Id.java
M fe/src/main/java/org/apache/impala/util/DisjointSet.java
A fe/src/main/java/org/apache/impala/util/ReferenceHashSet.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/util/TestDisjointSet.java
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
7 files changed, 127 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I609c8795c09becd78815605ea8e82e2f99e82212
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Jim Apple has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Jim Apple has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1: Code-Review+2

(1 comment)

Thanks for finding and fixing!

FYI: pre-review-test never talks back to gerrit.

http://gerrit.cloudera.org:8080/#/c/5978/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4920: custom cluster tests: remove unnecessary escaped quote 
chars
long line


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5957/1//COMMIT_MSG
Commit Message:

Line 12: to use cluster management software with a focus on governance.
> Why not always remove them?
We will eventually remove them from the source once we've sanity checked 
individual topics and made sure that what remains outside the hidden tags is 
not awkward and flows well. 

Removing all the 'hidden' content will be a global operation. For now though, 
with three writers working on the source, it's not a good idea to spot fix 
these references.


http://gerrit.cloudera.org:8080/#/c/5957/2/docs/topics/impala_lineage.xml
File docs/topics/impala_lineage.xml:

Line 61:   lineage data generated by Impala into graphs for easy 
visualization.
> These are no longer hidden and there is a mismatched ph tag at the end.
Thanks -- fixed the ph tag and another missing p tag -- the text is hidden in 
my build now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

2017-02-13 Thread David Knupp (Code Review)

David Knupp has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2

Michael Ho has posted comments on this change.

Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version 
V2
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5952/8/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

PS8, Line 228: other
not your change but it may be clearer to call this divisor.


http://gerrit.cloudera.org:8080/#/c/5952/8/tests/query_test/test_decimal_queries.py
File tests/query_test/test_decimal_queries.py:

PS8, Line 51: new_vector.get_value('exec_option')['decimal_v2'] = 
vector.get_value('decimal_v2')
: new_vector.get_value('exec_option')['batch_size'] = 
vector.get_value('batch_size')
or simply do 
cls.ImpalaTestMatrix.add_dimension(create_exec_option_dimension_from_dict({'decimal_v2':['false',
 'true'], 'batch':[0,1])) in add_test_dimensions().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Jim Apple has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5957/3/docs/topics/impala_lineage.xml
File docs/topics/impala_lineage.xml:

Line 63:   http://www.cloudera.com/documentation/enterprise/latest/topics/cn_iu_lineage.html";
 scope="external" format="html"/>.
Can this be removed, if it is CLoudera-specific?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Ambreen Kazi has uploaded a new patch set (#3).

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..

IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

This set of edits removes references and links to Cloudera Navigator
and Cloudera Manager from the auditing and lineage topics. Those
were either marked as 'hidden' or replaced with a generic suggestion
to use cluster management software with a focus on governance.

Some paragraphs with overflowing lines were also fixed.

Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
---
M docs/topics/impala_auditing.xml
M docs/topics/impala_lineage.xml
2 files changed, 57 insertions(+), 35 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] IMPALA-2020: Add rounding for decimal casts

Dan Hecht has posted comments on this change.

Change subject: IMPALA-2020: Add rounding for decimal casts
..


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

PS5, Line 303: return to_type(dv.whole_part(scale)); 
any reason not to have ToInt<>() handle this case too by passing in 'round' 
(similar to FromDouble())?


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/exprs/literal.cc
File be/src/exprs/literal.cc:

Line 185: value_.decimal4_val = Decimal4Value::FromDouble(type, v, 
&overflow, true);
where is this used? will it break compatibility?


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/runtime/decimal-value.h
File be/src/runtime/decimal-value.h:

PS5, Line 54: bool* overflow,
:   bool round
here and below, let's keep input parameters first, then output params.


PS5, Line 232:  bool& overflow
use 'bool* overflow'. we generally use pointers for output params to make it 
clearer at the callsite.


Line 232:   inline typename RESULT_T::underlying_type_t ToInt(int scale, bool& 
overflow) const;
add a function comment.


http://gerrit.cloudera.org:8080/#/c/5951/5/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 50: if (abs(d) >= max_value) {
is this correct? 'd' is already scaled here.  please be sure to test the upper 
bounds.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Michael Brown has posted comments on this change.

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..


Patch Set 1:

http://jenkins.impala.io:8080/view/Utility/job/pre-review-test/19/ but it was a 
draft when I kicked it off.  Not sure it'll post back here or not.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

Michael Brown has uploaded a new change for review.

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

Change subject: IMPALA-4920: custom cluster tests: remove unnecessary escaped 
quote chars
..

IMPALA-4920: custom cluster tests: remove unnecessary escaped quote chars

This patch fixes a problem in which unnecessary quote characters were
part of some impala-py.test arguments for running the custom cluster
tests. The bug was causing py.test metadata files as described by the
py.test options --junit-xml and --result-log to end up outside the
intended path for such artifacts. By removing the unnecessary escaped
quotes, custom cluster py.test metadata artifacts will now be in the
proper location. I doubly made sure it was safe to remove these quotes
by adding a space in the RESULTS_DIR. The space isn't expanded but is
instead properly treated as part of RESULTS_DIR.

Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
---
M tests/run-custom-cluster-tests.sh
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown

[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-4897: AnalysisException: specified cache pool does not 
exist
..


Patch Set 2:

> Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/264/

This test failed because while running the test, one of the partitions ended up 
having 2 files in it while it should have only 1.

This test passes locally always. I'll try to figure out why and where this 
extra file came from.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Jim Apple has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5957/2/docs/topics/impala_lineage.xml
File docs/topics/impala_lineage.xml:

Line 61:   
These are no longer hidden and there is a mismatched ph tag at the end.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5957/1/docs/topics/impala_lineage.xml
File docs/topics/impala_lineage.xml:

PS1, Line 60: Use a cluster manager with enhanced governance capabilities to 
transform
:   lineage data generated by Impala into graphs for easy 
visualization.
> This feels off-topic to me.
I've removed this reference to lineage diagrams since it's a Navigator feature.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

Ambreen Kazi has uploaded a new patch set (#2).

Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be 
generic.
..

IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.

This set of edits removes references and links to Cloudera Navigator
and Cloudera Manager from the auditing and lineage topics. Those
were either marked as 'hidden' or replaced with a generic suggestion
to use cluster management software with a focus on governance.

Some paragraphs with overflowing lines were also fixed.

Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
---
M docs/topics/impala_auditing.xml
M docs/topics/impala_lineage.xml
2 files changed, 56 insertions(+), 35 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Jim Apple has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 3: Code-Review+2

Thanks, Ambreen!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Ambreen Kazi has posted comments on this change.

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5962/2/docs/topics/impala_kerberos.xml
File docs/topics/impala_kerberos.xml:

PS2, Line 46: KDC
> Not defined yet
Added the full form with a link to relevant MIT KDC page.


Line 50: 
> Can this be removed?
Did you mean the entire paragraph that talks about Impala in a CM env? I've 
tagged it 'hidden' for now to be consistent with the rest of John's updates. We 
can deal with these occurrences all at once then.


http://gerrit.cloudera.org:8080/#/c/5962/2/docs/topics/impala_ldap.xml
File docs/topics/impala_ldap.xml:

Line 189: Specify the option on the impalad command 
line.
> This paragraph is confusing now.
Removed this line. Added this detail to a line before the ldap bind options are 
listed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

Ambreen Kazi has uploaded a new patch set (#3).

Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be 
generic
..

IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic

This is part 2 of the work being done to genericize the Impala security
topics. All references to Cloudera have been either marked 'hidden'
or replaced with links to the relevant open-source docs.

Note:
-Links to the standalone Cloudera ODBC driver doc have not been
removed.
-External links to the MIT Kerberos docs and Hadoop security
docs were added to impala_keydefs.

Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
---
M docs/impala_keydefs.ditamap
M docs/shared/impala_common.xml
M docs/topics/impala_delegation.xml
M docs/topics/impala_kerberos.xml
M docs/topics/impala_ldap.xml
5 files changed, 64 insertions(+), 42 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Ambreen Kazi 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Laurel Hale

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Jim Apple has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
..


Patch Set 1:

Started pre-merge tests in dry-run mode so they will be ready ASAP but won't 
submit this patch without your explicit action:

http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/265/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4546: Fix Moscow timezone conversion after 2014