diff --git a/datafusion-pg-catalog/export_pg_catalog_arrow.sh b/datafusion-pg-catalog/export_pg_catalog_arrow.sh index 2573377..2648ec7 100755 --- a/datafusion-pg-catalog/export_pg_catalog_arrow.sh +++ b/datafusion-pg-catalog/export_pg_catalog_arrow.sh @@ -9,7 +9,11 @@ DB_NAME="postgres" DB_USER="postgres" DB_PASSWORD="postgres" EXPORT_DIR="./pg_catalog_arrow_exports" -POSTGRES_PORT="5432" +# Host port to publish. The export Python runs *inside* the container and +# connects to the container's own postgres over `localhost`, so this host-side +# mapping is only needed for ad-hoc inspection. Override via env if 5432 is in +# use on the host (e.g. rootless docker's `passt` proxy). +POSTGRES_PORT="${POSTGRES_PORT:-5432}" # Export mode: "static" or "all" EXPORT_MODE="${1:-static}" @@ -160,12 +164,22 @@ import json import numpy as np def pg_type_to_arrow_type(pg_type, is_nullable=True): - """Map PostgreSQL types to Arrow types - - Every oid-alias type (oid, regproc, regtype, regclass, regnamespace, ...) - is stored as int32: the underlying object identifier. This lets the - `datafusion-pg-catalog` oid-coercion analyzer rule resolve name/numeric - string comparisons uniformly across all such columns. + """Map PostgreSQL types to Arrow types. + + oid-alias type storage (see OID_ALIAS_TYPES below): + + * `oid` columns are stored as int32 -- the raw object identifier. Postgres + displays `oid` as the integer, so the value round-trips faithfully. + * the `reg*` aliases (regproc, regtype, regclass, ...) are stored as their + **name (display) string** -- the form Postgres returns on a plain + `SELECT` (e.g. `pg_type.typinput` holds `textin`, not its oid). This + preserves the catalog's display semantics, which commit 0300310 had + inadvertently broken by casting `reg*` to `::oid`. + + Both families still carry `pg.oid_alias=` field metadata so the + wire layer (`arrow-pg`) reports the correct Postgres alias type in + RowDescription (e.g. REGPROC, OID 24); for `reg*` the text-protocol value + is then the name, matching real Postgres. """ type_mapping = { 'bigint': pa.int64(), @@ -179,19 +193,23 @@ def pg_type_to_arrow_type(pg_type, is_nullable=True): 'character varying': pa.string(), 'character': pa.string(), 'name': pa.string(), - # --- oid-alias family: all stored as int32 (the raw oid) --- + # --- oid-alias family --- + # `oid` is the raw integer identifier (Postgres displays it as int). 'oid': pa.int32(), - 'regproc': pa.int32(), - 'regprocedure': pa.int32(), - 'regoper': pa.int32(), - 'regoperator': pa.int32(), - 'regclass': pa.int32(), - 'regtype': pa.int32(), - 'regnamespace': pa.int32(), - 'regrole': pa.int32(), - 'regconfig': pa.int32(), - 'regdictionary': pa.int32(), - 'regcollation': pa.int32(), + # `reg*` aliases are stored as their NAME (display) string -- the form + # Postgres returns on a plain SELECT. We do NOT cast `::oid` (see + # select_expr), so e.g. pg_type.typinput holds "textin", not its oid. + 'regproc': pa.string(), + 'regprocedure': pa.string(), + 'regoper': pa.string(), + 'regoperator': pa.string(), + 'regclass': pa.string(), + 'regtype': pa.string(), + 'regnamespace': pa.string(), + 'regrole': pa.string(), + 'regconfig': pa.string(), + 'regdictionary': pa.string(), + 'regcollation': pa.string(), # --- end oid-alias family --- 'timestamp': pa.timestamp('us'), 'timestamp without time zone': pa.timestamp('us'), @@ -224,8 +242,15 @@ def pg_type_to_arrow_type(pg_type, is_nullable=True): return arrow_type # PostgreSQL types that are object-identifier aliases. For these we stamp Arrow -# field metadata `pg.oid_alias=` so the catalog's oid-coercion analyzer -# rule can resolve name/numeric string comparisons the way Postgres does. +# field metadata `pg.oid_alias=` so the wire layer (`arrow-pg`) reports +# the correct Postgres alias type in RowDescription (e.g. REGPROC, OID 24). +# +# Storage split (see pg_type_to_arrow_type above): +# * `oid` -> int32 (Postgres displays `oid` as the integer itself) +# * `reg*` -> string (the Postgres display NAME, e.g. "textin") +# We do NOT cast `reg*` to `::oid` (the former 0300310 approach): the display +# form is exactly the value we want to persist, so clients see `textin`, not +# the underlying oid integer. OID_ALIAS_TYPES = { 'oid', 'regproc', 'regprocedure', @@ -235,17 +260,17 @@ OID_ALIAS_TYPES = { 'regconfig', 'regdictionary', 'regcollation', } -# reg* aliases need an explicit `::oid` cast at SELECT time because Postgres -# returns their display (name) form, not the integer, on a plain SELECT. -REG_OID_TYPES = OID_ALIAS_TYPES - {'oid'} - def select_expr(col_name, data_type): - """Build a SELECT column expression, casting reg* columns to oid.""" - quoted = '"' + col_name.replace('"', '""') + '"' - if data_type in REG_OID_TYPES: - return f'{quoted}::oid AS {quoted}' - return quoted + """Build a SELECT column expression for a column. + + Columns are selected verbatim. `reg*` oid-alias columns are deliberately + NOT cast to `::oid`: Postgres returns their display (name) form on a plain + SELECT, which is the value we store (see pg_type_to_arrow_type). `oid` + columns are already integers, so no cast is needed for them either. + """ + _ = data_type # all columns are selected verbatim now + return '"' + col_name.replace('"', '""') + '"' def convert_value(value, arrow_type): """Convert PostgreSQL value to Arrow-compatible value""" @@ -389,8 +414,9 @@ def export_table_to_arrow(conn, schema_name, table_name, output_dir): arrow_schema = pa.schema(arrow_fields) - # Fetch all data. reg* oid-alias columns are cast to oid at SELECT time - # so they are returned as integers (matching their stored Arrow type). + # Fetch all data. Columns are selected verbatim; reg* oid-alias columns + # keep their Postgres display (name) form, matching their stored Arrow + # string type (see select_expr / pg_type_to_arrow_type). select_cols = ", ".join( select_expr(name, dtype) for (name, dtype, _, _, _) in columns diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_aggregate.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_aggregate.feather index bb42c29..e6065f0 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_aggregate.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_aggregate.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_am.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_am.feather index e444610..ec55444 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_am.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_am.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_amproc.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_amproc.feather index 67a59d7..718bd11 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_amproc.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_amproc.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_conversion.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_conversion.feather index 329c82a..1ebd515 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_conversion.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_conversion.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_operator.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_operator.feather index 00886e5..86c3b41 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_operator.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_operator.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_proc.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_proc.feather index 73bfd88..4894346 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_proc.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_proc.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_range.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_range.feather index 890a8ce..e6b599e 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_range.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_range.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_parser.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_parser.feather index c2cb87b..3c25bfc 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_parser.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_parser.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_template.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_template.feather index 3c04468..3a262eb 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_template.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_ts_template.feather differ diff --git a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_type.feather b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_type.feather index 4b36fce..8a86bef 100644 Binary files a/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_type.feather and b/datafusion-pg-catalog/pg_catalog_arrow_exports/pg_type.feather differ diff --git a/datafusion-pg-catalog/pg_to_arrow.py b/datafusion-pg-catalog/pg_to_arrow.py index 2bfe469..e0b99b0 100644 --- a/datafusion-pg-catalog/pg_to_arrow.py +++ b/datafusion-pg-catalog/pg_to_arrow.py @@ -15,10 +15,18 @@ def map_postgresql_to_arrow_type(type_oid: int) -> pa.DataType: """Map PostgreSQL data types to Arrow data types. - Every oid-alias type (oid, regproc, regtype, regclass, regnamespace, ...) - is stored as int32 (the raw object identifier) so the - `datafusion-pg-catalog` oid-coercion analyzer rule can resolve name/numeric - string comparisons uniformly. + oid-alias type storage: + + * `oid` (OID 26) is stored as int32 -- the raw object identifier, which + Postgres displays as the integer. + * the `reg*` aliases (regproc=24, regtype=2206, ...) are intentionally NOT + listed here, so they fall through to the `pa.string()` default: they are + stored as their **name (display) string**, the form Postgres returns on a + plain SELECT. This exporter fetches without a `::oid` cast, so a regproc + column e.g. holds "textin", not its oid. + + `pg.oid_alias=` field metadata is attached separately (see + is_oid_alias) so the wire layer reports the right Postgres alias type. """ # Map OIDs to Arrow types type_mapping = { @@ -67,9 +75,10 @@ def map_postgresql_to_arrow_type(type_oid: int) -> pa.DataType: return type_mapping.get(type_oid, pa.string()) # Fallback to string -# PostgreSQL `reg*` type names (and `oid`). Their OIDs are version-stable but -# kept here as (oid, name) pairs so we can both map them to int32 and stamp -# `pg.oid_alias=` field metadata for the catalog analyzer rule. +# PostgreSQL `reg*` type names (and `oid`). Used to stamp +# `pg.oid_alias=` field metadata on those columns so the wire layer +# (`arrow-pg`) reports the correct Postgres alias type. Storage itself is +# decided by map_postgresql_to_arrow_type: `oid` -> int32, `reg*` -> string. OID_ALIAS_TYPES_BY_NAME = { 'oid', 'regproc', 'regprocedure', diff --git a/datafusion-pg-catalog/src/pg_catalog.rs b/datafusion-pg-catalog/src/pg_catalog.rs index 89e7d36..f19f04e 100644 --- a/datafusion-pg-catalog/src/pg_catalog.rs +++ b/datafusion-pg-catalog/src/pg_catalog.rs @@ -1552,6 +1552,49 @@ mod test { assert_eq!(value, "my_database"); } + #[tokio::test] + async fn pg_type_regproc_columns_display_as_names() { + // Regression for issue #384: `pg_type.typinput`/`typoutput` are + // `regproc` columns. Postgres displays a regproc value as the function + // NAME (`textin`/`textout`), not the raw oid integer. Commit 0300310 + // had stored the integer via a `::oid` export cast, so a plain + // `SELECT typinput` returned a number. The feather export now stores + // the display name (string) instead, so the bare select returns the + // name again. + use crate::pg_catalog::context::EmptyContextProvider; + use datafusion::prelude::{SessionConfig, SessionContext}; + + let config = SessionConfig::new().with_default_catalog_and_schema("my_database", "public"); + let ctx = SessionContext::new_with_config(config); + setup_pg_catalog(&ctx, "my_database", EmptyContextProvider).unwrap(); + + // oid 25 == the `text` type, whose input/output procs are textin/textout. + let batches = ctx + .sql("SELECT typinput, typoutput FROM pg_catalog.pg_type WHERE oid = 25") + .await + .unwrap() + .collect() + .await + .unwrap(); + + assert_eq!(batches.len(), 1); + assert_eq!(batches[0].num_rows(), 1); + let typinput = batches[0] + .column(0) + .as_any() + .downcast_ref::() + .expect("typinput is a Utf8 column holding the regproc display name") + .value(0); + let typoutput = batches[0] + .column(1) + .as_any() + .downcast_ref::() + .expect("typoutput is a Utf8 column holding the regproc display name") + .value(0); + assert_eq!(typinput, "textin"); + assert_eq!(typoutput, "textout"); + } + #[test] fn test_load_arrow_data() { let table = ArrowTable::from_ipc_data(