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

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

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 more

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-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., Mohit

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
--- 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-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.

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 https://reviews.apache.org/r/26968/diff/2/?file=745523#file745523line58 Perhaps we can change the Type enum to seperate out the types we need and then

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 ---

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

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

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 ---