Skip to content

Conversation

@BiteTheDDDDt
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Problem Summary:

Describe the overview of changes.

Checklist(Required)

  1. Does it affect the original behavior: (Yes/No/I Don't know)
  2. Has unit tests been added: (Yes/No/No Need)
  3. Has document been added or modified: (Yes/No/No Need)
  4. Does it need to update dependencies: (Yes/No)
  5. Are there any changes that cannot be rolled back: (Yes/No)

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions github-actions bot added area/planner Issues or PRs related to the query planner kind/test labels Jul 12, 2022
@BiteTheDDDDt BiteTheDDDDt force-pushed the dev_mv branch 4 times, most recently from 33600e3 to 9625fc2 Compare July 20, 2022 03:11
@BiteTheDDDDt BiteTheDDDDt force-pushed the dev_mv branch 2 times, most recently from 0cf4fdd to 48b1149 Compare July 21, 2022 13:30
@BiteTheDDDDt BiteTheDDDDt marked this pull request as ready for review July 21, 2022 13:31
@github-actions github-actions bot added the area/sql/function Issues or PRs related to the SQL functions label Jul 22, 2022
} else if (_schema_mapping[i].materialized_function == "hll_hash") {
_do_materialized_transform = hll_hash;
} else if (_schema_mapping[i].materialized_function == "count_field") {
} else if (_schema_mapping[i].materialized_function == "is_not_null_pred") {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we update count_field name will cause update BE the function can not process the logic. keep it and remove after next version

for (int i = 0; i < row_tuples.size(); ++i) {
TupleDescriptor* tupleDesc = desc_tbl.get_tuple_descriptor(row_tuples[i]);

if (skip_delete) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do not treat each slot as a tuple


for (size_t i = 0; i < input_rows_count; ++i) {
res_data[i] = !src_data[i];
res_data[i] = src_data[i] == 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is better ?

mvColumnName = baseColumnName;
} else {
mvColumnName = mvColumnBuilder(functionName, baseColumnName);
if (!functionChild0.getType().isStringType()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why the origin do not have the logic

@BiteTheDDDDt BiteTheDDDDt force-pushed the dev_mv branch 3 times, most recently from 65eef10 to 96d242f Compare July 28, 2022 08:35
init_tuple_idx_map();
init_has_varlen_slots();

DCHECK(nullable_tuples.size() == _tuple_desc_map.size())
Copy link
Contributor

Choose a reason for hiding this comment

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

seems no need move the DCHECK to here

public int getAsInt() {
pendingMaxColUniqueId++;
return pendingMaxColUniqueId;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

1、now in rollup, we don't support light schema change, this is unnecessary.
2、in the end, we also need update tbl max unique id . for example in SchemaChangeJobV2.onFinished() , we do like this:
//update max column unique id
int maxColUniqueId = tbl.getMaxColUniqueId();
for (Column column : tbl.getFullSchema()) {
if (column.getUniqueId() > maxColUniqueId) {
maxColUniqueId = column.getUniqueId();
}
}
tbl.setMaxColUniqueId(maxColUniqueId);

3、i will refact the light schema change feature, here you can just delete this code about generating col_unique_id

BiteTheDDDDt and others added 4 commits August 3, 2022 12:18
update

update

update

update

update

update

update

update

update

update

update

update

fix fe ut

update

update

update

update

update

update

update
update

update

update
@HappenLee
Copy link
Contributor

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

PR approved by anyone and no changes requested.

@HappenLee HappenLee merged commit ec3c911 into apache:master Aug 4, 2022
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/planner Issues or PRs related to the query planner area/sql/function Issues or PRs related to the SQL functions area/vectorization kind/test reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants