-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[refine](column) remove IDataType get_default #62582
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -448,7 +448,7 @@ Status FixedReadPlan::fill_missing_columns( | |
| // If the control flow reaches this branch, the column neither has default value | ||
| // nor is nullable. It means that the row's delete sign is marked, and the value | ||
| // columns are useless and won't be read. So we can just put arbitary values in the cells | ||
| missing_col->insert(tablet_column.get_vec_type()->get_default()); | ||
| missing_col->insert_default(); | ||
| } | ||
| } else { | ||
| missing_col->insert_from(*old_value_block.get_by_position(i).column, | ||
|
|
@@ -618,7 +618,7 @@ static void fill_non_primary_key_cell_for_column_store( | |
| // store the generated auto-increment value in fixed partial update | ||
| new_col->insert_from(cur_col, block_pos); | ||
| } else { | ||
| new_col->insert(tablet_column.get_vec_type()->get_default()); | ||
| new_col->insert_default(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same issue exists in flexible partial update. |
||
| } | ||
| } else { | ||
| auto pos_in_old_block = read_index.at(cid).at(segment_pos); | ||
|
|
@@ -724,7 +724,7 @@ static void fill_non_primary_key_cell_for_row_store( | |
| // store the generated auto-increment value in fixed partial update | ||
| new_col->insert_from(cur_col, block_pos); | ||
| } else { | ||
| new_col->insert(tablet_column.get_vec_type()->get_default()); | ||
| new_col->insert_default(); | ||
|
Mryange marked this conversation as resolved.
|
||
| } | ||
| } else { | ||
| new_col->insert_from(old_value_col, pos_in_old_block); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1073,7 +1073,7 @@ Status BaseTablet::generate_new_block_for_partial_update( | |
| mutable_column.get()) | ||
| ->insert_default(); | ||
| } else { | ||
| mutable_column->insert(rs_column.get_vec_type()->get_default()); | ||
| mutable_column->insert_default(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This branch is not just filling dead payload. When a fixed partial update hits a previously delete-marked row, |
||
| } | ||
| } else { | ||
| mutable_column->insert_from(*old_block.get_by_position(i).column, | ||
|
|
@@ -1125,7 +1125,7 @@ static void fill_cell_for_flexible_partial_update( | |
| // keep consistency between replicas | ||
| new_col->insert_from(cur_col, read_index_update[cast_set<uint32_t>(idx)]); | ||
| } else { | ||
| new_col->insert(tablet_column.get_vec_type()->get_default()); | ||
| new_col->insert_default(); | ||
|
Mryange marked this conversation as resolved.
|
||
| } | ||
| } else { | ||
| new_col->insert_from(old_value_col, read_index_old[cast_set<uint32_t>(idx)]); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helper is no longer equivalent to the removed
get_default()for complex types. For Array, Map, and JSONB,insert_default()yields[],{}, and SQLNULLinstead of the previous synthetic defaults ([null],{null:null}, JSONnull), and the regression diff intest_partial_update_complex_type.outalready shows that behavior change. Because this helper is shared by many non-storage call sites, the PR is not behavior-preserving anymore.