subject:"\[kudu\-CR\] Fix client comaptibility with gcc 4.4"

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Todd Lipcon (Code Review)

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Reviewed-on: http://gerrit.cloudera.org:8080/9257
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/key_encoder.h
M src/kudu/common/partial_row.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
12 files changed, 38 insertions(+), 19 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 10 Feb 2018 00:26:57 +
Gerrit-HasComments: No

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Grant Henke (Code Review)

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9257/4/src/kudu/util/int128.h
File src/kudu/util/int128.h:

http://gerrit.cloudera.org:8080/#/c/9257/4/src/kudu/util/int128.h@24
PS4, Line 24: defined(__clang__) || \
:   (defined(__GNUC__) && \
:   (__GNUC__ * 1 + __GNUC_MINOR__ * 100) >= 40600)
> this probably needs parens around the whole thing or else it would cause pr
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 09 Feb 2018 20:23:26 +
Gerrit-HasComments: Yes

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Grant Henke (Code Review)

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/key_encoder.h
M src/kudu/common/partial_row.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
12 files changed, 38 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/9257/5
--
To view, visit http://gerrit.cloudera.org:8080/9257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9257/4/src/kudu/util/int128.h
File src/kudu/util/int128.h:

http://gerrit.cloudera.org:8080/#/c/9257/4/src/kudu/util/int128.h@24
PS4, Line 24: defined(__clang__) || \
:   (defined(__GNUC__) && \
:   (__GNUC__ * 1 + __GNUC_MINOR__ * 100) >= 40600)
this probably needs parens around the whole thing or else it would cause 
problems in an expression like:

#if KUDU_INT128_SUPPORTED && BLAH_BLAH

(due to operator precedence the && would apply only to the second half of the 
conjunction)

Alternatively change to:

#if ...
#define KUDU_INT128_SUPPORTED 1
#else
#define KUDU_INT128_SUPPORTED 0
#endif



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 09 Feb 2018 19:40:42 +
Gerrit-HasComments: Yes

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-09 Thread Grant Henke (Code Review)

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/key_encoder.h
M src/kudu/common/partial_row.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
12 files changed, 34 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/9257/4
--
To view, visit http://gerrit.cloudera.org:8080/9257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-08 Thread Grant Henke (Code Review)

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/key_encoder.cc
File src/kudu/common/key_encoder.cc:

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/key_encoder.cc@58
PS2, Line 58: #ifdef KUDU_INT128_SUPPORTED
> for these changes in .cc files is this required? We only build the client l
Done


http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/types.h
File src/kudu/common/types.h:

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/types.h@327
PS2, Line 327: #ifdef KUDU_INT128_SUPPORTED
> This header isn't part of the client library so I don't think it should be
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 09 Feb 2018 02:38:53 +
Gerrit-HasComments: Yes

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-08 Thread Grant Henke (Code Review)

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
13 files changed, 35 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/9257/3
--
To view, visit http://gerrit.cloudera.org:8080/9257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-08 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9257 )

Change subject: Fix client comaptibility with gcc 4.4
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/key_encoder.cc
File src/kudu/common/key_encoder.cc:

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/key_encoder.cc@58
PS2, Line 58: #ifdef KUDU_INT128_SUPPORTED
for these changes in .cc files is this required? We only build the client lib 
with compilers that support int128, so we should only need the changes in the 
headers that are included by end-user code


http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/types.h
File src/kudu/common/types.h:

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/common/types.h@327
PS2, Line 327: #ifdef KUDU_INT128_SUPPORTED
This header isn't part of the client library so I don't think it should be 
necessary (you can check the install() directives in client/CMakeLists.txt to 
see which ones get installed)


http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/util/int128.h
File src/kudu/util/int128.h:

http://gerrit.cloudera.org:8080/#/c/9257/2/src/kudu/util/int128.h@26
PS2, Line 26: #define KUDU_INT128_SUPPORTED
it might be safer to #define this to either 0 or 1 and then use #if 
KUDU_INT128_SUPPORTED everywehre instead of ifdef. That way you won't 
accidentally forgot to include int128.h and presume that it's not supported



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 09 Feb 2018 01:09:14 +
Gerrit-HasComments: Yes

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-08 Thread Grant Henke (Code Review)

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/column_predicate.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/decimal_util.cc
M src/kudu/util/decimal_util.h
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
22 files changed, 110 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/9257/2
--
To view, visit http://gerrit.cloudera.org:8080/9257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix client comaptibility with gcc 4.4

2018-02-08 Thread Grant Henke (Code Review)

Grant Henke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9257


Change subject: Fix client comaptibility with gcc 4.4
..

Fix client comaptibility with gcc 4.4

We expect the client lib to work with gcc 4.4 and C++98.
When the int128 type was added this broke compatibility
because __int128 support didn’t exist until gcc 4.6.

To work around this and still allow gcc 4.6+ and C++11
compilation to support int128, a preprocessor check
was added to define KUDU_INT128_SUPPORTED
which is used to exclude the incompatible client code.

Additionally the int128 insertion operator definitions
were moved to their own file to ensure they are not
required when working with Kudu’s int128 types.
This ensures there aren’t compatibily issues with clients
that define their own __int128 insertion operator.

Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
---
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/value-internal.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/column_predicate.cc
M src/kudu/common/key_encoder.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/key_util.cc
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/util/CMakeLists.txt
M src/kudu/util/decimal_util.cc
M src/kudu/util/decimal_util.h
M src/kudu/util/int128-test.cc
M src/kudu/util/int128.h
R src/kudu/util/int128_util.h
19 files changed, 103 insertions(+), 24 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/9257/1
--
To view, visit http://gerrit.cloudera.org:8080/9257
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I59b40a9718b321df1a5878160ac845d4cf3d9170
Gerrit-Change-Number: 9257
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

[kudu-CR] Fix client comaptibility with gcc 4.4

11 matches

Site Navigation

Mail list logo

Footer information