[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged.

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


IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Reviewed-on: http://gerrit.cloudera.org:8080/7414
Reviewed-by: Jim Apple 
Tested-by: Impala Public Jenkins
---
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/strings/join.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
7 files changed, 37 insertions(+), 185 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Jim Apple: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

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


Patch Set 6: Verified+1

-- 
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: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-15 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 6: Code-Review+2

-- 
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: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-15 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 6:

Thanks for fixing those last comments! I'm starting the pre-merge job now; it 
takes roughly four hours, and it will commit this patch if it passes all of the 
tests.

-- 
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: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-15 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 5: Code-Review+2

-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-14 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change.

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


Patch Set 5: Code-Review+1

(5 comments)

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

Line 101
Unnecessary include because of std::hash


Line 117
MSVC stuff is removed


Line 242
MSVC stuff is removed


Line 305
MSVC stuff is removed


Line 318
MSVC stuff is removed


-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-14 Thread Kim Jin Chul (Code Review)
Hello Tim Armstrong,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7414

to look at the new patch set (#5).

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

IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
---
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/strings/join.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
7 files changed, 37 insertions(+), 185 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-11 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change.

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


Patch Set 4:

(2 comments)

Sorry for the late. Let me answer Jim's comment soon.

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

PS3, Line 96: using std::hash;
: 
> What code requires this remain?
I answered in the PS#4


Line 189: 
> What code requires that this section remain?
I answered in the PS#4


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

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


Patch Set 4:

Did you have a chance to look at Jim's comments? We are very close to getting 
this merged.

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-01 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 4:

(5 comments)

I may not have expressed my previous comments in a simple way. Let me try again.

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

Line 96: using std::hash;
Can these two lines be deleted?


Line 187: #include 
Can this line be deleted?


Line 208:   bool operator()(const uint128& a, const uint128& b) const {
Can this MSCV stuff be deleted?


Line 215: // MSVC's STL requires an ever-so slightly different decl
Can this MSVC stuff be deleted?


Line 244:   bool operator()(const pair& a,
Can this MSVC stuff be deleted?


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-08-01 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

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


Patch Set 4: Code-Review+1

The change looks good to me. Thanks for contributing. It looks like you 
addressed Jim's remaining comments, but I'll let him confirm.

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-26 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded a new patch set (#4).

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

IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
---
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/strings/join.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
7 files changed, 37 insertions(+), 154 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-22 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 3:

(2 comments)

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

PS3, Line 96: #include 
: using std::hash;
What code requires this remain?


Line 189: namespace std {
What code requires that this section remain?


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-19 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

Line 278
> I looked at your comment. I did not respond yet because it is premature for
I found the specialized code from 218 to 328 is unused. My conclusion is the 
code can be removed because of the reason below. I believe there is no logical 
issue on my change.

1. hash functions are only used in hash_map/hash_set
(Checking with the command: git grep __gnu_cxx src)
2. hash_map/hash_set => unordered_map/unordered_set
The key type is only 'string'. std::hash is specialized in C++11.


-- 
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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-18 Thread Kim Jin Chul (Code Review)
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,
00083class _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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-17 Thread Jim Apple (Code Review)
Jim Apple 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 guess the reason is that __gnu_cxx::hash<> was not used anywhere.  I didn
I think you might be right. Where was it used before your patch? In the 
hash_set/hash_map definitions?


Line 278
> I got it. My idea is that some specializations in __gnu_cxx should be moved
My suggestion is to dig into this library and understand the details of what 
hash functions are used where. As-is, it seems a little bit like you are trying 
things just to see what seems to work, which is a dangerous way to live in C++.


-- 
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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-13 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has posted comments on this change.

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


Patch Set 3:

(2 comments)

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

PS3, Line 218: 
As I told in the previous comment, _gnu_ctx is replaced into std.

https://gerrit.cloudera.org/#/c/7414/1/be/src/gutil/hash/hash.h@a278


Line 251
You may know std already defined pointer type as below.

   /// Partial specializations for pointer types.
   template
 struct hash<_Tp*> : public __hash_base
 {
   size_t
   operator()(_Tp* __p) const noexcept
   { return reinterpret_cast(__p); }
 };

I guess your hash function is more faster than std's implementation, but there 
are pros and cons. I found the interesting article: 
https://stackoverflow.com/questions/20953390/what-is-the-fastest-hash-function-for-pointers

I have the following issue on this. Please answer it.

How to implement a customized hash specialization for pointer type.
I removed your code because it cannot be redefined. I think we need to 
introduce a new template class which extends std::hash if we want to keep 
own hash.


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-13 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded a new patch set (#3).

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

IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
---
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/strings/join.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
7 files changed, 37 insertions(+), 121 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-13 Thread Kim Jin Chul (Code Review)
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
> OK, so it looks like you've added it back in. Before we can decide if it sh
I guess the reason is that __gnu_cxx::hash<> was not used anywhere.  I didn't 
find any use case for this. Would you please double check?


Line 278
> So, do you need it to still exist, and if so, how do you make sure std::has
I got it. My idea is that some specializations in __gnu_cxx should be moved to 
std namespace. I can be sure this will not create a potential problem. Do you 
agree on this? Or any suggestion?


-- 
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 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-13 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 2:

(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
> You're right. std::hash template class is specialized between 1 byte and 8 
OK, so it looks like you've added it back in. Before we can decide if it should 
stay or not, let'd understand why it didn't break the build before.


Line 278
> You're right. It should make wrong result or corruption.
So, do you need it to still exist, and if so, how do you make sure std::hash, 
which may not be __gnu_cxx::hash, is specialized?


-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil

2017-07-13 Thread Kim Jin Chul (Code Review)
Kim Jin Chul has uploaded a new patch set (#2).

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

IMPALA-5116: Remove deprecated hash_* types in gutil

The following class templates are substituted from C++11 standard.
__gnu_cxx::hash_map => std::unordered_map
__gnu_cxx::hash_set => std::unordered_set
__gnu_cxx::hash => std::hash

Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
---
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/strings/join.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
7 files changed, 35 insertions(+), 106 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Jim Apple