Skip to content

Commit 65d1a2d

Browse files
committed
fix issue 9 from original repo
1 parent 0fd15ba commit 65d1a2d

File tree

3 files changed

+78
-24
lines changed

3 files changed

+78
-24
lines changed

ai_plans/FIX_EMPTY_SLICE.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Bug Fix: `carta.Map` Incorrectly Handles Empty Slices and Nil Associations
2+
3+
## Summary of the Issue
4+
5+
When using `carta.Map` with a `LEFT JOIN` where the "many" side of a `has-many` relationship had no corresponding entries, the mapper would incorrectly produce a slice containing a single, zero-valued struct instead of an empty slice. For example, a user with no posts would be mapped as:
6+
7+
```json
8+
{
9+
"user_id": 1,
10+
"posts": [
11+
{
12+
"post_id": null,
13+
"content": null
14+
}
15+
]
16+
}
17+
```
18+
19+
Instead of the expected:
20+
21+
```json
22+
{
23+
"user_id": 1,
24+
"posts": []
25+
}
26+
```
27+
28+
A related issue was discovered where `has-one` pointer associations were not being set to `nil` when the joined data was `NULL`, leading to an empty struct instead of a `null` value in the final JSON output.
29+
30+
The root cause was traced to the column allocation logic in `column.go`, where the sub-mapper for the nested struct was greedily and incorrectly claiming a non-`NULL` column from its parent (e.g., `user_id`), causing the `isNil` check to fail and an empty struct to be erroneously created.
31+
32+
## The Plan
33+
34+
1. **Analyze Core Logic:** Review `mapper.go`, `load.go`, and `setter.go` to understand the mapping and data loading process.
35+
2. **Isolate the Flaw:** Investigate `column.go` to pinpoint the exact flaw in the `allocateColumns` function that led to incorrect column claims by sub-mappers.
36+
3. **Refactor Column Allocation:** Modify the `allocateColumns` function in `column.go` to iterate through a struct's fields first, then search for a matching column. This makes column claims unambiguous and prevents sub-mappers from claiming columns belonging to their parents.
37+
4. **Address Nil Associations:** Update the `setDst` function in `setter.go` to check if the resolver for a `has-one` pointer association is empty. If it is, the field is left as `nil`, ensuring it marshals to `null`.
38+
5. **Add Verification Tests:**
39+
* Create a test case to verify that a `has-many` relationship with no results from a `LEFT JOIN` correctly produces an empty slice (`[]`).
40+
* Create a test case to verify that a `has-one` pointer relationship with no results from a `LEFT JOIN` correctly produces a `nil` value.
41+
6. **Implement and Verify:** Execute the plan by applying the code changes and running the new tests to confirm the fixes.
42+
7. **Cleanup:** Remove the temporary test cases to keep the test suite clean.
43+
44+
## The Result
45+
46+
The plan was executed successfully.
47+
48+
* The column allocation logic in `column.go` was refactored to be more precise, resolving the primary issue of incorrectly generated empty structs in slices.
49+
* The setter logic in `setter.go` was updated to correctly handle `nil` pointer associations.
50+
* All tests passed, confirming that both the empty slice and `nil` association issues have been fixed. The `carta` package now behaves as expected in these scenarios.

column.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,10 @@ type column struct {
1515
}
1616

1717
func allocateColumns(m *Mapper, columns map[string]column) error {
18-
var (
19-
candidates map[string]bool
20-
)
2118
presentColumns := map[string]column{}
22-
for cName, c := range columns {
23-
if m.IsBasic {
24-
candidates = getColumnNameCandidates("", m.AncestorNames, m.Delimiter)
19+
if m.IsBasic {
20+
candidates := getColumnNameCandidates("", m.AncestorNames, m.Delimiter)
21+
for cName, c := range columns {
2522
if _, ok := candidates[cName]; ok {
2623
presentColumns[cName] = column{
2724
typ: c.typ,
@@ -30,16 +27,18 @@ func allocateColumns(m *Mapper, columns map[string]column) error {
3027
}
3128
delete(columns, cName) // dealocate claimed column
3229
}
33-
} else {
34-
for i, field := range m.Fields {
35-
subMap, isSubMap := m.SubMaps[i]
36-
delimiter := m.Delimiter
37-
if isSubMap {
38-
delimiter = subMap.Delimiter
39-
}
40-
candidates = getColumnNameCandidates(field.Name, m.AncestorNames, delimiter)
41-
// can only allocate columns to basic fields
42-
if isBasicType(field.Typ) {
30+
}
31+
} else {
32+
for i, field := range m.Fields {
33+
subMap, isSubMap := m.SubMaps[i]
34+
delimiter := m.Delimiter
35+
if isSubMap {
36+
delimiter = subMap.Delimiter
37+
}
38+
candidates := getColumnNameCandidates(field.Name, m.AncestorNames, delimiter)
39+
// can only allocate columns to basic fields
40+
if isBasicType(field.Typ) {
41+
for cName, c := range columns {
4342
if _, ok := candidates[cName]; ok {
4443
presentColumns[cName] = column{
4544
typ: c.typ,

setter.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,23 @@ func setDst(m *Mapper, dst reflect.Value, rsv *resolver) error {
5151
childDst = elem.v.Field(int(fieldIndex)).Addr()
5252
}
5353
} else if subMap.Crd == Association {
54-
newChildElem = reflect.New(childTyp).Elem()
55-
if subMap.IsTypePtr {
56-
elem.v.Field(int(fieldIndex)).Set(newChildElem.Addr())
57-
childDst = elem.v.Field(int(fieldIndex))
58-
} else {
59-
elem.v.Field(int(fieldIndex)).Set(newChildElem)
60-
childDst = elem.v.Field(int(fieldIndex)).Addr()
54+
// Only set the association if it's not nil
55+
if len(subMapRsv.elements) > 0 {
56+
newChildElem = reflect.New(childTyp).Elem()
57+
if subMap.IsTypePtr {
58+
elem.v.Field(int(fieldIndex)).Set(newChildElem.Addr())
59+
childDst = elem.v.Field(int(fieldIndex))
60+
} else {
61+
elem.v.Field(int(fieldIndex)).Set(newChildElem)
62+
childDst = elem.v.Field(int(fieldIndex)).Addr()
63+
}
6164
}
6265
}
6366

6467
// setting the child
65-
setDst(subMap, childDst, subMapRsv)
68+
if len(subMapRsv.elements) > 0 {
69+
setDst(subMap, childDst, subMapRsv)
70+
}
6671
}
6772
}
6873

0 commit comments

Comments
 (0)