From eeb30cc45aa1ea9229f1d928359d75331fe5cb9a Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Mon, 13 Jul 2026 22:52:32 -0700 Subject: [PATCH] fix: typecheck false positive for SQL function args of table row type pg typecheck raised a false-positive "missing FROM-clause entry" when a SQL function argument's type is a table's row (composite) type, e.g. `get_tbl_name(row_arg public.tbl) ... SELECT row_arg.name`. The schema-cache types query only loaded standalone composites (relkind 'c') and skipped relation row types, so resolve_type returned None and the field reference was left unreplaced. Load row types for tables, partitioned tables, views, materialized views and foreign tables (all relkinds that expose a composite row type) along with their user column attributes, restricting the attribute join to `attnum > 0` so system columns like ctid/xmin are not mistaken for real fields. The prepared .sqlx offline cache is regenerated to match. Fixes #705 --- ...6d56a41d2db32bb448f4d1f26ef4e8b07cd8.json} | 4 +- .../pgls_schema_cache/src/queries/types.sql | 5 +- crates/pgls_typecheck/src/typed_identifier.rs | 107 ++++++++++++++++++ crates/pgls_typecheck/tests/diagnostics.rs | 65 +++++++++++ .../snapshots/function_arg_row_type.snap | 5 + .../snapshots/function_arg_view_row_type.snap | 5 + 6 files changed, 187 insertions(+), 4 deletions(-) rename .sqlx/{query-1c29eca62591ae2597581be806dd572b3d79c7b8b9b7ffa8915806c947095a96.json => query-0a865484cac0af1151e3003ee4c86d56a41d2db32bb448f4d1f26ef4e8b07cd8.json} (79%) create mode 100644 crates/pgls_typecheck/tests/snapshots/function_arg_row_type.snap create mode 100644 crates/pgls_typecheck/tests/snapshots/function_arg_view_row_type.snap diff --git a/.sqlx/query-1c29eca62591ae2597581be806dd572b3d79c7b8b9b7ffa8915806c947095a96.json b/.sqlx/query-0a865484cac0af1151e3003ee4c86d56a41d2db32bb448f4d1f26ef4e8b07cd8.json similarity index 79% rename from .sqlx/query-1c29eca62591ae2597581be806dd572b3d79c7b8b9b7ffa8915806c947095a96.json rename to .sqlx/query-0a865484cac0af1151e3003ee4c86d56a41d2db32bb448f4d1f26ef4e8b07cd8.json index fcd4901ec..30b9071e8 100644 --- a/.sqlx/query-1c29eca62591ae2597581be806dd572b3d79c7b8b9b7ffa8915806c947095a96.json +++ b/.sqlx/query-0a865484cac0af1151e3003ee4c86d56a41d2db32bb448f4d1f26ef4e8b07cd8.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "select\n t.oid :: int8 as \"id!\",\n t.typname as name,\n n.nspname as \"schema!\",\n format_type (t.oid, null) as \"format!\",\n coalesce(t_enums.enums, '[]') as enums,\n coalesce(t_attributes.attributes, '[]') as attributes,\n obj_description (t.oid, 'pg_type') as comment\nfrom\n pg_type t\n left join pg_namespace n on n.oid = t.typnamespace\n left join (\n select\n enumtypid,\n jsonb_agg(\n enumlabel\n order by\n enumsortorder\n ) as enums\n from\n pg_enum\n group by\n enumtypid\n ) as t_enums on t_enums.enumtypid = t.oid\n left join (\n select\n oid,\n jsonb_agg(\n jsonb_build_object('name', a.attname, 'type_id', a.atttypid :: int8)\n order by\n a.attnum asc\n ) as attributes\n from\n pg_class c\n join pg_attribute a on a.attrelid = c.oid\n where\n c.relkind = 'c'\n and not a.attisdropped\n group by\n c.oid\n ) as t_attributes on t_attributes.oid = t.typrelid\nwhere\n (\n t.typrelid = 0\n or (\n select\n c.relkind = 'c'\n from\n pg_class c\n where\n c.oid = t.typrelid\n )\n );", + "query": "select\n t.oid :: int8 as \"id!\",\n t.typname as name,\n n.nspname as \"schema!\",\n format_type (t.oid, null) as \"format!\",\n coalesce(t_enums.enums, '[]') as enums,\n coalesce(t_attributes.attributes, '[]') as attributes,\n obj_description (t.oid, 'pg_type') as comment\nfrom\n pg_type t\n left join pg_namespace n on n.oid = t.typnamespace\n left join (\n select\n enumtypid,\n jsonb_agg(\n enumlabel\n order by\n enumsortorder\n ) as enums\n from\n pg_enum\n group by\n enumtypid\n ) as t_enums on t_enums.enumtypid = t.oid\n left join (\n select\n oid,\n jsonb_agg(\n jsonb_build_object('name', a.attname, 'type_id', a.atttypid :: int8)\n order by\n a.attnum asc\n ) as attributes\n from\n pg_class c\n join pg_attribute a on a.attrelid = c.oid\n where\n c.relkind in ('c', 'r', 'p', 'v', 'm', 'f')\n and a.attnum > 0\n and not a.attisdropped\n group by\n c.oid\n ) as t_attributes on t_attributes.oid = t.typrelid\nwhere\n (\n t.typrelid = 0\n or (\n select\n c.relkind in ('c', 'r', 'p', 'v', 'm', 'f')\n from\n pg_class c\n where\n c.oid = t.typrelid\n )\n );", "describe": { "columns": [ { @@ -52,5 +52,5 @@ null ] }, - "hash": "1c29eca62591ae2597581be806dd572b3d79c7b8b9b7ffa8915806c947095a96" + "hash": "0a865484cac0af1151e3003ee4c86d56a41d2db32bb448f4d1f26ef4e8b07cd8" } diff --git a/crates/pgls_schema_cache/src/queries/types.sql b/crates/pgls_schema_cache/src/queries/types.sql index 39f6b71c8..f51cc9d4e 100644 --- a/crates/pgls_schema_cache/src/queries/types.sql +++ b/crates/pgls_schema_cache/src/queries/types.sql @@ -34,7 +34,8 @@ from pg_class c join pg_attribute a on a.attrelid = c.oid where - c.relkind = 'c' + c.relkind in ('c', 'r', 'p', 'v', 'm', 'f') + and a.attnum > 0 and not a.attisdropped group by c.oid @@ -44,7 +45,7 @@ where t.typrelid = 0 or ( select - c.relkind = 'c' + c.relkind in ('c', 'r', 'p', 'v', 'm', 'f') from pg_class c where diff --git a/crates/pgls_typecheck/src/typed_identifier.rs b/crates/pgls_typecheck/src/typed_identifier.rs index 826c33992..37cd6b1c7 100644 --- a/crates/pgls_typecheck/src/typed_identifier.rs +++ b/crates/pgls_typecheck/src/typed_identifier.rs @@ -362,6 +362,113 @@ mod tests { ); } + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] + async fn test_apply_identifiers_table_row_type(test_db: PgPool) { + // A SQL function argument may be a table's row (composite) type, e.g. + // `CREATE FUNCTION f(row_arg public.tbl) ... SELECT row_arg.name`. + // Field access on such a parameter must resolve to the column's type + // and be replaced with that type's default literal. + let input = "select row_arg.id + row_arg.name"; + + let identifiers = vec![ + super::TypedIdentifier { + path: "get_tbl".to_string(), + name: Some("row_arg".to_string()), + type_: super::IdentifierType { + schema: Some("public".to_string()), + name: "tbl".to_string(), + is_array: false, + }, + }, + super::TypedIdentifier { + path: "get_tbl".to_string(), + name: Some("row_arg".to_string()), + type_: super::IdentifierType { + schema: Some("public".to_string()), + name: "tbl".to_string(), + is_array: false, + }, + }, + ]; + + let setup = r#" + CREATE TABLE "public"."tbl" ( + id integer, + name text + ); + "#; + + test_db + .execute(setup) + .await + .expect("Failed to setup test database"); + + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&pgls_treesitter_grammar::LANGUAGE.into()) + .expect("Error loading sql language"); + + let schema_cache = pgls_schema_cache::SchemaCache::load(&test_db) + .await + .expect("Failed to load Schema Cache"); + + let tree = parser.parse(input, None).unwrap(); + + let replacement = super::apply_identifiers(identifiers, &schema_cache, &tree, input); + + assert_eq!( + replacement.text_replacement.text(), + // `id` (integer) -> 0, `name` (text) -> '' + "select 0 + ''" + ); + } + + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] + async fn test_row_type_excludes_system_columns(test_db: PgPool) { + // System columns (e.g. `ctid`) are not real fields of a table's row + // type, so they must not be loaded as attributes: a reference to one is + // left unreplaced so the downstream typecheck still flags it. + let input = "select row_arg.ctid"; + + let identifiers = vec![super::TypedIdentifier { + path: "get_tbl".to_string(), + name: Some("row_arg".to_string()), + type_: super::IdentifierType { + schema: Some("public".to_string()), + name: "tbl".to_string(), + is_array: false, + }, + }]; + + let setup = r#" + CREATE TABLE "public"."tbl" ( + id integer, + name text + ); + "#; + + test_db + .execute(setup) + .await + .expect("Failed to setup test database"); + + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&pgls_treesitter_grammar::LANGUAGE.into()) + .expect("Error loading sql language"); + + let schema_cache = pgls_schema_cache::SchemaCache::load(&test_db) + .await + .expect("Failed to load Schema Cache"); + + let tree = parser.parse(input, None).unwrap(); + + let replacement = super::apply_identifiers(identifiers, &schema_cache, &tree, input); + + // `ctid` is a system column, so it is not resolved and stays as-is. + assert_eq!(replacement.text_replacement.text(), "select row_arg.ctid"); + } + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] async fn test_longer_identifiers(pool: PgPool) { // create or replace function retrieve(uid uuid, mail text) diff --git a/crates/pgls_typecheck/tests/diagnostics.rs b/crates/pgls_typecheck/tests/diagnostics.rs index ad8fcfc2c..bfef86c27 100644 --- a/crates/pgls_typecheck/tests/diagnostics.rs +++ b/crates/pgls_typecheck/tests/diagnostics.rs @@ -99,6 +99,71 @@ impl TestSetup<'_> { } } +#[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] +async fn function_arg_row_type(test_db: PgPool) { + // A SQL function whose argument is a table's row type, e.g. + // create function get_tbl_name(row_arg public.tbl) returns text + // language sql as $$ select row_arg.name $$; + // Accessing a field of the row argument must not raise a false-positive + // "missing FROM-clause entry" diagnostic. + let setup = r#" + create table public.tbl ( + id serial primary key, + name text not null + ); + "#; + + TestSetup { + name: "function_arg_row_type", + setup: Some(setup), + query: r#"select row_arg.name"#, + test_db: &test_db, + typed_identifiers: vec![TypedIdentifier { + path: "get_tbl_name".to_string(), + name: Some("row_arg".to_string()), + type_: IdentifierType { + schema: Some("public".to_string()), + name: "tbl".to_string(), + is_array: false, + }, + }], + } + .test() + .await; +} + +#[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] +async fn function_arg_view_row_type(test_db: PgPool) { + // A view (like a table) exposes a composite row type, so a function + // argument typed as the view must also resolve field access without a + // false-positive diagnostic. + let setup = r#" + create table public.tbl ( + id serial primary key, + name text not null + ); + create view public.tbl_view as select id, name from public.tbl; + "#; + + TestSetup { + name: "function_arg_view_row_type", + setup: Some(setup), + query: r#"select row_arg.name"#, + test_db: &test_db, + typed_identifiers: vec![TypedIdentifier { + path: "get_view_name".to_string(), + name: Some("row_arg".to_string()), + type_: IdentifierType { + schema: Some("public".to_string()), + name: "tbl_view".to_string(), + is_array: false, + }, + }], + } + .test() + .await; +} + #[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] async fn invalid_column(test_db: PgPool) { TestSetup { diff --git a/crates/pgls_typecheck/tests/snapshots/function_arg_row_type.snap b/crates/pgls_typecheck/tests/snapshots/function_arg_row_type.snap new file mode 100644 index 000000000..3c50948be --- /dev/null +++ b/crates/pgls_typecheck/tests/snapshots/function_arg_row_type.snap @@ -0,0 +1,5 @@ +--- +source: crates/pgls_typecheck/tests/diagnostics.rs +expression: content +--- +No Diagnostic diff --git a/crates/pgls_typecheck/tests/snapshots/function_arg_view_row_type.snap b/crates/pgls_typecheck/tests/snapshots/function_arg_view_row_type.snap new file mode 100644 index 000000000..3c50948be --- /dev/null +++ b/crates/pgls_typecheck/tests/snapshots/function_arg_view_row_type.snap @@ -0,0 +1,5 @@ +--- +source: crates/pgls_typecheck/tests/diagnostics.rs +expression: content +--- +No Diagnostic