You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/21 07:39:04 UTC
[GitHub] [doris] jackwener opened a new pull request, #11073: [enhance](nereids): use optional for combine()
jackwener opened a new pull request, #11073:
URL: https://github.com/apache/doris/pull/11073
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
Describe the overview of changes.
use optional for combine()
If input list is empty, combine will return true. It's strange.
Use optional to correct it.
## 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](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener closed pull request #11073: [enhance](nereids): use optional for combine()
Posted by GitBox <gi...@apache.org>.
jackwener closed pull request #11073: [enhance](nereids): use optional for combine()
URL: https://github.com/apache/doris/pull/11073
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangshuo128 commented on pull request #11073: [enhance](nereids): use optional for combine()
Posted by GitBox <gi...@apache.org>.
wangshuo128 commented on PR #11073:
URL: https://github.com/apache/doris/pull/11073#issuecomment-1192087816
The current implementation looks reasonable to me.
A literal true/false predicate for empty input conjunctive/disjunctive predicates is right. It has no effect on the input data of the filter when the input predicate list is empty.
And you could check if the result is boolean literal, which has not much different from using an optional result.
A boolean literal result is much better because it has the opportunity to be optimized by boolean value rewrite rules further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on pull request #11073: [enhance](nereids): use optional for combine()
Posted by GitBox <gi...@apache.org>.
jackwener commented on PR #11073:
URL: https://github.com/apache/doris/pull/11073#issuecomment-1192107867
I don't think so, because this function is for combine expression by use `And` or `Or`.
Empty List is an abnormal input, it's invalid by using `LiteralTrue` as result.
And `LiteralTrue` defeat the intent of this function. We want to combine but return a `LiteralTrue`, it's strange.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org