Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,26 @@ FROM (
WHERE total_amount > 1000;
```

### Rule: implicit column name in references

```sql
create table u(
id int primary key
);
create table t(
u_id int references u
-- ^quickfix
);

-- becomes

create table t(
u_id int references u(id)
);
```

and vice versa

### Rule: aggregate free `having` condition

```sql
Expand Down
26 changes: 26 additions & 0 deletions crates/squawk_ide/src/classify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
let mut in_constraint_include_clause = false;
let mut in_constraint_where_clause = false;
let mut in_partition_item = false;
let mut in_set_null_columns = false;

// TODO: can we combine this if and the one that follows?
if let Some(parent) = name_ref.syntax().parent()
Expand Down Expand Up @@ -115,6 +116,7 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
// ^^^
.is_some_and(|field_name_ref| field_name_ref.syntax() == name_ref.syntax())
// we're not inside a call expr
&& field_expr.star_token().is_none()
&& field_expr
.syntax()
.parent()
Expand Down Expand Up @@ -220,6 +222,9 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
{
return Some(NameRefClass::Tablespace);
}
if ast::SetNullColumns::can_cast(ancestor.kind()) {
in_set_null_columns = true;
}
if let Some(foreign_key) = ast::ForeignKeyConstraint::cast(ancestor.clone()) {
if in_column_list {
// TODO: ast is too "flat" here, we need a unique node for to
Expand All @@ -241,10 +246,31 @@ pub(crate) fn classify_name_ref(name_ref: &ast::NameRef) -> Option<NameRefClass>
{
return Some(NameRefClass::ForeignKeyLocalColumn);
}
if in_set_null_columns {
return Some(NameRefClass::ForeignKeyLocalColumn);
}
} else {
return Some(NameRefClass::ForeignKeyTable);
}
}
if let Some(references_constraint) = ast::ReferencesConstraint::cast(ancestor.clone()) {
if let Some(column_ref) = references_constraint.column()
&& column_ref
.syntax()
.text_range()
.contains_range(name_ref.syntax().text_range())
{
return Some(NameRefClass::ForeignKeyColumn);
}
if let Some(path) = references_constraint.table()
&& path
.syntax()
.text_range()
.contains_range(name_ref.syntax().text_range())
{
return Some(NameRefClass::ForeignKeyTable);
}
}
if ast::CheckConstraint::can_cast(ancestor.kind()) {
return Some(NameRefClass::CheckConstraintColumn);
}
Expand Down
80 changes: 80 additions & 0 deletions crates/squawk_ide/src/goto_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,72 @@ create table bar(
");
}

#[test]
fn goto_foreign_key_on_delete_set_null_column() {
assert_snapshot!(goto("
create table users (
user_id integer not null,
primary key (user_id)
);

create table posts (
post_id integer not null,
author_id integer,
primary key (post_id),
foreign key (author_id) references users on delete set null (author_id$0)
);
"), @r"
╭▸
9 │ author_id integer,
│ ───────── 2. destination
10 │ primary key (post_id),
11 │ foreign key (author_id) references users on delete set null (author_id)
╰╴ ─ 1. source
");
}

#[test]
fn goto_references_constraint_table() {
assert_snapshot!(goto("
create table t (
id serial primary key
);

create table u (
id serial primary key,
t_id int references t$0
);
"), @r"
╭▸
2 │ create table t (
│ ─ 2. destination
8 │ t_id int references t
╰╴ ─ 1. source
");
}

#[test]
fn goto_references_constraint_column() {
assert_snapshot!(goto("
create table t (
id serial primary key
);

create table u (
id serial primary key,
t_id int references t(id$0)
);
"), @r"
╭▸
3 │ id serial primary key
│ ── 2. destination
8 │ t_id int references t(id)
╰╴ ─ 1. source
");
}

#[test]
fn goto_foreign_key_references_column() {
assert_snapshot!(goto("
Expand Down Expand Up @@ -2918,6 +2984,20 @@ select t$0 from t;
");
}

#[test]
fn goto_select_table_star_expansion() {
assert_snapshot!(goto("
create table t(id int, a int);
select t$0.* from t;
"), @r"
╭▸
2 │ create table t(id int, a int);
│ ─ 2. destination
3 │ select t.* from t;
╰╴ ─ 1. source
");
}

#[test]
fn goto_select_table_as_column_with_schema() {
assert_snapshot!(goto("
Expand Down
24 changes: 16 additions & 8 deletions crates/squawk_ide/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,29 @@ pub(crate) fn resolve_name_ref(
resolve_tablespace(binder, &tablespace_name).map(|ptr| smallvec![ptr])
}
NameRefClass::ForeignKeyTable => {
let foreign_key = name_ref
.syntax()
.ancestors()
.find_map(ast::ForeignKeyConstraint::cast)?;
let path = foreign_key.path()?;
let path = name_ref.syntax().ancestors().find_map(ast::Path::cast)?;
let table_name = extract_table_name(&path)?;
let schema = extract_schema_name(&path);
let position = name_ref.syntax().text_range().start();
resolve_table(binder, &table_name, &schema, position).map(|ptr| smallvec![ptr])
}
NameRefClass::ForeignKeyColumn => {
let foreign_key = name_ref
// TODO: the ast is too flat here
let path = if let Some(foreign_key) = name_ref
.syntax()
.ancestors()
.find_map(ast::ForeignKeyConstraint::cast)
{
foreign_key.path()?
} else if let Some(references_constraint) = name_ref
.syntax()
.ancestors()
.find_map(ast::ForeignKeyConstraint::cast)?;
let path = foreign_key.path()?;
.find_map(ast::ReferencesConstraint::cast)
{
references_constraint.table()?
} else {
return None;
};
let column_name = Name::from_node(name_ref);
resolve_column_for_path(binder, &path, column_name).map(|ptr| smallvec![ptr])
}
Expand Down Expand Up @@ -678,6 +685,7 @@ fn resolve_select_qualified_column_table(
let explicit_schema = if field_expr
.field()
.is_some_and(|f| f.syntax() == name_ref.syntax())
&& field_expr.star_token().is_none()
{
// if we're at the field `bar` in `foo.bar`
if let ast::Expr::NameRef(schema_name_ref) = field_expr.base()? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ fn not_valid_validate_in_transaction(
ast::AlterTableAction::AddConstraint(add_constraint) => {
if add_constraint.not_valid().is_some()
&& let Some(constraint) = add_constraint.constraint()
&& let Some(constraint_name) = constraint.name()
&& let Some(constraint_name) =
constraint.constraint_name().and_then(|c| c.name())
{
not_valid_names.insert(Identifier::new(&constraint_name.text()));
}
Expand Down
4 changes: 3 additions & 1 deletion crates/squawk_linter/src/rules/prefer_robust_stmts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse<SourceFile>) {
}
ast::AlterTableAction::AddConstraint(add_constraint) => {
let constraint = add_constraint.constraint();
if let Some(constraint_name) = constraint.and_then(|x| x.name())
if let Some(constraint_name) = constraint
.and_then(|x| x.constraint_name())
.and_then(|x| x.name())
&& let Some(constraint) = constraint_names
.get_mut(&Identifier::new(constraint_name.text().as_str()))
&& *constraint == Constraint::Dropped
Expand Down
1 change: 1 addition & 0 deletions crates/squawk_parser/src/generated/syntax_kind.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 13 additions & 9 deletions crates/squawk_parser/src/grammar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3760,9 +3760,7 @@ fn opt_column_constraint(p: &mut Parser<'_>) -> Option<CompletedMarker> {
return None;
}
let m = p.start();
if p.eat(CONSTRAINT_KW) {
name(p);
}
opt_constraint_name(p);
match opt_constraint_inner(p) {
Some(kind) => {
opt_constraint_option_list(p);
Expand Down Expand Up @@ -4087,9 +4085,7 @@ fn table_constraint(p: &mut Parser<'_>) -> CompletedMarker {
assert!(p.at_ts(TABLE_CONSTRAINT_FIRST));
let m = p.start();
// [ CONSTRAINT constraint_name ]
if p.eat(CONSTRAINT_KW) {
name(p);
}
opt_constraint_name(p);
// CHECK ( expression ) [ NO INHERIT ]
let kind = match p.current() {
CHECK_KW => {
Expand Down Expand Up @@ -7167,9 +7163,7 @@ fn alter_domain_action(p: &mut Parser<'_>) -> Option<CompletedMarker> {
// { NOT NULL | CHECK (expression) }
fn domain_constraint(p: &mut Parser<'_>) {
let m = p.start();
if p.eat(CONSTRAINT_KW) {
name(p);
}
opt_constraint_name(p);
if p.eat(NOT_KW) {
p.expect(NULL_KW);
m.complete(p, NOT_NULL_CONSTRAINT);
Expand All @@ -7186,6 +7180,16 @@ fn domain_constraint(p: &mut Parser<'_>) {
}
}

fn opt_constraint_name(p: &mut Parser<'_>) {
let m = p.start();
if p.eat(CONSTRAINT_KW) {
name(p);
m.complete(p, CONSTRAINT_NAME);
} else {
m.abandon(p);
}
}

// ALTER DEFAULT PRIVILEGES
// [ FOR { ROLE | USER } target_role [, ...] ]
// [ IN SCHEMA schema_name [, ...] ]
Expand Down
18 changes: 10 additions & 8 deletions crates/squawk_parser/tests/snapshots/tests__alter_domain_ok.snap
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,11 @@ SOURCE_FILE
ADD_KW "add"
WHITESPACE " "
CHECK_CONSTRAINT
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
CONSTRAINT_NAME
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
WHITESPACE " "
CHECK_KW "check"
WHITESPACE " "
Expand Down Expand Up @@ -169,10 +170,11 @@ SOURCE_FILE
ADD_KW "add"
WHITESPACE " "
CHECK_CONSTRAINT
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "a"
CONSTRAINT_NAME
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "a"
WHITESPACE " "
CHECK_KW "check"
WHITESPACE " "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1055,10 +1055,11 @@ SOURCE_FILE
ADD_KW "add"
WHITESPACE " "
CHECK_CONSTRAINT
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
CONSTRAINT_NAME
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
WHITESPACE " "
CHECK_KW "check"
WHITESPACE " "
Expand Down Expand Up @@ -1096,10 +1097,11 @@ SOURCE_FILE
ADD_KW "add"
WHITESPACE " "
CHECK_CONSTRAINT
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
CONSTRAINT_NAME
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "c"
WHITESPACE " "
CHECK_KW "check"
WHITESPACE " "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,11 @@ SOURCE_FILE
ADD_KW "add"
WHITESPACE " "
NOT_NULL_CONSTRAINT
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "id_not_null"
CONSTRAINT_NAME
CONSTRAINT_KW "constraint"
WHITESPACE " "
NAME
IDENT "id_not_null"
WHITESPACE " "
NOT_KW "not"
WHITESPACE " "
Expand Down
Loading
Loading