> On Aug. 28, 2014, 7:56 a.m., Prasanth_J wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java, 
> > line 202
> > <https://reviews.apache.org/r/24472/diff/1/?file=655367#file655367line202>
> >
> >     Why not just use Warehouse.getFileStatusesForSD(tbl.getSd())? It does 
> > the same thing.

True, this does seem to do the same thing. Will use 
Warehouse.getFileStatusesForSD(), though with your suggestion below this method 
will disappear.


> On Aug. 28, 2014, 7:56 a.m., Prasanth_J wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java, 
> > line 239
> > <https://reviews.apache.org/r/24472/diff/1/?file=655367#file655367line239>
> >
> >     If I undestand correctly, the difference between this method and the 
> > one below is FileStatus[]. If so factor out the common code and pass 
> > FileStatus[] as parameter. In case of tempTables you can use 
> > WareHouse.getFileStatusesFromSD() API to get FileStatus[]. Correct me if I 
> > am wrong.

Good, suggestion, I think this should work.


> On Aug. 28, 2014, 7:56 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java,
> >  line 396
> > <https://reviews.apache.org/r/24472/diff/1/?file=655368#file655368line396>
> >
> >     Is there any reason why you are not using FieldSchema's equals() here?

FieldSchema.equals() also compares the column comment, which could be changed 
during alter table. If just the column comment changed the columns are still 
relatively similar.


> On Aug. 28, 2014, 7:56 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java,
> >  line 391
> > <https://reviews.apache.org/r/24472/diff/1/?file=655368#file655368line391>
> >
> >     You can reuse the oldCols, newCols List above instead of using 
> > iterator. idx in the for loop is unused.

Thought the iterators would be better depending on what kind of List was used. 
I can redo the loop without using idx.


- Jason


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


On Aug. 26, 2014, 6:37 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24472/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 6:37 p.m.)
> 
> 
> Review request for hive and Prasanth_J.
> 
> 
> Bugs: HIVE-7649
>     https://issues.apache.org/jira/browse/HIVE-7649
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Update SessionHiveMetastoreClient to get column stats to work for temp tables.
> 
> 
> Diffs
> -----
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 5a56ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
>  37b1669 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 24f3710 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java fcfcf42 
>   ql/src/test/queries/clientnegative/temp_table_column_stats.q 9b7aa4a 
>   ql/src/test/queries/clientpositive/temp_table_display_colstats_tbllvl.q 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/temp_table_column_stats.q.out 4b0c0bc 
>   ql/src/test/results/clientpositive/temp_table_display_colstats_tbllvl.q.out 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24472/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Reply via email to