Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-12 Thread Alan Gates
On Sept. 9, 2014, 9:33 p.m., Eugene Koifman wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 11872 https://reviews.apache.org/r/25414/diff/1/?file=682029#file682029line11872 does this work if of implements a sublcass of AcidOutputFormat? Perhaps

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-12 Thread Alan Gates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/ --- (Updated Sept. 12, 2014, 7:14 p.m.) Review request for hive, Ashutosh Chauhan,

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-11 Thread Alan Gates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/ --- (Updated Sept. 11, 2014, 2:17 p.m.) Review request for hive, Ashutosh Chauhan,

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Alan Gates
On Sept. 9, 2014, 9:33 p.m., Eugene Koifman wrote: ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java, line 92 https://reviews.apache.org/r/25414/diff/1/?file=682021#file682021line92 is it because some form of write lock will be acquired on input? may be worth

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Alan Gates
On Sept. 10, 2014, 12:11 a.m., Thejas Nair wrote: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 305 https://reviews.apache.org/r/25414/diff/1/?file=682007#file682007line305 Pass another boolean param true to exclude it from the auto generated

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Alan Gates
On Sept. 10, 2014, 2:52 a.m., Thejas Nair wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java, line 74 https://reviews.apache.org/r/25414/diff/2/?file=682966#file682966line74 throw SemanticException here ? To me that's a RuntimeException. We

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Thejas Nair
On Sept. 6, 2014, 5:43 p.m., Brock Noland wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java, line 99 https://reviews.apache.org/r/25414/diff/1/?file=682028#file682028line99 looks like this can be final Alan Gates wrote: Sure, but why? I think

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Thejas Nair
On Sept. 10, 2014, 12:11 a.m., Thejas Nair wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 735 https://reviews.apache.org/r/25414/diff/2/?file=682964#file682964line735 fillDefaultStorageFormat uses the value of ConfVars.HIVEDEFAULTFILEFORMAT. If it

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Eugene Koifman
On Sept. 9, 2014, 9:33 p.m., Eugene Koifman wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java, line 46 https://reviews.apache.org/r/25414/diff/1/?file=682031#file682031line46 This should probably have @InterfaceAudience.Public

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-10 Thread Alan Gates
On Sept. 9, 2014, 9:33 p.m., Eugene Koifman wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 6108 https://reviews.apache.org/r/25414/diff/1/?file=682029#file682029line6108 Would ROWID.getTypeInfo() work? Seems to. On Sept. 9, 2014, 9:33 p.m., Eugene

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-09 Thread Eugene Koifman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52655 --- ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-09 Thread Thejas Nair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52559 --- common/src/java/org/apache/hadoop/hive/conf/HiveConf.java

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-09 Thread Thejas Nair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52810 ---

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-09 Thread Thejas Nair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52814 --- ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-08 Thread Alan Gates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/ --- (Updated Sept. 8, 2014, 11:37 p.m.) Review request for hive, Ashutosh Chauhan,

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-06 Thread Alan Gates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/ --- (Updated Sept. 6, 2014, 4:32 p.m.) Review request for hive, Ashutosh Chauhan,

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-06 Thread Alan Gates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/ --- (Updated Sept. 6, 2014, 4:32 p.m.) Review request for hive, Ashutosh Chauhan,

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-06 Thread Brock Noland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25414/#review52542 --- I obivously don't have context here but I do have a few items which

Re: Review Request 25414: HIVE-7788 Generate plans for insert, update, and delete

2014-09-06 Thread Alan Gates
On Sept. 6, 2014, 5:43 p.m., Brock Noland wrote: I obivously don't have context here but I do have a few items which I think should be addressed. Thx! Thanks for the review. On Sept. 6, 2014, 5:43 p.m., Brock Noland wrote: