-
Notifications
You must be signed in to change notification settings - Fork 447
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
fix(interactive): Support Index predicate for dynamic params in IR-Core #3282
Conversation
@@ -191,10 +191,16 @@ message Limit { | |||
// where the values referred by k1, k2, ... are indexed and hence the | |||
// predicate can be efficiently verified by leveraging the index. | |||
message IndexPredicate { | |||
message PkValue { |
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.
Looks like PKValue
is not quite necessary here, Simply define
message Triplet {
common.Property key = 1;
// The value can be either a constant value, or a parameter (in stored procedure)
one of value {
common.Value const = 2;
common.DynamicParam param = 3;
}
}
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.
Done
Committed-by: bingqing.lbq from Dev container
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3282 +/- ##
=======================================
Coverage 42.06% 42.06%
=======================================
Files 101 101
Lines 11021 11021
=======================================
Hits 4636 4636
Misses 6385 6385 Continue to review full report in Codecov by Sentry.
|
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.
Can merge as long as cis passed
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
What do these changes do?
As titled.
Related issue number
#3277