From 6a6075d811e13a102dd1f92630b8c19c99b92feb Mon Sep 17 00:00:00 2001 From: Sanskar Soni Date: Sat, 22 Aug 2026 13:25:01 +0530 Subject: [PATCH] feat: reject unsupported foreign table column types at DDL time --- supabase-wrappers/src/attrs.rs | 100 ++++++++++ supabase-wrappers/src/event_triggers.rs | 16 ++ supabase-wrappers/src/interface.rs | 10 +- supabase-wrappers/src/lib.rs | 2 + wrappers/build.rs | 54 ++++++ wrappers/src/event_triggers.rs | 242 ++++++++++++++++++++++++ wrappers/src/lib.rs | 1 + 7 files changed, 423 insertions(+), 2 deletions(-) create mode 100644 supabase-wrappers/src/attrs.rs create mode 100644 supabase-wrappers/src/event_triggers.rs create mode 100644 wrappers/src/event_triggers.rs diff --git a/supabase-wrappers/src/attrs.rs b/supabase-wrappers/src/attrs.rs new file mode 100644 index 00000000..fe325c16 --- /dev/null +++ b/supabase-wrappers/src/attrs.rs @@ -0,0 +1,100 @@ +//! Checks foreign table column types against what `Cell` can represent. + +use pgrx::PgSqlErrorCode; +use pgrx::pg_sys::panic::ErrorReport; +use pgrx::prelude::*; +use pgrx::rel::PgRelation; +use std::ffi::CStr; + +// Must match what Cell::into_datum()/from_polymorphic_datum() (interface.rs) actually +// handle: into_datum() writes straight into the output slot with no type check, so it +// only needs binary compatibility (text/varchar/bpchar share the varlena layout); but +// from_polymorphic_datum() matches on exact OID, so rowid/qual columns need a real arm +// there too or they silently parse as None instead of erroring. +const SUPPORTED_TYPE_OIDS: &[pg_sys::Oid] = &[ + pg_sys::BOOLOID, + pg_sys::CHAROID, + pg_sys::INT2OID, + pg_sys::FLOAT4OID, + pg_sys::INT4OID, + pg_sys::FLOAT8OID, + pg_sys::INT8OID, + pg_sys::NUMERICOID, + pg_sys::TEXTOID, + pg_sys::VARCHAROID, + pg_sys::BPCHAROID, + pg_sys::DATEOID, + pg_sys::TIMEOID, + pg_sys::TIMESTAMPOID, + pg_sys::TIMESTAMPTZOID, + pg_sys::INTERVALOID, + pg_sys::JSONBOID, + pg_sys::BYTEAOID, + pg_sys::UUIDOID, + pg_sys::BOOLARRAYOID, + pg_sys::INT2ARRAYOID, + pg_sys::INT4ARRAYOID, + pg_sys::INT8ARRAYOID, + pg_sys::FLOAT4ARRAYOID, + pg_sys::FLOAT8ARRAYOID, + pg_sys::TEXTARRAYOID, + pg_sys::VARCHARARRAYOID, + pg_sys::BPCHARARRAYOID, +]; + +const SUPPORTED_TYPES_HINT: &str = "supported column types are: boolean, \"char\", smallint, \ + integer, bigint, real, double precision, numeric, text, character varying, character, date, \ + time, timestamp, timestamp with time zone, interval, jsonb, bytea, uuid, arrays of these, \ + and domains over any of these"; + +/// Resolves domains to their base type first, so e.g. `CREATE DOMAIN my_text AS text` passes. +fn is_supported_type(typoid: pg_sys::Oid) -> bool { + let base = unsafe { pg_sys::getBaseType(typoid) }; + SUPPORTED_TYPE_OIDS.contains(&base) +} + +#[derive(thiserror::Error, Debug)] +pub enum AttrsError { + #[error("foreign table \"{table}\" has columns with unsupported data types: {columns}")] + UnsupportedColumnTypes { table: String, columns: String }, +} + +impl From for ErrorReport { + fn from(value: AttrsError) -> Self { + let message = format!("{value}"); + ErrorReport::new( + PgSqlErrorCode::ERRCODE_FDW_INVALID_DATA_TYPE, + message, + SUPPORTED_TYPES_HINT, + ) + } +} + +/// Checks every non-dropped column of `relid`, collecting all offending columns into a +/// single error rather than stopping at the first one. +pub fn check_foreign_table_column_types(relid: pg_sys::Oid) -> Result<(), AttrsError> { + let relation = unsafe { PgRelation::open(relid) }; + let tuple_desc = relation.tuple_desc(); + + let mut bad_columns = Vec::new(); + for attr in tuple_desc.iter().filter(|a| !a.is_dropped()) { + if is_supported_type(attr.atttypid) { + continue; + } + let type_name = unsafe { + CStr::from_ptr(pg_sys::format_type_be(attr.atttypid)) + .to_string_lossy() + .into_owned() + }; + bad_columns.push(format!("\"{}\" (type {type_name})", attr.name())); + } + + if bad_columns.is_empty() { + return Ok(()); + } + + Err(AttrsError::UnsupportedColumnTypes { + table: relation.name().to_string(), + columns: bad_columns.join(", "), + }) +} diff --git a/supabase-wrappers/src/event_triggers.rs b/supabase-wrappers/src/event_triggers.rs new file mode 100644 index 00000000..fbd948c5 --- /dev/null +++ b/supabase-wrappers/src/event_triggers.rs @@ -0,0 +1,16 @@ +//! Generic support for writing event trigger handlers in Rust (`pgrx` has `trigger_support` +//! for row triggers, but no equivalent for event triggers). + +use pgrx::nodes::is_a; +use pgrx::prelude::*; + +/// Mirrors `pg_sys::called_as_trigger`, for event triggers instead of row triggers. +/// +/// # Safety +/// +/// `fcinfo` must be a valid `pg_sys::FunctionCallInfo` for the duration of the call. +pub unsafe fn called_as_event_trigger(fcinfo: pg_sys::FunctionCallInfo) -> bool { + let fcinfo = unsafe { fcinfo.as_ref().expect("fcinfo was null") }; + !fcinfo.context.is_null() + && unsafe { is_a(fcinfo.context, pg_sys::NodeTag::T_EventTriggerData) } +} diff --git a/supabase-wrappers/src/interface.rs b/supabase-wrappers/src/interface.rs index 10c01ec7..935eb743 100644 --- a/supabase-wrappers/src/interface.rs +++ b/supabase-wrappers/src/interface.rs @@ -312,7 +312,11 @@ impl FromDatum for Cell { PgOid::BuiltIn(PgBuiltInOids::NUMERICOID) => { AnyNumeric::from_datum(datum, is_null).map(Cell::Numeric) } - PgOid::BuiltIn(PgBuiltInOids::TEXTOID) => { + PgOid::BuiltIn(PgBuiltInOids::TEXTOID) + | PgOid::BuiltIn(PgBuiltInOids::VARCHAROID) + | PgOid::BuiltIn(PgBuiltInOids::BPCHAROID) => { + // `text`, `varchar` and `bpchar` all share the same varlena + // representation, so it's safe to read any of them as a `String`. String::from_datum(datum, is_null).map(Cell::String) } PgOid::BuiltIn(PgBuiltInOids::DATEOID) => { @@ -361,7 +365,9 @@ impl FromDatum for Cell { PgOid::BuiltIn(PgBuiltInOids::FLOAT8ARRAYOID) => { Vec::>::from_datum(datum, false).map(Cell::F64Array) } - PgOid::BuiltIn(PgBuiltInOids::TEXTARRAYOID) => { + PgOid::BuiltIn(PgBuiltInOids::TEXTARRAYOID) + | PgOid::BuiltIn(PgBuiltInOids::VARCHARARRAYOID) + | PgOid::BuiltIn(PgBuiltInOids::BPCHARARRAYOID) => { Vec::>::from_datum(datum, false).map(Cell::StringArray) } PgOid::Custom(_) => { diff --git a/supabase-wrappers/src/lib.rs b/supabase-wrappers/src/lib.rs index 0fc483d5..081e0e63 100644 --- a/supabase-wrappers/src/lib.rs +++ b/supabase-wrappers/src/lib.rs @@ -292,6 +292,8 @@ //! - [SQL Server](https://github.com/supabase/wrappers/tree/main/wrappers/src/fdw/mssql_fdw): A FDW for [Microsoft SQL Server](https://www.microsoft.com/en-au/sql-server/) which supports data read only. //! - [Redis](https://github.com/supabase/wrappers/tree/main/wrappers/src/fdw/redis_fdw): A FDW for [Redis](https://redis.io/) which supports data read only. +pub mod attrs; +pub mod event_triggers; pub mod interface; pub mod options; pub mod qual; diff --git a/wrappers/build.rs b/wrappers/build.rs index 80c5529a..0927b9ef 100644 --- a/wrappers/build.rs +++ b/wrappers/build.rs @@ -9,6 +9,7 @@ fn main() { // otherwise leading it to report the include!() in s3vec.rs as unresolvable // even though the module is only ever compiled when the feature is on. generate_s3vec_type_sql(); + generate_event_triggers_sql(); } /// Generates `s3vec_type_sql.rs` in OUT_DIR, which contains the `pgrx::extension_sql!` call @@ -130,3 +131,56 @@ $s3vec_upgrade$; fs::write(&out_path, content) .unwrap_or_else(|e| panic!("failed to write {}: {e}", out_path.display())); } + +/// Generates `event_triggers_sql.rs` in OUT_DIR, which contains the `pgrx::extension_sql!` +/// call registering `check_supported_column_types` (the event trigger from +/// `event_triggers.rs`) with the correct versioned library name embedded as a string +/// literal. Same indirection as `generate_s3vec_type_sql`, same reason. +/// +/// The generated file is included in `event_triggers.rs` via: +/// `include!(concat!(env!("OUT_DIR"), "/event_triggers_sql.rs"));` +fn generate_event_triggers_sql() { + let out_dir = std::env::var("OUT_DIR").expect("OUT_DIR not set"); + let pkg_name = std::env::var("CARGO_PKG_NAME").expect("CARGO_PKG_NAME not set"); + let pkg_version = std::env::var("CARGO_PKG_VERSION").expect("CARGO_PKG_VERSION not set"); + let lib_name = format!("{pkg_name}-{pkg_version}"); + + // The outer r##"..."## delimiter allows r#"..."# to appear inside the format string. + let content = format!( + r##"// @generated by build.rs — do not edit by hand. +// Library name "{lib_name}" is embedded at compile time from CARGO_PKG_NAME/VERSION. +pgrx::extension_sql!( + r#"DO $register_event_triggers$ +BEGIN + + -- 1. Check supported types function + -- no OR REPLACE guard needed (never raises duplicate_function); no + -- IMMUTABLE/STRICT/PARALLEL SAFE either, since none of them are true here + CREATE OR REPLACE FUNCTION "check_supported_column_types"() + RETURNS event_trigger + LANGUAGE c + AS '{lib_name}', 'check_supported_column_types'; + + -- 2. Register event trigger + -- 'ALTER TABLE' is needed too: Postgres tags `ALTER TABLE ADD COLUMN` + -- (the common way to alter a foreign table) as 'ALTER TABLE', not 'ALTER FOREIGN TABLE'. + BEGIN + CREATE EVENT TRIGGER check_supported_column_types + ON ddl_command_end + WHEN TAG IN ('CREATE FOREIGN TABLE', 'ALTER FOREIGN TABLE', 'ALTER TABLE') + EXECUTE FUNCTION check_supported_column_types(); + EXCEPTION WHEN duplicate_object THEN NULL; + END; +END +$register_event_triggers$; +"#, + name = "register_event_triggers", + creates = [Function(check_supported_column_types)], +); +"## + ); + + let out_path = Path::new(&out_dir).join("event_triggers_sql.rs"); + fs::write(&out_path, content) + .unwrap_or_else(|e| panic!("failed to write {}: {e}", out_path.display())); +} diff --git a/wrappers/src/event_triggers.rs b/wrappers/src/event_triggers.rs new file mode 100644 index 00000000..08fde2bc --- /dev/null +++ b/wrappers/src/event_triggers.rs @@ -0,0 +1,242 @@ +use pgrx::pg_sys::panic::ErrorReportable; +use pgrx::prelude::*; +use std::collections::HashSet; +use supabase_wrappers::attrs::check_foreign_table_column_types; +use supabase_wrappers::event_triggers::called_as_event_trigger; + +// pgrx::extension_sql! requires a string literal, so build.rs generates +// event_triggers_sql.rs with the versioned library name (e.g. "wrappers-0.6.0") +// embedded as a literal from CARGO_PKG_NAME/VERSION at compile time. +include!(concat!(env!("OUT_DIR"), "/event_triggers_sql.rs")); + +/// Fired on `ddl_command_end` for `CREATE FOREIGN TABLE` / `ALTER FOREIGN TABLE`; rejects +/// the statement if a `wrappers`-backed foreign table it touched has an unsupported column +/// type. Scoped via `pg_depend` to tables whose FDW handler belongs to the `wrappers` +/// extension, so unrelated FDWs (`postgres_fdw`, etc.) are untouched. +#[unsafe(no_mangle)] +#[pg_guard] +pub unsafe extern "C-unwind" fn check_supported_column_types( + fcinfo: &pg_sys::FunctionCallInfoBaseData, +) -> pg_sys::Datum { + if unsafe { !called_as_event_trigger(fcinfo as *const _ as *mut _) } { + return pg_sys::Datum::from(0); + } + + let objids: Vec = Spi::connect(|client| { + client + .select( + "SELECT cmd.objid + FROM pg_event_trigger_ddl_commands() cmd + WHERE EXISTS ( + SELECT 1 + FROM pg_foreign_table ft + JOIN pg_foreign_server fs ON fs.oid = ft.ftserver + JOIN pg_foreign_data_wrapper fdw ON fdw.oid = fs.srvfdw + JOIN pg_depend dep + ON dep.classid = 'pg_proc'::regclass + AND dep.objid = fdw.fdwhandler + AND dep.refclassid = 'pg_extension'::regclass + AND dep.deptype = 'e' + JOIN pg_extension ext + ON ext.oid = dep.refobjid + AND ext.extname = 'wrappers' + WHERE ft.ftrelid = cmd.objid + )", + None, + &[], + ) + .and_then(|table| { + table + .map(|row| Ok(row.get::(1)?.unwrap_or_default())) + .collect::, pgrx::spi::Error>>() + }) + }) + .unwrap_or_report(); + + // dedupe: a single ALTER TABLE with multiple ADD COLUMN subcommands can + // yield one ddl_commands row per subcommand, all against the same table + let mut seen = HashSet::new(); + for objid in objids { + if seen.insert(objid) { + check_foreign_table_column_types(objid).unwrap_or_report(); + } + } + + pg_sys::Datum::from(0) +} + +#[unsafe(no_mangle)] +pub extern "C-unwind" fn pg_finfo_check_supported_column_types() -> *const pg_sys::Pg_finfo_record { + const MY_FINFO: pg_sys::Pg_finfo_record = pg_sys::Pg_finfo_record { api_version: 1 }; + &MY_FINFO +} + +// Minimal FDW so the tests below have a `wrappers`-backed foreign table to test against +// without needing an optional FDW feature enabled (CI's default test run skips helloworld_fdw). +#[cfg(any(test, feature = "pg_test"))] +mod test_fdw { + use pgrx::PgSqlErrorCode; + use pgrx::pg_sys::panic::ErrorReport; + use std::collections::HashMap; + use supabase_wrappers::prelude::*; + + #[wrappers_fdw( + version = "0.1.0", + author = "test", + website = "https://github.com/supabase/wrappers", + error_type = "EventTriggerTestFdwError" + )] + pub(crate) struct EventTriggerTestFdw; + + pub(crate) enum EventTriggerTestFdwError {} + + impl From for ErrorReport { + fn from(_value: EventTriggerTestFdwError) -> Self { + ErrorReport::new(PgSqlErrorCode::ERRCODE_FDW_ERROR, "", "") + } + } + + type EventTriggerTestFdwResult = Result; + + impl ForeignDataWrapper for EventTriggerTestFdw { + fn new(_server: ForeignServer) -> EventTriggerTestFdwResult { + Ok(Self) + } + + fn begin_scan( + &mut self, + _quals: &[Qual], + _columns: &[Column], + _sorts: &[Sort], + _limit: &Option, + _options: &HashMap, + ) -> EventTriggerTestFdwResult<()> { + Ok(()) + } + + fn iter_scan(&mut self, _row: &mut Row) -> EventTriggerTestFdwResult> { + Ok(None) + } + + fn end_scan(&mut self) -> EventTriggerTestFdwResult<()> { + Ok(()) + } + } +} + +#[cfg(any(test, feature = "pg_test"))] +#[pgrx::pg_schema] +mod tests { + use pgrx::prelude::*; + + // no IF NOT EXISTS: each #[pg_test] runs in its own rolled-back subtransaction, and + // CREATE FOREIGN DATA WRAPPER doesn't support IF NOT EXISTS anyway + fn setup_test_fdw() { + Spi::run( + "create foreign data wrapper event_trigger_test_wrapper \ + handler event_trigger_test_fdw_handler validator event_trigger_test_fdw_validator", + ) + .unwrap(); + Spi::run("create server event_trigger_test_server foreign data wrapper event_trigger_test_wrapper") + .unwrap(); + } + + #[pg_test] + fn test_create_foreign_table_with_supported_types_succeeds() { + setup_test_fdw(); + Spi::run( + "create foreign table ett_good ( + id bigint, + name varchar(50), + label char(3), + notes text, + tags text[], + meta jsonb + ) server event_trigger_test_server", + ) + .expect("foreign table with supported types should be created"); + } + + #[pg_test( + error = "foreign table \"ett_bad\" has columns with unsupported data types: \"loc\" (type point)" + )] + fn test_create_foreign_table_with_unsupported_type_is_rejected() { + setup_test_fdw(); + Spi::run( + "create foreign table ett_bad (id bigint, loc point) server event_trigger_test_server", + ) + .unwrap(); + } + + #[pg_test( + error = "foreign table \"ett_multi_bad\" has columns with unsupported data types: \"loc\" (type point), \"addr\" (type inet)" + )] + fn test_multiple_unsupported_columns_are_all_reported() { + setup_test_fdw(); + Spi::run( + "create foreign table ett_multi_bad (id bigint, loc point, addr inet) server event_trigger_test_server", + ) + .unwrap(); + } + + #[pg_test] + fn test_create_foreign_table_with_domain_over_supported_type_succeeds() { + setup_test_fdw(); + Spi::run("create domain ett_domain_text as text").unwrap(); + Spi::run("create foreign table ett_domain (id bigint, label ett_domain_text) server event_trigger_test_server") + .expect("foreign table with a domain over a supported base type should be created"); + } + + #[pg_test( + error = "foreign table \"ett_alter1\" has columns with unsupported data types: \"bad_col\" (type point)" + )] + fn test_alter_table_add_column_with_unsupported_type_is_rejected() { + setup_test_fdw(); + Spi::run("create foreign table ett_alter1 (id bigint) server event_trigger_test_server") + .unwrap(); + Spi::run("alter table ett_alter1 add column ok_col int, add column bad_col point").unwrap(); + } + + #[pg_test( + error = "foreign table \"ett_alter2\" has columns with unsupported data types: \"bad_col\" (type point)" + )] + fn test_alter_foreign_table_add_column_with_unsupported_type_is_rejected() { + setup_test_fdw(); + Spi::run("create foreign table ett_alter2 (id bigint) server event_trigger_test_server") + .unwrap(); + Spi::run("alter foreign table ett_alter2 add column bad_col point").unwrap(); + } + + #[pg_test] + fn test_regular_table_with_unsupported_type_is_unaffected() { + Spi::run("create table ett_plain (id bigint, loc point)") + .expect("regular tables aren't scoped by the check"); + Spi::run("alter table ett_plain add column loc2 point") + .expect("regular tables aren't scoped by the check"); + } + + #[pg_test(error = "event trigger \"check_supported_column_types\" already exists")] + fn test_reregistering_event_trigger_without_guard_fails() { + Spi::run( + "create event trigger check_supported_column_types on ddl_command_end \ + when tag in ('CREATE FOREIGN TABLE') execute function check_supported_column_types()", + ) + .unwrap(); + } + + // simulates ALTER EXTENSION wrappers UPDATE re-running build.rs's registration DO block + #[pg_test] + fn test_reregistering_event_trigger_with_guard_is_idempotent() { + Spi::run( + "DO $$ + BEGIN + CREATE EVENT TRIGGER check_supported_column_types + ON ddl_command_end + WHEN TAG IN ('CREATE FOREIGN TABLE') + EXECUTE FUNCTION check_supported_column_types(); + EXCEPTION WHEN duplicate_object THEN NULL; + END $$;", + ) + .expect("re-registering the event trigger through the guarded DO block must not error"); + } +} diff --git a/wrappers/src/lib.rs b/wrappers/src/lib.rs index d5526a91..bda22928 100644 --- a/wrappers/src/lib.rs +++ b/wrappers/src/lib.rs @@ -29,6 +29,7 @@ pg_module_magic!(); extension_sql_file!("../sql/bootstrap.sql", bootstrap); extension_sql_file!("../sql/finalize.sql", finalize); +mod event_triggers; /// FDW implementations for various data sources pub mod fdw; /// Statistics collection and reporting utilities