Skip to content

feat(comparison): Add contains / containedBy methods to Comparison expression#25

Merged
veewee merged 1 commit into
phpro:mainfrom
ElwynVdb:jsonb-comparison-contains
Apr 13, 2026
Merged

feat(comparison): Add contains / containedBy methods to Comparison expression#25
veewee merged 1 commit into
phpro:mainfrom
ElwynVdb:jsonb-comparison-contains

Conversation

@ElwynVdb
Copy link
Copy Markdown
Collaborator

@ElwynVdb ElwynVdb commented Apr 9, 2026

Q A
Type improvement
BC Break no
Fixed issues

Summary

  • Added a static contains method to Comparison that generates an expression using the @> operator for containment checks.
    • Added a static containedBy method to Comparison that generates an expression using the <@ operator for containment checks.

@ElwynVdb ElwynVdb requested review from Copilot and veewee and removed request for Copilot April 9, 2026 09:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds JSONB containment support to the expression layer by introducing a JsonbComparison::contains() helper that renders the PostgreSQL @> operator, with an accompanying integration test to validate expected query results.

Changes:

  • Add JsonbComparison::contains(Expression $jsonField, Expression $contained) producing jsonField @> contained.
  • Add integration coverage validating JSONB containment against a VALUES-backed dataset.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Expression/JsonbComparison.php Introduces contains() for JSONB containment using the @> operator.
tests/Integration/Expression/JsonbComparisonTest.php Adds an integration test asserting correct behavior of contains().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Integration/Expression/JsonbComparisonTest.php Outdated
@ElwynVdb ElwynVdb force-pushed the jsonb-comparison-contains branch from f17c536 to 9457fe0 Compare April 9, 2026 09:43
@veewee
Copy link
Copy Markdown
Contributor

veewee commented Apr 10, 2026

Thanks for the PR! The @> operator is useful, but I think JsonbComparison isn't the right place for it.

The @> (contains) operator in PostgreSQL isn't JSONB-specific — it works on arrays, ranges, hstore, and geometric types too. Compare this to ?| (has any keys), which is genuinely JSONB-only and belongs here.

I think Comparison is a better fit. It's already the home for binary comparison operators (=, !=, >, >=, <, <=), and @> / <@ follow exactly the same left operator right pattern. This way the methods work naturally regardless of the underlying type:

// jsonb
Comparison::contains(new Column('roles'), new SqlExpression("'[\"ROLE_USER\"]'::jsonb"));

// arrays
Comparison::contains(new Column('tags'), new SqlExpression("ARRAY['php']"));

Could you move contains to Comparison and also add containedBy for the <@ operator while you're at it?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Expression/Comparison.php
@ElwynVdb
Copy link
Copy Markdown
Collaborator Author

The changes have been made, please let me know if anything else must change 😄

@veewee veewee changed the title feat(Jsonb): Add contains method to JsonbComparison for JSONB field c… feat(comparison): Add contains / containedBy methods to Comparison expression Apr 13, 2026
@veewee veewee merged commit fbf7dc0 into phpro:main Apr 13, 2026
8 checks passed
@ElwynVdb ElwynVdb deleted the jsonb-comparison-contains branch April 20, 2026 11:25
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.

3 participants