Skip to content

Commit 8f58f3d

Browse files
authored
ide: fix goto def with temp table (#761)
1 parent 3809b5c commit 8f58f3d

File tree

3 files changed

+65
-29
lines changed

3 files changed

+65
-29
lines changed

crates/squawk_ide/src/binder.rs

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ impl Binder {
2727
symbols: Arena::new(),
2828
search_path_changes: vec![SearchPathChange {
2929
position: TextSize::from(0),
30-
search_path: vec![Schema::new("pg_temp"), Schema::new("public")],
30+
search_path: vec![Schema::new("public"), Schema::new("pg_temp")],
3131
}],
3232
}
3333
}
@@ -40,6 +40,14 @@ impl Binder {
4040
.expect("root scope must exist")
4141
}
4242

43+
fn current_search_path(&self) -> &[Schema] {
44+
&self
45+
.search_path_changes
46+
.last()
47+
.expect("search_path_changes should never be empty")
48+
.search_path
49+
}
50+
4351
pub(crate) fn search_path_at(&self, position: TextSize) -> &[Schema] {
4452
// We're assuming people don't actually use `set search_path` that much,
4553
// so linear search is fine
@@ -84,7 +92,9 @@ fn bind_create_table(b: &mut Binder, create_table: ast::CreateTable) {
8492
};
8593
let name_ptr = path_to_ptr(&path);
8694
let is_temp = create_table.temp_token().is_some() || create_table.temporary_token().is_some();
87-
let schema = schema_name(&path, is_temp);
95+
let Some(schema) = schema_name(b, &path, is_temp) else {
96+
return;
97+
};
8898

8999
let table_id = b.symbols.alloc(Symbol {
90100
kind: SymbolKind::Table,
@@ -121,36 +131,34 @@ fn path_to_ptr(path: &ast::Path) -> SyntaxNodePtr {
121131
SyntaxNodePtr::new(path.syntax())
122132
}
123133

124-
fn schema_name(path: &ast::Path, is_temp: bool) -> Schema {
125-
let default_schema = if is_temp { "pg_temp" } else { "public" };
126-
127-
let Some(segment) = path.qualifier().and_then(|q| q.segment()) else {
128-
return Schema::new(default_schema);
129-
};
134+
fn schema_name(b: &Binder, path: &ast::Path, is_temp: bool) -> Option<Schema> {
135+
if let Some(name_ref) = path
136+
.qualifier()
137+
.and_then(|q| q.segment())
138+
.and_then(|s| s.name_ref())
139+
{
140+
return Some(Schema(Name::new(name_ref.syntax().text().to_string())));
141+
}
130142

131-
let schema_name = if let Some(name) = segment.name() {
132-
Name::new(name.syntax().text().to_string())
133-
} else if let Some(name_ref) = segment.name_ref() {
134-
Name::new(name_ref.syntax().text().to_string())
135-
} else {
136-
return Schema::new(default_schema);
137-
};
143+
if is_temp {
144+
return Some(Schema::new("pg_temp"));
145+
}
138146

139-
Schema(schema_name)
147+
b.current_search_path().first().cloned()
140148
}
141149

142150
fn bind_set(b: &mut Binder, set: ast::Set) {
143151
let position = set.syntax().text_range().start();
144152

145153
// `set schema` is an alternative to `set search_path`
146154
if set.schema_token().is_some() {
147-
if let Some(literal) = set.literal() {
148-
if let Some(string_value) = extract_string_literal(&literal) {
149-
b.search_path_changes.push(SearchPathChange {
150-
position,
151-
search_path: vec![Schema::new(string_value)],
152-
});
153-
}
155+
if let Some(literal) = set.literal()
156+
&& let Some(string_value) = extract_string_literal(&literal)
157+
{
158+
b.search_path_changes.push(SearchPathChange {
159+
position,
160+
search_path: vec![Schema::new(string_value)],
161+
});
154162
}
155163
return;
156164
}
@@ -179,14 +187,17 @@ fn bind_set(b: &mut Binder, set: ast::Set) {
179187
if set.default_token().is_some() {
180188
b.search_path_changes.push(SearchPathChange {
181189
position,
182-
search_path: vec![Schema::new("pg_temp"), Schema::new("public")],
190+
search_path: vec![Schema::new("public"), Schema::new("pg_temp")],
183191
});
184192
} else {
185193
let mut search_path = vec![];
186194
for config_value in set.config_values() {
187195
match config_value {
188196
ast::ConfigValue::Literal(literal) => {
189197
if let Some(string_value) = extract_string_literal(&literal) {
198+
// You can unset the search path via `set search_path = ''`
199+
// so we want to skip over these, otherwise we'll
200+
// have a schema of value `''` which isn't valid.
190201
if !string_value.is_empty() {
191202
search_path.push(Schema::new(string_value));
192203
}

crates/squawk_ide/src/find_references.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ table users;
179179
}
180180

181181
#[test]
182-
fn temp_table_shadows_public() {
182+
fn temp_table_do_not_shadows_public() {
183183
assert_snapshot!(find_refs("
184184
create table t();
185185
create temp table t$0();
@@ -190,9 +190,7 @@ drop table t;
190190
│ ┬
191191
│ │
192192
│ 0. query
193-
│ 1. reference
194-
4 │ drop table t;
195-
╰╴ ─ 2. reference
193+
╰╴ 1. reference
196194
");
197195
}
198196

crates/squawk_ide/src/goto_definition.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,9 @@ create temp table t();
330330
drop table t$0;
331331
"), @r"
332332
╭▸
333+
2 │ create table t();
334+
│ ─ 2. destination
333335
3 │ create temp table t();
334-
│ ─ 2. destination
335336
4 │ drop table t;
336337
╰╴ ─ 1. source
337338
");
@@ -474,6 +475,32 @@ drop table t$0;
474475
");
475476
}
476477

478+
#[test]
479+
fn goto_with_search_path_and_unspecified_table() {
480+
assert_snapshot!(goto(r#"
481+
set search_path to foo,bar;
482+
create table t();
483+
drop table foo.t$0;
484+
"#), @r"
485+
╭▸
486+
3 │ create table t();
487+
│ ─ 2. destination
488+
4 │ drop table foo.t;
489+
╰╴ ─ 1. source
490+
");
491+
}
492+
493+
#[test]
494+
fn goto_with_search_path_empty() {
495+
goto_not_found(
496+
r#"
497+
set search_path = '';
498+
create table t();
499+
drop table t$0;
500+
"#,
501+
);
502+
}
503+
477504
#[test]
478505
fn goto_with_search_path_like_variable() {
479506
// not actually search path

0 commit comments

Comments
 (0)