Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 14, 2018, 9:51 a.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/package-info.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/8/ Changes: https://reviews.apache.org/r/65478/diff/7-8/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 14, 2018, 9:30 a.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/package-info.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/7/ Changes: https://reviews.apache.org/r/65478/diff/6-7/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 13, 2018, 3:43 p.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/package-info.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/6/ Changes: https://reviews.apache.org/r/65478/diff/5-6/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
> On Feb. 13, 2018, 8:50 a.m., Vihang Karajgaonkar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java > > Lines 253-254 (original), 255-256 (patched) > > <https://reviews.apache.org/r/65478/diff/4/?file=1957004#file1957004line261> > > > > before the patch, the code assumes that the ColumnReader's type matches > > with what the hive's type is. So these two lines made sense. But after the > > patch, it is possible that the underlying parquet type does not have > > decimalMetadata so it these lines will throw a NPE. Instead of throwing > > NPE, we should check if decimalMetadata is set and throw IOException here > > saying that underlying type cannot be converted to decimal. What do you > > think? > > > > Also, may be we should add a TODO here to use default precision and > > scale if its not available for the future. > > cheng xu wrote: > Why do we need the default precision and scale here? We can hardly read > data as decimal if those values are missing. > > Vihang Karajgaonkar wrote: > hmm .. I am not a 100% sure but I thought HiveDecimal defines something > like USER_DEFAULT_PRECISION and USER_DEFAULT_SCALE which is used when user > doesn't provide scale and precision. This is not important for this change so > you can ignore the comment related to default precision and scale. But I > think we should protect against a NPE which will be thrown if the user tries > to change say an int column to a decimal(10,2) (I know it doesn't work > anyways, but I think it would be better to throw an exception rather than > failing with a NPE) +1 for the checking part. Patch is updated addressing that. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/#review197313 --- On Feb. 13, 2018, 3:34 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65478/ > --- > > (Updated Feb. 13, 2018, 3:34 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > --- > > VectorizedParquetReader throws an exception when trying to reading from a > parquet table on which new columns are added. > > > Diffs > - > > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java > 907a9b8 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java > c36640d > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java > 08ac57b > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java > 39689f1 > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/package-info.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java > 9e414dc > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java > 3e5d831 > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java > 5d3ebd6 > > ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q > PRE-CREATION > > ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q > PRE-CREATION > ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65478/diff/5/ > > > Testing > --- > > Newly added UT passed and qtest passed locally. > > > Thanks, > > cheng xu > >
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 13, 2018, 3:34 p.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/package-info.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/5/ Changes: https://reviews.apache.org/r/65478/diff/4-5/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
> On Feb. 13, 2018, 8:50 a.m., Vihang Karajgaonkar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java > > Lines 253-254 (original), 255-256 (patched) > > <https://reviews.apache.org/r/65478/diff/4/?file=1957004#file1957004line261> > > > > before the patch, the code assumes that the ColumnReader's type matches > > with what the hive's type is. So these two lines made sense. But after the > > patch, it is possible that the underlying parquet type does not have > > decimalMetadata so it these lines will throw a NPE. Instead of throwing > > NPE, we should check if decimalMetadata is set and throw IOException here > > saying that underlying type cannot be converted to decimal. What do you > > think? > > > > Also, may be we should add a TODO here to use default precision and > > scale if its not available for the future. Why do we need the default precision and scale here? We can hardly read data as decimal if those values are missing. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/#review197313 ------- On Feb. 12, 2018, 4:25 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65478/ > --- > > (Updated Feb. 12, 2018, 4:25 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > --- > > VectorizedParquetReader throws an exception when trying to reading from a > parquet table on which new columns are added. > > > Diffs > - > > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java > 907a9b8 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java > c36640d > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java > 08ac57b > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java > 39689f1 > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java > 9e414dc > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java > 3e5d831 > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java > 5d3ebd6 > > ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q > PRE-CREATION > > ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q > PRE-CREATION > ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65478/diff/4/ > > > Testing > --- > > Newly added UT passed and qtest passed locally. > > > Thanks, > > cheng xu > >
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 12, 2018, 4:25 p.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/4/ Changes: https://reviews.apache.org/r/65478/diff/3-4/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 9, 2018, 8:07 p.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedListColumnReader.java c36640d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedPrimitiveColumnReader.java 39689f1 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q PRE-CREATION ql/src/test/queries/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_dictionary_encoding.q.out PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table_non_dictionary_encoding.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/3/ Changes: https://reviews.apache.org/r/65478/diff/2-3/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
> On Feb. 6, 2018, 3:06 p.m., Vihang Karajgaonkar wrote:
> > ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q
> > Lines 71 (patched)
> > <https://reviews.apache.org/r/65478/diff/2/?file=1952940#file1952940line71>
> >
> > Can you please test using timestamps as well. Specifically, the
> > following should work.
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts string);
> > select * from test_alter2;
> >
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts varchar(19));
> > -- this should truncate the microseconds
> > select * from test_alter2;
> >
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts char(25);
> > select * from test_alter2;
>
> cheng xu wrote:
> I test non vectorization path and all of these test cases failed due to
> not abled to type cast error. Any thoughts on this?
Never mind. It's passed in qtest. I will investigate on this.
- cheng
-------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65478/#review196867
---
On Feb. 5, 2018, 4:46 p.m., cheng xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65478/
> ---
>
> (Updated Feb. 5, 2018, 4:46 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> VectorizedParquetReader throws an exception when trying to reading from a
> parquet table on which new columns are added.
>
>
> Diffs
> -
>
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java
> 907a9b8
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java
> PRE-CREATION
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java
> PRE-CREATION
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java
> 08ac57b
>
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java
> 9e414dc
>
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java
> 5d3ebd6
> ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q PRE-CREATION
> ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out
> PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65478/diff/2/
>
>
> Testing
> ---
>
> Newly added UT passed and qtest passed locally.
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
> On Feb. 6, 2018, 3:06 p.m., Vihang Karajgaonkar wrote:
> > ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q
> > Lines 71 (patched)
> > <https://reviews.apache.org/r/65478/diff/2/?file=1952940#file1952940line71>
> >
> > Can you please test using timestamps as well. Specifically, the
> > following should work.
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts string);
> > select * from test_alter2;
> >
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts varchar(19));
> > -- this should truncate the microseconds
> > select * from test_alter2;
> >
> >
> > drop table test_alter2;
> > create table test_alter2 (ts timestamp) stored as parquet;
> > insert into test_alter2 values ('2018-01-01 13:14:15.123456'),
> > ('2018-01-02 14:15:16.123456'), ('2018-01-03 16:17:18.123456');
> > select * from test_alter2;
> > alter table test_alter2 replace columns (ts char(25);
> > select * from test_alter2;
I test non vectorization path and all of these test cases failed due to not
abled to type cast error. Any thoughts on this?
- cheng
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65478/#review196867
---
On Feb. 5, 2018, 4:46 p.m., cheng xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65478/
> ---
>
> (Updated Feb. 5, 2018, 4:46 p.m.)
>
>
> Review request for hive.
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> VectorizedParquetReader throws an exception when trying to reading from a
> parquet table on which new columns are added.
>
>
> Diffs
> -
>
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java
> 907a9b8
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java
> PRE-CREATION
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java
> PRE-CREATION
>
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java
> 08ac57b
>
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java
> 9e414dc
>
> ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java
> 5d3ebd6
> ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q PRE-CREATION
> ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out
> PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65478/diff/2/
>
>
> Testing
> ---
>
> Newly added UT passed and qtest passed locally.
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
> On Feb. 6, 2018, 1:46 a.m., Vihang Karajgaonkar wrote: > > Thanks Vihang for your review. Comments left below. > On Feb. 6, 2018, 1:46 a.m., Vihang Karajgaonkar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java > > Lines 24 (patched) > > <https://reviews.apache.org/r/65478/diff/1/?file=1951881#file1951881line24> > > > > Do we need to override the methods for other reads as well? What is the > > criteria to identify the methods which need to be overriden for this and > > TypesFromInt64PageReader? > > Jerry Chen wrote: > Current Ferdinand follows the same conversion principles of > ETypeConverter in Hive. The basic conversion implemented in ETypeConverter > is: low precision data type can convert to high precison data type. for > int32: it can be converted to int64, float, double. for int64: it can be > converted to float, double. For float: it can be converted to double. > > Although other conversions can logically supported such as int64 to int32 > or double to float. But that is just not always safe. Yes, as Jerry mentioned, our current implementation stays the same rules as non vectorization path. To support other conversion, we may need to ensure both vectorization and non vectorization have the same behavior since vectorization is just a performance feature. > On Feb. 6, 2018, 1:46 a.m., Vihang Karajgaonkar wrote: > > ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q > > Lines 9 (patched) > > <https://reviews.apache.org/r/65478/diff/1/?file=1951887#file1951887line9> > > > > can you please confirm that the result of this q file matches with > > non-vectorized execution? Yes, I checked the output as well by disabling the vectorization. And they have the same output as vectorization enabled case. > On Feb. 6, 2018, 1:46 a.m., Vihang Karajgaonkar wrote: > > ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out > > Lines 140 (patched) > > <https://reviews.apache.org/r/65478/diff/1/?file=1951888#file1951888line140> > > > > this is interesting. Do you know why this row is returned first? The order is the same as non vectorization read path. It may result from the order of generated files. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/#review196718 --- On Feb. 5, 2018, 4:46 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65478/ > --- > > (Updated Feb. 5, 2018, 4:46 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > --- > > VectorizedParquetReader throws an exception when trying to reading from a > parquet table on which new columns are added. > > > Diffs > - > > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java > 907a9b8 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java > 08ac57b > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java > 9e414dc > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java > 5d3ebd6 > ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q PRE-CREATION > ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65478/diff/2/ > > > Testing > --- > > Newly added UT passed and qtest passed locally. > > > Thanks, > > cheng xu > >
Re: Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- (Updated Feb. 5, 2018, 4:46 p.m.) Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/2/ Changes: https://reviews.apache.org/r/65478/diff/1-2/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Review Request 65478: HIVE-18553 VectorizedParquetReader fails after adding a new column to table
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65478/ --- Review request for hive. Repository: hive-git Description --- VectorizedParquetReader throws an exception when trying to reading from a parquet table on which new columns are added. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/BaseVectorizedColumnReader.java 907a9b8 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/DefaultParquetDataColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/ParquetDataColumnReaderFactory.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedDummyColumnReader.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/vector/VectorizedParquetRecordReader.java 08ac57b ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedColumnReader.java 9e414dc ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestVectorizedDictionaryEncodingColumnReader.java 3e5d831 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java 5d3ebd6 ql/src/test/queries/clientpositive/schema_evol_par_vec_table.q PRE-CREATION ql/src/test/results/clientpositive/schema_evol_par_vec_table.q.out PRE-CREATION Diff: https://reviews.apache.org/r/65478/diff/1/ Testing --- Newly added UT passed and qtest passed locally. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/temp_table.q.out 342d08c ql/src/test/results/clientpositive/temp_table_truncate.q.out b1af432 Diff: https://reviews.apache.org/r/63711/diff/7/ Changes: https://reviews.apache.org/r/63711/diff/6-7/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
> On Nov. 16, 2017, 4:44 a.m., Vihang Karajgaonkar wrote: > > ql/src/test/results/clientpositive/llap/sysdb.q.out > > Lines 2236 (patched) > > <https://reviews.apache.org/r/63711/diff/6/?file=1892471#file1892471line2236> > > > > not sure why this file is changing. Do you know? Actually this test case is failing in pre-commit due to difference in output. Locally I am not able to reproduce the output as the golden files. So I prefer to upload the local version instead and file other jira about this failing test case. Any thoughts on that? > On Nov. 16, 2017, 4:44 a.m., Vihang Karajgaonkar wrote: > > ql/src/test/results/clientpositive/llap/sysdb.q.out > > Line 3646 (original), 3706 (patched) > > <https://reviews.apache.org/r/63711/diff/6/?file=1892471#file1892471line3713> > > > > how come data values are changing here? So the same as above. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63711/#review191092 ------- On Nov. 15, 2017, 9:34 a.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63711/ > --- > > (Updated Nov. 15, 2017, 9:34 a.m.) > > > Review request for hive and Vihang Karajgaonkar. > > > Repository: hive-git > > > Description > --- > > Most of the vectorization related q-tests operate on ORC tables using Tez. It > would be good to add more coverage on a different combination of engine and > file-format. We can model existing q-tests using parquet tables and run it > using TestSparkCliDriver > > > Diffs > - > > data/scripts/q_test_cleanup.sql 4620dcd > data/scripts/q_test_init.sql f763c12 > itests/src/test/resources/testconfiguration.properties 1d16b65 > itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java f1d90ff > pom.xml dfb29ce > ql/src/test/queries/clientpositive/parquet_read_backward_compatible_files.q > 0abbc2f > ql/src/test/queries/clientpositive/parquet_vectorization_0.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_1.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_10.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_11.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_12.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_13.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_14.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_15.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_16.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_17.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_2.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_3.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_4.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_5.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_6.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_7.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_8.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_9.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_decimal_date.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_div0.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_limit.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_nested_udf.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_not.q PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_offset_limit.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_part.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_part_project.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_part_varchar.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_vectorization_pushdown.q > PRE-CREATION > ql/src/test/results/clientpositive/add_part_exist.q.out f8d50ca > ql/src/test/results/clientpositive/alter1.q.out c2efbe5 > ql/src/test/results/clientpositive/alter2.q.
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/clientpositive/spark/temp_table.q.out 342d08c ql/src/test/results/clientpositive/temp_table_truncate.q.out b1af432 Diff: https://reviews.apache.org/r/63711/diff/6/ Changes: https://reviews.apache.org/r/63711/diff/5-6/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/clientpositive/spark/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/temp_table.q.out 342d08c ql/src/test/results/clientpositive/temp_table_truncate.q.out b1af432 Diff: https://reviews.apache.org/r/63711/diff/5/ Changes: https://reviews.apache.org/r/63711/diff/4-5/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_1.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_10.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_11.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_12.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_13.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_14.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_15.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_16.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_17.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_2.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_3.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_4.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_5.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_6.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_7.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_8.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_9.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_decimal_date.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_div0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_nested_udf.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_pushdown.q.out PRE-CREATION Diff: https://reviews.apache.org/r/63711/diff/4/ Changes: https://reviews.apache.org/r/63711/diff/3-4/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_1.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_10.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_11.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_12.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_13.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_14.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_15.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_16.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_17.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_2.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_3.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_4.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_5.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_6.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_7.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_8.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_9.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_decimal_date.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_div0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_nested_udf.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_pushdown.q.out PRE-CREATION Diff: https://reviews.apache.org/r/63711/diff/3/ Changes: https://reviews.apache.org/r/63711/diff/2-3/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/parquet_vectorization_nested_udf.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/parquet_vectorization_short_regress.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_1.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_10.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_11.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_12.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_13.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_14.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_15.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_16.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_17.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_2.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_3.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_4.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_5.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_6.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_7.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_8.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_9.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_decimal_date.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_div0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_nested_udf.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_short_regress.q.out PRE-CREATION Diff: https://reviews.apache.org/r/63711/diff/2/ Changes: https://reviews.apache.org/r/63711/diff/1-2/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Review Request 63711: HIVE-17528 Add more q-tests for Hive-on-Spark with Parquet vectorized reader
/parquet_vectorization_decimal_date.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_div0.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_input_format_excludes.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_nested_udf.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_not.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_offset_limit.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_parquet_projection.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_project.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_part_varchar.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_pushdown.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/parquet_vectorization_short_regress.q.out PRE-CREATION Diff: https://reviews.apache.org/r/63711/diff/1/ Testing --- Qtest Added locally passed. Thanks, cheng xu
Re: Review Request 58501: HIVE-16469: Parquet timestamp table property is not always taken into account
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58501/#review174264 --- Ship it! Ship It! - cheng xu On May 4, 2017, 6:19 p.m., Barna Zsombor Klara wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58501/ > --- > > (Updated May 4, 2017, 6:19 p.m.) > > > Review request for hive, Sergio Pena and Zoltan Ivanfi. > > > Bugs: HIVE-16469 > https://issues.apache.org/jira/browse/HIVE-16469 > > > Repository: hive-git > > > Description > --- > > HIVE-16469: Parquet timestamp table property is not always taken into account > > > Diffs > - > > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java > 757b7fc0eaa39c956014aa446ab1b07fc4abf8d3 > ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java > 13750cdc34711d22f2adf2f483a6773ad05fb8d2 > ql/src/java/org/apache/hadoop/hive/ql/exec/StatsNoJobTask.java > 9c3a664b9aea2d6e050ffe2d7626127827dbc52a > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java > 1bd4db7805689ae1f91921ffbb5ff7da59f4bf60 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java > f4fadbb61bf45f62945700284c0b050f0984b696 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java > 2954601ce5bb25905cdb29ca0ca4551c2ca12b95 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java > 6413c5add6db2e8c9298285b15dba33ee74379a8 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java > b339cc4347eea143dca2f6d98f9aaafdc427 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java > dbd6fb3d0bc8c753abf86e99b52377617f248b5a > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java > c81499a91c84af3ba33f335506c1c44e7085f13d > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetRowGroupFilter.java > bf363f32a3ac0a4d790e2925d802c6e210adfb4b > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java > f2d79cf9d215e9a6e2a5e88cfc78378be860fd1f > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java > 1e10dbf18742524982606f1e6c6d447d683b2dc3 > ql/src/test/queries/clientnegative/parquet_int96_alter_invalid_timezone.q > PRE-CREATION > ql/src/test/queries/clientnegative/parquet_int96_create_invalid_timezone.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_int96_timestamp.q > 6eadd1b0a3313cbba7a798890b802baae302749e > > ql/src/test/results/clientnegative/parquet_int96_alter_invalid_timezone.q.out > PRE-CREATION > > ql/src/test/results/clientnegative/parquet_int96_create_invalid_timezone.q.out > PRE-CREATION > ql/src/test/results/clientpositive/parquet_int96_timestamp.q.out > b9a3664458a83f1856e4bc59eba5d56665df61cc > ql/src/test/results/clientpositive/spark/parquet_int96_timestamp.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58501/diff/5/ > > > Testing > --- > > Added qtests for the following cases: > - order by clause > - selfjoin > - calling UDFs with the timestamp values > - where clause with a constant cast as timestamp > - test for HoS > - implicit and explicit timestamp conversions in insert clause > > Tested manually but no qtests: > - join between 3 tables all parquet but with different/no timezone property > - subselect in from/where clauses > - exists / union / no exists > > > Thanks, > > Barna Zsombor Klara > >
Re: Review Request 58501: HIVE-16469: Parquet timestamp table property is not always taken into account
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58501/#review173857 --- ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java Lines 372 (patched) <https://reviews.apache.org/r/58501/#comment246903> Can we check the format type to see whether it's Parquet format? ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java Lines 101 (patched) <https://reviews.apache.org/r/58501/#comment246904> Can we rename this method like propagateParquetTimeZoneTablePorperty? Ususally method with prefix "check" should not have side-effect. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java Line 181 (original), 181 (patched) <https://reviews.apache.org/r/58501/#comment246902> Why not passing in the default value here when PARQUET_INT96_WRITE_ZONE_PROPERTY is not set? - cheng xu On May 3, 2017, 8:59 p.m., Barna Zsombor Klara wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58501/ > --- > > (Updated May 3, 2017, 8:59 p.m.) > > > Review request for hive, Sergio Pena and Zoltan Ivanfi. > > > Bugs: HIVE-16469 > https://issues.apache.org/jira/browse/HIVE-16469 > > > Repository: hive-git > > > Description > --- > > HIVE-16469: Parquet timestamp table property is not always taken into account > > > Diffs > - > > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java > 757b7fc0eaa39c956014aa446ab1b07fc4abf8d3 > ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java > 13750cdc34711d22f2adf2f483a6773ad05fb8d2 > ql/src/java/org/apache/hadoop/hive/ql/exec/StatsNoJobTask.java > 9c3a664b9aea2d6e050ffe2d7626127827dbc52a > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java > 1bd4db7805689ae1f91921ffbb5ff7da59f4bf60 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java > f4fadbb61bf45f62945700284c0b050f0984b696 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ParquetRecordReaderBase.java > 2954601ce5bb25905cdb29ca0ca4551c2ca12b95 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java > 6413c5add6db2e8c9298285b15dba33ee74379a8 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetTableUtils.java > b339cc4347eea143dca2f6d98f9aaafdc427 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/timestamp/NanoTimeUtils.java > dbd6fb3d0bc8c753abf86e99b52377617f248b5a > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java > c81499a91c84af3ba33f335506c1c44e7085f13d > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetRowGroupFilter.java > bf363f32a3ac0a4d790e2925d802c6e210adfb4b > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/VectorizedColumnReaderTestBase.java > f2d79cf9d215e9a6e2a5e88cfc78378be860fd1f > > ql/src/test/org/apache/hadoop/hive/ql/io/parquet/timestamp/TestNanoTimeUtils.java > 1e10dbf18742524982606f1e6c6d447d683b2dc3 > ql/src/test/queries/clientnegative/parquet_int96_alter_invalid_timezone.q > PRE-CREATION > ql/src/test/queries/clientnegative/parquet_int96_create_invalid_timezone.q > PRE-CREATION > ql/src/test/queries/clientpositive/parquet_int96_timestamp.q > 6eadd1b0a3313cbba7a798890b802baae302749e > > ql/src/test/results/clientnegative/parquet_int96_alter_invalid_timezone.q.out > PRE-CREATION > > ql/src/test/results/clientnegative/parquet_int96_create_invalid_timezone.q.out > PRE-CREATION > ql/src/test/results/clientpositive/parquet_int96_timestamp.q.out > b9a3664458a83f1856e4bc59eba5d56665df61cc > ql/src/test/results/clientpositive/spark/parquet_int96_timestamp.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58501/diff/4/ > > > Testing > --- > > Added qtests for the following cases: > - order by clause > - selfjoin > - calling UDFs with the timestamp values > - where clause with a constant cast as timestamp > - test for HoS > - implicit and explicit timestamp conversions in insert clause > > Tested manually but no qtests: > - join between 3 tables all parquet but with different/no timezone property > - subselect in from/where clauses > - exists / union / no exists > > > Thanks, > > Barna Zsombor Klara > >
Re: Review Request 52493: HIVE-13589 : beeline support prompt for password with '-p' option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52493/#review153648 --- Ship it! Ship It! - cheng xu On Oct. 22, 2016, 1:31 a.m., Vihang Karajgaonkar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52493/ > --- > > (Updated Oct. 22, 2016, 1:31 a.m.) > > > Review request for hive, cheng xu and Sergio Pena. > > > Bugs: HIVE-13589 > https://issues.apache.org/jira/browse/HIVE-13589 > > > Repository: hive-git > > > Description > --- > > HIVE-13589 : beeline support prompt for password with '-p' option > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java > fdbe0a2c6c37688ab511710bd43517908996f158 > beeline/src/java/org/apache/hive/beeline/Commands.java > 6c3e7f70c811ad81aa8f3ee2ec2e42d98da7d330 > > itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/52493/diff/ > > > Testing > --- > > > Thanks, > > Vihang Karajgaonkar > >
Re: Review Request 52493: HIVE-13589 : beeline support prompt for password with '-p' option
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52493/#review153482
---
Thank you for updating the patch. LGTM overall and just leave some minor
comments.
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 680)
<https://reviews.apache.org/r/52493/#comment222834>
hint: perfer HIVE_VAR_PREFIX.equals(arg)
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 926)
<https://reviews.apache.org/r/52493/#comment222836>
2 space indent
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 946)
<https://reviews.apache.org/r/52493/#comment222838>
Let's make it simple as following:
return url.indexOf('/', "jdbc:hive2://".length() < 0;
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java
(line 48)
<https://reviews.apache.org/r/52493/#comment222840>
Please move it before Line 45.
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java
(line 69)
<https://reviews.apache.org/r/52493/#comment222841>
hint: better to call "testPromptPasswordOptionAsFirst" in consist with the
later one.
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java
(line 243)
<https://reviews.apache.org/r/52493/#comment222839>
2 space indent
- cheng xu
On Oct. 21, 2016, 2:02 a.m., Vihang Karajgaonkar wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52493/
> ---
>
> (Updated Oct. 21, 2016, 2:02 a.m.)
>
>
> Review request for hive, cheng xu and Sergio Pena.
>
>
> Bugs: HIVE-13589
> https://issues.apache.org/jira/browse/HIVE-13589
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> HIVE-13589 : beeline support prompt for password with '-p' option
>
>
> Diffs
> -
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java
> fdbe0a2c6c37688ab511710bd43517908996f158
> beeline/src/java/org/apache/hive/beeline/Commands.java
> 6c3e7f70c811ad81aa8f3ee2ec2e42d98da7d330
>
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/52493/diff/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Vihang Karajgaonkar
>
>
Re: Review Request 52981: HIVE-14679: csv2/tsv2 output format disables quoting by default and it's difficult to enable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52981/#review153339 --- beeline/src/java/org/apache/hive/beeline/Commands.java (line 650) <https://reviews.apache.org/r/52981/#comment222631> No space needed after set. beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java (line 105) <https://reviews.apache.org/r/52981/#comment222634> Why default value is true? beeline/src/main/resources/BeeLine.properties (line 190) <https://reviews.apache.org/r/52981/#comment222635> If this option is not applicable for all outputformat, can you please make it clear what kind of outputformat the options can be applied? beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 48) <https://reviews.apache.org/r/52981/#comment222639> Please add a test like ""value"" beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 69) <https://reviews.apache.org/r/52981/#comment222642> AFAIK, this option doesn't affect csv/tsv/dsv. So you can see no change after enabling it. Please remove them if my understanding is correct. beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 115) <https://reviews.apache.org/r/52981/#comment222638> I am not sure why default value is true. If the default value is true, no need to set it again here. And we should test the case when quoting is disabled. beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 182) <https://reviews.apache.org/r/52981/#comment222641> hint: is that possible to reuse the code in TestBufferedRows? beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java (line 217) <https://reviews.apache.org/r/52981/#comment222637> Remove this line please. - cheng xu On Oct. 19, 2016, 7:55 p.m., Jianguo Tian wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52981/ > --- > > (Updated Oct. 19, 2016, 7:55 p.m.) > > > Review request for hive and cheng xu. > > > Bugs: HIVE-14679 > https://issues.apache.org/jira/browse/HIVE-14679 > > > Repository: hive-git > > > Description > --- > > 1. Quoting should be enabled by default for csv2, tsv2 and dsv. > 2. Disabling quoting should be possible using a beeline argument. > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 79922d2 > beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 57b9c46 > beeline/src/java/org/apache/hive/beeline/Commands.java 039e354 > beeline/src/java/org/apache/hive/beeline/SeparatedValuesOutputFormat.java > 66d9fd0 > beeline/src/main/resources/BeeLine.properties ad79c01 > beeline/src/test/org/apache/hive/beeline/TestOutputFormatWithQuotes.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/52981/diff/ > > > Testing > --- > > Test csv2,tsv2,dsv,csv and tsv output format with enabling double quotes. > > > Thanks, > > Jianguo Tian > >
Re: Review Request 52493: HIVE-13589 : beeline - support prompt for password with '-u' option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52493/#review151931 --- beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 861) <https://reviews.apache.org/r/52493/#comment220585> tailing space beeline/src/java/org/apache/hive/beeline/Commands.java (line 1040) <https://reviews.apache.org/r/52493/#comment220588> Checks -> Check beeline/src/java/org/apache/hive/beeline/Commands.java (line 1057) <https://reviews.apache.org/r/52493/#comment220586> This error msg is not user-friendly. Can you give some potential reason for such NPE like the comments do? beeline/src/java/org/apache/hive/beeline/Commands.java (line 1085) <https://reviews.apache.org/r/52493/#comment220589> Let's combine this "if" statement with the above one. beeline/src/java/org/apache/hive/beeline/Commands.java (line 1090) <https://reviews.apache.org/r/52493/#comment220587> Add space around "-" beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java (line 114) <https://reviews.apache.org/r/52493/#comment220595> Do we have to change the original format with "key=value;" pair? beeline/src/test/resources/hive-site.xml (line 48) <https://reviews.apache.org/r/52493/#comment220590> Do we need this change? itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java (line 59) <https://reviews.apache.org/r/52493/#comment220591> Why we need this configuration? itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java (line 254) <https://reviews.apache.org/r/52493/#comment220592> Please close this stream in finally block. itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java (line 262) <https://reviews.apache.org/r/52493/#comment220593> 2 space indents itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java (line 268) <https://reviews.apache.org/r/52493/#comment220594> 2 space indents - cheng xu On Oct. 4, 2016, 7:35 a.m., Vihang Karajgaonkar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52493/ > ------- > > (Updated Oct. 4, 2016, 7:35 a.m.) > > > Review request for hive, cheng xu and Sergio Pena. > > > Bugs: HIVE-13589 > https://issues.apache.org/jira/browse/HIVE-13589 > > > Repository: hive-git > > > Description > --- > > This change makes the -p option for beeline an option which has a > optionalArgument. This means that user can optionally provide a connection > url which has -p followed by no password and beeline will prompt for the > password on the console when it attempts the connection. There are no changes > to previous ways of making the connection. This way of connecting gives user > the ability to connect without logging the password in the command history or > showing up the password on the screen while connecting. > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java > 79922d267eaf13216e8a8e4826c050d334266579 > beeline/src/java/org/apache/hive/beeline/Commands.java > 039e3549209682b5ebdde937607b77c3ec094527 > beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java > a83f925df25be5801f885f4570fb8aaf5ab18c88 > beeline/src/test/resources/hive-site.xml > 5f310d68245275ac9dc24df45579784019eea332 > > itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeelinePasswordOption.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/52493/diff/ > > > Testing > --- > > > Thanks, > > Vihang Karajgaonkar > >
Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51695/#review150368 --- jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 100) <https://reviews.apache.org/r/51695/#comment218340> Since this import is not needed. jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 39) <https://reviews.apache.org/r/51695/#comment218339> Can we rename this class to TestJdbcDriver? - cheng xu On Sept. 23, 2016, 3:31 p.m., Jianguo Tian wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51695/ > --- > > (Updated Sept. 23, 2016, 3:31 p.m.) > > > Review request for hive and cheng xu. > > > Bugs: HIVE-5867 > https://issues.apache.org/jira/browse/HIVE-5867 > > > Repository: hive-git > > > Description > --- > > HIVE-5867: JDBC driver and beeline should support executing an initial SQL > script > > > Diffs > - > > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java > ad96a6466dd1aadab71fc261f55be4639dcbe2bf > jdbc/src/java/org/apache/hive/jdbc/Utils.java > 3161566994d6c6e01de9d88a6e87295684619ffa > jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java PRE-CREATION > > Diff: https://reviews.apache.org/r/51695/diff/ > > > Testing > --- > > TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() > method of HiveConnection.java. I test some positive cases and negative cases > to look that if these cases could be parse into SQL statement which could be > executed successfully. > > > Thanks, > > Jianguo Tian > >
Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51695/#review150122 --- jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 18) <https://reviews.apache.org/r/51695/#comment217997> Break line here please. jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 53) <https://reviews.apache.org/r/51695/#comment217991> This should a comment, right? e.g. //The cases above are some positive cases which could be executed. Here are some negative cases as below jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 67) <https://reviews.apache.org/r/51695/#comment217993> Can we do this one time setup in @BeforeClass method? jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 74) <https://reviews.apache.org/r/51695/#comment217996> You can change the expected variable as an array.And use the following to check equality: assertTrue(ArrayUtils.isEquals()) jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java (line 81) <https://reviews.apache.org/r/51695/#comment217992> Can we do this one time clean up in @AfterClass method? - cheng xu On Sept. 22, 2016, 2:49 p.m., Jianguo Tian wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51695/ > --- > > (Updated Sept. 22, 2016, 2:49 p.m.) > > > Review request for hive and cheng xu. > > > Bugs: HIVE-5867 > https://issues.apache.org/jira/browse/HIVE-5867 > > > Repository: hive-git > > > Description > --- > > HIVE-5867: JDBC driver and beeline should support executing an initial SQL > script > > > Diffs > - > > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java > ad96a6466dd1aadab71fc261f55be4639dcbe2bf > jdbc/src/java/org/apache/hive/jdbc/Utils.java > 3161566994d6c6e01de9d88a6e87295684619ffa > jdbc/src/test/org/apache/hive/jdbc/TestRunInitialSQL.java PRE-CREATION > > Diff: https://reviews.apache.org/r/51695/diff/ > > > Testing > --- > > TestRunInitialSQL.java is a JUnit test class which can test parseInitFile() > method of HiveConnection.java. I test some positive cases and negative cases > to look that if these cases could be parse into SQL statement which could be > executed successfully. > > > Thanks, > > Jianguo Tian > >
Re: Review Request 52111: HIVE-14388:Add number of rows inserted message after insert command in Beeline
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52111/#review149796 --- ql/src/java/org/apache/hadoop/hive/ql/Driver.java (line 1863) <https://reviews.apache.org/r/52111/#comment217512> Space needed around + ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java (line 379) <https://reviews.apache.org/r/52111/#comment217513> Remove it if no needed. service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TArrayTypeEntry.java (line 2) <https://reviews.apache.org/r/52111/#comment217514> Can you use Thrift 0.9.3 version? service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java (line 43) <https://reviews.apache.org/r/52111/#comment217515> Please use previous import which doesn't import unneeded package. service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java (line 628) <https://reviews.apache.org/r/52111/#comment217516> Remove debug info. For review, please don't include the generated files. :) I will review further it. - cheng xu On Sept. 21, 2016, 3:25 p.m., Ke Jia wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52111/ > --- > > (Updated Sept. 21, 2016, 3:25 p.m.) > > > Review request for hive and cheng xu. > > > Repository: hive-git > > > Description > --- > > Currently, when you run insert command on beeline, it returns a message > saying "No rows affected .." > A better and more intuitive msg would be "xxx rows inserted (26.068 seconds)" > > > Diffs > - > > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java a242501 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 42d398d > ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java > bb6ed84 > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 408c92e > service-rpc/if/TCLIService.thrift a4fa7b0 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TArrayTypeEntry.java > 358e322 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TBinaryColumn.java > a869cee > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TBoolColumn.java > 9bb6366 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TBoolValue.java > 87b3070 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TByteColumn.java > 68b3d3c > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TByteValue.java > a3d5951 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCLIService.java > 6dba051 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCLIServiceConstants.java > 930bed7 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenReq.java > a7d4e7d > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenResp.java > 611e92c > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelOperationReq.java > 4076c57 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelOperationResp.java > 7bcc765 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCloseOperationReq.java > 47a6b83 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCloseOperationResp.java > 0860a2b > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCloseSessionReq.java > 43ee87f > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCloseSessionResp.java > 38f82ac > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TColumn.java > 3c09c6b > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TColumnDesc.java > 31472c8 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TColumnValue.java > be99ce0 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TDoubleColumn.java > f93c9b4 > > service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TDoubleValue.java
Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51695/#review149793
---
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 237)
<https://reviews.apache.org/r/51695/#comment217505>
No need for "this."
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 250)
<https://reviews.apache.org/r/51695/#comment217501>
LOG.error
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 251)
<https://reviews.apache.org/r/51695/#comment217502>
Please throw SQLException.
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 256)
<https://reviews.apache.org/r/51695/#comment217511>
public method?
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 289)
<https://reviews.apache.org/r/51695/#comment217510>
private method?
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 295)
<https://reviews.apache.org/r/51695/#comment217509>
Please use if-else since you have only switch case.
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 747)
<https://reviews.apache.org/r/51695/#comment217503>
Do we need this method? It's private.
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 48)
<https://reviews.apache.org/r/51695/#comment217508>
Two space indents please.
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 53)
<https://reviews.apache.org/r/51695/#comment217506>
This is not a negative case.
"#negative cases:"->"#Some comments"
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 55)
<https://reviews.apache.org/r/51695/#comment217507>
Please add the case "#show tables; show/n tables"
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 75)
<https://reviews.apache.org/r/51695/#comment217504>
Assert.fail("Test was failed due to " + e);
- cheng xu
On Sept. 21, 2016, 11:47 a.m., Jianguo Tian wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> -------
>
> (Updated Sept. 21, 2016, 11:47 a.m.)
>
>
> Review request for hive and cheng xu.
>
>
> Bugs: HIVE-5867
> https://issues.apache.org/jira/browse/HIVE-5867
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> HIVE-5867: JDBC driver and beeline should support executing an initial SQL
> script
>
>
> Diffs
> -
>
> jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
> ad96a6466dd1aadab71fc261f55be4639dcbe2bf
> jdbc/src/java/org/apache/hive/jdbc/Utils.java
> 3161566994d6c6e01de9d88a6e87295684619ffa
> jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51695/diff/
>
>
> Testing
> ---
>
> TestInitSQL.java is JUnit test class which will test method initSql() in
> HiveConnection.java.
>
>
> Thanks,
>
> Jianguo Tian
>
>
Re: Review Request 48839: HIVE-14029: Update Spark version to 2.0.0
> On Sept. 21, 2016, 8:54 a.m., Szehon Ho wrote: > > This looks straight-forward and good to me (once 2.0.0 is the version in > > pom) Thanks Sezhon for your review. I have updated some versions required by Spark side. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48839/#review149771 --- On Sept. 21, 2016, 1:27 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48839/ > --- > > (Updated Sept. 21, 2016, 1:27 p.m.) > > > Review request for hive, Rui Li, Sergio Pena, Szehon Ho, and Xuefu Zhang. > > > Bugs: HIVE-14029 > https://issues.apache.org/jira/browse/HIVE-14029 > > > Repository: hive-git > > > Description > --- > > There are quite some new optimizations in Spark 2.0.0. We need to bump up > Spark to 2.0.0 to benefit those performance improvements. > > > Diffs > - > > itests/pom.xml a452db3 > pom.xml 2fb78cd > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java > 5b65036 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveMapFunction.java > 53c5c0e > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveReduceFunction.java > f6595f1 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SortByShuffler.java > a6350d3 > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/JobMetricsListener.java > 09c54c1 > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java > ee9f9b7 > spark-client/pom.xml 6cf3b17 > > spark-client/src/main/java/org/apache/hive/spark/client/MetricsCollection.java > e77aa78 > spark-client/src/main/java/org/apache/hive/spark/client/RemoteDriver.java > e3b88d1 > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/InputMetrics.java > e46b67d > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/Metrics.java > a7305cf > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleReadMetrics.java > be14c06 > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleWriteMetrics.java > 4420e4d > > spark-client/src/test/java/org/apache/hive/spark/client/TestMetricsCollection.java > 5146e91 > > Diff: https://reviews.apache.org/r/48839/diff/ > > > Testing > --- > > > Thanks, > > cheng xu > >
Re: Review Request 48839: HIVE-14029: Update Spark version to 2.0.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48839/ --- (Updated Sept. 21, 2016, 1:27 p.m.) Review request for hive, Rui Li, Sergio Pena, Szehon Ho, and Xuefu Zhang. Bugs: HIVE-14029 https://issues.apache.org/jira/browse/HIVE-14029 Repository: hive-git Description --- There are quite some new optimizations in Spark 2.0.0. We need to bump up Spark to 2.0.0 to benefit those performance improvements. Diffs (updated) - itests/pom.xml a452db3 pom.xml 2fb78cd ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java 5b65036 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveMapFunction.java 53c5c0e ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveReduceFunction.java f6595f1 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SortByShuffler.java a6350d3 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/JobMetricsListener.java 09c54c1 ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java ee9f9b7 spark-client/pom.xml 6cf3b17 spark-client/src/main/java/org/apache/hive/spark/client/MetricsCollection.java e77aa78 spark-client/src/main/java/org/apache/hive/spark/client/RemoteDriver.java e3b88d1 spark-client/src/main/java/org/apache/hive/spark/client/metrics/InputMetrics.java e46b67d spark-client/src/main/java/org/apache/hive/spark/client/metrics/Metrics.java a7305cf spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleReadMetrics.java be14c06 spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleWriteMetrics.java 4420e4d spark-client/src/test/java/org/apache/hive/spark/client/TestMetricsCollection.java 5146e91 Diff: https://reviews.apache.org/r/48839/diff/ Testing --- Thanks, cheng xu
Re: Review Request 48839: HIVE-14029: Update Spark version to 2.0.0
> On Sept. 21, 2016, 3:44 a.m., Sahil Takiar wrote: > > pom.xml, line 179 > > <https://reviews.apache.org/r/48839/diff/1/?file=1422055#file1422055line179> > > > > Can this be changed to `2.0.0` instead of `2.0.0-preview` > > Sahil Takiar wrote: > Looked at your updated patch, seems like you already did this. I forgot to update the review board entry. Reattach file to update it. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48839/#review149717 ------- On Sept. 21, 2016, 1:27 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48839/ > --- > > (Updated Sept. 21, 2016, 1:27 p.m.) > > > Review request for hive, Rui Li, Sergio Pena, Szehon Ho, and Xuefu Zhang. > > > Bugs: HIVE-14029 > https://issues.apache.org/jira/browse/HIVE-14029 > > > Repository: hive-git > > > Description > --- > > There are quite some new optimizations in Spark 2.0.0. We need to bump up > Spark to 2.0.0 to benefit those performance improvements. > > > Diffs > - > > itests/pom.xml a452db3 > pom.xml 2fb78cd > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java > 5b65036 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveMapFunction.java > 53c5c0e > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveReduceFunction.java > f6595f1 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SortByShuffler.java > a6350d3 > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/JobMetricsListener.java > 09c54c1 > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java > ee9f9b7 > spark-client/pom.xml 6cf3b17 > > spark-client/src/main/java/org/apache/hive/spark/client/MetricsCollection.java > e77aa78 > spark-client/src/main/java/org/apache/hive/spark/client/RemoteDriver.java > e3b88d1 > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/InputMetrics.java > e46b67d > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/Metrics.java > a7305cf > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleReadMetrics.java > be14c06 > > spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleWriteMetrics.java > 4420e4d > > spark-client/src/test/java/org/apache/hive/spark/client/TestMetricsCollection.java > 5146e91 > > Diff: https://reviews.apache.org/r/48839/diff/ > > > Testing > --- > > > Thanks, > > cheng xu > >
Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51695/#review148640
---
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 134)
<https://reviews.apache.org/r/51695/#comment216142>
Can you move the getter and setter after setupLoginTimeout method?
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 162)
<https://reviews.apache.org/r/51695/#comment216124>
Add a new line before this line.
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 183)
<https://reviews.apache.org/r/51695/#comment216145>
We should support it in embeded mode as well.
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 202)
<https://reviews.apache.org/r/51695/#comment216127>
remove "this."
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 252)
<https://reviews.apache.org/r/51695/#comment216125>
Use log
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 260)
<https://reviews.apache.org/r/51695/#comment216137>
Will this cover the following case?
show tables; show
tables;
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 273)
<https://reviews.apache.org/r/51695/#comment216131>
Please throw the original exception.
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 275)
<https://reviews.apache.org/r/51695/#comment216128>
if (br != null){
br.close();
}
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 1)
<https://reviews.apache.org/r/51695/#comment216126>
Please add license header.
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 16)
<https://reviews.apache.org/r/51695/#comment216129>
Why do you need mock?
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 18)
<https://reviews.apache.org/r/51695/#comment216140>
Can you add some negative cases?
e.g.
# show tables; show
tables
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 24)
<https://reviews.apache.org/r/51695/#comment216135>
L24-L29 is test case related and we should not add them in the setup method.
jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java (line 37)
<https://reviews.apache.org/r/51695/#comment216136>
Please remove L37-38 which is debug used only.
jdbc/src/test/resources/init.sql (line 1)
<https://reviews.apache.org/r/51695/#comment216147>
Please use echo to create some tmp file and remove them after test case
completed since we need to add many more cases like negative cases.
- cheng xu
On Sept. 13, 2016, 10:36 a.m., Jianguo Tian wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> -------
>
> (Updated Sept. 13, 2016, 10:36 a.m.)
>
>
> Review request for hive and cheng xu.
>
>
> Bugs: HIVE-5867
> https://issues.apache.org/jira/browse/HIVE-5867
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> HIVE-5867: JDBC driver and beeline should support executing an initial SQL
> script
>
>
> Diffs
> -
>
> jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
> ad96a6466dd1aadab71fc261f55be4639dcbe2bf
> jdbc/src/java/org/apache/hive/jdbc/Utils.java
> 3161566994d6c6e01de9d88a6e87295684619ffa
> jdbc/src/test/org/apache/hive/jdbc/TestInitSQL.java PRE-CREATION
> jdbc/src/test/resources/init.sql PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51695/diff/
>
>
> Testing
> ---
>
> TestInitSQL.java is JUnit test class which will test method initSql() in
> HiveConnection.java.
>
>
> Thanks,
>
> Jianguo Tian
>
>
Re: Review Request 51695: HIVE-5867: JDBC driver and beeline should support executing an initial SQL script
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51695/#review148124
---
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 128)
<https://reviews.apache.org/r/51695/#comment215548>
Extra Space
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 150)
<https://reviews.apache.org/r/51695/#comment215549>
Extra Space
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java (line 160)
<https://reviews.apache.org/r/51695/#comment215550>
Extra Space
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 116)
<https://reviews.apache.org/r/51695/#comment215552>
2 space indent from L116 to L166
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 127)
<https://reviews.apache.org/r/51695/#comment215557>
Will this handle the following case?
{noformat}
show
tables;
{noformat}
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 136)
<https://reviews.apache.org/r/51695/#comment215564>
Can we move it to the finally block?
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 147)
<https://reviews.apache.org/r/51695/#comment215559>
LOG.error
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 154)
<https://reviews.apache.org/r/51695/#comment215558>
Remove this line.
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 156)
<https://reviews.apache.org/r/51695/#comment215553>
Extra space
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 159)
<https://reviews.apache.org/r/51695/#comment215554>
Extra space
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 161)
<https://reviews.apache.org/r/51695/#comment21>
Extra space
jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java (line 167)
<https://reviews.apache.org/r/51695/#comment215560>
No ident needed here.
jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java (line 14)
<https://reviews.apache.org/r/51695/#comment215562>
Can this test pass without starting any server?
jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java (line 19)
<https://reviews.apache.org/r/51695/#comment215561>
Extra space.
jdbc/src/test/resources/init.sql (line 3)
<https://reviews.apache.org/r/51695/#comment215563>
Remove this line since it's commented out.
- cheng xu
On Sept. 8, 2016, 8:52 a.m., Jianguo Tian wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51695/
> -------
>
> (Updated Sept. 8, 2016, 8:52 a.m.)
>
>
> Review request for hive and cheng xu.
>
>
> Bugs: HIVE-5867
> https://issues.apache.org/jira/browse/HIVE-5867
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> HIVE-5867: JDBC driver and beeline should support executing an initial SQL
> script
>
>
> Diffs
> -
>
> jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
> ad96a6466dd1aadab71fc261f55be4639dcbe2bf
> jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java
> a349f8bdad09f526f66a71768ecd2ca019671167
> jdbc/src/java/org/apache/hive/jdbc/Utils.java
> 3161566994d6c6e01de9d88a6e87295684619ffa
> jdbc/src/test/org/apache/hive/jdbc/TestHiveDriver.java PRE-CREATION
> jdbc/src/test/resources/init.sql PRE-CREATION
>
> Diff: https://reviews.apache.org/r/51695/diff/
>
>
> Testing
> ---
>
> TestHiveDriver.java is JUnit test class which will print the rusult of
> executing initial SQL(init.sql locate in src/test/resources).
>
>
> Thanks,
>
> Jianguo Tian
>
>
Re: Review Request 48716: HIVE-13873 Column pruning for nested fields
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48716/ --- (Updated July 8, 2016, 9:40 a.m.) Review request for hive and Xuefu Zhang. Changes --- Remove unnecessary changes Bugs: HIVE-13873 https://issues.apache.org/jira/browse/HIVE-13873 Repository: hive-git Description --- Add group projection support for Parquet and this is the initial patch sharing my thoughts. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java dff1815 ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java 57b6c67 ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 6afe957 ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java 23a13d6 ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 227a051 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java db923fa ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java a89aa4d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3e38cc7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java 611a6b7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java a2a7f00 ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 8cf261d ql/src/test/queries/clientpositive/parquet_nested_field_pruning.q PRE-CREATION ql/src/test/results/clientpositive/parquet_nested_field_pruning.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java 0c7ac30 Diff: https://reviews.apache.org/r/48716/diff/ Testing --- Newly added qtest passed. Thanks, cheng xu
Re: Review Request 48716: HIVE-13873 Column pruning for nested fields
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48716/ --- (Updated July 8, 2016, 9:35 a.m.) Review request for hive and Xuefu Zhang. Changes --- Changes include: 1. skip isList cases and add TODO for the next step 2. add more tests 3. address some comments from Aihua Bugs: HIVE-13873 https://issues.apache.org/jira/browse/HIVE-13873 Repository: hive-git Description --- Add group projection support for Parquet and this is the initial patch sharing my thoughts. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java dff1815 ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java 57b6c67 ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 6afe957 ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java 23a13d6 ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 227a051 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java db923fa ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java a89aa4d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3e38cc7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java 611a6b7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java a2a7f00 ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 8cf261d ql/src/test/queries/clientpositive/parquet_nested_field_pruning.q PRE-CREATION ql/src/test/results/clientpositive/parquet_nested_field_pruning.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java 0c7ac30 Diff: https://reviews.apache.org/r/48716/diff/ Testing --- Newly added qtest passed. Thanks, cheng xu
Re: Review Request 48716: HIVE-13873 Column pruning for nested fields
> On July 6, 2016, 10:48 p.m., Aihua Xu wrote: > > serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java, > > line 122 > > <https://reviews.apache.org/r/48716/diff/1/?file=1419370#file1419370line122> > > > > Just try to understand the logic (not too familiar with Parquet). So > > the underneath parquet already supports "hive.io.file.readgroup.paths" or > > this is totally within hive? How are the struct data stored in parquet and > > pruned with the group path in general? Parquet doesn't support this configuration. We reconstruct the requested schema in Hive side by pruning unneeded columns like other projection does. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48716/#review140991 ------- On June 15, 2016, 11:34 a.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48716/ > --- > > (Updated June 15, 2016, 11:34 a.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-13873 > https://issues.apache.org/jira/browse/HIVE-13873 > > > Repository: hive-git > > > Description > --- > > Add group projection support for Parquet and this is the initial patch > sharing my thoughts. > > > Diffs > - > > ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java dff1815 > ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java 23abec3 > ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 6afe957 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java 24bf506 > ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java cfedf35 > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java > db923fa > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java > a89aa4d > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java > 3e38cc7 > > ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java > 74a1a82 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java > 611a6b7 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java > a2a7f00 > ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 8cf261d > ql/src/test/queries/clientpositive/parquet_struct.q PRE-CREATION > ql/src/test/results/clientpositive/parquet_struct.q.out PRE-CREATION > serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java > 0c7ac30 > > Diff: https://reviews.apache.org/r/48716/diff/ > > > Testing > --- > > Newly added qtest passed. > > > Thanks, > > cheng xu > >
Re: Review Request 49264: HIVE-14037: java.lang.ClassNotFoundException for the jar in hive.reloadable.aux.jars.path in mapreduce
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49264/#review139907 --- Ship it! Ship It! - cheng xu On June 29, 2016, 1:19 a.m., Aihua Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49264/ > --- > > (Updated June 29, 2016, 1:19 a.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > --- > > HIVE-14037: java.lang.ClassNotFoundException for the jar in > hive.reloadable.aux.jars.path in mapreduce > > > Diffs > - > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1d1306f > common/src/java/org/apache/hive/common/util/HiveStringUtils.java bba14e2 > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 528d663 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java 8a6499b > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java a42c2e9 > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 96c826b > ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java bb6a4e1 > ql/src/test/queries/clientpositive/reloadJar.q PRE-CREATION > ql/src/test/results/clientpositive/reloadJar.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/49264/diff/ > > > Testing > --- > > > Thanks, > > Aihua Xu > >
Re: Review Request 49264: HIVE-14037: java.lang.ClassNotFoundException for the jar in hive.reloadable.aux.jars.path in mapreduce
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49264/#review139712 --- Thanks Aihua for your patch. Left some comments here. ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java (lines 1741 - 1745) <https://reviews.apache.org/r/49264/#comment205041> Please change the indent to 4 space. ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java (line 153) <https://reviews.apache.org/r/49264/#comment205049> Maybe we should put the `reloadableAuxJars` in the first place so that it will override the existing addedJars if one jar is added alread in addedJars. If we don't allow users to override the addedJars or auxJars, we could describe this limitation in the configuration. Any thoughts for this? ql/src/test/queries/clientpositive/reloadJar.q (line 9) <https://reviews.apache.org/r/49264/#comment205044> Tailing space. ql/src/test/queries/clientpositive/reloadJar.q (line 12) <https://reviews.apache.org/r/49264/#comment205045> Tailing space. - cheng xu On June 27, 2016, 9:09 p.m., Aihua Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49264/ > --- > > (Updated June 27, 2016, 9:09 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > --- > > HIVE-14037: java.lang.ClassNotFoundException for the jar in > hive.reloadable.aux.jars.path in mapreduce > > > Diffs > - > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 1d1306ff6395a0504085dda98e96c3951519f299 > common/src/java/org/apache/hive/common/util/HiveStringUtils.java > bba14e21696af93dbee78d5073d130d09ec4e81c > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java > 528d663e701eaa13e2cd7da328973637457ff642 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java > 8a6499b3a42273b729991540ca7a07f6d5cd73e2 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java > a42c2e99fe62aac96c21e4c5590c1891c1ea529c > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java > 96c826b6fb245bf7286fad7d1275268ceac4dd78 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java > bb6a4e1e1ea63d48556dd88bd6e02f80a0adb261 > ql/src/test/queries/clientpositive/reloadJar.q PRE-CREATION > ql/src/test/results/clientpositive/reloadJar.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/49264/diff/ > > > Testing > --- > > > Thanks, > > Aihua Xu > >
Review Request 48839: HIVE-14029: Update Spark version to 2.0.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48839/ --- Review request for hive, Rui Li, Szehon Ho, and Xuefu Zhang. Bugs: HIVE-14029 https://issues.apache.org/jira/browse/HIVE-14029 Repository: hive-git Description --- There are quite some new optimizations in Spark 2.0.0. We need to bump up Spark to 2.0.0 to benefit those performance improvements. Diffs - pom.xml 63a5ae1 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java 5b65036 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveMapFunction.java 53c5c0e ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveReduceFunction.java f6595f1 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SortByShuffler.java a6350d3 ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/impl/JobMetricsListener.java 09c54c1 ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 4b34ebf ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java ee9f9b7 spark-client/src/main/java/org/apache/hive/spark/client/MetricsCollection.java e77aa78 spark-client/src/main/java/org/apache/hive/spark/client/RemoteDriver.java e3b88d1 spark-client/src/main/java/org/apache/hive/spark/client/metrics/InputMetrics.java e46b67d spark-client/src/main/java/org/apache/hive/spark/client/metrics/Metrics.java a7305cf spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleReadMetrics.java be14c06 spark-client/src/main/java/org/apache/hive/spark/client/metrics/ShuffleWriteMetrics.java 4420e4d spark-client/src/test/java/org/apache/hive/spark/client/TestMetricsCollection.java 5146e91 Diff: https://reviews.apache.org/r/48839/diff/ Testing --- Thanks, cheng xu
Re: Review Request 48716: HIVE-13873 Column pruning for nested fields
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48716/ --- (Updated June 15, 2016, 11:34 a.m.) Review request for hive and Xuefu Zhang. Summary (updated) - HIVE-13873 Column pruning for nested fields Bugs: HIVE-13873 https://issues.apache.org/jira/browse/HIVE-13873 Repository: hive-git Description --- Add group projection support for Parquet and this is the initial patch sharing my thoughts. Diffs - ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java dff1815 ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java 23abec3 ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 6afe957 ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java 24bf506 ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java cfedf35 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java db923fa ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java a89aa4d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3e38cc7 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 74a1a82 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java 611a6b7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java a2a7f00 ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 8cf261d ql/src/test/queries/clientpositive/parquet_struct.q PRE-CREATION ql/src/test/results/clientpositive/parquet_struct.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java 0c7ac30 Diff: https://reviews.apache.org/r/48716/diff/ Testing --- Newly added qtest passed. Thanks, cheng xu
Review Request 48716: HIVE-13873
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48716/ --- Review request for hive and Xuefu Zhang. Bugs: HIVE-13873 https://issues.apache.org/jira/browse/HIVE-13873 Repository: hive-git Description --- Add group projection support for Parquet and this is the initial patch sharing my thoughts. Diffs - ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java dff1815 ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java 23abec3 ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 6afe957 ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java 24bf506 ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java cfedf35 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/ProjectionPusher.java db923fa ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveStructConverter.java a89aa4d ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/DataWritableReadSupport.java 3e38cc7 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 74a1a82 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcCtx.java 611a6b7 ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java a2a7f00 ql/src/java/org/apache/hadoop/hive/ql/plan/TableScanDesc.java 8cf261d ql/src/test/queries/clientpositive/parquet_struct.q PRE-CREATION ql/src/test/results/clientpositive/parquet_struct.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/ColumnProjectionUtils.java 0c7ac30 Diff: https://reviews.apache.org/r/48716/diff/ Testing --- Newly added qtest passed. Thanks, cheng xu
[jira] [Created] (HIVE-13056) oozie-hive fails to authenticate
Cheng Xu created HIVE-13056:
---
Summary: oozie-hive fails to authenticate
Key: HIVE-13056
URL: https://issues.apache.org/jira/browse/HIVE-13056
Project: Hive
Issue Type: Bug
Components: Authentication
Affects Versions: 1.2.1
Reporter: Cheng Xu
Assignee: Sushanth Sowmyan
Priority: Critical
Fix For: 1.2.1
We're getting a HiveSQLException on secure windows clusters.
{code}
2016-02-08 13:48:09,535|beaver.machine|INFO|6114|140264674350912|MainThread|Job
ID : 000-160208134528402-oozie-oozi-W
2016-02-08
13:48:09,536|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08
13:48:09,536|beaver.machine|INFO|6114|140264674350912|MainThread|Workflow Name
: hive2-wf
2016-02-08 13:48:09,536|beaver.machine|INFO|6114|140264674350912|MainThread|App
Path :
wasb://oozie1-hb...@humbtestings5jp.blob.core.windows.net/user/hrt_qa/test_hiveserver2
2016-02-08
13:48:09,536|beaver.machine|INFO|6114|140264674350912|MainThread|Status
: KILLED
2016-02-08 13:48:09,537|beaver.machine|INFO|6114|140264674350912|MainThread|Run
: 0
2016-02-08
13:48:09,537|beaver.machine|INFO|6114|140264674350912|MainThread|User
: hrt_qa
2016-02-08
13:48:09,537|beaver.machine|INFO|6114|140264674350912|MainThread|Group
: -
2016-02-08
13:48:09,547|beaver.machine|INFO|6114|140264674350912|MainThread|Created
: 2016-02-08 13:47 GMT
2016-02-08
13:48:09,548|beaver.machine|INFO|6114|140264674350912|MainThread|Started
: 2016-02-08 13:47 GMT
2016-02-08
13:48:09,552|beaver.machine|INFO|6114|140264674350912|MainThread|Last Modified
: 2016-02-08 13:48 GMT
2016-02-08
13:48:09,553|beaver.machine|INFO|6114|140264674350912|MainThread|Ended
: 2016-02-08 13:48 GMT
2016-02-08
13:48:09,553|beaver.machine|INFO|6114|140264674350912|MainThread|CoordAction
ID: -
2016-02-08 13:48:09,566|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08
13:48:09,566|beaver.machine|INFO|6114|140264674350912|MainThread|Actions
2016-02-08
13:48:09,567|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08 13:48:09,567|beaver.machine|INFO|6114|140264674350912|MainThread|ID
StatusExt ID Ext Status Err Code
2016-02-08
13:48:09,567|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08
13:48:09,571|beaver.machine|INFO|6114|140264674350912|MainThread|000-160208134528402-oozie-oozi-W@:start:
OK- OK -
2016-02-08
13:48:09,572|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08
13:48:09,572|beaver.machine|INFO|6114|140264674350912|MainThread|000-160208134528402-oozie-oozi-W@hive-node
ERROR - ERROR
HiveSQLException
2016-02-08
13:48:09,572|beaver.machine|INFO|6114|140264674350912|MainThread|
2016-02-08
13:48:09,572|beaver.machine|INFO|6114|140264674350912|MainThread|000-160208134528402-oozie-oozi-W@fail
OK- OK
E0729
2016-02-08
13:48:09,572|beaver.machine|INFO|6114|140264674350912|MainThread|
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
Re: Review Request 39626: HIVE-12259: Command containing semicolon is broken in Beeline
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39626/#review103971
---
Thank you for working on this. I have a comment left.
beeline/src/java/org/apache/hive/beeline/BeeLine.java (line )
<https://reviews.apache.org/r/39626/#comment162086>
Method `exeCommandWithPrefix` failed to handle the commands like `!sh
test.sql;show\n tables;`. For connect command, it's possible to contain
semicolon in the connection string. So I am thinking about whether we can
escape connect command. How about the following code?
```
if
(!line.startsWith(COMMAND_PREFIX)&(";")&&!line.startWith("!connect"))
{
return commands.sql(line, getOpts().getEntireLineAsCommand());
} else {
// handle SQLLine command in beeline which starts with ! and does
not end with ;
return execCommandWithPrefix(line);
}
```
- cheng xu
On Oct. 25, 2015, 7:13 a.m., Chaoyu Tang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39626/
> ---
>
> (Updated Oct. 25, 2015, 7:13 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-12259
> https://issues.apache.org/jira/browse/HIVE-12259
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> The SQLLine commands (!cmd) and Connection URLs are broken if they contain
> the ";"
>
>
> Diffs
> -
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 69e9418
>
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java
> 0465ef3
>
> Diff: https://reviews.apache.org/r/39626/diff/
>
>
> Testing
> ---
>
> New unit tests passed
> Submitted precommit tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 39626: HIVE-12259: Command containing semicolon is broken in Beeline
> On Oct. 26, 2015, 9:14 a.m., cheng xu wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line
> > <https://reviews.apache.org/r/39626/diff/2/?file=1106510#file1106510line>
> >
> > Method `exeCommandWithPrefix` failed to handle the commands like `!sh
> > test.sql;show\n tables;`. For connect command, it's possible to contain
> > semicolon in the connection string. So I am thinking about whether we can
> > escape connect command. How about the following code?
> > ```
> > if
> > (!line.startsWith(COMMAND_PREFIX)&(";")&&!line.startWith("!connect"))
> > {
> > return commands.sql(line, getOpts().getEntireLineAsCommand());
> > } else {
> > // handle SQLLine command in beeline which starts with ! and
> > does not end with ;
> > return execCommandWithPrefix(line);
> > }
> > ```
>
> Chaoyu Tang wrote:
> A subset of BeeLine commands are actually SQLLine commands (see
> https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Clients). They
> include !connect which could be found in http://sqlline.sourceforge.net/.
> These commands are quite special since they start with ! and do NOT have to
> terminate with ;
> So for the approach you suggested, for the rest SQLLine commands, I think
> they will be prompted for more inputs till the line terminator ; is
> encountered (multi-lines command)
> As for the case you provided, both !sh and show are not SQLLine commands.
>
> So I wonder if it will make more sense and be clearer if we document that
> the SQLLine commands could not be used for multi-commands-in-one-line or
> mulit-lines command. Otherwise, combined with multi-lines command support, we
> will run into many backward issues.
LGTM. +1
- cheng
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39626/#review103971
---
On Oct. 25, 2015, 7:13 a.m., Chaoyu Tang wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39626/
> ---
>
> (Updated Oct. 25, 2015, 7:13 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-12259
> https://issues.apache.org/jira/browse/HIVE-12259
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> The SQLLine commands (!cmd) and Connection URLs are broken if they contain
> the ";"
>
>
> Diffs
> -
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 69e9418
>
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java
> 0465ef3
>
> Diff: https://reviews.apache.org/r/39626/diff/
>
>
> Testing
> ---
>
> New unit tests passed
> Submitted precommit tests
>
>
> Thanks,
>
> Chaoyu Tang
>
>
Re: Review Request 39626: HIVE-12259: Command containing semicolon is broken in Beeline
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39626/#review103978 --- Ship it! Ship It! - cheng xu On Oct. 25, 2015, 7:13 a.m., Chaoyu Tang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39626/ > --- > > (Updated Oct. 25, 2015, 7:13 a.m.) > > > Review request for hive. > > > Bugs: HIVE-12259 > https://issues.apache.org/jira/browse/HIVE-12259 > > > Repository: hive-git > > > Description > --- > > The SQLLine commands (!cmd) and Connection URLs are broken if they contain > the ";" > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 69e9418 > > itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java > 0465ef3 > > Diff: https://reviews.apache.org/r/39626/diff/ > > > Testing > --- > > New unit tests passed > Submitted precommit tests > > > Thanks, > > Chaoyu Tang > >
Re: Review Request 38247: HIVE-11778 Merge beeline-cli branch to trunk
> On Sept. 12, 2015, 6:46 a.m., Xuefu Zhang wrote: > > beeline/src/java/org/apache/hive/beeline/Commands.java, line 823 > > <https://reviews.apache.org/r/38247/diff/1/?file=1066968#file1066968line823> > > > > This seems weird as the method isn't new but shown as new here. Yes, it's quite strange why it was marked as new added lines. This code snippet is borrowed from old Hive CLI. > On Sept. 12, 2015, 6:46 a.m., Xuefu Zhang wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1090 > > <https://reviews.apache.org/r/38247/diff/1/?file=1066964#file1066964line1090> > > > > Again I have trouble understanding this: if cmdMap.size() > 1, then > > there must be at lease one match, right? Then, how cmdMap.get(line) can > > return null, as suggested in line 1088? Further, how can handle == null > > suggest multiple matches? I know this is old code, but let's clean it up if > > necessary. This code of block is very tricky. Assume there are two commands "connect" "myconnect". After going through the match block, there are two entries in the map. <<"connect", connectHander>,<"myconnect", myconnectHander>>. The code will allow user to execute the "connect" methods even through there are two matches since it's an exact match. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38247/#review98694 --- On Sept. 10, 2015, 11:35 a.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38247/ > --- > > (Updated Sept. 10, 2015, 11:35 a.m.) > > > Review request for hive and Xuefu Zhang. > > > Repository: hive-git > > > Description > --- > > The changes we made in beeline-cli branch > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 3cd2a8b > beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 3388391 > beeline/src/java/org/apache/hive/beeline/ClientCommandHookFactory.java > PRE-CREATION > beeline/src/java/org/apache/hive/beeline/ClientHook.java PRE-CREATION > beeline/src/java/org/apache/hive/beeline/Commands.java 3cdcfb8 > beeline/src/java/org/apache/hive/beeline/cli/CliOptionsProcessor.java > PRE-CREATION > beeline/src/java/org/apache/hive/beeline/cli/HiveCli.java PRE-CREATION > beeline/src/test/org/apache/hive/beeline/TestClientCommandHookFactory.java > PRE-CREATION > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java PRE-CREATION > beeline/src/test/resources/hive-site.xml PRE-CREATION > bin/ext/cli.sh 914aae3 > bin/ext/util/execHiveCmd.sh 167cc40 > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c > common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java > PRE-CREATION > common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java > PRE-CREATION > common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4030075 > > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java > 8b7a2e8 > ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java > e8b1d96 > ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java > 0558c53 > ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java > 25ce168 > > ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java > 9052c82 > ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d > ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 2414e12 > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 7ed8e5f > > service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java > bcc66cf > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > cc9df76 > > Diff: https://reviews.apache.org/r/38247/diff/ > > > Testing > --- > > UT and smoke test passed locally > > > Thanks, > > cheng xu > >
Re: Review Request 38247: HIVE-11778 Merge beeline-cli branch to trunk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38247/ --- (Updated Sept. 25, 2015, 9:43 a.m.) Review request for hive and Xuefu Zhang. Changes --- Update patch with the latest beeline-cli branch Repository: hive-git Description --- The changes we made in beeline-cli branch Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 3cd2a8b beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 3388391 beeline/src/java/org/apache/hive/beeline/ClientCommandHookFactory.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/ClientHook.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/Commands.java 3cdcfb8 beeline/src/java/org/apache/hive/beeline/cli/CliOptionsProcessor.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/cli/HiveCli.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/TestClientCommandHookFactory.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java PRE-CREATION beeline/src/test/resources/hive-site.xml PRE-CREATION bin/beeline 2fb05bc bin/ext/cli.cmd 9e85691 bin/ext/cli.sh 914aae3 bin/ext/util/execHiveCmd.sh 167cc40 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 938cf65 ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataPrettyFormatUtils.java 8f939e6 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 8b7a2e8 ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 2414e12 ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 014941e ql/src/test/results/clientpositive/describe_pretty.q.out 1c05e0d service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java bcc66cf service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java cc9df76 Diff: https://reviews.apache.org/r/38247/diff/ Testing --- UT and smoke test passed locally Thanks, cheng xu
Review Request 38247: HIVE-11778 Merge beeline-cli branch to trunk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38247/ --- Review request for hive and Xuefu Zhang. Repository: hive-git Description --- The changes we made in beeline-cli branch Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 3cd2a8b beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 3388391 beeline/src/java/org/apache/hive/beeline/ClientCommandHookFactory.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/ClientHook.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/Commands.java 3cdcfb8 beeline/src/java/org/apache/hive/beeline/cli/CliOptionsProcessor.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/cli/HiveCli.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/TestClientCommandHookFactory.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java PRE-CREATION beeline/src/test/resources/hive-site.xml PRE-CREATION bin/ext/cli.sh 914aae3 bin/ext/util/execHiveCmd.sh 167cc40 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4030075 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 8b7a2e8 ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 2414e12 ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 7ed8e5f service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java bcc66cf service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java cc9df76 Diff: https://reviews.apache.org/r/38247/diff/ Testing --- UT and smoke test passed locally Thanks, cheng xu
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for new CLI[Beeline-cli branch]
> On Sept. 7, 2015, 9:33 p.m., Xuefu Zhang wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1094
> > <https://reviews.apache.org/r/38044/diff/1/?file=1062103#file1062103line1094>
> >
> > I meant if it's possible to have multiple matches (cmdMap.size() > 1).
> > I asked this because in line 1080-1084 we detect if there is a match when
> > putting a match into the map and if so throw an error. Therefore, when we
> > reach line 1091, cmdMap.size() should be less than or equal to 1. Did I
> > miss anything?
I got your point. Line 1080-1084 handles the following case.
```java
new ReflectiveCommandHandler(this, new String[] {"A","B"},
new Completer[] {new TableNameCompletor(this)}),
new ReflectiveCommandHandler(this, new String[] {"A"},
new Completer[] {new TableNameCompletor(this)}),
```
If the line is "!A B;" then the error will be thrown. It's more like a internal
check for programmer to avoid them use the same keywords for a command hander.
Remove Line 1080-1084 since it's not reasonable to have single line match the
multiple ReflectiveCommandHandler.
- cheng
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38044/#review97919
---
On Sept. 8, 2015, 9:45 a.m., cheng xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38044/
> ---
>
> (Updated Sept. 8, 2015, 9:45 a.m.)
>
>
> Review request for hive and Xuefu Zhang.
>
>
> Bugs: HIVE-11640
> https://issues.apache.org/jira/browse/HIVE-11640
>
>
> Repository: hive-git
>
>
> Description
> ---
>
> Fixed issues includes:
> * Support the case that one line contains multi-commands with ";" seperated
> * Fix the nullempty string for new cli mode
> * Resolve command doesn't support issue when executing files
>
>
> Diffs
> -
>
> beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b
> beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec
> beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea
>
> Diff: https://reviews.apache.org/r/38044/diff/
>
>
> Testing
> ---
>
> Local smoke test passed and unit test passed locally
>
>
> Thanks,
>
> cheng xu
>
>
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for new CLI[Beeline-cli branch]
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38044/ --- (Updated Sept. 8, 2015, 9:45 a.m.) Review request for hive and Xuefu Zhang. Bugs: HIVE-11640 https://issues.apache.org/jira/browse/HIVE-11640 Repository: hive-git Description --- Fixed issues includes: * Support the case that one line contains multi-commands with ";" seperated * Fix the nullempty string for new cli mode * Resolve command doesn't support issue when executing files Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea Diff: https://reviews.apache.org/r/38044/diff/ Testing --- Local smoke test passed and unit test passed locally Thanks, cheng xu
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for new CLI[Beeline-cli branch]
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38044/#review97878 --- beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 1091) <https://reviews.apache.org/r/38044/#comment153947> That's possible. You can deep into the code of AbstractCommandHandler. The condition of match is whether the names is starting with the first part of the line. It means the command "!l;" is valid since there's only one match "list" exists. - cheng xu On Sept. 2, 2015, 3:16 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38044/ > --- > > (Updated Sept. 2, 2015, 3:16 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-11640 > https://issues.apache.org/jira/browse/HIVE-11640 > > > Repository: hive-git > > > Description > --- > > Fixed issues includes: > * Support the case that one line contains multi-commands with ";" seperated > * Fix the nullempty string for new cli mode > * Resolve command doesn't support issue when executing files > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b > beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea > > Diff: https://reviews.apache.org/r/38044/diff/ > > > Testing > --- > > Local smoke test passed and unit test passed locally > > > Thanks, > > cheng xu > >
Re: Review Request 38044: HIVE-11640 Shell command doesn't work for new CLI[Beeline-cli branch]
> On Sept. 3, 2015, 1:34 a.m., Xuefu Zhang wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 1094 > > <https://reviews.apache.org/r/38044/diff/1/?file=1062103#file1062103line1094> > > > > with line 1081-1085, is this still possible? I understand this is just > > moving code around. That's possible. You can deep into the code of AbstractCommandHandler. The condition of match is whether the names is starting with the first part of the line. It means the command "!l;" is valid since there's only one match "list" exists. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38044/#review97494 ------- On Sept. 2, 2015, 3:16 p.m., cheng xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38044/ > --- > > (Updated Sept. 2, 2015, 3:16 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-11640 > https://issues.apache.org/jira/browse/HIVE-11640 > > > Repository: hive-git > > > Description > --- > > Fixed issues includes: > * Support the case that one line contains multi-commands with ";" seperated > * Fix the nullempty string for new cli mode > * Resolve command doesn't support issue when executing files > > > Diffs > - > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b > beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec > beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea > > Diff: https://reviews.apache.org/r/38044/diff/ > > > Testing > --- > > Local smoke test passed and unit test passed locally > > > Thanks, > > cheng xu > >
Review Request 38044: HIVE-11640 Shell command doesn't work for new CLI[Beeline-cli branch]
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38044/ --- Review request for hive and Xuefu Zhang. Bugs: HIVE-11640 https://issues.apache.org/jira/browse/HIVE-11640 Repository: hive-git Description --- Fixed issues includes: * Support the case that one line contains multi-commands with ";" seperated * Fix the nullempty string for new cli mode * Resolve command doesn't support issue when executing files Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 1e4759b beeline/src/java/org/apache/hive/beeline/Commands.java 5e5cfec beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java e06d2ea Diff: https://reviews.apache.org/r/38044/diff/ Testing --- Local smoke test passed and unit test passed locally Thanks, cheng xu
Re: Review Request 36942: HIVE-11401: Predicate push down does not work with Parquet when partitions are in the expression
On July 31, 2015, 1:35 p.m., cheng xu wrote: Looks good to me. Just one minor question. Also I am wondering why this search argument works fine for ORC. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36942/#review93697 --- On July 31, 2015, 5:22 a.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36942/ --- (Updated July 31, 2015, 5:22 a.m.) Review request for hive, Aihua Xu, cheng xu, Dong Chen, and Szehon Ho. Bugs: HIVE-11401 https://issues.apache.org/jira/browse/HIVE-11401 Repository: hive-git Description --- The following patch reviews the predicate created by Hive, and removes any column that does not belong to the Parquet schema, such as partitioned columns. This way Parquet can filter the columns correctly. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetFilterPredicateConverter.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 49e52da2e26fd7213df1db88716eaee94cb536b8 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetRecordReaderWrapper.java 87dd344534f09c7fc565fdc467ac82a51f37ebba ql/src/test/org/apache/hadoop/hive/ql/io/parquet/read/TestParquetFilterPredicate.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestConvertAstToSearchArg.java 85e952fb6855a2a03902ed971f54191837b32dac ql/src/test/queries/clientpositive/parquet_predicate_pushdown.q PRE-CREATION ql/src/test/results/clientpositive/parquet_predicate_pushdown.q.out PRE-CREATION Diff: https://reviews.apache.org/r/36942/diff/ Testing --- Unit tests: TestParquetFilterPredicate.java Integration tests: parquet_predicate_pushdown.q Thanks, Sergio Pena
Re: Review Request 36942: HIVE-11401: Predicate push down does not work with Parquet when partitions are in the expression
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36942/#review93697 --- Looks good to me. Just one minor question. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetFilterPredicateConverter.java (lines 103 - 104) https://reviews.apache.org/r/36942/#comment148112 Why we need to create the leaf when columns is null? - cheng xu On July 31, 2015, 5:22 a.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36942/ --- (Updated July 31, 2015, 5:22 a.m.) Review request for hive, Aihua Xu, cheng xu, Dong Chen, and Szehon Ho. Bugs: HIVE-11401 https://issues.apache.org/jira/browse/HIVE-11401 Repository: hive-git Description --- The following patch reviews the predicate created by Hive, and removes any column that does not belong to the Parquet schema, such as partitioned columns. This way Parquet can filter the columns correctly. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetFilterPredicateConverter.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 49e52da2e26fd7213df1db88716eaee94cb536b8 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/TestParquetRecordReaderWrapper.java 87dd344534f09c7fc565fdc467ac82a51f37ebba ql/src/test/org/apache/hadoop/hive/ql/io/parquet/read/TestParquetFilterPredicate.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestConvertAstToSearchArg.java 85e952fb6855a2a03902ed971f54191837b32dac ql/src/test/queries/clientpositive/parquet_predicate_pushdown.q PRE-CREATION ql/src/test/results/clientpositive/parquet_predicate_pushdown.q.out PRE-CREATION Diff: https://reviews.apache.org/r/36942/diff/ Testing --- Unit tests: TestParquetFilterPredicate.java Integration tests: parquet_predicate_pushdown.q Thanks, Sergio Pena
Review Request 36443: HIVE-11226 BeeLine-Cli: support hive.cli.prompt in new CLI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36443/ --- Review request for hive and Xuefu Zhang. Bugs: HIVE-11226 https://issues.apache.org/jira/browse/HIVE-11226 Repository: hive-git Description --- Summary: 1. Use the HIVE as default value for CLI mode prompt 2. Add the mechanism to update prompt Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 7c53997 beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java 894f74f beeline/src/java/org/apache/hive/beeline/Commands.java b07388a beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java fa94c89 Diff: https://reviews.apache.org/r/36443/diff/ Testing --- Test passed locally Thanks, cheng xu
Re: Review Request 36300: HIVE-11191 Beeline-cli: support hive.cli.errors.ignore in new CLI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36300/ --- (Updated July 9, 2015, 3:07 p.m.) Review request for hive and Xuefu Zhang. Changes --- Summary for V2: 1. Add the fields of conf in beelineOpts which is used to parsing client side configuration 2. Add new API to update the configuration in client side Bugs: HIVE-11191 https://issues.apache.org/jira/browse/HIVE-11191 Repository: hive-git Description --- Summary: 1. Add option update stage after connection established in CLI mode 2. Update the getHiveConf API Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java c4dbcd4 beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java c1ec82a beeline/src/java/org/apache/hive/beeline/Commands.java d490273 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java ff8ab17 Diff: https://reviews.apache.org/r/36300/diff/ Testing --- Partial UT passed and local cluster test passed. Thanks, cheng xu
Review Request 36300: HIVE-11191 Beeline-cli: support hive.cli.errors.ignore in new CLI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36300/ --- Review request for hive and Xuefu Zhang. Bugs: HIVE-11191 https://issues.apache.org/jira/browse/HIVE-11191 Repository: hive-git Description --- Summary: 1. Add option update stage after connection established in CLI mode 2. Update the getHiveConf API Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 1d468eb beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java c1ec82a beeline/src/java/org/apache/hive/beeline/Commands.java d490273 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java ff8ab17 Diff: https://reviews.apache.org/r/36300/diff/ Testing --- Partial UT passed and local cluster test passed. Thanks, cheng xu
Re: Review Request 36300: HIVE-11191 Beeline-cli: support hive.cli.errors.ignore in new CLI
On July 8, 2015, 9:27 p.m., Xuefu Zhang wrote: beeline/src/java/org/apache/hive/beeline/Commands.java, line 759 https://reviews.apache.org/r/36300/diff/1/?file=1002091#file1002091line759 Why do we provide two way to do the same thing? When do we choose to use one or the other? This part of code is very tricky and blocked me quite some time. If you use the call method, seems data is not synchronized with the server side. And if you use the sql method, it will override the console of the beeline. I referred the code logic from executeInternal method. To be honest, I am not quite follow the original purpose of this code logic but it works. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36300/#review90889 --- On July 8, 2015, 4:44 p.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36300/ --- (Updated July 8, 2015, 4:44 p.m.) Review request for hive and Xuefu Zhang. Bugs: HIVE-11191 https://issues.apache.org/jira/browse/HIVE-11191 Repository: hive-git Description --- Summary: 1. Add option update stage after connection established in CLI mode 2. Update the getHiveConf API Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 1d468eb beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java c1ec82a beeline/src/java/org/apache/hive/beeline/Commands.java d490273 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java ff8ab17 Diff: https://reviews.apache.org/r/36300/diff/ Testing --- Partial UT passed and local cluster test passed. Thanks, cheng xu
Re: Review Request 35950: HIVE-11131: Get row information on DataWritableWriter once for better writing performance
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/#review90838 --- Ship it! Ship It! - cheng xu On July 8, 2015, 12:25 a.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/ --- (Updated July 8, 2015, 12:25 a.m.) Review request for hive, Ryan Blue, cheng xu, and Dong Chen. Bugs: HIVE-11131 https://issues.apache.org/jira/browse/HIVE-11131 Repository: hive-git Description --- Implemented data type writers that will be created before the first Hive row is written to Parquet. These writers contain information about object inspectors and schema of a specific data type, and calls the specific add() method used by Parquet for each data type. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java c195c3ec3ddae19bf255fc2c9633f8bf4390f428 Diff: https://reviews.apache.org/r/35950/diff/ Testing --- Tests from TestDataWritableWriter run OK. I run other tests with micro-becnhmarks, and I got some better results from this new implemntation: Using repeated rows across the file, this is the throughput increase using 1 million records: bigintboolean double float int string 7.598 7.491 7.488 7.588 7.530.270 (before) 10.13711.511 10.155 10.297 10.242 0.286 (after) Using random rows across the file, the is the throughput increase using 1 million records: bigintboolean double float int string 5.268 7.723 4.107 4.173 4.729 0.20 (before) 6.236 10.466 5.944 4.749 5.234 0.22 (after) Thanks, Sergio Pena
Re: Review Request 36156: HIVE-11053: Add more tests for HIVE-10844[Spark Branch]
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36156/#review90854 --- ql/src/test/queries/clientpositive/dynamic_rdd_cache.q (line 6) https://reviews.apache.org/r/36156/#comment144012 Minor issue: Please remove the tailing space in the qfile. Thank you! - cheng xu On July 8, 2015, 10:13 a.m., lun gao wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36156/ --- (Updated July 8, 2015, 10:13 a.m.) Review request for hive and chengxiang li. Bugs: HIVE-11053 https://issues.apache.org/jira/browse/HIVE-11053 Repository: hive-git Description --- Add some test cases for self union, self-join, CWE, and repeated sub-queries to verify the job of combining quivalent works in HIVE-10844. Diffs - itests/src/test/resources/testconfiguration.properties 4f2de12 ql/src/test/queries/clientpositive/dynamic_rdd_cache.q PRE-CREATION ql/src/test/results/clientpositive/dynamic_rdd_cache.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/dynamic_rdd_cache.q.out PRE-CREATION Diff: https://reviews.apache.org/r/36156/diff/ Testing --- Thanks, lun gao
Re: Review Request 36156: HIVE-11053: Add more tests for HIVE-10844[Spark Branch]
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36156/#review90860 --- Ship it! Ship It! - cheng xu On July 8, 2015, 11:05 a.m., lun gao wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36156/ --- (Updated July 8, 2015, 11:05 a.m.) Review request for hive and chengxiang li. Bugs: HIVE-11053 https://issues.apache.org/jira/browse/HIVE-11053 Repository: hive-git Description --- Add some test cases for self union, self-join, CWE, and repeated sub-queries to verify the job of combining quivalent works in HIVE-10844. Diffs - itests/src/test/resources/testconfiguration.properties 4f2de12 ql/src/test/queries/clientpositive/dynamic_rdd_cache.q PRE-CREATION ql/src/test/results/clientpositive/dynamic_rdd_cache.q.out PRE-CREATION ql/src/test/results/clientpositive/spark/dynamic_rdd_cache.q.out PRE-CREATION Diff: https://reviews.apache.org/r/36156/diff/ Testing --- Thanks, lun gao
Re: Review Request 35950: HIVE-11131: Get row information on DataWritableWriter once for better writing performance
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/#review89740 --- Thanks Sergio for this patch. Will this have negative impacts with the initial part? Thank you. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 416) https://reviews.apache.org/r/35950/#comment142514 Is that possible to use generic type to avoid creating DataWriter for each type since they are quite similar? - cheng xu On June 28, 2015, 8:29 a.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/ --- (Updated June 28, 2015, 8:29 a.m.) Review request for hive, Ryan Blue, cheng xu, and Dong Chen. Bugs: HIVE-11131 https://issues.apache.org/jira/browse/HIVE-11131 Repository: hive-git Description --- Implemented data type writers that will be created before the first Hive row is written to Parquet. These writers contain information about object inspectors and schema of a specific data type, and calls the specific add() method used by Parquet for each data type. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java c195c3ec3ddae19bf255fc2c9633f8bf4390f428 Diff: https://reviews.apache.org/r/35950/diff/ Testing --- Tests from TestDataWritableWriter run OK. I run other tests with micro-becnhmarks, and I got some better results from this new implemntation: Using repeated rows across the file, this is the throughput increase using 1 million records: bigintboolean double float int string 7.598 7.491 7.488 7.588 7.530.270 (before) 10.13711.511 10.155 10.297 10.242 0.286 (after) Using random rows across the file, the is the throughput increase using 1 million records: bigintboolean double float int string 5.268 7.723 4.107 4.173 4.729 0.20 (before) 6.236 10.466 5.944 4.749 5.234 0.22 (after) Thanks, Sergio Pena
Re: Review Request 35950: HIVE-11131: Get row information on DataWritableWriter once for better writing performance
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/#review89721 --- ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (lines 71 - 72) https://reviews.apache.org/r/35950/#comment142419 Add the comments before the declarations of messageWriter. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 73) https://reviews.apache.org/r/35950/#comment142418 No need to initialized by null val. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 107) https://reviews.apache.org/r/35950/#comment142422 I don't follow Why rename to schema here. ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 182) https://reviews.apache.org/r/35950/#comment142424 groupSchema - groupType ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 352) https://reviews.apache.org/r/35950/#comment142425 ByteDataWrter? ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java (line 538) https://reviews.apache.org/r/35950/#comment142420 DateDataWriter - cheng xu On June 28, 2015, 8:29 a.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35950/ --- (Updated June 28, 2015, 8:29 a.m.) Review request for hive, Ryan Blue, cheng xu, and Dong Chen. Bugs: HIVE-11131 https://issues.apache.org/jira/browse/HIVE-11131 Repository: hive-git Description --- Implemented data type writers that will be created before the first Hive row is written to Parquet. These writers contain information about object inspectors and schema of a specific data type, and calls the specific add() method used by Parquet for each data type. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/DataWritableWriter.java c195c3ec3ddae19bf255fc2c9633f8bf4390f428 Diff: https://reviews.apache.org/r/35950/diff/ Testing --- Tests from TestDataWritableWriter run OK. I run other tests with micro-becnhmarks, and I got some better results from this new implemntation: Using repeated rows across the file, this is the throughput increase using 1 million records: bigintboolean double float int string 7.598 7.491 7.488 7.588 7.530.270 (before) 10.13711.511 10.155 10.297 10.242 0.286 (after) Using random rows across the file, the is the throughput increase using 1 million records: bigintboolean double float int string 5.268 7.723 4.107 4.173 4.729 0.20 (before) 6.236 10.466 5.944 4.749 5.234 0.22 (after) Thanks, Sergio Pena
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 26, 2015, 2 p.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 26, 2015, 4:02 p.m.) Review request for hive, chinna and Xuefu Zhang. Changes --- Summary: 1) rebase code 2) add some java doc 3) do some code clean work Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 66fe322 beeline/src/java/org/apache/hive/beeline/Commands.java aaf6aec beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 669e6be ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java d271d6d service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java cc9df76 Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote: beeline/src/java/org/apache/hive/beeline/Commands.java, line 820 https://reviews.apache.org/r/35107/diff/3/?file=991102#file991102line820 Does this mean that env and sys variables are not being substituted for shell command? cheng xu wrote: No, this method is only used for retrieving hive configurations. For env and sys variables, they are subsituted by VariableSubstitution. Xuefu Zhang wrote: Not sure if I understand it. Could you outline the process in which the variables get substituted? Thanks. The class *VariableSubstitution* handles the case of hive variables. It's overriding the super class *SystemVariables* who deals with the system variables, env variables and hive conf variables. The process is as follows: 1. Construct a *VariableSubsitution* instance with the source where you could retrieve the hive variables 2. Call the method *substitute* from *VariableSubsitution*: SystemVariables#substitute(super.substitute) - SystemVariables#getSubstitute(get the eval from sys, env and hive conf) - VariableSubstitution#getSubstitute(get the eval from hive variables source) - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review89251 --- On June 25, 2015, 1:54 p.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 25, 2015, 1:54 p.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 26, 2015, 5:54 a.m., Xuefu Zhang wrote: beeline/src/java/org/apache/hive/beeline/Commands.java, line 855 https://reviews.apache.org/r/35107/diff/4/?file=991896#file991896line855 This used to call BeeLine.executeFile() now you have a new implementation. I don't quite follow why. Regardless, I see two problems. 1. we should remove the 2nd argument as all callers provide false. 2. Some part of the code in executeFile() is to fix some problem with the last line in the script file. Will your new implementation catches that? 3. It's my understanding that in Hive CLI compatible mode, commands in source file should follow Hive CLI syntax, while in normal mode, it should follow Beeline syntax. Is this what's done here? This is a little tricky code. The follows are the reasons I prefer not to use the *executeFile* in *BeeLine* to implement the source command: 1. In the executeFile method, it will reset the consoleReader. So the command after *source* will be skipped. Given source xx;desc tbl; as example, this desc tbl will be ignored since console reader is reset. It's OK for other places(execute initial file and execute file) to invoke this since it is called after the consoleReader initialized. This issue is not revealled in the last patch and I add one new test *testSourceCmd2* to cover this. 2. Furthermore, source command is executed in the cli side like sql and call command which is unlike the initial file and -f option. So I prefer to handle it in the command level. 3. WRT the last line issue, do you mean **output(); // dummy new line**? If so, I think it's not necessary since we treat it as a single command. The content should not be displayed in the cli as the old CLI. ``` cat /root/workspace/test.sql create table test2(a string, b string); ``` In old cli ``` hive source /root/workspace/test.sql ; OK Time taken: 0.192 seconds hive show tables; OK test2 Time taken: 0.117 seconds, Fetched: 1 row(s) ``` - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review89421 --- On June 25, 2015, 1:54 p.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 25, 2015, 1:54 p.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 24, 2015, 3:39 p.m.) Review request for hive, chinna and Xuefu Zhang. Changes --- Update patch addressing Xuefu's comments Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 25, 2015, 1:54 p.m.) Review request for hive, chinna and Xuefu Zhang. Changes --- code clean Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote: ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java, line 146 https://reviews.apache.org/r/35107/diff/3/?file=991115#file991115line146 Could we keep @Override at a separate line? Same for other places. I am using the code-style file(eclipse-styles.xml) under the dev-support folder. Seems annotation before class/method is not wrapped. Anyway, I just update all the places in this patch. Thank you for figuring this out. On June 25, 2015, 6:21 a.m., Xuefu Zhang wrote: beeline/src/java/org/apache/hive/beeline/Commands.java, line 820 https://reviews.apache.org/r/35107/diff/3/?file=991102#file991102line820 Does this mean that env and sys variables are not being substituted for shell command? No, this method is only used for retrieving hive configurations. For env and sys variables, they are subsituted by VariableSubstitution. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review89251 --- On June 25, 2015, 1:54 p.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 25, 2015, 1:54 p.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java b7d2f2e beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c common/src/java/org/apache/hadoop/hive/conf/HiveVariableSource.java PRE-CREATION common/src/java/org/apache/hadoop/hive/conf/VariableSubstitution.java PRE-CREATION common/src/test/org/apache/hadoop/hive/conf/TestVariableSubstitution.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/Driver.java 338e755 ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java a5f0a7f ql/src/java/org/apache/hadoop/hive/ql/parse/VariableSubstitution.java e8b1d96 ql/src/java/org/apache/hadoop/hive/ql/processors/AddResourceProcessor.java 0558c53 ql/src/java/org/apache/hadoop/hive/ql/processors/CompileProcessor.java 25ce168 ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 9052c82 ql/src/java/org/apache/hadoop/hive/ql/processors/DfsProcessor.java cc0414d ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java bc9254c service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 33ee16b Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 9, 2015, 12:41 p.m., Xuefu Zhang wrote: Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. cheng xu wrote: Thanks for your comments. `I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective.` The substitution works well in HS2 currently. There are two reasons for me to add API getting the conf from HS2. One is to support substitution in sh and source command. In the old cli, source command and sh command worked well with substitution. So this part of this patch is addressing this purpose. Another consideration is for https://issues.apache.org/jira/browse/HIVE-10847 which required some configuration from hive-site.xml. Xuefu Zhang wrote: Yeah. It's a little trickier than thought. Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands. More importantly, Beeline can connect to multiple serves at the same time, so which configurations should be used to substitue the variables? User should be able to execute shell commands w/o any server connection. For CLI, server and client are together, so these don't matter. But for beeline + HS2 deployment, story will be different. I don't know what's the best, and all I'm saying is that we need to be very careful on what we doing. Before we decide what to do, we need to clearly define the problem we are trying to solve first. cheng xu wrote: Thank you for your prompt reply. `Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands.` I'm not sure whether substitution for sh and source is useful. We can enable the support of substitution after connection established for beeline unless connected. For the new CLI who is using an embedded connection, it should be supported WRT the backwards compatibility. I am a little confused about `connect to multiple serves at the same time`. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command */opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline* with specify any hostname) If so, I think it may cause some errors if no connection available since the current implementation is based on connection by using **SetProcessor**. AFAIK, it's safe to get the configurations from HS2 via **SetProcessor** which is what beeline actually did after connection is established. Connection(session) should only be assiocated with one server. If user didn't connect to any HS2, the substitution for *sh* and *source* should be disabled. To be honest, it will have some negative impacts for the performance since it requires to execute set command. WRT the performance, we can make this support configurable. In summary, substitution is enabled unless connection is established for source or sh command considering the backwards compatibility. And we can disable the support for beeline if not reasonable or brings lower performce. For HIVE-10847, I think we still need one way to access the configuration from server side but it is only needed when start a connection. Any thoughts? cheng xu wrote: Sorry for below typo. I am a little confused about connect to multiple serves at the same time. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command /opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline **without** specify any hostname) Xuefu Zhang wrote: I think it's possible to start beeline without any connection. To do that, just run beeline w/o -u parameter. Once beeline starts, you can run !connect jdbc url to make a connection. I also believe it's also possible to make another connection using !connect jdbc url w/o disconnecting from the previous connection. You can run !list to get a list of connections, and !go index to select a particular
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 9, 2015, 12:41 p.m., Xuefu Zhang wrote: Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. cheng xu wrote: Thanks for your comments. `I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective.` The substitution works well in HS2 currently. There are two reasons for me to add API getting the conf from HS2. One is to support substitution in sh and source command. In the old cli, source command and sh command worked well with substitution. So this part of this patch is addressing this purpose. Another consideration is for https://issues.apache.org/jira/browse/HIVE-10847 which required some configuration from hive-site.xml. Xuefu Zhang wrote: Yeah. It's a little trickier than thought. Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands. More importantly, Beeline can connect to multiple serves at the same time, so which configurations should be used to substitue the variables? User should be able to execute shell commands w/o any server connection. For CLI, server and client are together, so these don't matter. But for beeline + HS2 deployment, story will be different. I don't know what's the best, and all I'm saying is that we need to be very careful on what we doing. Before we decide what to do, we need to clearly define the problem we are trying to solve first. cheng xu wrote: Thank you for your prompt reply. `Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands.` I'm not sure whether substitution for sh and source is useful. We can enable the support of substitution after connection established for beeline unless connected. For the new CLI who is using an embedded connection, it should be supported WRT the backwards compatibility. I am a little confused about `connect to multiple serves at the same time`. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command */opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline* with specify any hostname) If so, I think it may cause some errors if no connection available since the current implementation is based on connection by using **SetProcessor**. AFAIK, it's safe to get the configurations from HS2 via **SetProcessor** which is what beeline actually did after connection is established. Connection(session) should only be assiocated with one server. If user didn't connect to any HS2, the substitution for *sh* and *source* should be disabled. To be honest, it will have some negative impacts for the performance since it requires to execute set command. WRT the performance, we can make this support configurable. In summary, substitution is enabled unless connection is established for source or sh command considering the backwards compatibility. And we can disable the support for beeline if not reasonable or brings lower performce. For HIVE-10847, I think we still need one way to access the configuration from server side but it is only needed when start a connection. Any thoughts? cheng xu wrote: Sorry for below typo. I am a little confused about connect to multiple serves at the same time. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command /opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline **without** specify any hostname) Xuefu Zhang wrote: I think it's possible to start beeline without any connection. To do that, just run beeline w/o -u parameter. Once beeline starts, you can run !connect jdbc url to make a connection. I also believe it's also possible to make another connection using !connect jdbc url w/o disconnecting from the previous connection. You can run !list to get a list of connections, and !go index to select a particular
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 9, 2015, 12:41 p.m., Xuefu Zhang wrote: Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. Thanks for your comments. `I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective.` The substitution works well in HS2 currently. There are two reasons for me to add API getting the conf from HS2. One is to support substitution in sh and source command. In the old cli, source command and sh command worked well with substitution. So this part of this patch is addressing this purpose. Another consideration is for https://issues.apache.org/jira/browse/HIVE-10847 which required some configuration from hive-site.xml. - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 --- On June 5, 2015, 10:09 a.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 5, 2015, 10:09 a.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 45a7e87 beeline/src/java/org/apache/hive/beeline/BeelineVariableSubstitution.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 9, 2015, 12:41 p.m., Xuefu Zhang wrote: Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. cheng xu wrote: Thanks for your comments. `I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective.` The substitution works well in HS2 currently. There are two reasons for me to add API getting the conf from HS2. One is to support substitution in sh and source command. In the old cli, source command and sh command worked well with substitution. So this part of this patch is addressing this purpose. Another consideration is for https://issues.apache.org/jira/browse/HIVE-10847 which required some configuration from hive-site.xml. Xuefu Zhang wrote: Yeah. It's a little trickier than thought. Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands. More importantly, Beeline can connect to multiple serves at the same time, so which configurations should be used to substitue the variables? User should be able to execute shell commands w/o any server connection. For CLI, server and client are together, so these don't matter. But for beeline + HS2 deployment, story will be different. I don't know what's the best, and all I'm saying is that we need to be very careful on what we doing. Before we decide what to do, we need to clearly define the problem we are trying to solve first. Thank you for your prompt reply. `Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands.` I'm not sure whether substitution for sh and source is useful. We can enable the support of substitution after connection established for beeline unless connected. For the new CLI who is using an embedded connection, it should be supported WRT the backwards compatibility. I am a little confused about `connect to multiple serves at the same time`. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command */opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline* with specify any hostname) If so, I think it may cause some errors if no connection available since the current implementation is based on connection by using **SetProcessor**. AFAIK, it's safe to get the configurations from HS2 via **SetProcessor** which is what beeline actually did after connection is established. Connection(session) should only be assiocated with one server. If user didn't connect to any HS2, the substitution for *sh* and *source* should be disabled. To be honest, it will have some negative impacts for the performance since it requires to execute set command. WRT the performance, we can make this support configurable. In summary, substitution is enabled unless connection is established for source or sh command considering the backwards compatibility. And we can disable the support for beeline if not reasonable or brings lower performce. For HIVE-10847, I think we still need one way to access the configuration from server side but it is only needed when start a connection. Any thoughts? - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 --- On June 5, 2015, 10:09 a.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 5, 2015, 10:09 a.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
On June 9, 2015, 12:41 p.m., Xuefu Zhang wrote: Besides the two minor issues I found in the patch, I was wondering if the approach we are taking is the best. Variable substitution is a server (HS2) behavior, and on this ground I think this should happen in HS2 instead of beeline. Please note that JDBC client may also submit queries with $var in it, and such a case should be also supported. I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective. Shell command (starting with !sh) is executed in the client (Beeline). I think we are fine if variable substituion doesn't work for shell command. We can address that as a followup taks if desirable. cheng xu wrote: Thanks for your comments. `I also noticed that in Driver class, there is code handling variable substitution. I'm wondering why it's not effective.` The substitution works well in HS2 currently. There are two reasons for me to add API getting the conf from HS2. One is to support substitution in sh and source command. In the old cli, source command and sh command worked well with substitution. So this part of this patch is addressing this purpose. Another consideration is for https://issues.apache.org/jira/browse/HIVE-10847 which required some configuration from hive-site.xml. Xuefu Zhang wrote: Yeah. It's a little trickier than thought. Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands. More importantly, Beeline can connect to multiple serves at the same time, so which configurations should be used to substitue the variables? User should be able to execute shell commands w/o any server connection. For CLI, server and client are together, so these don't matter. But for beeline + HS2 deployment, story will be different. I don't know what's the best, and all I'm saying is that we need to be very careful on what we doing. Before we decide what to do, we need to clearly define the problem we are trying to solve first. cheng xu wrote: Thank you for your prompt reply. `Shell command is executed at client side (Beeline) and it doesn't seem making sense to use server specific variables such (env, sys, hiveconf, hivevar) in the shell commands.` I'm not sure whether substitution for sh and source is useful. We can enable the support of substitution after connection established for beeline unless connected. For the new CLI who is using an embedded connection, it should be supported WRT the backwards compatibility. I am a little confused about `connect to multiple serves at the same time`. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command */opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline* with specify any hostname) If so, I think it may cause some errors if no connection available since the current implementation is based on connection by using **SetProcessor**. AFAIK, it's safe to get the configurations from HS2 via **SetProcessor** which is what beeline actually did after connection is established. Connection(session) should only be assiocated with one server. If user didn't connect to any HS2, the substitution for *sh* and *source* should be disabled. To be honest, it will have some negative impacts for the performance since it requires to execute set command. WRT the performance, we can make this support configurable. In summary, substitution is enabled unless connection is established for source or sh command considering the backwards compatibility. And we can disable the support for beeline if not reasonable or brings lower performce. For HIVE-10847, I think we still need one way to access the configuration from server side but it is only needed when start a connection. Any thoughts? Sorry for below typo. I am a little confused about connect to multiple serves at the same time. Does it mean you can use beeline to connect any server in one connection and you can have multiple beeline instances running? (It's the case that user executes the command /opt/apache-hive-1.2.0-SNAPSHOT-bin/bin/hive --service beeline **without** specify any hostname) - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/#review87090 --- On June 5, 2015, 10:09 a.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107
Re: Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- (Updated June 5, 2015, 10:09 a.m.) Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 45a7e87 beeline/src/java/org/apache/hive/beeline/BeelineVariableSubstitution.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Review Request 35107: HIVE-6791 Support variable substition for Beeline shell command
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35107/ --- Review request for hive, chinna and Xuefu Zhang. Bugs: HIVE-6791 https://issues.apache.org/jira/browse/HIVE-6791 Repository: hive-git Description --- Summary: 1) move the beeline-cli convertor to the place where cli is executed(class **Commands**) 2) support substitution for source command 3) add some unit test for substitution 4) add one way to get the configuration from HS2 Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 45a7e87 beeline/src/java/org/apache/hive/beeline/BeelineVariableSubstitution.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/Commands.java a42baa3 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java 6cbb030 Diff: https://reviews.apache.org/r/35107/diff/ Testing --- Unit test passed Thanks, cheng xu
Re: Review Request 34922: HIVE-10705 Update tests for HIVE-9302 after removing binaries
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34922/ --- (Updated June 3, 2015, 10:52 a.m.) Review request for hive, Hari Sankar Sivarama Subramaniyan, Sushanth Sowmyan, and Vaibhav Gumashta. Bugs: HIVE-10705 https://issues.apache.org/jira/browse/HIVE-10705 Repository: hive-git Description --- Summary: 1. remove binaries and make jar file in the runtime 2. move some common utilities to the HiveTestUtils Diffs (updated) - beeline/pom.xml 352f561 beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java 7a354f3 beeline/src/test/resources/DummyDriver.txt PRE-CREATION common/src/java/org/apache/hive/common/util/HiveTestUtils.java db34494 ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ad22a Diff: https://reviews.apache.org/r/34922/diff/ Testing --- UT passed locally Thanks, cheng xu
Re: Review Request 34922: HIVE-10705 Update tests for HIVE-9302 after removing binaries
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34922/ --- (Updated June 3, 2015, 10:56 a.m.) Review request for hive, Hari Sankar Sivarama Subramaniyan, Sushanth Sowmyan, and Vaibhav Gumashta. Bugs: HIVE-10705 https://issues.apache.org/jira/browse/HIVE-10705 Repository: hive-git Description --- Summary: 1. remove binaries and make jar file in the runtime 2. move some common utilities to the HiveTestUtils Diffs (updated) - beeline/pom.xml 352f561 beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java 7a354f3 beeline/src/test/resources/DummyDriver-1.0-SNAPSHOT.jar 3dadc9e beeline/src/test/resources/DummyDriver.txt PRE-CREATION beeline/src/test/resources/postgresql-9.3.jdbc3.jar f537b98 common/src/java/org/apache/hive/common/util/HiveTestUtils.java db34494 ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ad22a Diff: https://reviews.apache.org/r/34922/diff/ Testing --- UT passed locally Thanks, cheng xu
Re: Review Request 34922: HIVE-10705 Update tests for HIVE-9302 after removing binaries
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34922/ --- (Updated June 2, 2015, 1:15 p.m.) Review request for hive, Hari Sankar Sivarama Subramaniyan, Sushanth Sowmyan, and Vaibhav Gumashta. Bugs: HIVE-10705 https://issues.apache.org/jira/browse/HIVE-10705 Repository: hive-git Description --- Summary: 1. remove binaries and make jar file in the runtime 2. move some common utilities to the HiveTestUtils Diffs (updated) - beeline/pom.xml 352f561 beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java 7a354f3 beeline/src/test/resources/DummyDriver-1.0-SNAPSHOT.jar 3dadc9e beeline/src/test/resources/DummyDriver.txt PRE-CREATION beeline/src/test/resources/postgresql-9.3.jdbc3.jar f537b98 common/src/java/org/apache/hive/common/util/HiveTestUtils.java db34494 ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ad22a Diff: https://reviews.apache.org/r/34922/diff/ Testing --- UT passed locally Thanks, cheng xu
Review Request 34922: HIVE-10705 Update tests for HIVE-9302 after removing binaries
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34922/ --- Review request for hive, Hari Sankar Sivarama Subramaniyan, Sushanth Sowmyan, and Vaibhav Gumashta. Bugs: HIVE-10705 https://issues.apache.org/jira/browse/HIVE-10705 Repository: hive-git Description --- Summary: 1. remove binaries and make jar file in the runtime 2. move some common utilities to the HiveTestUtils Diffs - beeline/pom.xml 352f561 beeline/src/test/org/apache/hive/beeline/DummyDriver.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/TestBeelineArgParsing.java 7a354f3 beeline/src/test/resources/DummyDriver-1.0-SNAPSHOT.jar 3dadc9e beeline/src/test/resources/DummyDriver.txt PRE-CREATION beeline/src/test/resources/postgresql-9.3.jdbc3.jar f537b98 common/src/java/org/apache/hive/common/util/HiveTestUtils.java db34494 ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ad22a Diff: https://reviews.apache.org/r/34922/diff/ Testing --- UT passed locally Thanks, cheng xu
Re: Review Request 34752: Beeline-CLI: Implement CLI source command using Beeline functionality
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34752/ --- (Updated June 1, 2015, 11:17 a.m.) Review request for hive, chinna and Xuefu Zhang. Changes --- Add source support for beeline as well Bugs: HIVE-10821 https://issues.apache.org/jira/browse/HIVE-10821 Repository: hive-git Description --- Add source command support for CLI using beeline Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 4a82635 beeline/src/java/org/apache/hive/beeline/Commands.java 4c60525 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java cc0b598 Diff: https://reviews.apache.org/r/34752/diff/ Testing --- Newly created UT passed Thanks, cheng xu
Re: Review Request 34752: Beeline-CLI: Implement CLI source command using Beeline functionality
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34752/ --- (Updated May 29, 2015, 2:44 a.m.) Review request for hive and Xuefu Zhang. Bugs: HIVE-10821 https://issues.apache.org/jira/browse/HIVE-10821 Repository: hive-git Description --- Add source command support for CLI using beeline Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 4a82635 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java cc0b598 Diff: https://reviews.apache.org/r/34752/diff/ Testing --- Newly created UT passed Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 28, 2015, 8:29 a.m.) Review request for hive, Alan Gates, Owen O'Malley, and Sergio Pena. Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java c6fb26c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java f513572 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ObjectArrayWritableObjectInspector.java 571f993 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 34248: HIVE-10684 Fix the unit test failures for HIVE-7553 after HIVE-10674 removed the binary jar files
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34248/ --- (Updated May 28, 2015, 2:31 a.m.) Review request for hive and Sushanth Sowmyan. Bugs: HIVE-10684 https://issues.apache.org/jira/browse/HIVE-10684 Repository: hive-git Description --- Remove binaries from source and fix the failed cases Diffs (updated) - ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ba07e ql/src/test/resources/RefreshedJarClassV1.txt PRE-CREATION ql/src/test/resources/RefreshedJarClassV2.txt PRE-CREATION Diff: https://reviews.apache.org/r/34248/diff/ Testing --- UT passed Thanks, cheng xu
Review Request 34752: Beeline-CLI: Implement CLI source command using Beeline functionality
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34752/ --- Review request for hive and Xuefu Zhang. Bugs: HIVE-10821 https://issues.apache.org/jira/browse/HIVE-10821 Repository: hive-git Description --- Add source command support for CLI using beeline Diffs - beeline/src/java/org/apache/hive/beeline/BeeLine.java 4a82635 beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java cc0b598 Diff: https://reviews.apache.org/r/34752/diff/ Testing --- Newly created UT passed Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
On May 21, 2015, 7:18 p.m., Sergio Pena wrote: ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetStructObjectInspector.java, line 1 https://reviews.apache.org/r/34473/diff/2/?file=966163#file966163line1 I am waiting to commit the patch from HIVE-10749 that uses a similar class named ObjectArrayWritableObjectInspector. Also, I think this is already part o the parquet branch. It's https://issues.apache.org/jira/browse/HIVE-9658 - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/#review84758 --- On May 22, 2015, 6:26 a.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 22, 2015, 6:26 a.m.) Review request for hive, Alan Gates, Owen O'Malley, and Sergio Pena. Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java c6fb26c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java f513572 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ObjectArrayWritableObjectInspector.java 571f993 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
On May 21, 2015, 7:18 p.m., Sergio Pena wrote: ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java, line 59 https://reviews.apache.org/r/34473/diff/2/?file=966160#file966160line59 Could you separate words with _? Like ENABLE_ACID_SCHEMA_INFO. It helps to read the constant more easily. Do we have to enable transactions exclusively for parquet? Isn't there another variable that enables trasnactions on Hive that we can use? This variable is used for setting the schema for parquet. It's only related to whether you need to write data to base file or not. So we have to use this way to append the original data with ACID info. On May 21, 2015, 7:18 p.m., Sergio Pena wrote: ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java, lines 98-103 https://reviews.apache.org/r/34473/diff/2/?file=966160#file966160line98 You can use this one line to return the column list: return (ListString) StringUtils.getStringCollection(tableProperties.getProperty(IOConstants.COLUMNS)); It will return an empty list array if COLUMN is empty. Great suggestion! - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/#review84758 --- On May 22, 2015, 6:26 a.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 22, 2015, 6:26 a.m.) Review request for hive, Alan Gates, Owen O'Malley, and Sergio Pena. Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java c6fb26c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java f513572 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ObjectArrayWritableObjectInspector.java 571f993 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 22, 2015, 6:26 a.m.) Review request for hive, Alan Gates, Owen O'Malley, and Sergio Pena. Changes --- Summary: 1. use some utility to reduce LOC 2. remove *ParquetRecordReaderWrapper.java* and use *ObjectArrayWritableObjectInspector* instead Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java c6fb26c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java f513572 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ObjectArrayWritableObjectInspector.java 571f993 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 21, 2015, 7:45 a.m.) Review request for hive, Alan Gates and Sergio Pena. Changes --- Summary: 1. fix code style issues 2. remove codes irrelevant to the insert statement 3. fix one issue about SetParquetSchema from previous patch Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java c6fb26c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java f513572 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetStructObjectInspector.java PRE-CREATION ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 34473: HIVE-10749 Implement Insert statement for parquet
On May 20, 2015, 8:45 p.m., Alexander Pivovarov wrote: ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetStructObjectInspector.java, line 207 https://reviews.apache.org/r/34473/diff/1/?file=965270#file965270line207 you can use final ArrayListObject list = new ArrayListObject(Collections.nCopies(fields.size(), null)); instead I don't think so because only in the insert statement, we can't understand how to inspect the row object until creating parquet writer. This is why I create the new constructor in ParquetStructObjectInspector. Thank yoU! - cheng --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/#review84574 --- On May 20, 2015, 2:54 p.m., cheng xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- (Updated May 20, 2015, 2:54 p.m.) Review request for hive, Alan Gates and Sergio Pena. Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java 000eb38 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 8380117 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/VectorizedParquetInputFormat.java 4e1820c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRawRecordMerger.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java 43c772f ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 0a5edbb ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetStructObjectInspector.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java 0d32e49 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java 5f7f597 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 33956: HIVE-9614: Encrypt mapjoin tables
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33956/#review84671 --- Thank you for this patch. I have some questions and will have another round of review after understanding these questions. Thank you! common/src/java/org/apache/hive/common/util/HdfsEncryptionUtilities.java https://reviews.apache.org/r/33956/#comment136026 Why not use the isPathEncrypted from HdfsEncryptionShim directly? common/src/java/org/apache/hive/common/util/HdfsEncryptionUtilities.java https://reviews.apache.org/r/33956/#comment136027 The same as above. ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java https://reviews.apache.org/r/33956/#comment136025 Is it possible to get the FsPermission from org.apache.hadoop.fs.FileContext? ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java https://reviews.apache.org/r/33956/#comment136022 I am a little confused here. How can a local path be converted to a hdfs path? The original code is trying to create a tar file from a local path and uploading it to the hdfs with replication information. The new code path will lose the replication information. And the previous code path will only be executed in a local file or pfile schema in test. ql/src/test/queries/clientpositive/encryption_map_join_select.q https://reviews.apache.org/r/33956/#comment136021 drop table encryptedTable PURGE; - cheng xu On May 7, 2015, 9:23 p.m., Sergio Pena wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33956/ --- (Updated May 7, 2015, 9:23 p.m.) Review request for hive, Brock Noland and cheng xu. Bugs: HIVE-9614 https://issues.apache.org/jira/browse/HIVE-9614 Repository: hive-git Description --- The security issue here is that encrypted tables used on MAP-JOIN queries, and stored on the distribute cache, are first copied to the client local filesystem in an unencrypted form in order to compress it there. This patch avoids the local copy if the table is encrypted on HDFS. It keeps the hash table on HDFS, compresses the table in HDFS, and then adds it to the distributed cache. Files that are copied to the datanodes by the distributed cache are still unencrypted. This is a limitation we have from HDFS. Diffs - common/src/java/org/apache/hadoop/hive/common/CompressionUtils.java 0e0d538c2faf1c52c4d8378df013294ae4efa41c common/src/java/org/apache/hive/common/util/HdfsEncryptionUtilities.java PRE-CREATION itests/src/test/resources/testconfiguration.properties 3eff7d010923a4e07d5024904f1531ca52473aa2 ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ad5c8f8302de2a15b1703161799f71cd81a94475 ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java d7a08ecf1c183fe56b5ca41c2c69d413874418bb ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java 4d84f0f76ce17711077ceadf23e6b9ed12e6a414 ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/MapJoinResolver.java c0a72b69df3871bbcc870af286774aee5269668b ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java cbc5466261f749fe7b84d7533dc0ff3274b6777f ql/src/java/org/apache/hadoop/hive/ql/plan/MapredLocalWork.java 82143a64db163da766dcc138231b4d4174603470 ql/src/test/queries/clientpositive/encryption_map_join_select.q PRE-CREATION ql/src/test/results/clientpositive/encrypted/encryption_map_join_select.q.out PRE-CREATION Diff: https://reviews.apache.org/r/33956/diff/ Testing --- Thanks, Sergio Pena
Review Request 34473: HIVE-10749 Implement Insert statement for parquet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34473/ --- Review request for hive, Alan Gates and Sergio Pena. Bugs: HIVE-10749 https://issues.apache.org/jira/browse/HIVE-10749 Repository: hive-git Description --- Implement the insert statement for parquet format. Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetInputFormat.java 000eb38 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/MapredParquetOutputFormat.java 8380117 ql/src/java/org/apache/hadoop/hive/ql/io/parquet/VectorizedParquetInputFormat.java 4e1820c ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRawRecordMerger.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/acid/ParquetRecordUpdater.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java 43c772f ql/src/java/org/apache/hadoop/hive/ql/io/parquet/read/ParquetRecordReaderWrapper.java 0a5edbb ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetStructObjectInspector.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/parquet/write/ParquetRecordWriterWrapper.java 0d32e49 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/AbstractTestParquetDirect.java 5f7f597 ql/src/test/org/apache/hadoop/hive/ql/io/parquet/acid/TestParquetRecordUpdater.java PRE-CREATION ql/src/test/queries/clientpositive/acid_parquet_insert.q PRE-CREATION ql/src/test/results/clientpositive/acid_parquet_insert.q.out PRE-CREATION Diff: https://reviews.apache.org/r/34473/diff/ Testing --- Newly added qtest and UT passed locally Thanks, cheng xu
Re: Review Request 33881: HIVE-10623 Implement hive cli options using beeline functionality
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33881/ --- (Updated May 14, 2015, 3:28 p.m.) Review request for hive and Xuefu Zhang. Bugs: HIVE-10623 https://issues.apache.org/jira/browse/HIVE-10623 Repository: hive-git Description --- Changes: 1. Support the hive cli options including database, e, !, H, f. 2. Add error handler for using f and e together 3. Add error handler for invalid option Diffs (updated) - beeline/src/java/org/apache/hive/beeline/BeeLine.java 0da15f6 beeline/src/java/org/apache/hive/beeline/cli/CliOptionsProcessor.java PRE-CREATION beeline/src/java/org/apache/hive/beeline/cli/HiveCli.java PRE-CREATION beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java PRE-CREATION beeline/src/test/resources/hive-site.xml PRE-CREATION Diff: https://reviews.apache.org/r/33881/diff/ Testing --- Newly add unit test passed locally. Thanks, cheng xu
Review Request 34248: HIVE-10684 Fix the unit test failures for HIVE-7553 after HIVE-10674 removed the binary jar files
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34248/ --- Review request for hive and Sushanth Sowmyan. Bugs: HIVE-10684 https://issues.apache.org/jira/browse/HIVE-10684 Repository: hive-git Description --- Remove binaries from source and fix the failed cases Diffs - ql/pom.xml f1a6f7d ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java 45ba07e ql/src/test/resources/RefreshedJarClassV1.txt PRE-CREATION ql/src/test/resources/RefreshedJarClassV2.txt PRE-CREATION Diff: https://reviews.apache.org/r/34248/diff/ Testing --- UT passed Thanks, cheng xu
Re: Review Request 34235: HIVE-10687 Fix avro deserialization issues for evolved unions
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34235/#review83883 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java https://reviews.apache.org/r/34235/#comment134958 tail space serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java https://reviews.apache.org/r/34235/#comment134959 tailing space serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java https://reviews.apache.org/r/34235/#comment134964 Do you need to cover null value case? serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java https://reviews.apache.org/r/34235/#comment134960 tailing spaces serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java https://reviews.apache.org/r/34235/#comment134962 remove space pls Some minor issues and a question - cheng xu On May 14, 2015, 10:07 p.m., Swarnim Kulkarni wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34235/ --- (Updated May 14, 2015, 10:07 p.m.) Review request for hive and Brock Noland. Bugs: HIVE-10687 https://issues.apache.org/jira/browse/HIVE-10687 Repository: hive-git Description --- HIVE-10687 Fix avro deserialization issues for evolved unions Diffs - serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java e94cd83c064199ba719cc2de222edd0e12401c8c serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java eb495b4e1fc5874b30936f646b5bdb5aa8734130 serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java c9e7d68b211ebc8c66af243fe85f4f89c6fd6cf3 Diff: https://reviews.apache.org/r/34235/diff/ Testing --- Thanks, Swarnim Kulkarni
