[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration

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

Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
..


Patch Set 5: Verified+1

Build Successful

https://jenkins.impala.io/job/gerrit-docs-auto-test/384/ : Doc tests passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e
Gerrit-Change-Number: 13776
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 12 Jul 2019 02:40:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration

2019-07-11 Thread Alex Rodoni (Code Review)
Hello Thomas Tauber-Marshall, Mike Percy, Andrew Wong, Grant Henke, Hao Hao, 
Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
..

IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration

Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e
---
M docs/shared/impala_common.xml
M docs/topics/impala_kudu.xml
M docs/topics/impala_tables.xml
3 files changed, 84 insertions(+), 64 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e
Gerrit-Change-Number: 13776
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration

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

Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
..


Patch Set 5:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/384/

Testing docs change - this change appears to modify docs/ and no code. This is 
experimental - please report any issues to tarmstr...@cloudera.com or on this 
JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e
Gerrit-Change-Number: 13776
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 12 Jul 2019 02:16:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration

2019-07-11 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13776 )

Change subject: IMPALA-8727: [DOCS] Impala-side changes for Kudu HMS integration
..


Patch Set 4:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/13776/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/13776/4/docs/shared/impala_common.xml@2116
PS4, Line 2116: in Kudu that is integrated with Hive
  : Metastore.
> nit: maybe rephrase, "...if the Kudu service is integrated with the Hive Me
Done


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/shared/impala_common.xml@2115
PS4, Line 2115: This
  : operation is not supported in Kudu that is integrated 
with Hive
  : Metastore.
> We have been using the term "orphaned table" to refer to a Kudu table that
@andrew So this statement is correct, right?


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/shared/impala_common.xml@2115
PS4, Line 2115: This
  : operation is not supported in Kudu that is integrated 
with Hive
  : Metastore.
> I don't think that this is correct. The design doc says that this applies t
According to Andrew's comment, this is correct, no?


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_kudu.xml
File docs/topics/impala_kudu.xml:

http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_kudu.xml@1116
PS4, Line 1116: This section only applies the Kudu services that 
are not
> This patch mostly just points out things that no longer apply. Is there an
Added a section and a link to kudu doc.


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_kudu.xml@1116
PS4, Line 1116: This section only applies the Kudu services that 
are not
> Upstream Kudu has just updated its docs to include both of these:
Done


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_tables.xml
File docs/topics/impala_tables.xml:

http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_tables.xml@289
PS4, Line 289: Some
 : information about the table is stored in the metastore 
database for use
 : by Impala. Other table metadata is managed internally by 
Kudu.
> Actually all data that Impala needs is stored in the HMS (including things
Changed to: All data that Impala needs is stored in the HMS.


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_tables.xml@293
PS4, Line 293:   
 : In the Kudu services enabled to use Hive Metastore 
(HMS), all metadata
 : is managed by HMS.
 :   
> This isn't quite right. Mostly it's managed by Kudu. Some Impala-specific t
Done


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_tables.xml@336
PS4, Line 336: In the Kudu integrated with HMS, the 
impala:: prefix
> Maybe move this up to be directly after the paragraph that talks about '::'
Done


http://gerrit.cloudera.org:8080/#/c/13776/4/docs/topics/impala_tables.xml@336
PS4, Line 336: In the Kudu integrated with HMS, the 
impala:: prefix
> +1
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieec79ac0bbb860c6394a3bf0617b285a7d23ca9e
Gerrit-Change-Number: 13776
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 12 Jul 2019 02:15:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

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

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..

IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists generation

iTbl.getMetaStoreTable() returned null for tables that failed to load,
which led to several test failures in Hive 3 builds.

Also changed the name of member loadedTbls_ to indicate that loading may
have failed.

Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Reviewed-on: http://gerrit.cloudera.org:8080/13845
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
1 file changed, 20 insertions(+), 17 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

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

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 12 Jul 2019 01:12:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] Support SPNEGO for Impala webserver

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

Change subject: Support SPNEGO for Impala webserver
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3865/ : 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/13774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f
Gerrit-Change-Number: 13774
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jul 2019 21:11:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8729: [DOCS] Describe on-demand metadata feature

2019-07-11 Thread Alex Rodoni (Code Review)
Alex Rodoni has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/13802 )

Change subject: IMPALA-8729: [DOCS] Describe on-demand metadata feature
..

IMPALA-8729: [DOCS] Describe on-demand metadata feature

- Overview of on-demand metadata.
- Config flags to enable/disable on-demand metadata.

Change-Id: I64261625c1d9b122c7cca59f9b004dda05810351
Reviewed-on: http://gerrit.cloudera.org:8080/13802
Tested-by: Impala Public Jenkins 
Reviewed-by: Bharath Vissapragada 
---
M docs/topics/impala_metadata.xml
1 file changed, 98 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I64261625c1d9b122c7cca59f9b004dda05810351
Gerrit-Change-Number: 13802
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] Support SPNEGO for Impala webserver

2019-07-11 Thread Todd Lipcon (Code Review)
Hello Thomas Tauber-Marshall, Impala Public Jenkins,

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

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

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

Change subject: Support SPNEGO for Impala webserver
..

Support SPNEGO for Impala webserver

This ports over changes from kudu commit
1f291b77ef0868ac888a850678adc2d7cce65529 which implemented SPNEGO for
the Kudu webserver.

Unfortunately, thorough testing of this is difficult given that curl
isn't currently in the toolchain. I was able to manually test this by
adding a 'sleep(1000)' call into the newly added test case, then setting
up $KRB5_CONFIG in my shell to point to the temporary KDC's environment,
and using 'curl -u : --negotiate http://...' to authenticate.

Strangely, using the version of curl on el7 didn't seem to work properly
(perhaps an el7 curl bug) but using curl on my Ubuntu 18 laptop I was
able to authenticate with SPNEGO.

Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f
---
M be/src/exec/kudu-util.h
M be/src/gutil/strings/escaping.cc
M be/src/util/CMakeLists.txt
A be/src/util/kudu-status-util.h
M be/src/util/webserver-test.cc
M be/src/util/webserver.cc
M be/src/util/webserver.h
7 files changed, 267 insertions(+), 42 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife2b04310e1571d231bf8ee1bcfd3b7afc2edd8f
Gerrit-Change-Number: 13774
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

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

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:54:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

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

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:54:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

2019-07-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13845 )

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 1: Code-Review+2

(1 comment)

Carry +2

http://gerrit.cloudera.org:8080/#/c/13845/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:

http://gerrit.cloudera.org:8080/#/c/13845/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@61
PS1, Line 61: loadedOrFailedTbls_
> nit: maybe requestedTbls_?
I like the current name better even if it is long and ugly. :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:54:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

2019-07-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py@669
PS7, Line 669: if HIVE_MAJOR_VERSION > 2:
 :   del properties['OBJCAPABILITIES']
We could also check OBJCAPABILITIES here, as it should have a fix values.

It may also make sense to add some other tests for OBJCAPABILITIES, e.g. what 
is its value for insert only transactional tables or upgraded tables created by 
Impala.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:45:50 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1

2019-07-11 Thread Attila Jeges (Code Review)
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13722 )

Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..


Patch Set 4:

(31 comments)

> Did some benchmarking on the IsoSql parsing and there was one thing
 > that caused a decent performance drop compared to the SimpleDate
 > format version:
 > - For SimpleDateFormat everything is a separator that is not a
 > digit, so checking the end of a pattern section is done via calling
 > isdigit(). On the other hand my implementation had an unordered_set
 > to contain the separator characters and for each character in the
 > input I made a lookup in this set. Apparently isdigit() outperforms
 > the set implementation so I made some massaging on the
 > IsSeparator() function.
 > - The improvement was to get rid of the unordered_set for
 > separators and simply do comparisons on hard-coded characters
 > within IsSeparator(). This gave a significant performance
 > improvement. (Still doesn;t reach the efficiency of isdigit())
 >
 > Still the SimpleDateFormat implementation has some performance
 > advantage over the IsoSql implementation but this is due to the
 > fact that the latter offers more flexibility:
 > - The length of the separator sequences is flexible (matching is
 > not strict char by char).
 > - There is a defined set of characters that can serve as a
 > separator (not taking everything non-digit as separator).
 > I feel that taking these extra functionalities into account the
 > performance difference is reasonable and acceptable.

Thanks for the improvements and the analysis!

http://gerrit.cloudera.org:8080/#/c/13722/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13722/2//COMMIT_MSG@49
PS2, Line 49: In a string type to timestamp conversion the timezone offset 
tokens
:   are parsed, expected to match with the input but they don't 
adjust
:   the result as the input is already expected to be in UTC format.
> Not really since e.g. Oracle have different types for timestamp with timezo
ok, thanks for the explanation.


http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/benchmarks/parse-timestamp-benchmark.cc
File be/src/benchmarks/parse-timestamp-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/benchmarks/parse-timestamp-benchmark.cc@46
PS3, Line 46: //
nit: put a space after // in this line and below.


http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/benchmarks/parse-timestamp-benchmark.cc@64
PS3, Line 64: //  ImpalaSimpleDateFormatTZTimeStamp   16.2 16.6 
17.2  67.2X  65.3X  66.3X
: //ImpalaIsoSqlFormatTimeStamp
Maybe add 'ImpalaIsoSqlFormatTZTimestamp' to this micro-benchmark?


http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/benchmarks/parse-timestamp-benchmark.cc@134
PS3, Line 134: TestImpalaDate
I think this should be called 'TestImpalaSimpleDateFormat' for consistency (or 
something similar).


http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/exprs/cast-functions-ir.cc@172
PS3, Line 172:
Add 'const' specifier to the type. Here and below in L192, L309, L344.


http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/runtime/date-test.cc
File be/src/runtime/date-test.cc:

http://gerrit.cloudera.org:8080/#/c/13722/3/be/src/runtime/date-test.cc@36
PS3, Line 36: using namespace datetime_parse_util;
I think, this test should cover the new iso-sql date parser as well.


http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:

http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-parser.cc@141
PS2, Line 141: if (!ParseAndValidate(current_pos, group_len, 0, 86399, 
_in_day)) {
 :   return false;
 : }
 : result->second = second_in_day % 60;
 : int minutes_in_day = second_in_day / 60;
 : result->minute = minutes_in_day % 60;
 : result->hour = minutes_in_day / 60;
 : break;
 :   }
> The idea was to only move logic to functions that are used both by SimpleDa
Ok, I get it.


http://gerrit.cloudera.org:8080/#/c/13722/2/be/src/runtime/datetime-iso-sql-format-parser.cc@219
PS2, Line 219: DCHECK(input_str != nullptr);
> No need to check if 'input_len' > 2 as the length of a  TIMEZONE_HOUR is by
What happens if the input string is invalid?

e.g.: At the end of the input string where the TIMEZONE_HOUR token should be, 
the input string has just one character (a digit). In this case 
GetNextTokenGroupFromInput() is called with input_len = 1, tok.type = 
TIMEZONE_HOUR and tok.len = 3. In L219 we set input_len to 2. Then in L225 we 

[Impala-ASF-CR] IMPALA-8729: [DOCS] Describe on-demand metadata feature

2019-07-11 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13802 )

Change subject: IMPALA-8729: [DOCS] Describe on-demand metadata feature
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64261625c1d9b122c7cca59f9b004dda05810351
Gerrit-Change-Number: 13802
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:25:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8571[WIP]: improve QueryEventHook execution

2019-07-11 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13748 )

Change subject: IMPALA-8571[WIP]: improve QueryEventHook execution
..


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13748/14/fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java
File 
fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java:

http://gerrit.cloudera.org:8080/#/c/13748/14/fe/src/main/java/org/apache/impala/hooks/FixedCapacityQueryHookExecutor.java@327
PS14, Line 327: final Future f = hookExecutor_.submit(() -> 
{
> I think I'm missing something.  If I just do new FutureTask<>(callHook).run
Like we discussed offline, I meant new Thread(Future).run() but we will have to 
bear the thread creation cost repeatedly. So probably fine to leave it as is 
(Also consider using Executors.newCachedThreadPool)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb88422f7cfe86947d11ce57d2b4c63e57d1b643
Gerrit-Change-Number: 13748
Gerrit-PatchSet: 14
Gerrit-Owner: radford nguyen 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: radford nguyen 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:21:36 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

2019-07-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13845 )

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13845/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:

http://gerrit.cloudera.org:8080/#/c/13845/1/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@61
PS1, Line 61: loadedOrFailedTbls_
nit: maybe requestedTbls_?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:05:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

2019-07-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..


Patch Set 7: Code-Review+1

(10 comments)

Lgtm, found some nits, but nothing serious.

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS7, Line 78:   private static final String CONNECTORREAD = "CONNECTORREAD";
:   private static final String CONNECTORWRITE = "CONNECTORWRITE";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@83
PS7, Line 83:   private static final String HIVEFULLACIDREAD = 
"HIVEFULLACIDREAD";
:   private static final String HIVEFULLACIDWRITE = 
"HIVEFULLACIDWRITE";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@87
PS7, Line 87:   private static final String HIVEMANAGESTATS = "HIVEMANAGESTATS";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: MAJOR_VERSION
nit: Technically this class doesn't have a version number, so maybe we should 
rename it to IMPALA_MAJOR_VERSION


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@470
PS7, Line 470: HIVEMANAGEDINSERTREAD,
nit: maybe add TODO for HIVEMANAGEDINSERTWRITE once IMPALA-8636 goes in.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@532
PS7, Line 532: shim
nit: impala


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

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@130
PS7, Line 130:   public static final byte REQUIRE_READ = (byte)2;
 :   public static final byte REQUIRE_WRITE = (byte)4;
 :   public static final byte REQUIRE_READWRITE = (byte)8;
nit: maybe we could use the same naming as in Hive, i.e. ACCESSTYPE_READONLY, 
etc.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@198
PS7, Line 198: OperationType
nit: please add comment


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@270
PS7, Line 270:   case READ:
 :   case ANY:
 :   default:
 : ensureTableSupported(table);
nit: ensureTableSupported() also succeeds if the table has 
ACCESSTYPE_WRITEONLY, however having a write only table might not be a real 
life scenario.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   static {
: if (MetastoreShim.getMajorVersion() > 2) {
:   MetastoreShim.setHiveClientCapabilities();
: }
:   }
nit: Can you place this to MetastoreShim 3 only?

 static {
   setHiveClientCapabilities();
 }



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 17:45:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

2019-07-11 Thread Yongzhi Chen (Code Review)
Yongzhi Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13845 )

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 1: Code-Review+1

The change looks good.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 16:42:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

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

Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3864/ : 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/13845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 15:42:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

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

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3863/ : 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/13558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 15:28:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8681: Fix null pointer exception in ValidWriteIdLists generation

2019-07-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13845


Change subject: IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists 
generation
..

IMPALA-8681: Fix null pointer exception in  ValidWriteIdLists generation

iTbl.getMetaStoreTable() returned null for tables that failed to load,
which led to several test failures in Hive 3 builds.

Also changed the name of member loadedTbls_ to indicate that loading may
have failed.

Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
---
M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
1 file changed, 20 insertions(+), 17 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id9d1dcf9b7496c4a80e9af82cadad8682085d849
Gerrit-Change-Number: 13845
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

2019-07-11 Thread Yongzhi Chen (Code Review)
Yongzhi Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..


Patch Set 7:

(3 comments)

Patch set 7 addresses review issues.

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG@7
PS6, Line 7: Support table capabilities handling with Hive
> I think that bucketed tables are now a secondary feature compared to capabi
Done


http://gerrit.cloudera.org:8080/#/c/13558/6/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/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS6, Line 2210: he table cannot be found after created.
> This means that HMS doesn't return Kudu tables where OBJCAPABILITIES is set
Done


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS6, Line 2211: ed jira
> typo: adding
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 14:50:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

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

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS7, Line 20: import static 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS7, Line 21: import static 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS7, Line 22: import static 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS7, Line 23: import static 
org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 14:48:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

2019-07-11 Thread Yongzhi Chen (Code Review)
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, 
Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
..

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 393 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1

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

Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3862/ : 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/13722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
Gerrit-Change-Number: 13722
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 11 Jul 2019 14:27:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1

2019-07-11 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13722 )

Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..


Patch Set 5:

Did some benchmarking on the IsoSql parsing and there was one thing that caused 
a decent performance drop compared to the SimpleDate format version:
- For SimpleDateFormat everything is a separator that is not a digit, so 
checking the end of a pattern section is done via calling isdigit(). On the 
other hand my implementation had an unordered_set to contain the separator 
characters and for each character in the input I made a lookup in this set. 
Apparently isdigit() outperforms the set implementation so I made some 
massaging on the IsSeparator() function.
- The improvement was to get rid of the unordered_set for separators and simply 
do comparisons on hard-coded characters within IsSeparator(). This gave a 
significant performance improvement. (Still doesn;t reach the efficiency of 
isdigit())

Still the SimpleDateFormat implementation has some performance advantage over 
the IsoSql implementation but this is due to the fact that the latter offers 
more flexibility:
 - The length of the separator sequences is flexible (matching is not strict 
char by char).
 - There is a defined set of characters that can serve as a separator (not 
taking everything non-digit as separator).
I feel that taking these extra functionalities into account the performance 
difference is reasonable and acceptable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
Gerrit-Change-Number: 13722
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 11 Jul 2019 13:56:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1

2019-07-11 Thread Gabor Kaszab (Code Review)
Hello Attila Jeges, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..

IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1

This enhancement introduces FORMAT clause for CAST() operator that is
applicable for casts between string types and timestamp types. Instead
of accepting SimpleDateFormat patterns the FORMAT clause supports
datetime patterns following the ISO:SQL:2016 standard.
Note, the CAST() operator without the FORMAT clause still uses
Impala's implementation of SimpleDateFormat handling. Similarly, the
existing conversion functions such as to_timestamp(), from_timestamp()
etc. remain unchanged and use SimpleDateFormat.

Milestone 1 contains all the format tokens covered by the SQL
standard. Further milestones will add more functionality on top of
this list to cover functionality provided by other RDBMS systems.

List of tokens implemented by this change:
- , YYY, YY, Y: Year tokens
- , RR: Round year tokens
- MM: Month
- DD: Day
- DDD: Day of year
- HH, HH12: Hour of day (1-12)
- HH24: Hour of day (0-23)
- MI: Minute
- SS: Second
- S: Second of day
- FF, FF1, ..., FF9: Fractional second
- AM, PM, A.M., P.M.: Meridiem indicators
- TZH: Timezone hour
- TZM: Timezone minute
- Separators: - . / , ' ; : space
- ISO8601 date indicators (T, Z)

Some notes about the matching algorithm:
- The parsing algorithm uses these tokens in a case insensitive
  manner.
- The separators are interchangeable with each other. For example a
  '-' separator in the format will match with a '.' character in the
  input.
- The length of the separator sequences is handled flexibly meaning
  that a single separator character in the format for instance would
  match with a multi-separator sequence in the input.
- In a string type to timestamp conversion the timezone offset tokens
  are parsed, expected to match with the input but they don't adjust
  the result as the input is already expected to be in UTC format.

Usage example:
SELECT CAST('01-02-2019' AS TIMESTAMP FORMAT 'MM-DD-');
SELECT CAST('2019.10.10 13:30:40.123456 +01:30' AS TIMESTAMP
FORMAT '-MM-DD HH24:MI:SS.FF9 TZH:TZM');
SELECT CAST(timestamp_column as STRING
FORMAT " MM HH12 YY") from some_table;

Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
---
M be/src/benchmarks/convert-timestamp-benchmark.cc
M be/src/benchmarks/parse-timestamp-benchmark.cc
M be/src/common/init.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/CMakeLists.txt
A be/src/exprs/cast-expr.cc
A be/src/exprs/cast-expr.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/date-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.h
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/date-parse-util.cc
M be/src/runtime/date-parse-util.h
M be/src/runtime/date-test.cc
M be/src/runtime/date-value.cc
M be/src/runtime/date-value.h
A be/src/runtime/datetime-iso-sql-format-parser.cc
A be/src/runtime/datetime-iso-sql-format-parser.h
A be/src/runtime/datetime-iso-sql-format-tokenizer.cc
A be/src/runtime/datetime-iso-sql-format-tokenizer.h
D be/src/runtime/datetime-parse-util.h
A be/src/runtime/datetime-parser-common.cc
A be/src/runtime/datetime-parser-common.h
R be/src/runtime/datetime-simple-date-format-parser.cc
A be/src/runtime/datetime-simple-date-format-parser.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/testutil/random-vector-generators.h
M be/src/util/dict-test.cc
M be/src/util/min-max-filter-test.cc
M be/src/util/string-parser.h
M common/thrift/Exprs.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A 
testdata/workloads/functional-query/queries/QueryTest/cast_format_from_table.test
M testdata/workloads/functional-query/queries/QueryTest/date.test
A tests/query_test/test_cast_with_format.py
54 files changed, 3,333 insertions(+), 853 deletions(-)


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

Gerrit-Project: Impala-ASF

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

2019-07-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
..


Patch Set 6: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG@7
PS6, Line 7: Prohibit write operations for bucketed tables
I think that bucketed tables are now a secondary feature compared to capability 
handling. Maybe something like "Support table capabilities handling with Hive 
3" would be more descriptive.


http://gerrit.cloudera.org:8080/#/c/13558/6/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/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS6, Line 2210: The table cannot be found after created.
This means that HMS doesn't return Kudu tables where OBJCAPABILITIES is set? If 
there is a jira for this, can you mention it in the comment?


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS6, Line 2211: addiing
typo: adding



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Sudhanshu Arora 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Reviewer: Yongzhi Chen 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Thu, 11 Jul 2019 12:05:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer

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

Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
..

IMPALA-8543: More diagnostics for TAcceptQueueServer

This change adds more logging information in TAcceptQueueServer
to help diagnose issues at various stages of client connections
establishment.

Two new metrics are also added to measure the connection setup time
and the wait time for service threads to be available.

Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Reviewed-on: http://gerrit.cloudera.org:8080/13790
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M common/thrift/metrics.json
3 files changed, 189 insertions(+), 28 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer

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

Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Thu, 11 Jul 2019 06:55:52 +
Gerrit-HasComments: No