-
Notifications
You must be signed in to change notification settings - Fork 110
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
feat(1329): System table for system variables #1342
Conversation
7da35b4
to
563a3d3
Compare
563a3d3
to
35a2075
Compare
I think you need to mark this as potentially breaking because the table |
|
||
/// The type of a system variable in `st_var` | ||
#[repr(u8)] | ||
pub enum StVarType { |
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.
Why do we need another type
for this? I think just matching on AlgebraicType
if wanna restrict to primitive
types is enough
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 a column be typed as AlgebraicValue
? I was under the assumption it needed a concrete type. In this case, AlgebraicType::Sum(...)
.
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.
We could conceivably impl SpacetimeType for AlgebraicValue
by having an AlgebraicType
which describes AlgebraicValue
.
) -> Result<(), DBError> { | ||
if auth.caller != auth.owner { | ||
if let Some(limit) = config.row_limit { | ||
if let Some(limit) = StVarTable::row_limit(ctx, db, tx)? { |
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.
We use the row_limit
in st_var
when estimating cardinality.
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
Closes #1329. Used to persist global parameters for slow query logging and cardinality limits.
44c7daf
to
6e89743
Compare
Closes #1329.
Used to persist global parameters for slow query logging and cardinality limits.
Expected complexity level and risk
2
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!