> On 2010-06-22 15:34:57, John Sichi wrote: > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java, > > line 408 > > <http://review.hbase.org/r/223/diff/1/?file=1551#file1551line408> > > > > Rather than repeating the HiveConf.getVar in several places, it would > > be cleaner to just pass the configuration down into the Utilities method as > > the new parameter and have it do the configuration check.
I didn't that before, but changed the way in the patch later. The reason is that the getting the value of localMode is HiveConf.getVar is a hash lookup and and a string comparison. It is quite expensive if it is called many times. In the current patch, the HiveConf.getVar() and string comparison are called only once and passed to the for-loop. - Ning ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/223/#review268 ----------------------------------------------------------- On 2010-06-22 15:24:38, John Sichi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/223/ > ----------------------------------------------------------- > > (Updated 2010-06-22 15:24:38) > > > Review request for Hive Developers. > > > Summary > ------- > > jvs reviewing Ning's change > > > This addresses bug HIVE-1416. > http://issues.apache.org/jira/browse/HIVE-1416 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/JoinOperator.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/shims/src/0.17/java/org/apache/hadoop/hive/shims/Hadoop17Shims.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/shims/src/0.18/java/org/apache/hadoop/hive/shims/Hadoop18Shims.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/shims/src/0.19/java/org/apache/hadoop/hive/shims/Hadoop19Shims.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java > 956664 > > http://svn.apache.org/repos/asf/hadoop/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java > 956664 > > Diff: http://review.hbase.org/r/223/diff > > > Testing > ------- > > > Thanks, > > John > >
