Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-17 Thread Mohit Sabharwal
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review61847 --- Ship it! Thanks for making the changes! - Mohit Sabharwal On Nov

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-15 Thread cheng xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/ --- (Updated Nov. 16, 2014, 3:52 a.m.) Review request for hive. Changes ---

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-15 Thread cheng xu
> On Nov. 16, 2014, 12:49 a.m., Mohit Sabharwal wrote: > > Thank you for making the changes! I had a few more comments on error > > handling and readability. But otherwise, looks good to me. Thanks! > > > > Could you please add import statements for all the inner classes? This > > makes things

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-15 Thread Mohit Sabharwal
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review61668 --- Thank you for making the changes! I had a few more comments on error

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-14 Thread cheng xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/ --- (Updated Nov. 15, 2014, 2:51 a.m.) Review request for hive. Changes ---

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-14 Thread cheng xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/ --- (Updated Nov. 14, 2014, 3:55 p.m.) Review request for hive. Changes ---

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-14 Thread cheng xu
> On Nov. 14, 2014, 12:33 a.m., Mohit Sabharwal wrote: > > Had a few high level questions. Will take another pass after that. I'd > > appreciate adding more comments to the code. Thanks for your review. I added some inline comments below and update the patch. > On Nov. 14, 2014, 12:33 a.m., M

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-13 Thread Mohit Sabharwal
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review61267 --- Had a few high level questions. Will take another pass after that. I

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-03 Thread Szehon Ho
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review59711 --- ql/src/java/org/apache/hadoop/hive/ql/io/parquet/FilterPredicateLea

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-03 Thread cheng xu
> On Nov. 1, 2014, 8:21 p.m., Brock Noland wrote: > > serde/src/java/org/apache/hadoop/hive/ql/io/sarg/PredicateLeaf.java, line 58 > > > > > > Perhaps we can change the Type enum to seperate out the types we need > > a

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-11-01 Thread Brock Noland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review59500 --- Hi, This approach looks great! I think we should try and avoid crea

Re: Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-10-28 Thread Brock Noland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/#review58914 --- Hi, I looked into this a little more, and I think that you should a

Review Request 26968: HIVE-8122: convert ExprNode to Parquet supported FilterPredict

2014-10-21 Thread cheng xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26968/ --- Review request for hive. Repository: hive-git Description --- HIVE-8122: