You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2016/11/16 05:36:49 UTC
[kudu-CR] [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5104
Change subject: [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
[WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
This patch introduces a Decimal type to Kudu. It's based on the
column attributes, which are bits of information attached to a
ColumnSchema. For a Decimal column, the attributes are the
precision and scale of the Decimal column. Decimal attributes
determine
1. The server-side storage/physical type of the column, either
INT32 or INT64. This should be invisible to clients.
2. How clients should interpret the values of a decimal column.
No other types use attributes at this time.
The design of the Decimal column is based on Impala's decimal type.
The maximum precision is 18, the largest precision whose maximum
value can be stored in a 64-bit integer.
This patch contains only server-side and C++ client Decimal support.
Java support will follow.
Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
---
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client-unittest.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema-internal.h
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
A src/kudu/common/decimal_value-test.cc
A src/kudu/common/decimal_value.cc
A src/kudu/common/decimal_value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/server/webui_util.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
30 files changed, 1,156 insertions(+), 53 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5104/1
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
Patch Set 2:
(16 comments)
I, for one, welcome our new bot overlord.
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/client/client-unittest.cc
File src/kudu/client/client-unittest.cc:
Line 264: Status TestFunc(const MonoTime& deadline, bool* retry, int* counter) {
> warning: parameter 'deadline' is unused [misc-unused-parameters]
Pass
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/client/scan_batch.cc
File src/kudu/client/scan_batch.cc:
Line 184: } else if (col.type_info()->type() == DECIMAL64) {
> warning: don't use else after return [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:
Line 164: Status GetUnixTimeMicros(const Slice& col_name, int64_t* micros_since_utc_epoch)
> warning: function 'kudu::client::KuduScanBatch::RowPtr::GetUnixTimeMicros'
Done
Line 196: Status GetUnixTimeMicros(int col_idx, int64_t* micros_since_utc_epoch)
> warning: function 'kudu::client::KuduScanBatch::RowPtr::GetUnixTimeMicros'
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/decimal_value-test.cc
File src/kudu/common/decimal_value-test.cc:
Line 187: DecimalValue::MAX_UNSCALED_DECIMAL32 + 1, &overflow);
> warning: either cast from 'int' to 'int64_t' (aka 'long') is ineffective, o
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/partial_row.cc
File src/kudu/common/partial_row.cc:
Line 283: return Set<TypeTraits<DECIMAL32> >(col_idx, (int32_t) val.value());
> warning: C-style casts are discouraged; use static_cast [google-readability
Done
Line 284: } else if (col.type_info()->type() == DECIMAL64) {
> warning: don't use else after return [readability-else-after-return]
Done
Line 612: } else if (col.type_info()->type() == DECIMAL64) {
> warning: don't use else after return [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 93: Status SetUnixTimeMicros(const Slice& col_name,
> warning: function 'kudu::KuduPartialRow::SetUnixTimeMicros' has a definitio
Done
Line 123: Status SetUnixTimeMicros(int col_idx, int64_t micros_since_utc_epoch) WARN_UNUSED_RESULT;
> warning: function 'kudu::KuduPartialRow::SetUnixTimeMicros' has a definitio
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:
Line 20: #include <set>
> warning: #includes are not sorted properly [llvm-include-order]
Done
Line 73: } else {
> warning: don't use else after return [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/schema.h
File src/kudu/common/schema.h:
Line 170: attributes_(std::move(attributes)),
> warning: std::move of the variable of a trivially-copyable type has no effe
Done
Line 171: storage_attributes_(std::move(storage_attributes)) {
> warning: std::move of the variable of a trivially-copyable type has no effe
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:
Line 247: const ColumnAttributesPB attributesPB = pb.column_attributes();
> warning: the const qualified variable 'attributesPB' is copy-constructed fr
Done
http://gerrit.cloudera.org:8080/#/c/5104/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 866: ColumnSchema col = client_schema.column(i);
> warning: the variable 'col' is copy-constructed from a const reference but
Done
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has abandoned this change.
Change subject: KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5104
to look at the new patch set (#2).
Change subject: [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
[WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
This patch introduces a Decimal type to Kudu. It's based on the
column attributes, which are bits of information attached to a
ColumnSchema. For a Decimal column, the attributes are the
precision and scale of the Decimal column. Decimal attributes
determine
1. The server-side storage/physical type of the column, either
INT32 or INT64. This should be invisible to clients.
2. How clients should interpret the values of a decimal column.
No other types use attributes at this time.
The design of the Decimal column is based on Impala's decimal type.
The maximum precision is 18, the largest precision whose maximum
value can be stored in a 64-bit integer.
This patch contains only server-side and C++ client Decimal support.
Java support will follow.
Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
---
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client-unittest.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema-internal.h
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
A src/kudu/common/decimal_value-test.cc
A src/kudu/common/decimal_value.cc
A src/kudu/common/decimal_value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/server/webui_util.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
30 files changed, 1,157 insertions(+), 53 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5104/2
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5104
to look at the new patch set (#5).
Change subject: KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
KUDU-721 Support for Decimal type: Part 1 (C++)
This patch introduces a Decimal type to Kudu. It's based on
column attributes, which are bits of information attached to a
ColumnSchema. For a Decimal column, the attributes are the
precision and scale of the Decimal column. Decimal attributes
determine
1. The server-side storage/physical type of the column, either
INT32 or INT64. This should be invisible to clients.
2. How clients should interpret the values of a decimal column.
No other types use attributes at this time.
The design of the Decimal column is based on Impala's decimal type.
The maximum precision is 18, the largest precision whose maximum
value can be stored in a 64-bit integer.
This patch contains only server-side and C++ client Decimal support.
Java support will follow.
Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
---
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client-unittest.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema-internal.h
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
A src/kudu/common/decimal_value-test.cc
A src/kudu/common/decimal_value.cc
A src/kudu/common/decimal_value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/server/webui_util.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
30 files changed, 1,174 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5104/5
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5104
to look at the new patch set (#3).
Change subject: [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
[WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
This patch introduces a Decimal type to Kudu. It's based on the
column attributes, which are bits of information attached to a
ColumnSchema. For a Decimal column, the attributes are the
precision and scale of the Decimal column. Decimal attributes
determine
1. The server-side storage/physical type of the column, either
INT32 or INT64. This should be invisible to clients.
2. How clients should interpret the values of a decimal column.
No other types use attributes at this time.
The design of the Decimal column is based on Impala's decimal type.
The maximum precision is 18, the largest precision whose maximum
value can be stored in a 64-bit integer.
This patch contains only server-side and C++ client Decimal support.
Java support will follow.
Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
---
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client-unittest.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema-internal.h
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
A src/kudu/common/decimal_value-test.cc
A src/kudu/common/decimal_value.cc
A src/kudu/common/decimal_value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/server/webui_util.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
30 files changed, 1,163 insertions(+), 60 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5104/3
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5104
to look at the new patch set (#4).
Change subject: [WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
......................................................................
[WIP] KUDU-721 Support for Decimal type: Part 1 (C++)
This patch introduces a Decimal type to Kudu. It's based on
column attributes, which are bits of information attached to a
ColumnSchema. For a Decimal column, the attributes are the
precision and scale of the Decimal column. Decimal attributes
determine
1. The server-side storage/physical type of the column, either
INT32 or INT64. This should be invisible to clients.
2. How clients should interpret the values of a decimal column.
No other types use attributes at this time.
The design of the Decimal column is based on Impala's decimal type.
The maximum precision is 18, the largest precision whose maximum
value can be stored in a 64-bit integer.
This patch contains only server-side and C++ client Decimal support.
Java support will follow.
Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
---
M src/kudu/client/CMakeLists.txt
M src/kudu/client/client-unittest.cc
M src/kudu/client/predicate-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema-internal.h
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/value.cc
M src/kudu/client/value.h
M src/kudu/common/CMakeLists.txt
M src/kudu/common/common.proto
A src/kudu/common/decimal_value-test.cc
A src/kudu/common/decimal_value.cc
A src/kudu/common/decimal_value.h
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/types.cc
M src/kudu/common/types.h
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/log-test.cc
M src/kudu/integration-tests/all_types-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/server/webui_util.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
30 files changed, 1,174 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5104/4
--
To view, visit http://gerrit.cloudera.org:8080/5104
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f7829cdad883f5c124a437327062b2b45b6841b
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>