Skip to content

Conversation

@bobhan1
Copy link
Contributor

@bobhan1 bobhan1 commented Aug 28, 2024

Proposed changes

picks #39958

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@bobhan1
Copy link
Contributor Author

bobhan1 commented Aug 28, 2024

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


Status Segment::lookup_row_key(const Slice& key, bool with_seq_col, bool with_rowid,
RowLocation* row_location) {
Status Segment::lookup_row_key(const Slice& key, const TabletSchema* latest_schema,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'lookup_row_key' exceeds recommended size/complexity thresholds [readability-function-size]

Status Segment::lookup_row_key(const Slice& key, const TabletSchema* latest_schema,
                ^
Additional context

be/src/olap/rowset/segment_v2/segment.cpp:695: 87 lines including whitespace and comments (threshold 80)

Status Segment::lookup_row_key(const Slice& key, const TabletSchema* latest_schema,
                ^

zhannngchen pushed a commit that referenced this pull request Aug 28, 2024
… column for merge-on-write table (#39958)

## Proposed changes
Currently, `BaseTablet::lookup_row_key()` use tablet_meta's schema to
decide whether a tablet has sequence column. But users can use `ALTER
TABLE tbl ENABLE FEATURE "SEQUENCE_LOAD" WITH ...` to add hidden
sequence column on MOW table. This is a light schema change which will
not change the BE's tablet meta, thus causing wrong behavior in
`BaseTablet::lookup_row_key()`.
This PR use the schema of the current load, which is the latest schema,
to decide whether a tablet has sequence column and correct the lookup
procedure in `BaseTablet::lookup_row_key()` and
`Segment::lookup_row_key()`.

branch-2.1-pick: #40010
branch-2.0-pick: #40015
@bobhan1
Copy link
Contributor Author

bobhan1 commented Aug 28, 2024

run buildall

@bobhan1
Copy link
Contributor Author

bobhan1 commented Aug 28, 2024

run beut

@dataroaring dataroaring merged commit bb709ad into apache:branch-2.1 Aug 28, 2024
dataroaring pushed a commit that referenced this pull request Sep 3, 2024
… column for merge-on-write table (#39958)

## Proposed changes
Currently, `BaseTablet::lookup_row_key()` use tablet_meta's schema to
decide whether a tablet has sequence column. But users can use `ALTER
TABLE tbl ENABLE FEATURE "SEQUENCE_LOAD" WITH ...` to add hidden
sequence column on MOW table. This is a light schema change which will
not change the BE's tablet meta, thus causing wrong behavior in
`BaseTablet::lookup_row_key()`.
This PR use the schema of the current load, which is the latest schema,
to decide whether a tablet has sequence column and correct the lookup
procedure in `BaseTablet::lookup_row_key()` and
`Segment::lookup_row_key()`.

branch-2.1-pick: #40010
branch-2.0-pick: #40015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants