-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Bug] Add regular column when materialized slot is empty in tuple #4719
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
Conversation
If the materilized slot is empty in inline view, the scanner will throw exception 'no materialized slot!'. When the outer query does not need any real columns of the inner query at all, such as the outer query only contains constant calculations, all slots in the inner query will not be materialized. For example: ``` select c1 from (select 'xx' c1, k1 from test) t1; ``` But this does not mean that the inner query (select 'xx' c1, k1 from test) does not need to be executed. Because the number of rows in the outer result needs to be determined by the number of rows in the inner query. In this case, the inner query scan node needs to add a column to ensure the correct result. At the same time it can avoid the scan node reporting errors. Fixed apache#4716 Change-Id: I86bb52efab65b658ea78094037543cb23dbff1d4
|
+1. But please add a UT for this bug. |
This single test is a bit difficult to add, because the slot information is not displayed in the explain. Or should I just add a single test for this function |
OK |
kangkaisen
left a comment
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.
+1
| * @param tblRef | ||
| * @param analyzer | ||
| */ | ||
| private void materializeBaseTableRefResultForCrossJoinOrCountStar(BaseTableRef tblRef, Analyzer analyzer) { |
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.
I think this method name should be changed to a more common name?
like: materializeSlotForEmptyMaterializedTableRef()?
And add examples in comment to explain why we call it.
Change-Id: I51d5e71acdabfb4e48389436391e19917c36f52c
morningman
left a comment
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.
LGTM
If the materialized slot is empty in inline view, the scanner will throw exception 'no materialized slot!'.
When the outer query does not need any real columns of the inner query at all, such as the outer query only contains constant calculations,
all slots in the inner query will not be materialized.
For example:
But this does not mean that the inner query (select 'xx' c1, k1 from test) does not need to be executed.
Because the number of rows in the outer result needs to be determined by the number of rows in the inner query.
In this case, the inner query scan node needs to add a column to ensure the correct result.
At the same time it can avoid the scan node reporting errors.
Fixed #4716
Change-Id: I86bb52efab65b658ea78094037543cb23dbff1d4
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Types of changes
What types of changes does your code introduce to Doris?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.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...