Skip to content

fix: identity equality for expression nodes; eager equijoin collision check - #3

Merged
Xof merged 1 commit into
mainfrom
fix/node-identity-and-equijoin-validation
Jun 21, 2026
Merged

fix: identity equality for expression nodes; eager equijoin collision check#3
Xof merged 1 commit into
mainfrom
fix/node-identity-and-equijoin-validation

Conversation

@Xof

@Xof Xof commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

P2 API/semantics fixes (closes A1, A2)

A1 — expression nodes had value-equality enabled, and it raised

The node dataclasses used the default eq=True. The generated structural
__eq__ compares fields, reaching Attr.__eq__, which returns a Predicate
whose __bool__ raises PredicateError. So any of these blew up:

node1 == node2          # PredicateError
node1 in [node2]        # PredicateError
set([node1, node2])     # PredicateError on collision

eq=True + frozen=True also synthesized a __hash__ over the fields,
making Grouping and Rename (dict-valued fields) unhashable
hash(node) raised TypeError.

Expression-tree equivalence is identity, not structure (the engine check
uses is; Attr.__hash__ already chose identity). Fix: eq=False on all
12 node dataclasses, so __eq__/__hash__ fall back to object identity.

A2 — Equijoin deferred its collision check past construction

Equijoin.__post_init__ validated only that the join attributes exist; the
ambiguous-output-name SchemaError lived in _schema() and surfaced only on
a later .schema() call — unlike every other node, which validates eagerly.

Fix: move the collision check into __post_init__ (byte-identical logic),
leaving _schema() to only shape the result schema.

Tests

  • TestNodeIdentity (7): == is identity (self True, distinct False, no
    raise) even with predicate-bearing children; membership checks don't raise;
    Grouping/Rename are hashable and identity-keyed; a guard asserts every
    one of the 12 node classes inherits object.__eq__/object.__hash__.
  • TestEquijoinEagerValidation (2): colliding non-join column raises
    SchemaError at construction; the dropped right join column is not a
    false collision.

Local gate green: ruff, mypy, pytest (106 passed, 15 skipped).

… check

A1: The node dataclasses used the default eq=True, generating a structural
__eq__ that reaches Attr.__eq__ (which returns a Predicate whose __bool__
raises) — so node1 == node2, "node in [...]", and set(nodes) raised
PredicateError, and the derived __hash__ raised TypeError on the dict fields
of Grouping/Rename. Expression-tree equivalence is identity, not structure,
so all 12 node dataclasses now use eq=False (object identity for __eq__ and
__hash__).

A2: Equijoin validated only join-attribute existence at construction and
deferred its ambiguous-output-name check to _schema(), unlike every other
node. Move the collision check into __post_init__ (eager-validation
invariant); _schema() now only shapes the result.

Adds TestNodeIdentity and TestEquijoinEagerValidation. Closes A1, A2.
@Xof
Xof merged commit 34a5a2f into main Jun 21, 2026
5 checks passed
@Xof
Xof deleted the fix/node-identity-and-equijoin-validation branch June 21, 2026 09:05
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