Skip to content
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

[GLUTEN-1874][CH] Fixes nullable mismatch in union #1901

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

lgbo-ustc
Copy link
Contributor

@lgbo-ustc lgbo-ustc commented Jun 9, 2023

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

Fixes: #1874

main changes

add a new field output_schema in RelRoot

This field will not affect velox

add a nullable projections after parsing root node in CH

new implementation for finding the non-nullable columns

This has the largest changes, since we remove all the required_columns in many functions

fixed some bugs about pushing down filter in merge tree

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

unit tests

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

@github-actions
Copy link

github-actions bot commented Jun 9, 2023

Run Gluten Clickhouse CI

@github-actions
Copy link

github-actions bot commented Jun 9, 2023

#1874

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

1 similar comment
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@lgbo-ustc lgbo-ustc marked this pull request as ready for review June 15, 2023 10:30
@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Run Gluten Clickhouse CI

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Run Gluten Clickhouse CI

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Run Gluten Clickhouse CI

@lgbo-ustc
Copy link
Contributor Author

@jinchengchenghh @rui-mo Do you have any more questions?

@rui-mo
Copy link
Contributor

rui-mo commented Jul 10, 2023

There are some conflicts need to be resolved.

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

return childCtx
// Since some columns' nullability will be removed after this filter, we need to update the
// outputAttributes of child context.
TransformContext(childCtx.inputAttributes, output, childCtx.root)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe can not return the output of the filter when all the filter are pushdowned, some attrs may be removed in this case. @rui-mo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, this fiter is no sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Velox backend, if all the conditions are pushed down, the filter's computing is ignored.

@github-actions
Copy link

Run Gluten Clickhouse CI

@github-actions
Copy link

Run Gluten Clickhouse CI

Copy link
Contributor

@zzcclp zzcclp left a comment

Choose a reason for hiding this comment

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

LGTM

@zzcclp zzcclp merged commit d942ee1 into apache:main Jul 14, 2023
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.

[CH] Mismatch nullable in union streams
4 participants