[Impala-ASF-CR] Improve error handling for validation of unified backend executable

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12895 )

Change subject: Improve error handling for validation of unified backend 
executable
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia84cd074be79bc9d99b0621a1ae216f3debf5833
Gerrit-Change-Number: 12895
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 03 Apr 2019 03:47:31 +
Gerrit-HasComments: No


[Impala-ASF-CR] Improve error handling for validation of unified backend executable

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12895 )

Change subject: Improve error handling for validation of unified backend 
executable
..

Improve error handling for validation of unified backend executable

bin/validate-unified-backend-test-filters.py currently does not
handle the case where the call to run the unified backend test
executable fails. Instead, it produces a very confusing message
about invalid filters that does not mention that the execution
failed.

This checks the return code when running the unified backend
test executable and produces a reasonable message if the
return code is not 0.

Change-Id: Ia84cd074be79bc9d99b0621a1ae216f3debf5833
Reviewed-on: http://gerrit.cloudera.org:8080/12895
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M bin/validate-unified-backend-test-filters.py
1 file changed, 8 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia84cd074be79bc9d99b0621a1ae216f3debf5833
Gerrit-Change-Number: 12895
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 


[Impala-ASF-CR] Don't build with shared objects in bootstrap build.sh

2019-04-02 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12910 )

Change subject: Don't build with shared objects in bootstrap_build.sh
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/12910/1//COMMIT_MSG@9
PS1, Line 9: various
   : problems
Is there a newer JIRA? The last I remember about this was years ago.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id89a19c6fc29b7ac21e5d6174d490ddc8a6f9c0b
Gerrit-Change-Number: 12910
Gerrit-PatchSet: 1
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Comment-Date: Wed, 03 Apr 2019 01:59:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Give each config change in bootstrap system.sh its own line

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12900 )

Change subject: Give each config change in bootstrap_system.sh its own line
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3973/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d3ae6c0b816113b7bf690adff4f1cd905388776
Gerrit-Change-Number: 12900
Gerrit-PatchSet: 5
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 03 Apr 2019 01:39:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] Give each config change in bootstrap system.sh its own line

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12900 )

Change subject: Give each config change in bootstrap_system.sh its own line
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d3ae6c0b816113b7bf690adff4f1cd905388776
Gerrit-Change-Number: 12900
Gerrit-PatchSet: 5
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 03 Apr 2019 01:39:53 +
Gerrit-HasComments: No


[native-toolchain-CR] Check for thirft.protocol.fastbinary in both lib and lib64

2019-04-02 Thread Thomas Marshall (Code Review)
Thomas Marshall has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12915 )

Change subject: Check for thirft.protocol.fastbinary in both lib and lib64
..

Check for thirft.protocol.fastbinary in both lib and lib64

db301f6b992d36ea315c037fde954e01340be1e2 introduced an assertion on
the thrift build script which ensures that thrift.protocol.fastbinary
gets compiled in thrift's python bindings, however in some distros, the
shared object is placed on $PY_PREFIX/lib64 (instead of $PY_PREFIX/lib)
so we check in both locations.

Change-Id: Ib2294eff2f945af94de0345322f5496777fb1b08
Reviewed-on: http://gerrit.cloudera.org:8080/12915
Reviewed-by: Thomas Marshall 
Tested-by: Thomas Marshall 
---
M source/thrift/build.sh
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Thomas Marshall: Looks good to me, approved; Verified

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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib2294eff2f945af94de0345322f5496777fb1b08
Gerrit-Change-Number: 12915
Gerrit-PatchSet: 2
Gerrit-Owner: Hector Acosta 
Gerrit-Reviewer: Thomas Marshall 


[native-toolchain-CR] Check for thirft.protocol.fastbinary in both lib and lib64

2019-04-02 Thread Hector Acosta (Code Review)
Hector Acosta has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12915


Change subject: Check for thirft.protocol.fastbinary in both lib and lib64
..

Check for thirft.protocol.fastbinary in both lib and lib64

db301f6b992d36ea315c037fde954e01340be1e2 introduced an assertion on
the thrift build script which ensures that thrift.protocol.fastbinary
gets compiled in thrift's python bindings, however in some distros, the
shared object is placed on $PY_PREFIX/lib64 (instead of $PY_PREFIX/lib)
so we check in both locations.

Change-Id: Ib2294eff2f945af94de0345322f5496777fb1b08
---
M source/thrift/build.sh
1 file changed, 2 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/15/12915/1
--
To view, visit http://gerrit.cloudera.org:8080/12915
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib2294eff2f945af94de0345322f5496777fb1b08
Gerrit-Change-Number: 12915
Gerrit-PatchSet: 1
Gerrit-Owner: Hector Acosta 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-8014: Revise FK/PK cardinality estimation

2019-04-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12535 )

Change subject: IMPALA-8014: Revise FK/PK cardinality estimation
..


Patch Set 5:

I've been meaning to make some time to look at this. I think my high-level 
concern is about potential regressions (i.e. where we were wrong for the right 
reasons). I know you've argued against having a flag for everything because of 
the configuration space, testing and code complexity problems, but I wonder if 
there's a way we can guard against the worst issues.

E.g. can we factor out the cardinality code into a strategy class with a simple 
interface to encapsulate the different versions of the logic? Can we limit the 
configuration space by using something coarser-grained than planner versioning, 
e.g. have a planner "epoch" that is incremented for a group of significant 
planner changes.

Definitely don't want to be stuck being overly conservative with preserving 
semi-broken logic, but it would be good to mitigate the risk if it can be done 
with reasonable effort.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0db82564596b0a9271b89b8e3f7a3cf92d306da
Gerrit-Change-Number: 12535
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 03 Apr 2019 00:01:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12914 )

Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2618/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: radford nguyen 
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:52:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12916 )

Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2617/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:57:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12885 )

Change subject: IMPALA-8371: Return appropriate error code for unified backend 
tests
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2616/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:31:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger

2019-04-02 Thread Austin Nobis (Code Review)
Austin Nobis has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/12914 )

Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..

IMPALA-8226: Add grant/revoke to/from group for Ranger

This patch adds fupport for GRANT privilege statements to GROUP and
REVOKE privilege statements from GROUP.  The grammar has been updated to
support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e:

GRANT  ON  TO GROUP 
REVOKE  ON  FROM GROUP 

Currently, only Ranger's authorization implementation supports GROUP
based privileges. Sentry will throw an UnsupportedOperationException if
it is the enabled authorization provider and this new grammar is used.

Testing:
- AuthorizationStmtTest was updated to also test for GROUP
  authorization.
- ToSqlTest was updated to test for GROUP changes to the grammar.
- A GROUP based E2E test was added to test_ranger.py
- Ran all FE tests
- Ran authorization E2E tests

Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
M 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/resources/ranger-hive-security.xml
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group.json.template
M testdata/cluster/ranger/setup/impala_user.json.template
M tests/authorization/test_ranger.py
15 files changed, 212 insertions(+), 83 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: radford nguyen 


[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12914 )

Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12914/2/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

http://gerrit.cloudera.org:8080/#/c/12914/2/testdata/bin/create-load-data.sh@305
PS2, Line 305:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/12914/2/testdata/bin/create-load-data.sh@312
PS2, Line 312:
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/12914/2/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/12914/2/tests/authorization/test_ranger.py@58
PS2, Line 58: ,
flake8: E501 line too long (91 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/12914/2/tests/authorization/test_ranger.py@82
PS2, Line 82: ,
flake8: E501 line too long (91 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: radford nguyen 
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:20:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8226: Add grant/revoke to/from group for Ranger

2019-04-02 Thread Austin Nobis (Code Review)
Austin Nobis has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12914


Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger
..

IMPALA-8226: Add grant/revoke to/from group for Ranger

This patch adds fupport for GRANT privilege statements to GROUP and
REVOKE privilege statements from GROUP.  The grammar has been updated to
support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e:

GRANT  ON  TO GROUP 
REVOKE  ON  FROM GROUP 

Currently, only Ranger's authorization implementation supports GROUP
based privileges. Sentry will throw an UnsupportedOperationException if
it is the enabled authorization provider and this new grammar is used.

Testing:
- AuthorizationStmtTest was updated to also test for GROUP
  authorization.
- ToSqlTest was updated to test for GROUP changes to the grammar.
- A GROUP based E2E test was added to test_ranger.py
- Ran all FE tests
- Ran authorization E2E tests

Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
M 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/resources/ranger-hive-security.xml
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group.json.template
M testdata/cluster/ranger/setup/impala_user.json.template
M tests/authorization/test_ranger.py
15 files changed, 212 insertions(+), 83 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Gerrit-Change-Number: 12914
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis 
Gerrit-Reviewer: Austin Nobis 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: radford nguyen 


[Impala-ASF-CR] IMPALA-8360: Fix race conditions in thread-pool-test

2019-04-02 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12916


Change subject: IMPALA-8360: Fix race conditions in thread-pool-test
..

IMPALA-8360: Fix race conditions in thread-pool-test

There are race conditions in thread-pool-test between the caller
thread and the worker thread. Specifically, in some cases, the
worker thread seems to be slow in freeing resources. This can
lead to asserts failing because the work item has not been
freed or because the submit of a task failed when it should
not have.

This fixes the race conditions:
 - It breaks up the existing SynchronousThreadPoolTest into two
   smaller tests so that the two can't interfere with each other.
 - It adds the ability to sleep and recheck that the work item
   has been freed.

Testing:
 - Ran thread-pool-test in a loop for 20k iterations

Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
---
M be/src/util/thread-pool-test.cc
1 file changed, 16 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Gerrit-Change-Number: 12916
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 


[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

2019-04-02 Thread Joe McDonnell (Code Review)
Hello Andrew Sherman, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend 
tests
..

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 83 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 


[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12902 )

Change subject: IMPALA-7211: Fix the between predicate for decimals
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:55:10 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12902 )

Change subject: IMPALA-7211: Fix the between predicate for decimals
..

IMPALA-7211: Fix the between predicate for decimals

Before this patch, some queries would fail where the inputs to the
between predicate were decimal types that are not compatible with each
other. We would needlessly cast them to the same type in the FE. This
was not necessary because we are able to handle decimals of different
types in the backend already for this predicate. This patch should even
result in a slight performance improvement.

Testing:
- Added some tests to AnalyzeExprsTest.

Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Reviewed-on: http://gerrit.cloudera.org:8080/10898
Reviewed-by: Vuk Ercegovac 
Tested-by: Impala Public Jenkins 
Reviewed-on: http://gerrit.cloudera.org:8080/12902
Reviewed-by: Tim Armstrong 
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
3 files changed, 66 insertions(+), 63 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

2019-04-02 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12885 )

Change subject: IMPALA-8371: Return appropriate error code for unified backend 
tests
..


Patch Set 3: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:52:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

2019-04-02 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12885 )

Change subject: IMPALA-8371: Return appropriate error code for unified backend 
tests
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@21
PS2, Line 21: # 1: The file location to write the generated script
> Aren't these parameters now named (--gtest_filter,--test_script_output) rat
Good point, I updated this comment to reflect the new args.


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@32
PS2, Line 32: # The script template requires substitutions to set the variables 
used by the body.
> Nit: Clearer maybe to say "the script header template".
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:51:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Improve error handling for validation of unified backend executable

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12895 )

Change subject: Improve error handling for validation of unified backend 
executable
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3972/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia84cd074be79bc9d99b0621a1ae216f3debf5833
Gerrit-Change-Number: 12895
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:47:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] Improve error handling for validation of unified backend executable

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12895 )

Change subject: Improve error handling for validation of unified backend 
executable
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia84cd074be79bc9d99b0621a1ae216f3debf5833
Gerrit-Change-Number: 12895
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:47:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.

2019-04-02 Thread Paul Rogers (Code Review)
Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12889 )

Change subject: IMPALA-7971: Add support for insert events in event processor.
..


Patch Set 5: Code-Review+1

The code here is OK. Discussed in person the issue of cross-system race 
conditions which we agreed to address another time. Would like Vihang to take a 
close look at the notification code itself since he is most knowledgeable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c48c5ca4bde18d532c582980aebbc25f1bf1c52
Gerrit-Change-Number: 12889
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Bharath Krishna 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:24:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.

2019-04-02 Thread Bharath Krishna (Code Review)
Bharath Krishna has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12889 )

Change subject: IMPALA-7971: Add support for insert events in event processor.
..


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@601
PS5, Line 601: Preconditions.checkNotNull(insertMessage.getTableObj());
These following two lines can be

Preconditions.checkNotNull(insertMessage.getTableObj());
msTbl_ = insertMessage.getTableObj();
written as :
msTbl_ = Preconditions.checkNotNull(insertMessage.getTableObj());


http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:

http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@204
PS5, Line 204:   // Metric name for number of refreshes by event processor sofar
Typo sofar


http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3551
PS5, Line 3551:   // For non-partitioned tables parts below will contain a 
single value. The
nit : comma after tables.


http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3766
PS5, Line 3766: LOG.debug("Firing insert event... for %s", tbl.getName());
LOG.debug("Firing insert event for {}", tbl.getName());


http://gerrit.cloudera.org:8080/#/c/12889/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3780
PS5, Line 3780:   LOG.error("Failed to fire insert event. This may cause 
some table refreshes to be"
I think it's better to have:
Some Tables might not be refreshed on other Impala clusters for this event.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c48c5ca4bde18d532c582980aebbc25f1bf1c52
Gerrit-Change-Number: 12889
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Bharath Krishna 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 02 Apr 2019 21:55:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12889 )

Change subject: IMPALA-7971: Add support for insert events in event processor.
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2615/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c48c5ca4bde18d532c582980aebbc25f1bf1c52
Gerrit-Change-Number: 12889
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Bharath Krishna 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 02 Apr 2019 21:42:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().

2019-04-02 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12672 )

Change subject: IMPALA-8143: Enhance DoRpcWithRetry().
..


Patch Set 8:

Reviewers please wait for a future Patch Ste with a simplified test


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
Gerrit-Change-Number: 12672
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 02 Apr 2019 21:15:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.

2019-04-02 Thread Anurag Mantripragada (Code Review)
Anurag Mantripragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12889 )

Change subject: IMPALA-7971: Add support for insert events in event processor.
..


Patch Set 5:

(24 comments)

Thanks for the comments Vihang. I cleaned up code in CatalogOpExecutor. Still 
figuring out a way to write tests to verify if tables are refreshed because of 
insert events.

http://gerrit.cloudera.org:8080/#/c/12889/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12889/4//COMMIT_MSG@24
PS4, Line 24: Existing self-events logic cannot be used for insert events since
:firing insert event does not allow us to modify
> I think it is more appropriate to say existing self-events logic cannot be
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/12889/4/be/src/service/client-request-state.cc@1106
PS4, Line 1106:   // is_overwrite is used to know the type of insert in FE.
> add a comment here explaining why this is needed.
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@839
PS4, Line 839: HashSet<>(f
> would be appropriate to intialize the set with the capacity fdList.size() s
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@842
PS4, Line 842:
> suggest you to use Path.SEPARATOR instead of "/"
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@159
PS4, Line 159: refresh
> refresh on a table/partition
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@587
PS4, Line 587: public static class InsertEvent extends MetastoreTableEvent {
> IIUC, the reason you are extending TableInvalidatingEvent is because you wa
You are right. Changing it back to old TableInvalidatingEvent.


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@588
PS4, Line 588:
> nit, add new line above the constructor. Add a javadoc
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@606
PS4, Line 606:   }
> add a // TODO : to handle self-events for insert case
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@613
PS4, Line 613:
> nit, just saying refresh is good enough. No need to say reload here.
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@616
PS4, Line 616:
> same as above. Just say refresh since I don't think reload means anything e
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@623
PS4, Line 623:   }
> Add a // TODO : One way to do this would be to change hive source code to r
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@965
PS4, Line 965: t object parameters used for self-
> Why do we need to rename? Currently, all the implementations of this sub-cl
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
File 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java:

http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@118
PS4, Line 118:
> Ignore to keep it consistent with other entries of this table
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@118
PS4, Line 118:   |||
 :  * | INSERT EVENT| Refres
> Just use Refresh unless Reload means something else.
Done


http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/12889/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3554
PS4, Line 3554: FeCatalogUtils.loadAllPartitions((HdfsTable) table);
  :   // Map of partition ids to file names of all existing 
partitions touched by the
  :   // insert
> 

[Impala-ASF-CR] IMPALA-7971: Add support for insert events in event processor.

2019-04-02 Thread Anurag Mantripragada (Code Review)
Anurag Mantripragada has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/12889 )

Change subject: IMPALA-7971: Add support for insert events in event processor.
..

IMPALA-7971: Add support for insert events in event processor.

This patch adds support for detecting and processing insert events
triggered by impala as well as external engines (eg.Hive).

Inserts from Impala will fire an insert event notification.
The event-processor will refresh tables using this event. Both insert
into and overwrite are supported for tables/partitions. Also, renamed
TableInvalidatingEvent class to TableInvalidatingOrRefreshingEvent
to reflect new behaviour.

Known Issues:
1. There is an unnecessary table invalidate when insert is done in Hive
   as the insert operation creates an ALTER and an INSERT notification
   event. Currently there is no way for the Event Processor to identify
   and prevent the unnecessary invalidate. IMPALA-7973 may potentially
   solve this issue.
2. Existing self-events logic cannot be used for insert events since
   firing insert event does not allow us to modify table parameters in
   HMS. This means we cannot get the CatalogServiceIdentifiers in insert
   events. Therefore, the event-processor will also refresh the tables
   for which insert operation is performed through Impala.

Testing:
Added new custom cluster tests to run different insert commands from
hive and verified new data is available in Impala without invalidate
metadata.

Change-Id: I7c48c5ca4bde18d532c582980aebbc25f1bf1c52
---
M be/src/service/client-request-state.cc
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M 
fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
A tests/custom_cluster/test_event_processing.py
7 files changed, 282 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c48c5ca4bde18d532c582980aebbc25f1bf1c52
Gerrit-Change-Number: 12889
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada 
Gerrit-Reviewer: Anurag Mantripragada 
Gerrit-Reviewer: Bharath Krishna 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-7368: Add initial support for DATE type

2019-04-02 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12481 )

Change subject: IMPALA-7368: Add initial support for DATE type
..


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG@20
PS11, Line 20: - from STRING to DATE. The string value must be formatted as
 :   -MM-dd.
This seems to be stricter than CAST() in postgres, mysql, and hive (see 
https://docs.google.com/spreadsheets/d/1IkceWy8lBSkF5NaL-Jls2y86zosyD9pwxtO7qKpSxZw/edit#gid=0
 row 31)


http://gerrit.cloudera.org:8080/#/c/12481/11//COMMIT_MSG@38
PS11, Line 38: E.g: year('2019-02-15') must resolve to
 :year(TIMESTAMP) instead of year(DATE). Note, that 
year(DATE) is
 :not implemented yet, so this is not an issue at the 
moment but
 :it will be in the future.
> Not necessarily. The valid range for TIMESTAMP is from 1400-01-01 to -1
Should we be emitting a WARNING for any out-of-range values? it seems 
surprising to silently emit NULL for out-of-range casts (I had a customer 
complain about this last week wrt timestamp)


http://gerrit.cloudera.org:8080/#/c/12481/11/fe/src/main/java/org/apache/impala/catalog/Function.java
File fe/src/main/java/org/apache/impala/catalog/Function.java:

http://gerrit.cloudera.org:8080/#/c/12481/11/fe/src/main/java/org/apache/impala/catalog/Function.java@191
PS11, Line 191:   public boolean compare(Function other, CompareMode mode) {
> I agree that the function overload resolution algorithm needs to be redesig
I'm afraid about introducing new semantics here which we'll need to break again 
later. I did some testing on your patch relative to Hive 2, Hive 3, postgres, 
and MariaDB, and the results were pretty interesting:

https://docs.google.com/spreadsheets/d/1IkceWy8lBSkF5NaL-Jls2y86zosyD9pwxtO7qKpSxZw/edit#gid=0

It seems like at least in the cases where Hive3 and Postgres agree, we should 
probably be giving the same results as well? It certainly seems like Impala is 
giving NULL in some places I wouldn't have expected.

Maybe we can add a few more queries here, and a column for Impala (prior to the 
patch) to make sure we aren't introducing any backward-incompatible behavior?

In other words, I think the priorities should be:

1) don't break existing queries
2) for queries where postgres and Hive3 agree, we should give the same result
3) for queries where postgres and Hive differ, we should probably go with Hive3 
or collaborate with the Hive team to see if they think this is a bug

In the case that this would require a big restructuring, I'd prefer if in the 
meantime we result these queries in an error like "ambiguous types detected, 
insert casts to resolve". Later, if we can figure out correct implicit casting 
rules, we can loosen it. But I'm against introducing new behavior which we 
already know that we'll have to break.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea8155ef09557e0afa2f8b2d0b2dc9d0896dc30f
Gerrit-Change-Number: 12481
Gerrit-PatchSet: 11
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 02 Apr 2019 19:35:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12247 )

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..


Patch Set 14:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2614/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Tue, 02 Apr 2019 19:15:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12672 )

Change subject: IMPALA-8143: Enhance DoRpcWithRetry().
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2613/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
Gerrit-Change-Number: 12672
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 02 Apr 2019 18:58:48 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12902 )

Change subject: IMPALA-7211: Fix the between predicate for decimals
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3971/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 02 Apr 2019 18:54:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] Give each config change in bootstrap system.sh its own line

2019-04-02 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12900 )

Change subject: Give each config change in bootstrap_system.sh its own line
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d3ae6c0b816113b7bf690adff4f1cd905388776
Gerrit-Change-Number: 12900
Gerrit-PatchSet: 4
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Tue, 02 Apr 2019 18:50:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().

2019-04-02 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12672 )

Change subject: IMPALA-8143: Enhance DoRpcWithRetry().
..


Patch Set 5:

(3 comments)

Thanks Michael. Please see the upcoming Patch Set 8.

http://gerrit.cloudera.org:8080/#/c/12672/5/be/src/rpc/rpc-mgr-test.cc
File be/src/rpc/rpc-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/12672/5/be/src/rpc/rpc-mgr-test.cc@333
PS5, Line 333:   // Call DoRpcWithRetry with no backoff on our busy service.
 :   const int retries = 3;
 :   impala::Status rpc_status = RpcMgr::DoRpcWithRetry(proxy, 
::Ping,
 :   request, , query_ctx, "ping failed", retries, 
MonoDelta::FromSeconds(100));
 :   EXPECT_ERROR(rpc_status, TErrorCode::GENERAL);
 :   EXPECT_STR_CONTAINS(rpc_status.msg().msg(),
 :   "dropped due to backpressure. The service queue contains 1 
items out of a maximum "
 :   "of 1; memory consumption is ");
 :   ASSERT_EQ(overflow_metric->GetValue(), retries);
 :
 :   // Call DoRpcWithRetry, but this time we do backoff on a busy 
service, and this waiting
 :   // allows the outstanding aysnc calls to complete, so that 
then this call will succeed.
 :   impala::Status rpc_status_backoff =
 :   RpcMgr::DoRpcWithRetry(proxy, ::Ping, 
request, ,
 :   query_ctx, "ping failed", 10, 
MonoDelta::FromSeconds(100), 2);
 :   ASSERT_OK(rpc_status_backoff);
 :   ASSERT_GT(overflow_metric->GetValue(), retries);
 :
 :   // Check that async calls did complete.
 :   ASSERT_FALSE(async1_done.pending());
 :   ASSERT_FALSE(async2_done.pending());
> How about the following idea:
I did think about this. So the nice thing about your suggestion (thanks) is 
that we can set up the full queue.
Now we want to have a call to DoRpcWithRetry that will block (because queue is 
full) and then succeed (after retry).
So concurrently with the call to DoRpcWithRetry we need to unblock the queue.
We could start the call to DoRpcWithRetry in a thread, and then sleep for a 
while, and then unblock the queue.
That would work but is functionally equivalent to what we have now in that it 
depends on a sleep call.
So I understand the desire for a completely deterministic test but I still 
don't see a simple way to achieve that. I am happy to listen to more 
suggestions, or explanations of what I am not understanding.


http://gerrit.cloudera.org:8080/#/c/12672/7/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/12672/7/be/src/rpc/rpc-mgr.h@186
PS7, Line 186: og' fun
> timeout_ms
Done


http://gerrit.cloudera.org:8080/#/c/12672/5/be/src/rpc/rpc-mgr.h
File be/src/rpc/rpc-mgr.h:

http://gerrit.cloudera.org:8080/#/c/12672/5/be/src/rpc/rpc-mgr.h@178
PS5, Line 178:   // A no-op method that is used as part of overloading 
DoRpcWithRetry().
 :   static void logging_noop(){};
 :
 :   // The type of the log method passed to DoRpcWithRetry().
 :   typedef boost::function RpcLogFn;
> They still seem to be in PS7
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
Gerrit-Change-Number: 12672
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 02 Apr 2019 18:17:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().

2019-04-02 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/12672 )

Change subject: IMPALA-8143: Enhance DoRpcWithRetry().
..

IMPALA-8143: Enhance DoRpcWithRetry().

Allow callers of RpcMgr::DoRpcWithRetry to specify a time to sleep if
the remote service is busy. DoRpcWithRetry now only sleeps if the remote
service is busy.

TESTING:

Ran all end-to-end tests.

Add a new test to rpc-mgr-test.cc which tests RpcMgr::DoRpcWithRetry in
two ways. Firstly we test the case where the remote service is busy
(which we force by filling up the queue), and secondly we exercise
DoRpcWithRetry by using a fake Proxy that simulates failures.

Clean up unused values of FaultInjectionUtil.RpcCallType and match
values with test_rpc_timeout.py.

Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
---
M be/src/rpc/impala-service-pool.cc
M be/src/rpc/impala-service-pool.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr-test.h
M be/src/rpc/rpc-mgr.h
M be/src/rpc/rpc-mgr.inline.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/query-state.h
M be/src/service/client-request-state.cc
M be/src/service/control-service.cc
M be/src/service/control-service.h
M be/src/testutil/fault-injection-util.h
M common/protobuf/rpc_test.proto
M tests/custom_cluster/test_rpc_timeout.py
14 files changed, 268 insertions(+), 62 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390
Gerrit-Change-Number: 12672
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7368: Add initial support for DATE type

2019-04-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12481 )

Change subject: IMPALA-7368: Add initial support for DATE type
..


Patch Set 16: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12481/11/be/src/exprs/conditional-functions-ir.cc
File be/src/exprs/conditional-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/12481/11/be/src/exprs/conditional-functions-ir.cc@37
PS11, Line 37: IS_NULL_COMPUTE_
> Done
Sorry for the churn here induced by my original comment. I do think there's 
something worth revisiting later just as a maintainability tihng - there would 
be some value in infrastructure to "stamp out" these more mechanical aspects of 
a data type, but it requires some thought. E.g. for something like a numeric 
type, where there's a set of common operations and functions, it seems like we 
should be able to define a new numeric type in one place and have the required 
definitions added automatically elsewhere, rather than having to chase down all 
the locations in the codebase.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea8155ef09557e0afa2f8b2d0b2dc9d0896dc30f
Gerrit-Change-Number: 12481
Gerrit-PatchSet: 16
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 02 Apr 2019 17:49:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

2019-04-02 Thread Andrew Sherman (Code Review)
Andrew Sherman has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12885 )

Change subject: IMPALA-8371: Return appropriate error code for unified backend 
tests
..


Patch Set 2: Code-Review+1

(2 comments)

LGTM

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@21
PS2, Line 21: # 1: The file location to write the generated script
Aren't these parameters now named (--gtest_filter,--test_script_output) rather 
than positional?


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@32
PS2, Line 32: # The script template requires substitutions to set the variables 
used by the body.
Nit: Clearer maybe to say "the script header template".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Comment-Date: Tue, 02 Apr 2019 16:38:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12247 )

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..


Patch Set 14:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/2612/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Tue, 02 Apr 2019 15:53:33 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals

2019-04-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12902 )

Change subject: IMPALA-7211: Fix the between predicate for decimals
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 02 Apr 2019 15:21:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12247 )

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..


Patch Set 14:

> Build Failed
 >
 > https://jenkins.impala.io/job/gerrit-code-review-checks/2609/ :
 > Initial code review checks failed. See linked job for details on
 > the failure.

The error during code review check seems to be unrelated:
E: Failed to fetch 
http://us-west-2.ec2.archive.ubuntu.com/ubuntu/pool/universe/o/objenesis/libobjenesis-java_2.2-1_all.deb
  504  Gateway Time-out [IP: 34.210.25.51 80]


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Tue, 02 Apr 2019 15:16:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] Don't build with shared objects in bootstrap build.sh

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12910 )

Change subject: Don't build with shared objects in bootstrap_build.sh
..


Patch Set 1:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/2610/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id89a19c6fc29b7ac21e5d6174d490ddc8a6f9c0b
Gerrit-Change-Number: 12910
Gerrit-PatchSet: 1
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 02 Apr 2019 14:49:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6216: Make PYTHON EGG CACHE configurable

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12911 )

Change subject: IMPALA-6216: Make PYTHON_EGG_CACHE configurable
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/2611/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I695b2b31d9045eef1a53268f6516858935aed508
Gerrit-Change-Number: 12911
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 02 Apr 2019 14:45:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6216: Make PYTHON EGG CACHE configurable

2019-04-02 Thread Yongzhi Chen (Code Review)
Yongzhi Chen has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12911


Change subject: IMPALA-6216: Make PYTHON_EGG_CACHE configurable
..

IMPALA-6216: Make PYTHON_EGG_CACHE configurable

User can set environment variable PYTHON_EGG_CACHE before call
impala-shell.

Testing:
Run impala-shell with or w/o PYTHON_EGG_CACHE configured

Change-Id: I695b2b31d9045eef1a53268f6516858935aed508
---
M shell/impala-shell
1 file changed, 7 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I695b2b31d9045eef1a53268f6516858935aed508
Gerrit-Change-Number: 12911
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen 


[Impala-ASF-CR] Don't build with shared objects in bootstrap build.sh

2019-04-02 Thread Laszlo Gaal (Code Review)
Laszlo Gaal has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12910


Change subject: Don't build with shared objects in bootstrap_build.sh
..

Don't build with shared objects in bootstrap_build.sh

Building with shared objects is not supported currently because of various
problems. All tested configurations of Impala are built with the supporting
libraries being linked statically.

The patch removes the '-so' flag from bootstrap_build.sh to make
the compile-only check work the same way as the regular builds.

Change-Id: Id89a19c6fc29b7ac21e5d6174d490ddc8a6f9c0b
---
M bin/bootstrap_build.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id89a19c6fc29b7ac21e5d6174d490ddc8a6f9c0b
Gerrit-Change-Number: 12910
Gerrit-PatchSet: 1
Gerrit-Owner: Laszlo Gaal 


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12247 )

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..


Patch Set 14:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/2609/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Tue, 02 Apr 2019 13:00:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12247 )

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12247/9/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:

http://gerrit.cloudera.org:8080/#/c/12247/9/be/src/runtime/timestamp-test.cc@740
PS9, Line 740: EQ(MI
> I feel I'm slightly in favor of adding proper distinct variables instead of
All the names I came up with were awkward, so I reused the same name but put 
all the tests to different blocks.


http://gerrit.cloudera.org:8080/#/c/12247/13/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:

http://gerrit.cloudera.org:8080/#/c/12247/13/be/src/runtime/timestamp-test.cc@743
PS13, Line 743: Add 250ns
> The comments should now reflect that you're setting it explicitly instead o
Done


http://gerrit.cloudera.org:8080/#/c/12247/13/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/12247/13/be/src/runtime/timestamp-value.h@196
PS13, Line 196: 1'000'000'000LL
> We have NANOS_PER_SEC in timestamp-value-inline.h, use here, too, and elsew
NANOS_PER_DAY comes from walltime.h, which is included in most places, but I 
did not want to include it in a frequently used header like this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi 
Gerrit-Comment-Date: Tue, 02 Apr 2019 12:18:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5051: Add INT64 timestamp write support in Parquet

2019-04-02 Thread Csaba Ringhofer (Code Review)
Hello Lars Volker, Zoltan Borok-Nagy, Zoltan Ivanfi, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5051: Add INT64 timestamp write support in Parquet
..

IMPALA-5051: Add INT64 timestamp write support in Parquet

Add query option "parquet_timestamp_type" that chooses the
Parquet type used when writing TIMESTAMP columns. This is an
experimental feature at the moment, because these types are not
widely adopted in other Hadoop components yet. For this reason
the query option is added as "development" level, and the default
behavior is not changed.

The following options can be used:
INT96_NANOS (default):
  This is the same as the old behavior, can represent any
  timestamp that can be handled by Impala.
INT64_MILLIS, INT64_MICROS:
  Can encode the whole [1400..1) range handled by Impala
  at the cost of reduced precision. Values are rounded towards
  minus infinity during writing.
INT64_NANOS:
  Can encode a reduced range without losing nanosecond precision:
  [1677-09-21 00:12:43.145224192 .. 2262-04-11 23:47:16.854775807]
  Values outside this range are converted to NULLs without warning.

The change was done completely in the backend and all TIMESTAMP
columns are written using the type set in the query option.
An alternative design would have been to implement some parts
in the fronted by adding TIMESTAMP->BIGINT conversion functions
to the query plan, which would make it easier to add the possibility
of per-column setting in the future. I choose the current design
because it seemed much simpler and there are no clear plans for the
per-column setting. Most of the code will be still useful if we
decide to go the other way in the future.

All types are written without conversion to UTC (the way Impala
always wrote timestamps), and this information is expressed in the
new Parquet logical types by setting isAdjustedToUTC to false. The
old logical type (converted_type) is not set, because old readers do
not read isAdjustedToUTC, and assume that TIMESTAMP_MILLIS and
TIMESTAMP_MICROS are written in UTC. These readers can still read
int64 timestamp columns as INT_64.

Testing:
- added unit tests for new TimestampValue->int64 functions
- add EE tests for checking values / min-max stats / metadata
  written for int64 Parquet timestamps
- ran core tests

Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.h
M be/src/exec/parquet/parquet-common.cc
M be/src/exec/parquet/parquet-common.h
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/exec/parquet/parquet-metadata-utils.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.h
M be/src/runtime/timestamp-value.inline.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M 
testdata/workloads/functional-query/queries/QueryTest/parquet-int64-timestamps.test
M tests/query_test/test_insert_parquet.py
M tests/util/get_parquet_metadata.py
18 files changed, 611 insertions(+), 117 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/12247/14
--
To view, visit http://gerrit.cloudera.org:8080/12247
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib41ad532ec902ed5a9a1528513726eac1c11441f
Gerrit-Change-Number: 12247
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: Zoltan Ivanfi