You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/10 14:06:16 UTC
[GitHub] [incubator-doris] cambyzju opened a new pull request, #10070: [enhancement] Change array offset type from UInt32 to Int64
cambyzju opened a new pull request, #10070:
URL: https://github.com/apache/incubator-doris/pull/10070
# Proposed changes
Issue Number: close #7570
## Problem Summary:
Now column `Array<T>` contains column `offsets` and `data`, and type of column `offsets` is UInt32 now.
In compute layer, if we call array_union to merge arrays, the size may overflow.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#issuecomment-1157183563
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#discussion_r897790755
##########
be/src/runtime/collection_value.h:
##########
@@ -123,21 +123,21 @@ class CollectionValue {
const bool* null_signs() const { return _null_signs; }
void* mutable_data() { return _data; }
bool* mutable_null_signs() { return _null_signs; }
- void set_length(uint32_t length) { _length = length; }
+ void set_length(int64_t length) { _length = length; }
void set_has_null(bool has_null) { _has_null = has_null; }
void set_data(void* data) { _data = data; }
void set_null_signs(bool* null_signs) { _null_signs = null_signs; }
private:
using AllocateMemFunc = std::function<uint8_t*(size_t size)>;
static Status init_collection(CollectionValue* value, const AllocateMemFunc& allocate,
- uint32_t size, PrimitiveType child_type);
+ int64_t size, PrimitiveType child_type);
ArrayIterator internal_iterator(PrimitiveType child_type) const;
private:
// child column data
void* _data;
- uint32_t _length;
+ int64_t _length;
Review Comment:
done
##########
be/src/runtime/collection_value.h:
##########
@@ -160,7 +160,7 @@ class ArrayIterator {
}
return false;
}
- bool seek(uint32_t n) const {
+ bool seek(int64_t n) const {
Review Comment:
u64
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on a diff in pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
HappenLee commented on code in PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#discussion_r898649471
##########
be/src/vec/columns/column_vector.cpp:
##########
@@ -327,7 +327,7 @@ ColumnPtr ColumnVector<T>::replicate(const IColumn::Offsets& offsets) const {
// vectorized this code to speed up
IColumn::Offset counts[size];
- for (size_t i = 0; i < size; ++i) {
+ for (ssize_t i = 0; i < size; ++i) {
Review Comment:
size is `size_t`, not need change `i` to `sszie_t`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] github-actions[bot] commented on pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#issuecomment-1157183549
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#discussion_r898656705
##########
be/src/vec/columns/column_vector.cpp:
##########
@@ -327,7 +327,7 @@ ColumnPtr ColumnVector<T>::replicate(const IColumn::Offsets& offsets) const {
// vectorized this code to speed up
IColumn::Offset counts[size];
- for (size_t i = 0; i < size; ++i) {
+ for (ssize_t i = 0; i < size; ++i) {
Review Comment:
We use `offsets[i - 1]` inside this for loop, so it is better to use `ssize_t` for `index -1`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on pull request #10070: [enhancement] Change array offset type from UInt32 to Int64
Posted by GitBox <gi...@apache.org>.
cambyzju commented on PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#issuecomment-1154678492
>
int64 is already enough, and JAVA do not have uint64.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #10070: [enhancement] Change array offset type from UInt32 to UInt64
Posted by GitBox <gi...@apache.org>.
morningman merged PR #10070:
URL: https://github.com/apache/doris/pull/10070
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju closed pull request #10070: [enhancement] Change array offset type from UInt32 to Int64
Posted by GitBox <gi...@apache.org>.
cambyzju closed pull request #10070: [enhancement] Change array offset type from UInt32 to Int64
URL: https://github.com/apache/incubator-doris/pull/10070
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] adonis0147 commented on a diff in pull request #10070: [enhancement] Change array offset type from UInt32 to Int64
Posted by GitBox <gi...@apache.org>.
adonis0147 commented on code in PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#discussion_r896319479
##########
be/src/runtime/collection_value.h:
##########
@@ -123,21 +123,21 @@ class CollectionValue {
const bool* null_signs() const { return _null_signs; }
void* mutable_data() { return _data; }
bool* mutable_null_signs() { return _null_signs; }
- void set_length(uint32_t length) { _length = length; }
+ void set_length(int64_t length) { _length = length; }
void set_has_null(bool has_null) { _has_null = has_null; }
void set_data(void* data) { _data = data; }
void set_null_signs(bool* null_signs) { _null_signs = null_signs; }
private:
using AllocateMemFunc = std::function<uint8_t*(size_t size)>;
static Status init_collection(CollectionValue* value, const AllocateMemFunc& allocate,
- uint32_t size, PrimitiveType child_type);
+ int64_t size, PrimitiveType child_type);
ArrayIterator internal_iterator(PrimitiveType child_type) const;
private:
// child column data
void* _data;
- uint32_t _length;
+ int64_t _length;
Review Comment:
Using `uint64_t` is better. For some places which need signed value, we use signed value there.
##########
be/src/runtime/collection_value.h:
##########
@@ -160,7 +160,7 @@ class ArrayIterator {
}
return false;
}
- bool seek(uint32_t n) const {
+ bool seek(int64_t n) const {
Review Comment:
If using `int64_t` here, the function `seek` should check whether `n` is negative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee commented on pull request #10070: [enhancement] Change array offset type from UInt32 to Int64
Posted by GitBox <gi...@apache.org>.
HappenLee commented on PR #10070:
URL: https://github.com/apache/incubator-doris/pull/10070#issuecomment-1154670790
why int64 not uint64？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org