Kim Jin Chul has posted comments on this change.

Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h
File be/src/gutil/hash/hash.h:

Line 222
> I think you might be right. Where was it used before your patch? In the has
Yes, it seems the specializations for hash are only used in the hash_* 
container.

e.g.
00082   template<class _Key, class _Tp, class _HashFn = hash<_Key>,
00083        class _EqualKey = equal_to<_Key>, class _Alloc = allocator<_Tp> >
00084     class hash_map

By the way, I have not found any use case to create an object for hash_* 
container, so the code is now unused. Do you think we should leave it to use it 
later? Or it is better to remove the code to improve code coverage?


Line 278
> My suggestion is to dig into this library and understand the details of wha
I understand what you worry about. Let me look into the code more deeply. 

By the way, did you check my comment in the previous patch set? It would be 
nice if you take a look at the comment: 
https://gerrit.cloudera.org/#/c/7414/3/be/src/gutil/hash/hash.h@a251


-- 
To view, visit http://gerrit.cloudera.org:8080/7414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com>
Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to