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
3 changes: 3 additions & 0 deletions changelog.d/3349-postgres-system-schema-columns.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- The PostgreSQL adapter no longer picks up columns from other schemas when introspecting a table. `cfdbinfo(type="columns")` applies no schema restriction, so JDBC matched the table name across every schema on the connection — and PostgreSQL, YugabyteDB and CockroachDB all ship catalog views named `sequences`, `tables`, `columns`, `views`, `triggers` and more. An application table sharing one of those names silently collected a second batch of phantom columns typed `"information_schema"."sql_identifier"`, which no `$getType()` case matched, and the model failed to initialise with the opaque `key [RV] doesn't exist`. Rows from `information_schema`, `pg_catalog`, `crdb_internal` and `pg_extension` are now dropped — inside the `cacheDatabaseSchema` memo, so the filtering costs one pass per datasource+table per application lifetime. Reported against YugabyteDB (PostgreSQL 15 wire protocol) and reproducible on stock PostgreSQL (#3349)
- The migrator's column lookup got the same guard. `vendor/wheels/migrator/Base.cfc` calls `$dbinfo(type="columns")` directly rather than through the model adapter, so `changeTable(name="sequences")` adding a column named `data_type` or `start_value` could see the catalog view's column and treat it as already present (#3349)
- An unmapped PostgreSQL column type now throws `Wheels.UnknownColumnType` naming the type, instead of `key [RV] doesn't exist` from an unassigned return variable — the failure names the column type and points at catalog bleed as the likely cause rather than reading like a framework bug (#3349)
37 changes: 37 additions & 0 deletions vendor/wheels/Global.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,43 @@ return local.$wheels;
return local.rv;
}

/**
* Drops rows belonging to a database's system schemas from a `$dbinfo(type="columns")`
* result.
*
* `cfdbinfo(type="columns")` passes no schema restriction to JDBC's `getColumns()`, so the
* table name is matched across EVERY schema on the connection. PostgreSQL and YugabyteDB
* ship real ANSI `information_schema` views named `sequences`, `tables`, `columns`,
* `views`, `triggers` and more, so an application table sharing one of those names silently
* collects a second batch of phantom columns from the catalog (issue #3349). No application
* table lives in a system schema, so filtering them out is always safe.
*
* A result set that carries no `table_schem` column — several engines omit it — is returned
* untouched. Yes, JDBC really does spell it `table_schem`, not `table_schema`.
*/
public query function $excludeSystemSchemaRows(
required query columns,
string schemas = "information_schema,pg_catalog,crdb_internal,pg_extension"
) {
if (!ListFindNoCase(arguments.columns.columnList, "table_schem")) {
return arguments.columns;
}
local.rv = QueryNew(arguments.columns.columnList);
local.columnNames = ListToArray(arguments.columns.columnList);
local.iEnd = arguments.columns.recordCount;
for (local.i = 1; local.i <= local.iEnd; local.i++) {
if (!ListFindNoCase(arguments.schemas, arguments.columns["table_schem"][local.i])) {
QueryAddRow(local.rv);
local.jEnd = ArrayLen(local.columnNames);
for (local.j = 1; local.j <= local.jEnd; local.j++) {
local.item = local.columnNames[local.j];
QuerySetCell(local.rv, local.item, arguments.columns[local.item][local.i]);
}
}
}
return local.rv;
}

public any function $wddx(required any input, string action = "cfml2wddx", boolean useTimeZoneInfo = true) {
arguments.output = "local.output";
local.args = {};
Expand Down
37 changes: 37 additions & 0 deletions vendor/wheels/databaseAdapters/PostgreSQL/PostgreSQLModel.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,47 @@ component extends="wheels.databaseAdapters.Base" output=false {
case "geography":
local.rv = "cf_sql_other";
break;
default:
// Without this branch `local.rv` is never assigned and the return throws
// `key [RV] doesn't exist` — an error that names nothing useful and reads
// like a framework bug. The classic source was catalog bleed: a table whose
// name collides with an `information_schema` view picked up phantom columns
// typed `"information_schema"."sql_identifier"` (issue #3349, fixed in
// `$getColumnInfo()` below). Anything reaching here now is a genuinely
// unmapped PostgreSQL type, so say so.
Throw(
type = "Wheels.UnknownColumnType",
message = "The PostgreSQL column type `#arguments.type#` is not mapped to a CFML SQL type.",
extendedInfo = "Add a case for `#arguments.type#` to `$getType()` in `vendor/wheels/databaseAdapters/PostgreSQL/PostgreSQLModel.cfc`. If the type name looks schema-qualified (e.g. `""information_schema"".""sql_identifier""`), the column is not yours — it came from a catalog view sharing your table's name."
);
}
return local.rv;
}

/**
* Override Base adapter's function.
*
* `cfdbinfo(type="columns")` applies no schema restriction, so JDBC matches the table name
* across every schema on the connection. PostgreSQL and YugabyteDB both ship ANSI
* `information_schema` views named `sequences`, `tables`, `columns`, `views`, `triggers`
* and more, so an application table named `sequences` collected a second batch of columns
* from `information_schema.sequences` — typed `"information_schema"."sql_identifier"`,
* which nothing in `$getType()` matched (issue #3349). Via `CockroachDBModel`, the
* `crdb_internal` and `pg_extension` view names collide the same way.
*
* Filtering here rather than in `$getColumns()` keeps the work behind the
* `cacheDatabaseSchema` memo that `$getColumns()` wraps around this call — once per
* datasource+table per application lifetime instead of on every read.
*/
public query function $getColumnInfo(
required string table,
required string datasource,
required string username,
required string password
) {
return $excludeSystemSchemaRows(columns = super.$getColumnInfo(argumentCollection = arguments));
}

/**
* Call functions to make adapter specific changes to arguments before executing query.
*/
Expand Down
8 changes: 8 additions & 0 deletions vendor/wheels/migrator/Base.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,14 @@ component extends="wheels.Global"{
type = "columns",
table = arguments.tableName
);
// This path calls $dbinfo directly rather than going through the model adapter, so it
// needs its own catalog-bleed guard: an unrestricted cfdbinfo matches the table name in
// every schema on the connection, and a table named after an `information_schema` view
// (`sequences`, `tables`, `columns`, …) picks up that view's columns too. A
// changeTable() adding a column whose name collides with one of them — `data_type`,
// `start_value` — would then see it as already present (issue #3349). No application
// table lives in a system schema, so this cannot drop a real column.
local.columns = $excludeSystemSchemaRows(columns = local.columns);
local.columnList = ValueList(local.columns.COLUMN_NAME);
if (!StructKeyExists(request, "$wheelsMigratorColumns")) {
request.$wheelsMigratorColumns = {};
Expand Down
115 changes: 115 additions & 0 deletions vendor/wheels/tests/specs/model/SystemSchemaColumnFilterSpec.cfc
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
component extends="wheels.WheelsTest" {

function run() {
g = application.wo

// Regression for issue #3349.
//
// `cfdbinfo(type="columns")` passes no schema restriction to JDBC's getColumns(), so the
// table name is matched across every schema on the connection. PostgreSQL and YugabyteDB
// ship real ANSI `information_schema` views named `sequences`, `tables`, `columns`,
// `views`, `triggers` and more — so an application table named `sequences` silently
// collected a second batch of columns from `information_schema.sequences`.
//
// These specs drive the filter with a hand-built catalog result rather than a live
// PostgreSQL connection, so the logic is covered on every engine × database leg
// including the SQLite-only ones. The end-to-end behaviour is verified separately
// against a real PostgreSQL container.
describe("Tests that $excludeSystemSchemaRows", () => {

$catalogResult = function() {
var q = QueryNew("table_schem,table_name,column_name,type_name")
var rows = [
["public", "sequences", "id", "uuid"],
["public", "sequences", "name", "varchar"],
["public", "sequences", "value", "int8"],
["information_schema", "sequences", "sequence_catalog", '"information_schema"."sql_identifier"'],
["information_schema", "sequences", "start_value", '"information_schema"."character_data"'],
["pg_catalog", "sequences", "oid", "oid"],
["crdb_internal", "sequences", "descriptor_id", "int8"],
["pg_extension", "sequences", "ext_col", "int8"]
]
for (var row in rows) {
QueryAddRow(q)
QuerySetCell(q, "table_schem", row[1])
QuerySetCell(q, "table_name", row[2])
QuerySetCell(q, "column_name", row[3])
QuerySetCell(q, "type_name", row[4])
}
return q
}

it("keeps only the application schema's columns", () => {
result = g.$excludeSystemSchemaRows(columns = $catalogResult())

expect(result.recordCount).toBe(3)
expect(ValueList(result.column_name)).toBe("id,name,value")
})

it("drops every system schema, not just information_schema", () => {
result = g.$excludeSystemSchemaRows(columns = $catalogResult())

// `crdb_internal` and `pg_extension` collide too, reached via
// CockroachDBModel extends PostgreSQLModel
expect(ValueList(result.table_schem)).toBe("public,public,public")
})

it("preserves every column of the source result", () => {
source = $catalogResult()
result = g.$excludeSystemSchemaRows(columns = source)

expect(ListSort(result.columnList, "textnocase")).toBe(ListSort(source.columnList, "textnocase"))
})

it("returns a result with no table_schem column untouched", () => {
// several engines' cfdbinfo omits it; filtering must not blank the result
noSchema = QueryNew("column_name")
QueryAddRow(noSchema)
QuerySetCell(noSchema, "column_name", "id")

result = g.$excludeSystemSchemaRows(columns = noSchema)

expect(result.recordCount).toBe(1)
})

it("is case-insensitive about the schema name", () => {
upper = QueryNew("table_schem,column_name")
QueryAddRow(upper)
QuerySetCell(upper, "table_schem", "INFORMATION_SCHEMA")
QuerySetCell(upper, "column_name", "sequence_catalog")

expect(g.$excludeSystemSchemaRows(columns = upper).recordCount).toBe(0)
})
})

describe("Tests that the PostgreSQL adapter", () => {

// Before this, an unmapped type left `local.rv` unassigned and the return threw
// `key [RV] doesn't exist` — which names neither the column, the type, nor the
// table, and reads like a framework bug rather than a schema problem.
it("names the offending type instead of throwing key [RV] doesn't exist", () => {
adapter = CreateObject("component", "wheels.databaseAdapters.PostgreSQL.PostgreSQLModel")
thrown = {type: "", message: ""}

try {
adapter.$getType(type = '"information_schema"."sql_identifier"')
} catch (any e) {
thrown.type = e.type
thrown.message = e.message
}

expect(thrown.type).toBe("Wheels.UnknownColumnType")
expect(thrown.message).toInclude("sql_identifier")
})

it("still maps the types it knows", () => {
adapter = CreateObject("component", "wheels.databaseAdapters.PostgreSQL.PostgreSQLModel")

expect(adapter.$getType(type = "int8")).toBe("cf_sql_bigint")
expect(adapter.$getType(type = "uuid")).toBe("cf_sql_varchar")
expect(adapter.$getType(type = "jsonb")).toBe("cf_sql_longvarchar")
})
})
}

}
Loading