-
Couldn't load subscription status.
- Fork 579
validate default constraint contains only constant expressions #3788
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
base: main
Are you sure you want to change the base?
validate default constraint contains only constant expressions #3788
Conversation
|
Nyrkiö Report for Commit: 20f8c62
|
f9fcc9b to
7f71300
Compare
| } | ||
|
|
||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] | ||
| pub enum ConstantFlags { |
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.
Please document what this does
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.
added some more docs
| Expr::Parenthesized(exprs) => { | ||
| let new_flag = if matches!(flags, ConstantFlags::ExprFunction { .. }) { | ||
| ConstantFlags::ExprFunction { | ||
| // disallow ID inside parametrized |
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.
parametrized? also, why?
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.
CREATE TABLE t(x DEFAULT (adfgdfg)); VS CREATE TABLE t(x DEFAULT adfgdfg);
The first is parenthesized and the second one is not. The second one is interpreted as a String literal in this case, despite being an Identifier in the AST, so it is allowed and constant. This also applies to functions and other expressions like CASE
I thought at first this was a parser bug, but I compared the AST output with sqlite3-parser and the AST is correct. It's just some crazy rule SQLite decided to follow. Maybe it is a compatibility thing with some other DBMS.
| arg.is_constant( | ||
| resolver, | ||
| ConstantFlags::ExprFunction { | ||
| // disallow ID in arguments |
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?
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.
maybe the Parenthesized is already enough because you need to add a Parenthesis to have functions in DEFAULT, but I just to be sure I did this here as well.
7f71300 to
20f8c62
Compare

Closes #3636
Did something similar to SQLite and added a flag when checking for a constant expression, that changes the behaviour slightly for functions and identifiers.
Also asked Claude to generate some tests to check our
DEFAULTbehaviour