feat(rsql): add arrayContains operator for PostgreSQL array filtering - #39
Merged
Conversation
- Introduced `FilterBuilder.arrayContains()` and `arrayContainsIfPresent()`. - Added `=arrayContains=` / `=ac=` operator syntax. - Implemented `ArrayContainsOperatorHandler` for SQL generation. - Updated tests, documentation, and operator registry.
sant1ago-da-hanoi
left a comment
sant1ago-da-hanoi
left a comment
Contributor
Author
There was a problem hiding this comment.
Code Review
Files changed: 8 | Lines: +206 / -64
Summary
PR thêm operator =arrayContains= / =ac= cho PostgreSQL TEXT[] columns, generate SQL 'value' = ANY(column). Logic handler đúng, test coverage đủ cho alias/no-alias/dedup. README bị corrupt ở 1 chỗ cần fix trước khi merge, và thiếu null guard trên arrayContains().
Findings
Phải sửa
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 1 | README.md:591-598 |
Standards | Code block bị corrupt — DatabaseContextHolder.clear() ngắt dòng, }finally{ thiếu khoảng trắng |
Sửa lại đúng format } finally { DatabaseContextHolder.clear(); } |
Nên sửa
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 2 | ArrayContainsOperatorHandler.java:9 |
Code Quality | Không có guard/Javadoc — operator chỉ hợp lệ với PostgreSQL TEXT[] nhưng không validate, sẽ fail ở runtime nếu dùng sai column type |
Thêm Javadoc note rõ "PostgreSQL only". Tốt hơn: check dialect + throw DbException |
| 3 | FilterBuilder.java:313 |
Code Quality | arrayContains(field, null) throw NPE — List.of(null) không hợp lệ trong Java 9+ |
Thêm Objects.requireNonNull(value, "value must not be null") đầu method |
Nhỏ
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 4 | FilterBuilder.java:313,445 |
Standards | Thiếu Javadoc trên arrayContains() và arrayContainsIfPresent() |
Thêm Javadoc mô tả PostgreSQL-only, = ANY(column), và null/blank skip behavior |
| 5 | ArrayContainsOperatorHandler.java:1 |
Standards | Thiếu Javadoc class-level | Thêm /** Handles =arrayContains= / =ac=. Generates :param = ANY(column) for PostgreSQL TEXT[] columns. */ |
| 6 | README.md:210,233 |
Standards | Indentation lệch ở 2 code block không liên quan tính năng chính | Khôi phục indent gốc |
Positive Patterns
reviewAndSetParam()tái dùng đúng cách — param deduplication hoạt động chính xác, có test case xác nhận- Test coverage đủ 3 nhánh: with alias, without alias, duplicate param key
Đánh giá: Nên sửa trước khi merge
1 phải sửa, 2 nên sửa, 3 nhỏ. Fix #1 (README corrupt) và #3 (NPE risk) trước — cái còn lại làm kèm được.
- Add null guard on FilterBuilder.arrayContains() - Add Javadoc on ArrayContainsOperatorHandler and FilterBuilder methods - Fix corrupted DatabaseContextHolder code block in README
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.
Summary
ARRAY_CONTAINSRSQL operator (=arrayContains=/=ac=) that generates'value' = ANY(column)SQL for PostgreSQLTEXT[]columnsarrayContains()andarrayContainsIfPresent()convenience methods toFilterBuilderRSQLOperatorHandlers, update README with docs and examplesCloses #38
Test plan
ArrayContainsOperatorHandler(with/without alias, param deduplication)FilterBuilder.arrayContains/arrayContainsIfPresentCustomRSQLOperatorsTestupdated for new operator symbols and countmvn testpasses