Skip to content

Relocate statement-property merging out of Connection #166

Description

#Relocate statement-property merging out of Connection

Motivation

Connection._resolve_properties() currently welds together two responsibilities that belong to
different layers:

  1. Pure property-domain policy — validating a raw properties dict (is-a-dict, keys are str,
    values are str | int | bool) and knowing which keys are driver-owned and therefore
    rejected from caller input.
  2. Connection/execution overlay — stamping sql.current-catalog / sql.current-database from
    connection identity and sql.snapshot.mode = now from the execution mode.

Layer 1 needs nothing from self; it's leaking property-domain knowledge into Connection. As we
build out the property types (#162's enums, #163's StatementProperties), that logic reads as
misplaced — it wants a home next to the properties it reasons about.

Scope

  • Move the driver-owned-key declaration into the property domain. Replace
    Connection._RESERVED_STATEMENT_PROPERTIES with a DRIVER_OWNED_PROPERTIES frozenset of
    Property members declared in confluent_sql.statement_properties, imported by Connection.
    (Name is a proposal — it must be literally true of the set: these are the keys the driver owns
    and overlays, not merely "reserved.") Delete the Connection-local constant.
  • Move the stateless validation into the property domain, e.g. a
    validate_properties_dict(props) free function (or fold it into the normalization step below).
    Keep the exact error messages and field paths the current tests pin.
  • Add a normalization front-step to the merge: accept either a raw PropertiesDict or a
    StatementProperties (downgrading the latter via .to_properties_dict() from Frozen dataclass StatementProperties #163) before
    validating and overlaying.
  • Keep the overlay in Connection (or extract to a small _overlay_driver_properties() /
    ConnectionContext if Connection keeps accreting) — it is built from connection identity +
    execution mode and has no other honest home.
  • Rename _resolve_properties if "resolve" stops fitting once it's thin orchestration —
    candidate _build_statement_properties. The method should read as normalize → validate →
    overlay
    .

Explicit non-goal: do not put the overlay on StatementProperties

StatementProperties (#163) is a typed view over a curated subset that deliberately excludes
catalog/database/snapshot-mode — its own docstring commits to "never fights the driver-owned
overlay." Giving it a resolve() method would force it to emit keys it does not model and to reach
for environment_id / _database / execution_mode that aren't its data; the name would lie. Its
only role in this pipeline is rendering itself via to_properties_dict().

Invariant worth a test

DRIVER_OWNED_PROPERTIES and the set of keys the overlay actually stamps must stay in lockstep:
every rejected key is one the driver sets, and every key the driver sets is rejected from caller
input. Add a test asserting the two sets are equal so a future overlay key can't be added without
also being reserved (and vice versa).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions