[Impala-ASF-CR] Improve error handling for validation of unified backend executable
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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().
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.
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.
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
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
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().
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
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
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().
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().
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
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
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
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
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
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
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
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
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
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
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
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
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