Skip to content

Support add new key column for LinkedSchemaChange#432

Merged
chaoyli merged 2 commits intoapache:masterfrom
kangkaisen:add-key-column
Dec 14, 2018
Merged

Support add new key column for LinkedSchemaChange#432
chaoyli merged 2 commits intoapache:masterfrom
kangkaisen:add-key-column

Conversation

@kangkaisen
Copy link
Copy Markdown
Contributor

Fix 332

In PR 337, I didn't think about this case: user add a new key column to base table.

This PR supports add new key column for LinkedSchemaChange.

I'm not sure SegmentGroup include "olap/schema_change.h" is reasonable and the code smell is good.

@chaoyli Please review this PR. Thanks you.

Comment thread be/src/olap/segment_group.cpp Outdated
if (schema_mapping[i].ref_column == -1) {
num_new_keys++;

if (true == column_schema.is_allow_null && column_schema.default_value.length() == 0) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

first->copy(schema_mapping[i].default_value);
second->copy(schema_mapping[i].default_value);
will be ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK.

@kangkaisen
Copy link
Copy Markdown
Contributor Author

Update the commit. I wanted to move ColumnMapping to olap_common.h initially, but I found the ColumnMapping relayed wrapper_field. So I moved ColumnMapping to a single file.

}
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

empty line

}
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

empty line

@chaoyli chaoyli merged commit 0bedd33 into apache:master Dec 14, 2018
GoGoWen pushed a commit to GoGoWen/incubator-doris that referenced this pull request Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants