refactor: replace oid-coercion analyzer rule with a SQL rewrite rule#380
Merged
Conversation
The oid-coercion analyzer rule (OidStringCoercion) resolved oid-alias
strings to oids, but doing so at the analyzer layer forced it to build a
LogicalPlan scan -- which can only be done with a TableProvider -- so the
rule carried an OidLookupProvider trait that implementers had to wire up.
That TableProvider dependency made the feature hard to integrate into
user code.
Move name->oid resolution back to the SQL layer, where a scan can be
emitted as plain SQL text that DataFusion plans normally through the
catalog -- no TableProvider, no provider trait:
* Add RewriteRegCastToSubquery (rules.rs), which rewrites the FORWARD
oid-alias cast direction at the AST layer:
- numeric string ('16417') -> literal oid (matches Postgres, and
avoids embedding a subquery in VALUES/IN contexts DF can't
decorrelate);
- name string ('public') -> (SELECT oid FROM pg_catalog.<table>
WHERE <name_col> = <operand>).
A direction guard leaves reverse/column casts (prorettype::regtype,
prorettype::regtype::text) untouched; PgOidTypePlanner still maps
those cast types to Int32 so they parse.
* Delete oid_coercion_rule.rs entirely (the analyzer rule, the
OidLookupProvider trait, and their tests -- ~598 lines).
* Drop the OidLookupProvider impl, the oid_lookup_target /
build_oid_name_subquery helpers, their test, the analyzer-rule
registration in setup_pg_catalog, and the now-unused imports from
pg_catalog.rs.
* Register RewriteRegCastToSubquery in parser.rs (before
RemoveSubqueryFromProjection so its subqueries get the LIMIT 1
scalar stamp).
Functional difference: the implicit `oid_col = 'str'` form (bare string
vs an oid column) is no longer resolved, because the SQL layer has no
schema. That case is covered by the analyzer rule's Field-metadata path
which is now gone; exactly one client query (dbeaver's relation-size
`c.relnamespace = 'public'`) used it, and that query is re-added to the
blacklist with a Blocked by / Remove when comment. Every explicit
`'...'::regclass`-style cast still resolves to a real oid.
PgOidTypePlanner and oid_field are left in place: the TypePlanner is
still needed so reverse/column casts parse, and the pg.oid_alias column
metadata it stamps is now unused but harmless (cosmetic cleanup deferred
to keep this change focused).
All 15 workspace test suites pass (62+45+24 lib tests + 8 client
integration tests: dbeaver, metabase, pgcli, pgadbc, psql, grafana,
metabase, array_bounds); clippy clean; fmt clean. Net -396 lines.
sunng87
added a commit
that referenced
this pull request
Jul 20, 2026
… the oid (#385) * fix(catalog): store reg* oid-alias columns as their display name, not the oid PostgreSQL `reg*` columns (regproc, regtype, regclass, ...) display as the object NAME, not the integer oid -- e.g. `pg_type.typinput` for the `text` type shows `textin`. Commit 0300310 had exported these as `::oid` integers so the oid-coercion analyzer rule could resolve name/numeric string comparisons. That benefit is now mostly gone (the coercion analyzer was replaced by SQL rewrites in #380, and the implicit comparison it served is blacklisted), while the display regression it introduced remains: clients saw `42` where real Postgres returns `textin` (issue #384). Revert the `reg*` storage to the display name (string), keeping the `pg.oid_alias=<kind>` field metadata so the wire layer (arrow-pg, fixed separately) still reports the correct alias type in RowDescription. Genuine `oid` columns stay int32 (Postgres displays oid as the integer). export_pg_catalog_arrow.sh (generates the committed feathers): - reg* family -> pa.string(); drop the `::oid` SELECT cast (select_expr). - oid stays pa.int32(); metadata stamping unchanged. - POSTGRES_PORT is now env-overridable (host-side publish is only for ad-hoc inspection; the export runs in-container). pg_to_arrow.py (generic exporter): it already stored reg* as string (the OIDs fall through to the pa.string() default); only the now-stale docstrings claimed int32. Updated the docstrings. Regenerated the 10 affected feathers against postgres:17.6 (unchanged version): pg_type, pg_proc, pg_aggregate, pg_operator, pg_am, pg_amproc, pg_conversion, pg_range, pg_ts_parser, pg_ts_template. Verified pg_type.typinput/typoutput/ typreceive/typsend are now `string` + `pg.oid_alias=regproc` holding the name (`textin`, `textout`, ...); oid/typnamespace remain int32. Added end-to-end test pg_type_regproc_columns_display_as_names: selecting typinput/typoutput for oid=25 returns `textin`/`textout`. Trade-off (documented in the issue/option choice): numeric oid equality on reg* columns (`WHERE typinput = 42`) and an explicit `col::regproc` cast on a (now string) reg* column no longer work the int path. The common display and name-comparison forms -- `SELECT typinput`, `typinput::text`, `WHERE typinput = 'textin'` -- are restored. Refs: #384 * chore: fmt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
revert the string auto coersion to oid support, it doesn't work in a multi catalog setup, at least for now.