-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
expression: support ConstItem() for expression #10004
Conversation
} | ||
return false | ||
} | ||
|
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.
Seems ExtractColumns
can provide what you want? CorrelatedColumn
should be treated as constant actually?
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.
Oh, my fault. We need to check also non-deterministic functions and statement parameters
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.
Oh from this comment, you need to check if the expression is "purely" constant, i.e, CorrelatedColumn
should not be treated as constant because we are not in executor, so ExtractColumns
cannot work, sorry for the misleading.
Why would non-deterministic functions matter here? though their return value is non-deterministic, the return type is deterministic?
What does statement parameters mean?
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.
For non-deterministic functions, their return value is non-deterministic, so you can't use its value to computer the frac part or somethings else.
I believe statement parameters
is the parameters used in prepare statement.
Codecov Report
@@ Coverage Diff @@
## master #10004 +/- ##
================================================
+ Coverage 77.9619% 78.0633% +0.1014%
================================================
Files 405 405
Lines 82244 82223 -21
================================================
+ Hits 64119 64186 +67
+ Misses 13392 13332 -60
+ Partials 4733 4705 -28 |
55d9c83
to
6ea2ec8
Compare
3cad36a
to
fde3a79
Compare
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
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
/run-all-tests |
What problem does this PR solve?
Add a method for expression.
Sometimes we get a expression, we may want to know if the expression refers to a table.
Presently we have
IsCorrelated()
, but it's not enough.For example, See #9751
We want to call evalString, but We cannot eval scalar functions that contain tables.
In the source code of MySQL,
bool const_item() const { return used_tables() == 0; }
And
used_tables()
has been used many times in MySQL, I thinkReferTable
would be helpful for further development.What is changed and how it works?
Add a method for expression
Check List
Tests
Code changes
Side effects
Related changes