Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]fix the crash of checksum task #3735 #3738

Merged
merged 4 commits into from
Jun 10, 2020

Conversation

marising
Copy link
Contributor

@marising marising commented Jun 1, 2020

  1. the table include key column 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

#3735

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
@marising marising changed the title fix the crash of checksum task [Bug]fix the crash of checksum task #3735 Jun 1, 2020
@morningman
Copy link
Contributor

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.

@morningman morningman added area/storage Issues or PRs related to storage engine kind/fix Categorizes issue or PR as related to a bug. labels Jun 1, 2020
@marising
Copy link
Contributor Author

marising commented Jun 1, 2020

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 | |
....
+--------------+-------------+------+-------+---------+-----------

@imay
Copy link
Contributor

imay commented Jun 1, 2020

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.

be/src/olap/row.h Outdated Show resolved Hide resolved
@morningman
Copy link
Contributor

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?

@imay
Copy link
Contributor

imay commented Jun 1, 2020

@morningman I'm not sure about it either. However I think it is OK to make double/float column as a sort key.

@marising
Copy link
Contributor Author

marising commented Jun 1, 2020

Aggregate Key, not Duplicate Key @morningman

CREATE TABLE xxx_sku_AB (
......
price double NULL COMMENT "",
......
) ENGINE=OLAP
...
AGGREGATE KEY(..., price, ...)
...

@marising
Copy link
Contributor Author

marising commented Jun 2, 2020

  1. 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

@marising marising requested a review from imay June 3, 2020 03:48
be/src/olap/row.h Outdated Show resolved Hide resolved
imay
imay previously approved these changes Jun 5, 2020
Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imay imay added the approved Indicates a PR has been approved by one committer. label Jun 5, 2020
@morningman morningman removed the approved Indicates a PR has been approved by one committer. label Jun 5, 2020
@morningman
Copy link
Contributor

hi @marising , if you finished this PR, please let me know, thank u.

@marising
Copy link
Contributor Author

marising commented Jun 6, 2020

@morningman Ok,I'll tell you, if our internal review passes.

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morningman morningman added the approved Indicates a PR has been approved by one committer. label Jun 9, 2020
@morningman morningman merged commit ef94c25 into apache:master Jun 10, 2020
morningman pushed a commit to morningman/doris that referenced this pull request Jun 22, 2020
1. the table include key column 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

apache#3735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. area/storage Issues or PRs related to storage engine kind/fix Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants