Skip to content

refactor: move oid name-lookup machinery out of the coercion rule#379

Merged
sunng87 merged 1 commit into
masterfrom
refactor/simplify-oid-coercion-rule
Jul 19, 2026
Merged

refactor: move oid name-lookup machinery out of the coercion rule#379
sunng87 merged 1 commit into
masterfrom
refactor/simplify-oid-coercion-rule

Conversation

@sunng87

@sunng87 sunng87 commented Jul 19, 2026

Copy link
Copy Markdown
Member

Problem

The oid-coercion analyzer rule (oid_coercion_rule.rs) builds a TableProvider-backed LogicalPlan subquery inline to resolve name strings to oids (e.g. relnamespace = 'public'(SELECT oid FROM pg_namespace WHERE nspname = 'public')). Depending on TableProvider and LogicalPlanBuilder is out of scope for an analyzer rule, which should be a pure expression rewriter.

Change

Move all data-access concerns behind the OidLookupProvider trait:

  • Trait method changes from
    build_table_provider(name) -> Option<Arc<dyn TableProvider>>
    to
    resolve_name_expr(kind, name) -> Option<Expr> — the rule asks for a resolved oid-valued expression and never sees a TableProvider, a LogicalPlanBuilder, or the kind→table mapping.
  • The kind→table mapping (oid_lookup_target) and the subquery construction (build_oid_name_subquery) move to pg_catalog.rs, next to the provider that already owns build_table_by_name — the natural home for catalog data access.
  • resolve_operand in the rule collapses to: numeric string → literal Int32 (inline, no catalog); name string → provider.resolve_name_expr.

The rule file drops its imports of TableProvider, provider_as_source, LogicalPlanBuilder, Subquery, and Spans, and shrinks by ~54 net lines.

Why the subquery mechanism stays

Name resolution for dynamic tables (pg_namespace, pg_class) must stay lazy: the oids for those tables are assigned during execution, not at plan time, so the resolution cannot be collapsed to an eager lookup. The scalar-subquery approach is still used — it just now lives in the provider rather than the rule. The rule's goal ("support Postgres's hidden coercion from string to oid") is unchanged; only where the lookup is implemented moves.

Test plan

Behavior is unchanged — the same lazy scalar subquery is produced.

  • cargo test --workspace — 47 pg-catalog lib tests + 8 client integration tests pass, including every query that depends on name resolution:
    • dbeaver: c.relnamespace = 'public'
    • metabase: classoid = 'pg_class'::regclass
    • pgcli: prorettype::regtype != 'trigger'::regtype
    • pgadbc: cls.oid = 'clubs'::regclass::oid
  • cargo clippy -p datafusion-pg-catalog --lib — no new warnings
  • cargo fmt --all --check — clean
  • moved lookup_target unit test → oid_lookup_target_maps_kinds_to_backing_tables in pg_catalog.rs

The oid-coercion analyzer rule (oid_coercion_rule.rs) was building a
TableProvider-backed LogicalPlan subquery inline to resolve name strings
to oids (e.g. `relnamespace = 'public'` -> `(SELECT oid FROM
pg_namespace WHERE nspname = 'public')`). Having the rule depend on
TableProvider and LogicalPlanBuilder is out of scope for an analyzer
rule, which should be a pure expression rewriter.

This moves all the data-access concerns behind the OidLookupProvider
trait:

* The trait's method changes from
    build_table_provider(name) -> Option<Arc<dyn TableProvider>>
  to
    resolve_name_expr(kind, name) -> Option<Expr>
  so the rule asks for a resolved oid-valued expression and never sees a
  TableProvider, a LogicalPlanBuilder, or the kind->table mapping.

* The kind->table mapping (`oid_lookup_target`) and the subquery
  construction (`build_oid_name_subquery`) move to pg_catalog.rs, next
  to the provider that already owns build_table_by_name -- the natural
  home for catalog data access.

* resolve_operand in the rule collapses to: numeric string -> literal
  Int32 (handled inline, no catalog), name string -> provider.resolve_name_expr.

The rule file drops its imports of TableProvider, provider_as_source,
LogicalPlanBuilder, Subquery, and Spans, and shrinks by ~54 net lines.

Behavior is unchanged: name resolution still produces the same lazy
scalar subquery (oids for dynamic tables like pg_namespace are assigned
at execution, so the subquery must stay lazy). All 47 pg-catalog lib
tests and all 8 client integration tests (dbeaver `relnamespace='public'`,
metabase `'pg_class'::regclass`, pgcli `'trigger'::regtype`, pgadbc
`'clubs'::regclass::oid`, ...) remain green; zero new clippy warnings.
@sunng87
sunng87 merged commit 97754a5 into master Jul 19, 2026
7 checks passed
@sunng87
sunng87 deleted the refactor/simplify-oid-coercion-rule branch July 19, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant