Skip to content

Commit 66fba2b

Browse files
authored
fix: propagate null through dict and map bracket access (#7079)
* fix: propagate null through dict and map bracket access Change dict [] and map [] operators to propagate null when the parent element is nil, instead of returning a runtime error. This aligns the behavior of [] with the existing []? (conditional access) operator. Before this change, expressions like: dict["domain"]["key"] would fail with "cannot access field key, parent element is null" when the parent key did not exist in the dict/map. After this change, the expression evaluates to null, which then correctly evaluates to false in boolean comparisons. This is the expected behavior for dict/map chains where intermediate keys may not be present. The []? operator already had this behavior; now [] matches it. The isConditional parameter is removed since both paths are identical. * refactor: collapse duplicate index handlers
1 parent da25948 commit 66fba2b

File tree

3 files changed

+73
-39
lines changed

3 files changed

+73
-39
lines changed

llx/builtin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ func init() {
539539
string("*" + types.Time): {f: dictTimesTimeV2, Label: "*"},
540540
// fields
541541
"[]": {f: dictGetIndex},
542-
"[]?": {f: dictGetConditionalIndex},
542+
"[]?": {f: dictGetIndex},
543543
"first": {f: dictGetFirstIndexV2},
544544
"last": {f: dictGetLastIndexV2},
545545
"{}": {f: dictBlockCallV2},
@@ -724,7 +724,7 @@ func init() {
724724
},
725725
types.MapLike: {
726726
"[]": {f: mapGetIndex},
727-
"[]?": {f: mapGetConditionalIndex},
727+
"[]?": {f: mapGetIndex},
728728
"length": {f: mapLengthV2},
729729
"where": {f: mapWhereV2},
730730
"$whereNot": {f: mapWhereNotV2},

llx/builtin_map.go

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,6 @@ import (
1818
var mapFunctions map[string]chunkHandlerV2 //nolint:unused
1919

2020
func mapGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64) (*RawData, uint64, error) {
21-
return _mapGetIndex(e, bind, chunk, ref, false)
22-
}
23-
24-
func mapGetConditionalIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64) (*RawData, uint64, error) {
25-
return _mapGetIndex(e, bind, chunk, ref, true)
26-
}
27-
28-
func _mapGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64, isConditional bool) (*RawData, uint64, error) {
2921
args := chunk.Function.Args
3022
// TODO: all this needs to go into the compile phase
3123
if len(args) < 1 {
@@ -36,13 +28,10 @@ func _mapGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64, isC
3628
}
3729
// ^^ TODO
3830

31+
childType := bind.Type.Child()
3932
if bind.Value == nil {
40-
if isConditional {
41-
return &RawData{Type: bind.Type.Child()}, 0, nil
42-
} else {
43-
field := args[0].LabelV2(e.ctx.code)
44-
return nil, 0, errors.New("cannot access map field " + field + ", map is null")
45-
}
33+
// Propagate null through map access chains instead of erroring.
34+
return &RawData{Type: childType}, 0, nil
4635
}
4736

4837
var key string
@@ -65,15 +54,6 @@ func _mapGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64, isC
6554
return nil, 0, errors.New("Called [] with wrong type " + t.Label())
6655
}
6756

68-
childType := bind.Type.Child()
69-
70-
if bind.Value == nil {
71-
return &RawData{
72-
Type: childType,
73-
Value: nil,
74-
}, 0, nil
75-
}
76-
7757
m, ok := bind.Value.(map[string]any)
7858
if !ok {
7959
return nil, 0, errors.New("failed to typecast " + bind.Type.Label() + " into map")
@@ -372,14 +352,6 @@ func mapValuesV2(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64) (*Ra
372352
}
373353

374354
func dictGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64) (*RawData, uint64, error) {
375-
return _dictGetIndex(e, bind, chunk, ref, false)
376-
}
377-
378-
func dictGetConditionalIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64) (*RawData, uint64, error) {
379-
return _dictGetIndex(e, bind, chunk, ref, true)
380-
}
381-
382-
func _dictGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64, isConditional bool) (*RawData, uint64, error) {
383355
args := chunk.Function.Args
384356
// TODO: all this needs to go into the compile phase
385357
if len(args) < 1 {
@@ -390,12 +362,10 @@ func _dictGetIndex(e *blockExecutor, bind *RawData, chunk *Chunk, ref uint64, is
390362
}
391363

392364
if bind.Value == nil {
393-
if isConditional {
394-
return &RawData{Type: bind.Type}, 0, nil
395-
} else {
396-
field := args[0].LabelV2(e.ctx.code)
397-
return nil, 0, errors.New("cannot access field " + field + ", parent element is null")
398-
}
365+
// Propagate null through dict access chains instead of erroring.
366+
// This enables expressions like dict['a']['b'] to evaluate to null
367+
// when dict['a'] is nil, rather than failing with a runtime error.
368+
return &RawData{Type: bind.Type}, 0, nil
399369
}
400370

401371
switch x := bind.Value.(type) {

llx/builtin_map_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright (c) Mondoo, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package llx
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"go.mondoo.com/mql/v13/types"
12+
)
13+
14+
func newTestBlockExecutor() *blockExecutor {
15+
return &blockExecutor{
16+
ctx: &MQLExecutorV2{code: &CodeV2{}},
17+
}
18+
}
19+
20+
func newStringKeyChunk() *Chunk {
21+
return &Chunk{
22+
Function: &Function{
23+
Args: []*Primitive{{Value: []byte("key"), Type: string(types.String)}},
24+
},
25+
}
26+
}
27+
28+
func runIndexHandler(t *testing.T, bind *RawData, operator string) (*RawData, uint64, error) {
29+
t.Helper()
30+
31+
handler, err := BuiltinFunctionV2(bind.Type, operator)
32+
require.NoError(t, err)
33+
34+
return handler.f(newTestBlockExecutor(), bind, newStringKeyChunk(), 0)
35+
}
36+
37+
func TestDictGetIndex_NilValue(t *testing.T) {
38+
for _, operator := range []string{"[]", "[]?"} {
39+
t.Run(operator+" returns typed null when parent dict is nil", func(t *testing.T) {
40+
bind := &RawData{Type: types.Dict, Value: nil}
41+
42+
res, ref, err := runIndexHandler(t, bind, operator)
43+
require.NoError(t, err)
44+
assert.Equal(t, uint64(0), ref)
45+
assert.Equal(t, types.Dict, res.Type)
46+
assert.Nil(t, res.Value, "null dict access should propagate null, not error")
47+
})
48+
}
49+
}
50+
51+
func TestMapGetIndex_NilValue(t *testing.T) {
52+
for _, operator := range []string{"[]", "[]?"} {
53+
t.Run(operator+" returns typed null when parent map is nil", func(t *testing.T) {
54+
mapType := types.Map(types.String, types.String)
55+
bind := &RawData{Type: mapType, Value: nil}
56+
57+
res, ref, err := runIndexHandler(t, bind, operator)
58+
require.NoError(t, err)
59+
assert.Equal(t, uint64(0), ref)
60+
assert.Equal(t, types.String, res.Type)
61+
assert.Nil(t, res.Value, "null map access should propagate null, not error")
62+
})
63+
}
64+
}

0 commit comments

Comments
 (0)