feat(expression): support multi-level JSONB path in SELECT fields - #36
Conversation
sant1ago-da-hanoi
left a comment
There was a problem hiding this comment.
Code Review
Files changed: 2 | Lines: +313 / -113
Summary
Rewrite getAlias() thành recursive parser hỗ trợ chained JSONB operators. Code rõ ràng, tách method hợp lý, test coverage tốt cho happy path. Có 2 edge case thiếu xử lý có thể gây invalid SQL runtime.
Findings
Phải sửa
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 1 | DbTable.java:109-110 |
Code Quality | "data**" (trailing **, no segments) → split("\\*\\*") trả về [""] → output ->'' — SQL invalid. Tương tự "data*" ở line 126-127. |
Thêm guard: nếu remainder.isBlank() hoặc segments rỗng → throw DbException. Thêm test case cho trailing delimiter. |
| 2 | DbTable.java:191-194 |
Code Quality | extractKey — quoted key thiếu closing quote (data->'feedback) → trả về substring kèm ' mở → isQuoted() return false → wrap thêm quotes → silent SQL corruption. |
Throw exception khi không tìm thấy closing quote. Fail fast > produce garbage SQL. Thêm test case. |
| 3 | DbTable.java:81 |
Code Quality | name.split(":") — input nhiều hơn 1 colon (vd data->>name:alias:extra) → aliasParts[1] chỉ lấy phần giữa, phần sau bị drop im lặng. |
Dùng split(":", 2) để limit 2 phần. Hoặc throw nếu aliasParts.length > 2. |
Nên sửa
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 4 | DbTable.java:80-136 |
Standards | getAlias() 56 lines, 5 nhánh if-else. Mỗi nhánh là parser riêng — nên tách method giống đã làm với parseChainedArrows. |
Tách mỗi nhánh thành method riêng. getAlias() chỉ còn dispatch. |
| 5 | DbTableTest.java:151-231 |
Standards | 7 test mới đều tạo DbTable + DbColumn inline — duplicate setup. |
Rút fixture ra @BeforeEach hoặc dùng @ParameterizedTest + @CsvSource. |
Nhỏ
| # | File | Category | Issue | Suggestion |
|---|---|---|---|---|
| 6 | DbTable.java:80 |
Standards | Param name shadows record component name (line 14). |
Rename → expression hoặc fieldExpression. |
Đánh giá: Chưa sẵn sàng merge
3 phải sửa (edge case gây invalid SQL), 2 nên sửa, 1 nhỏ. Fix 3 cái đầu rồi push lại.
sant1ago-da-hanoi
left a comment
There was a problem hiding this comment.
Re-review (commit 4ae8a43)
3/3 Phải sửa đã fix:
- Trailing delimiter →
validateNotBlank()+ throw ✅ - Unclosed quote →
extractKeythrow thay vì return garbage ✅ split(":", 2)✅- Bonus: param rename
name→fieldExpression✅
Test coverage cho cả 3 edge case. OK merge.
Còn 2 Nên sửa chưa address (tách method getAlias, test fixture dedup) — không block merge, track riêng nếu cần.
sant1ago-da-hanoi
left a comment
There was a problem hiding this comment.
Re-review (commit b9cc837)
2/2 Nên sửa còn lại đã fix:
getAlias()tách thànhparsePathArray()+parseAsteriskShorthand()— dispatch gọn ✅- Test fixture rút ra
static finalJSONB_TABLE— hết duplicate ✅
Không có issue mới. Tất cả 6 findings đã resolved. OK merge.
sant1ago-da-hanoi
left a comment
There was a problem hiding this comment.
Re-review (commits fc78afc → ba5188a)
4 commits mới:
- Edge case tests mở rộng (empty key, trailing arrow, single char, no operator) ✅
@Buildertrên record + dùng trongcopyWithAliasvàPostgreSQLDataExclusion— readable hơn constructor 7 params ✅parseChainedArrowsgọn hơn,quoteIfNeededrõ intent hơnisQuoted✅
Không có issue mới. OK merge.
Summary
Rewrite
DbTable.getAlias()to support multi-level JSONB path chaining in SELECT fields.Closes #18
Changes
->/->>operators#>>/#>path syntax: Now produces valid PostgreSQL path array format#>>'{feedback,type}'(was missing'{}'wrapping)->/->>operators: Auto-quote unquoted keys, support unlimited chaining depth**/*shorthand: Support multi-level via repeated delimiters (data**a**b→->'a'->>'b')Breaking Change
Output format for single-level arrow operators now includes quotes:
data->>name→ jsonParts =->>namedata->>name→ jsonParts =->>'name'This produces valid PostgreSQL syntax. The old format was technically invalid SQL (missing quotes around key names). All existing tests and RSQL module tests pass with the new format.
Supported Syntax
data->>name->>'name'data->feedback->>type->'feedback'->>'type'data->'feedback'->>'type'->'feedback'->>'type'meta->config->settings->>theme->'config'->'settings'->>'theme'data**feedback**type->'feedback'->>'type'data*feedback*type->'feedback'->'type'data#>>feedback.type#>>'{feedback,type}'Tests
All 275 tests pass (core + rsql + jdbc).