Skip to content

Conversation

@kangkaisen
Copy link
Contributor

@kangkaisen kangkaisen commented Aug 29, 2019

For #1610, make bitmap_union support insert into select and broker load

1 Add a check for select * with bitmap_union
2 Define the bitmap_union column type to varchar(0), because the bitmap real length is variable, when insert into, the bitmap real length maybe larger than origin varchar length.

Test the following case:

1

insert into bitmap_test_2 select id, id2 from bitmap_test_3;

2

insert into bitmap_test_2 select id, to_bitmap(id2) from bitmap_int;

3

INSERT INTO bitmap_test_2 (id, id2) VALUES (1000, to_bitmap(1000)), (1000, to_bitmap(2000));

@imay
Copy link
Contributor

imay commented Aug 29, 2019

@kangkaisen
I have seen this PR.
I think you should check source column is a valid bitmap in InsertStatement like HLL here

@kangkaisen
Copy link
Contributor Author

@kangkaisen
I have seen this PR.
I think you should check source column is a valid bitmap in InsertStatement like HLL here
@imay Update

}
} else if (expr instanceof FunctionCallExpr) {
final FunctionCallExpr functionExpr = (FunctionCallExpr) expr;
if (!functionExpr.getFnName().getFunction().equalsIgnoreCase(FunctionSet.TO_BITMAP)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think BITMAP_UNION() is OK too.

And you should add in
fe/src/main/java/org/apache/doris/planner/BrokerScanNode.java:452
fe/src/main/java/org/apache/doris/planner/StreamLoadScanNode.java:278

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update

@kangkaisen kangkaisen changed the title Make bitmap_union agg column support insert into Make bitmap_union agg column support insert into and broker load Aug 29, 2019
protected final TupleDescriptor desc;
protected Map<String, PartitionColumnFilter> columnFilters;
protected String sortColumn = null;
protected Map<String, SlotDescriptor> slotDescByName = Maps.newHashMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to unify BrokerScanNode and StreamScanNode, I think you'd better to make another class which is subclass of ScanNode. Because not all ScanNode has these properties, they only are used in loading.

@morningman
Copy link
Contributor

There are a lot change related to load process in my pull request #1695 .
So...Please do not change any class name or class inheritance relationship at this time.
T.T

@kangkaisen
Copy link
Contributor Author

There are a lot change related to load process in my pull request #1695 .
So...Please do not change any class name or class inheritance relationship at this time.
T.T

OK. I has minimized the change.

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 merged commit 3a33f3d into apache:master Aug 30, 2019
@imay imay mentioned this pull request Sep 26, 2019
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.

4 participants