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 2020/06/01 07:48:49 UTC
[GitHub] [incubator-doris] marising opened a new pull request #3738: fix the crash of checksum task
marising opened a new pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738
1. the table include key colum of double/float type
2. when run checksum task, will use all of key columns to compare
3. schema.column(idx) of double/float type is NULL
----------------------------------------------------------------
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.
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] imay commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
imay commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636901913
> 2\. The table of customer
> alter table add column xxx double key null after xxx，but no error message and execute success!
@marising
Should create an issue to fix it.
----------------------------------------------------------------
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.
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] morningman commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-639541105
hi @marising , if you finished this PR, please let me know, thank u.
----------------------------------------------------------------
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.
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] marising commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-637211321
> > 2. The table of customer
> > alter table add column xxx double key null after xxx，but no error message and execute success!
>
> @marising
> Should create an issue to fix it.
I can't reproduce this problem in the online cluster of customer, and the time for adding columns is a little long audit.log be cleaned up, I will pay attention to this problem
----------------------------------------------------------------
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.
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] marising commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-639932459
@morningman Ok，I'll tell you, if our internal review passes.
----------------------------------------------------------------
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.
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] marising commented on a change in pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on a change in pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#discussion_r433302450
##########
File path: be/src/olap/row.h
##########
@@ -59,6 +59,10 @@ bool equal_row(const std::vector<uint32_t>& ids,
template<typename LhsRowType, typename RhsRowType>
int compare_row(const LhsRowType& lhs, const RhsRowType& rhs) {
for (uint32_t cid = 0; cid < lhs.schema()->num_key_columns(); ++cid) {
+ //because the num_column_ids include the column of double/float type
Review comment:
Ignore the column of double/float type in engine_checksum_task.cpp, but num_key_columns include the double/float column index.
In other ways, I can only add related functions such as reader.init, compare function for sort and aggregation function for checksum. The code volume is a little large.
```
// ignore float and double type considering to precision lose
for (size_t i = 0; i < tablet->tablet_schema().num_columns(); ++i) {
FieldType type = tablet->tablet_schema().column(i).type();
if (type == OLAP_FIELD_TYPE_FLOAT || type == OLAP_FIELD_TYPE_DOUBLE) {
continue;
}
reader_params.return_columns.push_back(i);
}
```
----------------------------------------------------------------
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.
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] morningman commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636848312
This CL looks good to me.
But I had a question in your issue: #3735, that why key columns include float or double.
We don't support using float or double column as key column.
----------------------------------------------------------------
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.
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] marising edited a comment on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising edited a comment on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636877119
It's very confusing for me，doris does not support the key column as double or float，but I've met twice.
1. I test alter table add double key column
MySQL [checksum]> alter table catalog_sales modify column `cs_sales_price` double KEY null after `cs_item_sk`;
ERROR 1064 (HY000): Float or double can not used as a key, use decimal instead.
2. The table of customer
alter table add column xxx double key null after xxx，but no error message and execute success!
MySQL [search]> desc xxx_sku_AB;
+--------------+-------------+------+-------+---------+-----------+
| Field | Type | Null | Key | Default | Extra |
+--------------+-------------+------+-------+---------+-----------+
...
| price | DOUBLE | Yes | true | N/A | |
....
+--------------+-------------+------+-------+---------+-----------
----------------------------------------------------------------
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.
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] imay commented on a change in pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#discussion_r434973104
##########
File path: be/src/olap/row.h
##########
@@ -186,7 +186,12 @@ void agg_finalize_row(const std::vector<uint32_t>& ids, RowType* row, MemPool* m
template<typename RowType>
uint32_t hash_row(const RowType& row, uint32_t seed) {
+ FieldType type;
for (uint32_t cid : row.schema()->column_ids()) {
+ type = row.schema()->column(cid)->type();
Review comment:
Should add comment for this `if` logical to let others know why.
----------------------------------------------------------------
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.
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] morningman merged pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738
----------------------------------------------------------------
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.
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] marising edited a comment on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising edited a comment on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636912790
Aggregate Key, not Duplicate Key
CREATE TABLE `xxx_sku_AB` (
......
`price` double NULL COMMENT "",
......
) ENGINE=OLAP
...
AGGREGATE KEY(..., `price`, ...)
...
----------------------------------------------------------------
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.
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] imay commented on a change in pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#discussion_r433283161
##########
File path: be/src/olap/row.h
##########
@@ -59,6 +59,10 @@ bool equal_row(const std::vector<uint32_t>& ids,
template<typename LhsRowType, typename RhsRowType>
int compare_row(const LhsRowType& lhs, const RhsRowType& rhs) {
for (uint32_t cid = 0; cid < lhs.schema()->num_key_columns(); ++cid) {
+ //because the num_column_ids include the column of double/float type
Review comment:
This function is used in many senarios. And changing here to fix the problem is not a good idea.
Why not to make column in the schema is not NULL.
----------------------------------------------------------------
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.
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] marising commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636877119
It's very confusing for me，doris does not support the key column as double or float，but I've met twice.
1. I create table and alter table add column
MySQL [checksum]> alter table catalog_sales modify column `cs_sales_price` double KEY null after `cs_item_sk`;
ERROR 1064 (HY000): Float or double can not used as a key, use decimal instead.
2. The table of customer
alter table add column xxx double key null after xxx，but no error message and execute success!
MySQL [search]> desc xxx_sku_AB;
+--------------+-------------+------+-------+---------+-----------+
| Field | Type | Null | Key | Default | Extra |
+--------------+-------------+------+-------+---------+-----------+
...
| price | DOUBLE | Yes | true | N/A | |
....
+--------------+-------------+------+-------+---------+-----------
----------------------------------------------------------------
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.
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] morningman commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636906110
> 2\. The table of customer
> alter table add column xxx double key null after xxx，but no error message and execute success!
I guess the table `customer` is a DUPLICATE KEY table, right? So that the `key` in `alter table` stmt is not exactly a `key` column, but just a sort column.
I think its ok to allow a float type column to be a sort column in DUPLICATE KEY table.
@imay , is that right?
----------------------------------------------------------------
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.
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] imay commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
imay commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636907424
@morningman I'm not sure about it either. However I think it is OK to make double/float column as a sort key.
----------------------------------------------------------------
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.
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] marising commented on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636912790
CREATE TABLE `xxx_sku_AB` (
......
`price` double NULL COMMENT "",
......
) ENGINE=OLAP
...
AGGREGATE KEY(..., `price`, ...)
...
----------------------------------------------------------------
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.
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] marising edited a comment on pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising edited a comment on pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#issuecomment-636912790
Aggregate Key, not Duplicate Key @morningman
CREATE TABLE `xxx_sku_AB` (
......
`price` double NULL COMMENT "",
......
) ENGINE=OLAP
...
AGGREGATE KEY(..., `price`, ...)
...
----------------------------------------------------------------
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.
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] marising commented on a change in pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
marising commented on a change in pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#discussion_r433856840
##########
File path: be/src/olap/row.h
##########
@@ -59,6 +59,10 @@ bool equal_row(const std::vector<uint32_t>& ids,
template<typename LhsRowType, typename RhsRowType>
int compare_row(const LhsRowType& lhs, const RhsRowType& rhs) {
for (uint32_t cid = 0; cid < lhs.schema()->num_key_columns(); ++cid) {
+ //because the num_column_ids include the column of double/float type
Review comment:
1. remove the code of ignore the column of float/double type's in engine_checksum.cpp
2. ignore the cell of float/double type when compute row's hash code
3. the value of checksum is independent of the sorting of data rows
----------------------------------------------------------------
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.
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] morningman commented on a change in pull request #3738: [Bug]fix the crash of checksum task #3735
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3738:
URL: https://github.com/apache/incubator-doris/pull/3738#discussion_r435191500
##########
File path: be/src/olap/task/engine_checksum_task.cpp
##########
@@ -118,8 +113,10 @@ OLAPStatus EngineChecksumTask::_compute_checksum() {
OLAP_LOG_WARNING("fail to read in reader. [res=%d]", res);
return res;
}
-
- row_checksum = hash_row(row, row_checksum);
+ one_checksum = 0;
Review comment:
`one_checksum = 0;` this line is meaningless.
----------------------------------------------------------------
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.
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