Re: Review Request 70524: Break up DDLTask - extract Workload Management related operations

2019-04-23 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70524/#review214816 --- ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/Alte

Re: Review Request 69054: HIVE-20740 : Remove global lock in ObjectStore.setConf method

2018-11-27 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69054/#review210893 --- LGTM standalone-metastore/metastore-server/src/test/java/org/apa

Re: Review Request 69167: HIVE-20796: jdbc URL can contain sensitive information that should not be logged

2018-10-25 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69167/#review210043 --- standalone-metastore/metastore-server/src/main/java/org/apache/ha

Re: Review Request 69054: HIVE-20740 : Remove global lock in ObjectStore.setConf method

2018-10-22 Thread Andrew Sherman via Review Board
> On Oct. 16, 2018, 8:59 p.m., Andrew Sherman wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Lines 291 (patched) > > > > > > What is someone has s

Re: Review Request 69054: HIVE-20740 : Remove global lock in ObjectStore.setConf method

2018-10-16 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69054/#review209662 --- This all looks good, I just have annoying questions standalone-m

Re: Review Request 68969: HIVE-20307 : Add support for filterspec to the getPartitions with projection API

2018-10-09 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68969/#review209385 --- Ship it! standalone-metastore/metastore-server/src/test/java/o

Re: Review Request 68969: HIVE-20307 : Add support for filterspec to the getPartitions with projection API

2018-10-09 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68969/#review209380 --- All looks fine within the bounds of what I understand standalone

Re: Review Request 68664: HIVE-20306: Implement projection spec for fetching only requested fields from partitions

2018-10-02 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68664/#review209171 --- Fix it, then Ship it! All looks good, just a few nits which I n

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

2018-10-02 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68889/#review209165 --- Ship it! Ship It! - Andrew Sherman On Oct. 2, 2018, 9:55 p.m

Re: Review Request 68889: HIVE-20610 : TestDbNotificationListener should not use /tmp directory

2018-10-01 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68889/#review209131 --- itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/liste

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68827/#review209102 --- standalone-metastore/metastore-common/src/main/java/org/apache/ha

Re: Review Request 68827: HIVE-20545 : Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-28 Thread Andrew Sherman via Review Board
> On Sept. 26, 2018, 5:51 p.m., Andrew Sherman wrote: > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 917 (patched) > > > > > > make privat

Re: Review Request 68827: Exclude large-sized parameters from serialization of Table and Partition thrift objects in HMS notifications

2018-09-26 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68827/#review209031 --- Code looks clean, but I have some scary questions. standalone-me

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-25 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68710/#review208993 --- service-rpc/pom.xml Lines 156 (patched)

Re: Review Request 68767: HIVE-20551: Create PreparedStatement query dynamically when IN clause is used

2018-09-24 Thread Andrew Sherman via Review Board
> On Sept. 20, 2018, 10:56 p.m., Andrew Sherman wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > > Lines 316 (patched) > > > > > > 1) Does setO

Re: Review Request 68664: HIVE-20306: Implement projection spec for fetching only requested fields from partitions

2018-09-21 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68664/#review208866 --- Basically this looks good. All my issues are pretty picky. stand

Re: Review Request 68710: HIVE-20544: TOpenSessionReq logs password and username

2018-09-21 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68710/#review208862 --- service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/r

Re: Review Request 68767: HIVE-20551: Create PreparedStatement query dynamically when IN clause is used

2018-09-21 Thread Andrew Sherman via Review Board
> On Sept. 20, 2018, 10:56 p.m., Andrew Sherman wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > > Lines 316 (patched) > > > > > > 1) Does setO

Re: Review Request 68767: HIVE-20551: Create PreparedStatement query dynamically when IN clause is used

2018-09-21 Thread Andrew Sherman via Review Board
> On Sept. 21, 2018, 12:04 a.m., denys kuzmenko wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > > Line 311 (original), 311 (patched) > > > > > >

Re: Review Request 68767: HIVE-20551: Create PreparedStatement query dynamically when IN clause is used

2018-09-20 Thread Andrew Sherman via Review Board
> On Sept. 20, 2018, 10:56 p.m., Andrew Sherman wrote: > > This all looks good, I haver questions... - Andrew --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68767/#review208821 -

Re: Review Request 68767: HIVE-20551: Create PreparedStatement query dynamically when IN clause is used

2018-09-20 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68767/#review208821 --- standalone-metastore/metastore-server/src/main/java/org/apache/ha

Re: Review Request 68365: HIVE-19253: HMS ignores tableType property for external tables

2018-08-15 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68365/#review207383 --- Ship it! Ship It! - Andrew Sherman On Aug. 15, 2018, 11:13 p

Re: Review Request 68365: HIVE-19253: HMS ignores tableType property for external tables

2018-08-15 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68365/#review207354 --- standalone-metastore/metastore-server/src/test/java/org/apache/ha

Re: Review Request 68027: HIVE-19986: Add logging of runtime statistics indicating when Hdfs Erasure Coding is used by MR. These stats are not avalable until the unreleased Hadoop 3.2 so the shim is u

2018-07-24 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68027/ --- (Updated July 24, 2018, 9:15 p.m.) Review request for hive and Sahil Takiar.

Re: Review Request 67468: HIVE-18118: provide supportability support for Erasure Coding Update number of Erasure Coded Files in a directory as part of Basic (aka Quick) Stats This information is then

2018-06-21 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67468/ --- (Updated June 21, 2018, 4:32 p.m.) Review request for hive and Sahil Takiar.

Re: Review Request 67468: FIXES TO MAKE INTELLIJ HAPPY DO NOT PUSH

2018-06-20 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67468/ --- (Updated June 21, 2018, 12:33 a.m.) Review request for hive and Sahil Takiar.

Re: Review Request 67468: HIVE-18118: provide supportability support for Erasure Coding Update number of Erasure Coded Files in a directory as part of Basic (aka Quick) Stats This information is then

2018-06-15 Thread Andrew Sherman via Review Board
> On June 15, 2018, 1:37 p.m., Sahil Takiar wrote: > > Why not show the #of EC files for regular explain plans too? To decrease > > the # of q file updates, it can be omitted if the # of EC files = 0 I saw that regular explain did not report numFiles so I did not report numEcFiles there. I thi

Review Request 67468: HIVE-18118: provide supportability support for Erasure Coding Update number of Erasure Coded Files in a directory as part of Basic (aka Quick) Stats This information is then (mos

2018-06-05 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67468/ --- Review request for hive and Sahil Takiar. Repository: hive-git Description --

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-22 Thread Andrew Sherman via Review Board
> On May 10, 2018, noon, Sahil Takiar wrote: > > testutils/ptest2/conf/deployed/master-mr2.properties > > Line 75 (original), 75 (patched) > > > > > > have you manually deployed these changes to the ptest server? this

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-15 Thread Andrew Sherman via Review Board
> On May 10, 2018, noon, Sahil Takiar wrote: > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > > Lines 674 (patched) > > > > > > is this necessary since you set the cluster type to mr

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-15 Thread Andrew Sherman via Review Board
> On May 14, 2018, 4:57 p.m., Sahil Takiar wrote: > > itests/src/test/resources/testconfiguration.properties > > Line 1693 (original), 1693 (patched) > > > > > > why would we want the ec commands to work outside t

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-15 Thread Andrew Sherman via Review Board
> On May 10, 2018, noon, Sahil Takiar wrote: > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > > Lines 674 (patched) > > > > > > is this necessary since you set the cluster type to mr

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-14 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67023/ --- (Updated May 14, 2018, 9:44 p.m.) Review request for hive and Sahil Takiar. R

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-14 Thread Andrew Sherman via Review Board
> On May 14, 2018, 4:57 p.m., Sahil Takiar wrote: > > itests/src/test/resources/testconfiguration.properties > > Line 1693 (original), 1693 (patched) > > > > > > why would we want the ec commands to work outside t

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-14 Thread Andrew Sherman via Review Board
> On May 10, 2018, noon, Sahil Takiar wrote: > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java > > Lines 674 (patched) > > > > > > is this necessary since you set the cluster type to mr

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-11 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67023/#review202981 --- ql/src/test/queries/clientpositive/erasure_simple.q Lines 33 (pat

Re: Review Request 67023: HIVE-18117: Add a new Test Driver "TestErasureCodingHDFSCliDriver" that can be used to run tests over hdfs directories that employ Erasure Coding.

2018-05-11 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67023/ --- (Updated May 11, 2018, 11:38 p.m.) Review request for hive and Sahil Takiar.

Re: Review Request 65745: HIVE-18743: CREATE TABLE on S3 data can be extremely slow. DO_NOT_UPDATE_STATS workaround is buggy.

2018-03-02 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65745/#review198560 --- standalone-metastore/src/main/java/org/apache/hadoop/hive/metasto

Re: Review Request 65075: HIVE-18426: Memory leak in RoutingAppender for every hive operation

2018-01-12 Thread Andrew Sherman via Review Board
> On Jan. 11, 2018, 9:51 p.m., Andrew Sherman wrote: > > common/src/java/org/apache/hadoop/hive/common/LogUtils.java > > Lines 259 (patched) > > > > > > Does deleteAppender() call stop() internally? If so then can we

Re: Review Request 65075: HIVE-18426: Memory leak in RoutingAppender for every hive operation

2018-01-11 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65075/#review195255 --- common/src/java/org/apache/hadoop/hive/common/LogUtils.java Lines

Re: Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-12-08 Thread Andrew Sherman via Review Board
> On Dec. 7, 2017, 8:12 p.m., Sahil Takiar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRProcContext.java > > Line 201 (original), 208 (patched) > > > > > > Can we use `parseContext.getQueryState()`

Re: Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-12-07 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64193/ --- (Updated Dec. 7, 2017, 7:12 p.m.) Review request for hive. Repository: hive-g

Re: Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-12-06 Thread Andrew Sherman via Review Board
> On Dec. 2, 2017, 12:22 a.m., Sahil Takiar wrote: > > Since we touch the `LoadSemanticAnalyzer` could we add a q-test (could be > > added to one of the existing `lineage*.q` files) for `LOAD` statements. > > Same for import / export statements (as far as I can tell there are no > > existing o

Re: Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-12-04 Thread Andrew Sherman via Review Board
> On Dec. 2, 2017, 12:22 a.m., Sahil Takiar wrote: > > Since we touch the `LoadSemanticAnalyzer` could we add a q-test (could be > > added to one of the existing `lineage*.q` files) for `LOAD` statements. > > Same for import / export statements (as far as I can tell there are no > > existing o

Re: Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-12-01 Thread Andrew Sherman via Review Board
> On Dec. 1, 2017, 5:37 p.m., Sahil Takiar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/Driver.java > > Lines 365 (patched) > > > > > > Why do we need constructors to explicitly pass in a `LineageState` > > obje

Review Request 64193: HIVE-18054: Make Lineage work with concurrent queries on a Session

2017-11-29 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64193/ --- Review request for hive. Repository: hive-git Description --- A Hive Ses

Re: Review Request 63586: Fix HIVE-17942: HiveAlterHandler should use the conf from HMS Handler

2017-11-08 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63586/#review190483 --- Ship it! Ship It! - Andrew Sherman On Nov. 8, 2017, 7:17 p.m

Re: Review Request 63586: Fix HIVE-17942: HiveAlterHandler should use the conf from HMS Handler

2017-11-08 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63586/#review190454 --- itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/T

Re: Review Request 62995: HIVE-17806 Create directory for metrics file if it doesn't exist

2017-10-16 Thread Andrew Sherman via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62995/#review188223 --- common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Js

Re: Review Request 62693: HIVE-17635: Add unit tests to CompactionTxnHandler and use PreparedStatements for queries

2017-10-13 Thread Andrew Sherman via Review Board
> On Oct. 12, 2017, 5:34 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Line 387 (original), 411 (patched) > > > > > > What is this for? The 'ques