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
21 changes: 21 additions & 0 deletions docs/SOUNDNESS.md
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,27 @@ what it is or refused:
`s.t`.
An `INSERT`'s target is replaced by its source, so it is put through the same check beside the
relations the two sources read.
- **A system relation is not the DDL's table.** Postgres searches `pg_catalog` before the search
path unless the path names it, so a bare `pg_attribute` reads `pg_catalog.pg_attribute` even
after `CREATE TABLE pg_attribute` has put a table of that name in `public`. The declared table's
key would make `SELECT DISTINCT attname` and `SELECT attname` one query, and over the catalog's
rows they are not (#202). So a bare relation name that starts with `pg_` is refused, whatever the
DDL declares, as a query's relation and as a DML target. So is a name under a system schema,
`information_schema` or any schema starting with `pg_` (a prefix Postgres reserves). Postgres
refuses to create a table in `pg_catalog` or `pg_toast`, and `information_schema.tables` is its
view whatever a DDL says. Those qualifiers are never stripped: `pg_toast.t` is not the DDL's
`t`. The rule is the prefix, not a list of one major's catalog relations. Postgres documents
that its system relations keep the `pg_` prefix, and a list that missed a relation a later major
adds would lower that relation against the DDL. What it costs is refusals of tables Postgres
would find: a user's table named `pg_*` read bare (read through its schema, `public.pg_x`, it
lowers), a temporary table of such a name (the session's temporary schema is searched first), a
table a DDL creates as `pg_temp.t`, a new table a superuser adds to `information_schema`, and a
query over the catalog under an inferred catalog, which
would type the catalog's columns from the query rather than from Postgres. A `WITH` binding named
like a catalog relation is the binding, as in Postgres. Relations no input creates, a temporary
table or one an extension installs into `pg_catalog`, stay outside the model, as everywhere: the
DDL is taken as the tables there are. A `search_path` that names `pg_catalog` after the user's
schemas would let a bare `pg_*` name reach the DDL's table, and no input states one (#91).
- **`ONLY` is a keyword.** In `FROM ONLY t`, `UPDATE ONLY t` or `DELETE FROM ONLY t` it leaves `t`'s
inheritance children out. The parser reads it only in `TRUNCATE`, `ALTER TABLE` and `LOCK`, and
elsewhere takes it as a table named `ONLY` with the alias `t`, which would resolve to a table or CTE
Expand Down
22 changes: 22 additions & 0 deletions sqleq-lean/src/recognize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,14 @@ fn extract(i: &Insert) -> Result<Parts, Refusal> {
let TableObject::TableName(name) = &i.table else {
return Err(unsupported("INSERT target is not a plain table"));
};
// As the frontend refuses it (#202): a bare `pg_*` target is `pg_catalog`'s relation, which
// Postgres searches before the search path, and a target under a system schema is that schema's.
// The schema is keyed by the bare name, so either would be read as the DDL's table of that name.
if sqleq_frontend::internals::system_relation(name) {
return Err(unsupported(format!(
"INSERT target {name} names a system relation, which Postgres writes in place of any table the DDL declares"
)));
}
let Some(target) = last_name(name) else {
return Err(unsupported("INSERT target is not a plain name"));
};
Expand Down Expand Up @@ -723,6 +731,20 @@ mod tests {
}
}

/// A target Postgres resolves into a system schema is not the DDL's table of that bare name
/// (#202), and a `pg_*` table in a user's schema still is.
#[test]
fn a_system_relation_target_is_refused() {
for sql in [
"INSERT INTO pg_attribute (attname) VALUES ($1)",
"INSERT INTO pg_catalog.t (a) VALUES ($1)",
"INSERT INTO information_schema.t (a) VALUES ($1)",
] {
assert!(matches!(rec(sql), Err(Refusal::Unsupported(r)) if r.contains("system relation")), "{sql}");
}
rec("INSERT INTO public.pg_attribute (attname) VALUES ($1)").unwrap();
}

#[test]
fn an_insert_without_a_column_list_is_read_and_re_rendered() {
for sql in ["INSERT INTO t VALUES ($1, $2)", "INSERT INTO t SELECT * FROM unnest($1::int[], $2::text[])"] {
Expand Down
29 changes: 26 additions & 3 deletions src/dml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,9 +1101,13 @@ fn update_pair(cat: &Catalog, a: &Update, b: &Update) -> Result<(Query, Query, [
///
/// That is how [`resolve_tables`][crate::normalize::resolve_tables] resolves the queries' own
/// references before [`lower`][crate::lower] reads them, so this resolves the target the same way the
/// rest of the pipeline will. Where the pair's guards keep a reference qualified (a system schema, or
/// one bare name reached through two qualifiers) `lower` then fails to resolve the target and the pair
/// is refused, so a mismatch here costs a refusal and never a proof.
/// rest of the pipeline will. Where the pair's guards keep a reference as written (one bare name
/// reached through two qualifiers) `lower` reads the name as written, and a target that is not
/// declared so fails to resolve and the pair is refused, so a mismatch here costs a refusal and never
/// a proof. A relation Postgres reads from a system schema, a bare `pg_*` or one under a system
/// schema ([`crate::normalize::system_relation`]), *is* found here when the DDL names it, and so it is
/// refused by name: by `lower` where the target becomes a scan (`UPDATE`, `DELETE`), and by
/// [`insert_pair`], whose target never does.
fn find_target(cat: &Catalog, name: &str) -> Option<usize> {
cat.resolve(name)
}
Expand Down Expand Up @@ -1351,6 +1355,17 @@ fn insert_pair(cat: &Catalog, a: &Insert, b: &Insert) -> Result<(Query, Query, [
ControlFlow::<()>::Continue(())
});
}
// SOUNDNESS GUARD (#202), before the declared short-circuit below: the target is replaced by its
// source, so `lower`'s refusal of a system relation never sees it. `INSERT INTO pg_attribute`
// writes `pg_catalog.pg_attribute`, whose columns, types and keys are not the DDL's.
for n in [na, nb] {
if crate::normalize::system_relation(n) {
return Err(unsupported(format!(
"the INSERT target {} names a system relation, which Postgres writes in place of any table the DDL declares",
obj_name(n)
)));
}
}
// As in `resolve_tables`, a name the DDL declares exactly as written is that table and needs no
// stripping, so only a target that would be stripped is held to the check.
let declared = cat.declares(&name) && cat.declares(&other);
Expand Down Expand Up @@ -1379,6 +1394,14 @@ fn insert_pair(cat: &Catalog, a: &Insert, b: &Insert) -> Result<(Query, Query, [

let idx = find_target(cat, &name)
.ok_or_else(|| schema(format!("no declared schema for INSERT target {name}")))?;
// A bare target can resolve to a table the DDL puts under a system schema (`t` to a declared
// `pg_catalog.t`), as a query's `FROM t` does in `resolve_tables` before `lower` refuses it.
if crate::normalize::system_key(&cat.tables[idx].name) {
return Err(unsupported(format!(
"the INSERT target {name} resolves to {}, a system relation, not a table the DDL declares",
cat.tables[idx].name
)));
}
no_trigger(cat, &name)?;
let t = &cat.tables[idx];
// The declared prefix, for the reason [`update_pair`] gives: a system column is not written by
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ mod verify;
pub mod internals {
pub use crate::catalog::{enforced_per_statement, obj_name, Catalog, SYSTEM_COLUMNS};
pub use crate::infer::{declares, nid};
pub use crate::normalize::system_relation;

/// Aliased rather than re-exported: `DIALECT` sits at the crate root, where making it
/// `pub` would widen the default API even with this feature off.
Expand Down
11 changes: 11 additions & 0 deletions src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,17 @@ fn factor_instance(cat: &Catalog, fns: &Fns, tf: &TableFactor, offset: usize, ou
return Err(unsupported(format!("table factor with {m}")));
}
let tn = obj_name(name);
// SOUNDNESS GUARD (#202): a bare `pg_*` name reads `pg_catalog`'s relation, which Postgres
// searches before the search path, and a name under a system schema reads that schema's.
// Neither reads the table the DDL declares, so its columns and keys say nothing about the
// rows read: `SELECT DISTINCT attname FROM pg_attribute` is not made redundant by a
// declared `pg_attribute (attname PRIMARY KEY)`. Before the lookup, so the refusal is the
// same whether the DDL declares the name or not.
if crate::normalize::system_relation(name) {
return Err(unsupported(format!(
"{tn} names a system relation, which Postgres reads in place of any table the DDL declares"
)));
}
let unknown = || schema(format!("unknown table {tn}{}", same_last_name(cat, &tn)));
let idx = cat.find(&tn).ok_or_else(unknown)?;
// An alias's column list renames the table's columns in order, and may stop short:
Expand Down
98 changes: 92 additions & 6 deletions src/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1610,9 +1610,54 @@ impl VisitorMut for ReplaceCteRefs<'_> {
}
}

/// Schemas whose contents are the server's, not the user's. A table under one of these is not the
/// table the DDL declares even if the bare names collide.
const SYS_SCHEMAS: [&str; 4] = ["pg_catalog", "information_schema", "pg_temp", "sys"];
/// Whether a schema is the server's rather than the user's: `information_schema`, or any name under
/// the `pg_` prefix, which Postgres reserves for its own schemas (`CREATE SCHEMA pg_x` is an error).
/// That is `pg_catalog`, `pg_toast`, the session's `pg_temp` and every `pg_temp_N` and
/// `pg_toast_temp_N`. A table under one of these is not the table the DDL declares, even if the bare
/// names collide.
///
/// Over-approximated, which only refuses more: compared lower-cased, though a quoted `"PG_CATALOG"`
/// is a schema a user may create, and `information_schema` as a whole, though a superuser may add a
/// table of their own to it.
pub(crate) fn system_schema(schema: &str) -> bool {
let s = schema.to_lowercase();
s.starts_with("pg_") || s == "information_schema"
}

/// Whether a relation reference names a relation Postgres reads from a system schema, whatever the
/// DDL declares under that name:
///
/// * **A bare `pg_*` name.** Postgres searches `pg_catalog` before the search path unless the path
/// names it, so `FROM pg_attribute` reads `pg_catalog.pg_attribute` even after a `CREATE TABLE
/// pg_attribute` has put a table of that name in `public`. Postgres documents that its system
/// relations keep the `pg_` prefix so that they cannot collide with user tables that avoid it, so
/// the prefix is the rule, not a list of one major's catalog: a list that misses a relation a newer
/// major adds would lower that relation against the DDL. A user table named `pg_*` is read
/// through its schema (`public.pg_x`), which this does not refuse.
/// * **Any qualifier that is a system schema** (`system_schema`): `information_schema`, or any name
/// under the `pg_` prefix Postgres reserves. `pg_catalog.pg_attribute` is the catalog's even
/// when the DDL "declares" it, since Postgres would not run that `CREATE TABLE`.
///
/// Lower-cased, as `safe_to_strip` keys a bare name, so `"PG_X"` (not `pg_x` to Postgres) is
/// refused too; that costs only a refusal. A part that is not an identifier counts as no name.
pub fn system_relation(name: &ObjectName) -> bool {
let parts: Vec<String> =
name.0.iter().map(|p| p.as_ident().map(|id| id.value.to_lowercase()).unwrap_or_default()).collect();
system_path(&parts)
}

/// [`system_relation`] over a catalog key, the dotted, lower-cased name `catalog::scan_ddl` stores.
pub(crate) fn system_key(key: &str) -> bool {
system_path(&key.split('.').map(str::to_string).collect::<Vec<_>>())
}

fn system_path(parts: &[String]) -> bool {
match parts.split_last() {
Some((bare, [])) => bare.starts_with("pg_"),
Some((_, qualifier)) => qualifier.iter().any(|q| system_schema(q)),
None => false,
}
}

/// Drop the schema/database qualifier from table references: `part_16.orders` -> `orders`.
///
Expand Down Expand Up @@ -1765,9 +1810,10 @@ pub(crate) fn safe_to_strip(names: &[ObjectName]) -> bool {
.iter()
.map(|p| p.as_ident().map(crate::dml::fold_ident).unwrap_or_default())
.collect::<Vec<_>>();
// The system schemas by their lower-cased name: `"PG_CATALOG"` is not one, but stopping on it
// costs only a refusal.
if qualifier.iter().any(|q| SYS_SCHEMAS.contains(&q.to_lowercase().as_str())) {
// The system schemas, as `system_relation` reads them, and `sys`, which is no Postgres schema
// but names a catalog elsewhere. Every `pg_` schema counts: `pg_toast.t` or `pg_temp_3.t`
// stripped to `t` would read the DDL's `t`, which neither is.
if qualifier.iter().any(|q| system_schema(q) || q.eq_ignore_ascii_case("sys")) {
return false;
}
// Two spellings of one bare name are two tables until proven otherwise.
Expand Down Expand Up @@ -2405,11 +2451,51 @@ mod tests {
"SELECT a FROM part_1.t JOIN pg_catalog.pg_class c ON c.oid = part_1.t.oid",
"SELECT a FROM part_1.t JOIN information_schema.columns c ON c.x = part_1.t.y",
"SELECT a FROM part_1.t JOIN pg_class c ON c.oid = part_1.t.oid",
// Every `pg_` schema is the server's, not only the ones a list names (#202).
"SELECT a FROM pg_toast.t",
"SELECT a FROM pg_temp_3.t",
"SELECT a FROM pg_toast_temp_3.t",
] {
assert_eq!(unschemad(sql), sql, "should not be rewritten");
}
}

/// The relations Postgres reads from a system schema whatever the DDL says (#202): a bare `pg_*`
/// name, which `pg_catalog` holds ahead of the search path, and any name under a system schema.
/// A `pg_*` name in a user's schema is the user's table.
#[test]
fn system_relations_are_bare_pg_names_and_system_schemas() {
let name = |sql: &str| {
let q = parse_queries(sql, sql).remove(0);
let mut names = CollectTableNames(Vec::new());
let _ = Visit::visit(&q, &mut names);
names.0.remove(0)
};
for sql in [
"SELECT 1 FROM pg_attribute",
r#"SELECT 1 FROM "pg_attribute""#,
"SELECT 1 FROM PG_CLASS",
"SELECT 1 FROM pg_catalog.pg_attribute",
"SELECT 1 FROM pg_catalog.t",
"SELECT 1 FROM information_schema.tables",
"SELECT 1 FROM pg_toast.t",
"SELECT 1 FROM pg_temp.t",
"SELECT 1 FROM db.pg_catalog.t",
] {
assert!(system_relation(&name(sql)), "{sql}");
}
for sql in [
"SELECT 1 FROM t",
"SELECT 1 FROM public.pg_attribute",
"SELECT 1 FROM s.pg_x",
"SELECT 1 FROM catalog.t",
"SELECT 1 FROM pgx",
] {
assert!(!system_relation(&name(sql)), "{sql}");
}
assert!(system_key("pg_attribute") && system_key("pg_catalog.t") && !system_key("public.pg_attribute"));
}

/// `a.orders` and `b.orders` are two tables. Stripping would merge them, turning a join between
/// two relations into a self-join — a different query, not a different spelling.
#[test]
Expand Down
18 changes: 18 additions & 0 deletions tests/pairs/names/catalog_qualified_declared_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Unless explicitly stated otherwise all files in this repository are licensed under the
-- Apache License Version 2.0.
-- This product includes software developed at Datadog (https://www.datadoghq.com/).
-- Copyright 2026-Present Datadog, Inc.

-- truth: not-equivalent
-- expect frontend: refuse:unsupported
-- expect fuzz: error
-- expect qed: no-plan
-- expect sqleq-solver: no-plan
-- expect lean: unsupported
-- origin: issue #202: a DDL that declares a table under pg_catalog was read as that table, though
-- Postgres refuses to create it and the queries read the catalog's relation
-- witness: any database: pg_catalog.pg_attribute holds many rows per attname, so A returns fewer
-- rows than B
create table "pg_catalog"."pg_attribute" ("attname" TEXT PRIMARY KEY, "attnum" INTEGER);
SELECT DISTINCT "attname" FROM "pg_catalog"."pg_attribute";
SELECT "attname" FROM "pg_catalog"."pg_attribute";
18 changes: 18 additions & 0 deletions tests/pairs/names/catalog_relation_shadows_declared_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Unless explicitly stated otherwise all files in this repository are licensed under the
-- Apache License Version 2.0.
-- This product includes software developed at Datadog (https://www.datadoghq.com/).
-- Copyright 2026-Present Datadog, Inc.

-- truth: not-equivalent
-- expect frontend: refuse:unsupported
-- expect fuzz: counterexample
-- expect qed: no-plan
-- expect sqleq-solver: no-plan
-- expect lean: unsupported
-- origin: issue #202: Postgres searches pg_catalog before the search path, so a bare pg_attribute
-- read the catalog's relation while the frontend lowered the DDL's table and its key
-- witness: any database: pg_catalog.pg_attribute holds many rows per attname, so A returns fewer
-- rows than B (on a fresh Postgres 17 database, 1188 against 3146)
create table "pg_attribute" ("attname" TEXT PRIMARY KEY, "attnum" INTEGER);
SELECT DISTINCT "attname" FROM "pg_attribute";
SELECT "attname" FROM "pg_attribute";
17 changes: 17 additions & 0 deletions tests/pairs/names/public_qualified_pg_named_table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- Unless explicitly stated otherwise all files in this repository are licensed under the
-- Apache License Version 2.0.
-- This product includes software developed at Datadog (https://www.datadoghq.com/).
-- Copyright 2026-Present Datadog, Inc.

-- truth: equivalent
-- expect frontend: emit
-- expect fuzz: no-counterexample
-- expect qed: proved
-- expect sqleq-solver: proved
-- expect lean: unsupported
-- origin: issue #202: the control for the refusal of a bare pg_* name: a pg_* table read through
-- the schema the DDL declares it in is that table
-- argument: public.pg_attribute is the declared table, and its key makes DISTINCT redundant
create table "public"."pg_attribute" ("attname" TEXT PRIMARY KEY, "attnum" INTEGER);
SELECT DISTINCT "attname" FROM "public"."pg_attribute";
SELECT "attname" FROM "public"."pg_attribute";
18 changes: 18 additions & 0 deletions tests/pairs/names/temp_schema_qualifier_not_stripped.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Unless explicitly stated otherwise all files in this repository are licensed under the
-- Apache License Version 2.0.
-- This product includes software developed at Datadog (https://www.datadoghq.com/).
-- Copyright 2026-Present Datadog, Inc.

-- truth: not-equivalent
-- expect frontend: refuse:unsupported
-- expect fuzz: error
-- expect qed: no-plan
-- expect sqleq-solver: no-plan
-- expect lean: unsupported
-- origin: issue #202: only pg_catalog, information_schema and pg_temp stopped the schema strip, so
-- pg_temp_3.t was stripped to t and read as the DDL's table and its key
-- witness: a session whose temporary schema is pg_temp_3, holding a temporary t = {(1), (1)}: A
-- returns 1 once and B twice
create table "t" ("a" INTEGER PRIMARY KEY);
SELECT DISTINCT "a" FROM "pg_temp_3"."t";
SELECT "a" FROM "pg_temp_3"."t";
3 changes: 3 additions & 0 deletions tests/refusal_kinds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ const ACCEPTED: &[Row] = &[
// Postgres syntax the frontend's parser does not read.
row(Unsupported, "SELECT", "SELECT 1", "parser does not read", "prepares: a select list may be empty"),
row(Unsupported, r#"SELECT "a" FROM ONLY "t""#, r#"SELECT "a" FROM "t""#, "ONLY in a table reference", "prepares"),
// A catalog relation Postgres reads and the frontend does not model, not an unknown table (#202).
row(Unsupported, r#"SELECT "relname" FROM "pg_class""#, r#"SELECT "relname" FROM "pg_class" WHERE true"#,
"system relation", "prepares: reads pg_catalog.pg_class"),
row(Unsupported, r#"TABLE "t""#, r#"SELECT * FROM "t""#, "parser does not read", "prepares"),
row(Unsupported, r#"DELETE FROM "t""#, r#"SELECT "a" FROM "t""#, "DELETE paired with non-DELETE", "prepares"),
row(Unsupported, r#"MERGE INTO "t" USING "u" ON "t"."a" = "u"."a" WHEN MATCHED THEN DELETE"#,
Expand Down
Loading
Loading