You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Na Yang <ny...@maprtech.com> on 2014/09/03 06:45:35 UTC

Re: Review Request 25176: HIVE-7870: Insert overwrite table query does not generate correct task plan [Spark Branch]


> On Aug. 29, 2014, 5:30 p.m., Brock Noland wrote:
> > Hi Na,
> > 
> > Thank you very much for the patch! I have one high level question:
> > 
> > It appears we created the union_remove_spark* files because we wanted to add an additional property to the union_remove .q file? Meaning what is the delta beween union_remove_spark_1.q and union_remove_?
> > 
> > Cheers!
> 
> Na Yang wrote:
>     Hi Brock,
>     
>     That is correct. the union_remove_spark* files include an extra config property hive.merge.sparkfile comparing to the corresponding union_remove_* files. Except that extra config property, all other queries in the union_remove_spark* file are same as the queries in the union_remove_* file. 
>     
>     The hive.merge.sparkfile value is set according to the hive.merge.mapfile and hive.merge.mapredfile properity values in the orginal union_remove_* file. Regarding to the test result, we expect to see the same data are returned from the union_remove_spark* queries and the corresponding union_remove_* queries.
>     
>     Thanks,
>     Na
> 
> Brock Noland wrote:
>     Hi,
>     
>     Thank you very much for the information! I think instead of adding the new union_remove_spark tests we should just add the hive.merge.sparkfile property to the union_remove q files. The extra property won't impact the existng tests other than an extra line of outpit.
>     
>     If instead we'd like to keep the hive_remove_spark* properties then we'd need to add a check to QTestUtil that does not run spark files for MR:
>     
>     https://github.com/apache/hive/blob/trunk/itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java#L431
>     
>     as the tests are currently running for both spark and MR. As such, I think the first solution (just add the property to the existing tests) makes sense.
>     
>     Thoughts?
> 
> Na Yang wrote:
>     Hi Brock,
>     
>     Thank you for your suggestion. I also prefer the first solution. Let me modify the existing union_remove q files and re-genenrate the .q.out files for both MR and Spark.
>     
>     Thanks,
>     Na
> 
> Brock Noland wrote:
>     Awesome, thanks!!

Hi Brock,

I have uploaded a new patch according to our previously discussion by modifying the existing union_remove q files. Can you please review the new changes?

Thanks & Regards,
Na


- Na


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25176/#review51889
-----------------------------------------------------------


On Aug. 29, 2014, 8:59 p.m., Na Yang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25176/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 8:59 p.m.)
> 
> 
> Review request for hive, Brock Noland, Szehon Ho, and Xuefu Zhang.
> 
> 
> Bugs: HIVE-7870
>     https://issues.apache.org/jira/browse/HIVE-7870
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-7870: Insert overwrite table query does not generate correct task plan [Spark Branch]
> 
> The cause of this problem is during spark/tez task generation, the union file sink operator are cloned to two new filesink operator. The linkedfilesinkdesc info for those new filesink operators are missing. In addition, the two new filesink operators also need to be linked together.   
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties 88ef4f8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java 9c808d4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/spark/GenSparkProcContext.java 5ddc16d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/spark/GenSparkUtils.java 379a39c 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/spark/SparkCompiler.java 76fc290 
>   ql/src/test/queries/clientpositive/union_remove_1.q c87b3fe 
>   ql/src/test/queries/clientpositive/union_remove_10.q 6701952 
>   ql/src/test/queries/clientpositive/union_remove_11.q 4b2fa42 
>   ql/src/test/queries/clientpositive/union_remove_12.q 69d0d0a 
>   ql/src/test/queries/clientpositive/union_remove_13.q 7605f0e 
>   ql/src/test/queries/clientpositive/union_remove_14.q a4fdfc8 
>   ql/src/test/queries/clientpositive/union_remove_15.q e3c937b 
>   ql/src/test/queries/clientpositive/union_remove_16.q 537078b 
>   ql/src/test/queries/clientpositive/union_remove_17.q d70f3d3 
>   ql/src/test/queries/clientpositive/union_remove_18.q 6352bc3 
>   ql/src/test/queries/clientpositive/union_remove_19.q 8c45953 
>   ql/src/test/queries/clientpositive/union_remove_2.q 83cd288 
>   ql/src/test/queries/clientpositive/union_remove_20.q f80f7c1 
>   ql/src/test/queries/clientpositive/union_remove_21.q 8963c25 
>   ql/src/test/queries/clientpositive/union_remove_22.q b0c1ccd 
>   ql/src/test/queries/clientpositive/union_remove_23.q a1b989a 
>   ql/src/test/queries/clientpositive/union_remove_24.q ec561e0 
>   ql/src/test/queries/clientpositive/union_remove_25.q 76c1ff5 
>   ql/src/test/queries/clientpositive/union_remove_3.q 9617f73 
>   ql/src/test/queries/clientpositive/union_remove_4.q cae323b 
>   ql/src/test/queries/clientpositive/union_remove_5.q 5df84e1 
>   ql/src/test/queries/clientpositive/union_remove_6.q bfce26d 
>   ql/src/test/queries/clientpositive/union_remove_7.q 3a95674 
>   ql/src/test/queries/clientpositive/union_remove_8.q a83a43e 
>   ql/src/test/queries/clientpositive/union_remove_9.q e71f6dd 
>   ql/src/test/results/clientpositive/spark/union10.q.out 20c681e 
>   ql/src/test/results/clientpositive/spark/union18.q.out 3f37a0a 
>   ql/src/test/results/clientpositive/spark/union19.q.out 6922fcd 
>   ql/src/test/results/clientpositive/spark/union28.q.out 8bd5218 
>   ql/src/test/results/clientpositive/spark/union29.q.out b9546ef 
>   ql/src/test/results/clientpositive/spark/union3.q.out 3ae6536 
>   ql/src/test/results/clientpositive/spark/union30.q.out 12717a1 
>   ql/src/test/results/clientpositive/spark/union33.q.out b89757f 
>   ql/src/test/results/clientpositive/spark/union4.q.out 6341cd9 
>   ql/src/test/results/clientpositive/spark/union6.q.out 263d9f4 
>   ql/src/test/results/clientpositive/spark/union_remove_10.q.out 927a15d 
>   ql/src/test/results/clientpositive/spark/union_remove_11.q.out 96651e1 
>   ql/src/test/results/clientpositive/spark/union_remove_16.q.out 0954ae4 
>   ql/src/test/results/clientpositive/spark/union_remove_4.q.out cc46dda 
>   ql/src/test/results/clientpositive/spark/union_remove_5.q.out f6cdeb3 
>   ql/src/test/results/clientpositive/spark/union_remove_9.q.out 1f0260c 
> 
> Diff: https://reviews.apache.org/r/25176/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Na Yang
> 
>