From ef89a5c5892efed34ca761a6517e26bedc712ff2 Mon Sep 17 00:00:00 2001 From: Jan Veltmaat Date: Wed, 22 Apr 2026 14:46:20 -0400 Subject: [PATCH] fix nested between operator --- generators/esgen/bridgeutil.go | 59 ++-------- generators/esgen/esgenerator_test.go | 156 ++++++++++++++++++++++++++- 2 files changed, 165 insertions(+), 50 deletions(-) diff --git a/generators/esgen/bridgeutil.go b/generators/esgen/bridgeutil.go index d2ff1bc..fc43aa8 100644 --- a/generators/esgen/bridgeutil.go +++ b/generators/esgen/bridgeutil.go @@ -131,59 +131,20 @@ func makeRange(lhs *gentypes.FieldType, op lex.TokenType, rhs expr.Node) (any, e // makeBetween returns a range filter for Elasticsearch given the 3 nodes that // make up a comparison. func makeBetween(lhs *gentypes.FieldType, lower, upper any) (any, error) { - /* - "nested": { - "query": { - "bool": { - "must": [ - { - "term": { - "k": "open" - } - }, - { - "range": { - "f": {"gt": 7} - } - }, - { - "range": { - "f": {"lt": 15} - } - } - ] - } - }, - "path": "map_events" - } - - "must": [ - { - "range": { - "f": {"gt": 7} - } - }, - { - "range": { - "f": {"lt": 15} - } - } - ] - */ + fieldName := lhs.Field + if lhs.Nested() { + fieldName, lower = lhs.PrefixAndValue(lower) + _, upper = lhs.PrefixAndValue(upper) + } - lr := &RangeFilter{Range: map[string]RangeQry{lhs.Field: {GT: lower}}} - ur := &RangeFilter{Range: map[string]RangeQry{lhs.Field: {LT: upper}}} - fl := []any{lr, ur} + lr := &RangeFilter{Range: map[string]RangeQry{fieldName: {GT: lower}}} + ur := &RangeFilter{Range: map[string]RangeQry{fieldName: {LT: upper}}} + inner := &boolean{must{[]any{lr, ur}}} if lhs.Nested() { - fl = append(fl, Term("k", lhs.Field)) - return &nested{&NestedQuery{ - Query: &boolean{must{fl}}, - Path: lhs.Path, - IgnoreUnmapped: true, - }}, nil + return Nested(lhs, inner), nil } - return &boolean{must{fl}}, nil + return inner, nil } // makeWildcard returns a wildcard/like query diff --git a/generators/esgen/esgenerator_test.go b/generators/esgen/esgenerator_test.go index f9a6de7..86758ca 100644 --- a/generators/esgen/esgenerator_test.go +++ b/generators/esgen/esgenerator_test.go @@ -3,6 +3,7 @@ package esgen import ( + "encoding/json" "testing" "time" @@ -74,15 +75,22 @@ func TestWalk(t *testing.T) { } type schema struct { - cols map[string]value.ValueType + cols map[string]value.ValueType + fields map[string]*gentypes.FieldType // pre-built FieldTypes (e.g. for nested map fields) } func (s schema) Column(f string) (value.ValueType, bool) { + if ft, ok := s.fields[f]; ok { + return ft.Type, true + } c, ok := s.cols[f] return c, ok } func (s schema) ColumnInfo(f string) (*gentypes.FieldType, bool) { + if ft, ok := s.fields[f]; ok { + return ft, true + } c, ok := s.cols[f] if !ok { return nil, ok @@ -93,3 +101,149 @@ func (s schema) ColumnInfo(f string) (*gentypes.FieldType, bool) { TypeName: c.String(), }, true } + +// TestBetween covers the BETWEEN operator for both top-level and nested +// (map) fields. Regression test for a bug where nested BETWEEN emitted +// range/term queries on bare field names ("foo", "k") instead of the +// prefixed nested paths ("user_data.i", "user_data.k"), so no documents +// matched. +func TestBetween(t *testing.T) { + tests := []struct { + name string + lhs *gentypes.FieldType + want string + }{ + { + name: "simple numeric", + lhs: &gentypes.FieldType{Field: "visitct", Type: value.IntType, TypeName: "int"}, + want: `{ + "bool": { + "must": [ + {"range": {"visitct": {"gt": 5}}}, + {"range": {"visitct": {"lt": 10}}} + ] + } + }`, + }, + { + name: "nested map numeric", + lhs: &gentypes.FieldType{ + Field: "foo", + Path: "user_data", + Prefix: "i", + Type: value.MapIntType, + TypeName: "map[string]int", + }, + want: `{ + "nested": { + "path": "user_data", + "ignore_unmapped": true, + "query": { + "bool": { + "must": [ + {"term": {"user_data.k": "foo"}}, + {"bool": { + "must": [ + {"range": {"user_data.i": {"gt": 5}}}, + {"range": {"user_data.i": {"lt": 10}}} + ] + }} + ] + } + } + } + }`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := makeBetween(tt.lhs, 5, 10) + require.NoError(t, err) + assertJSONEqual(t, tt.want, got) + }) + } +} + +// TestBetweenWalk exercises BETWEEN end-to-end through the filterql parser +// and the generator's WalkExpr, ensuring the dispatcher still reaches +// makeBetween with the correct FieldType. +func TestBetweenWalk(t *testing.T) { + s := schema{ + cols: map[string]value.ValueType{ + "visitct": value.IntType, + }, + fields: map[string]*gentypes.FieldType{ + "user_data.foo": { + Field: "foo", + Path: "user_data", + Prefix: "i", + Type: value.MapIntType, + TypeName: "map[string]int", + }, + }, + } + g := NewGenerator(time.Now(), nil, s) + + tests := []struct { + name string + filter string + want string + }{ + { + name: "simple", + filter: `FILTER visitct BETWEEN 5 AND 10`, + want: `{ + "bool": { + "must": [ + {"range": {"visitct": {"gt": 5}}}, + {"range": {"visitct": {"lt": 10}}} + ] + } + }`, + }, + { + name: "nested", + filter: `FILTER user_data.foo BETWEEN 5 AND 10`, + want: `{ + "nested": { + "path": "user_data", + "ignore_unmapped": true, + "query": { + "bool": { + "must": [ + {"term": {"user_data.k": "foo"}}, + {"bool": { + "must": [ + {"range": {"user_data.i": {"gt": 5}}}, + {"range": {"user_data.i": {"lt": 10}}} + ] + }} + ] + } + } + } + }`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs, err := rel.ParseFilterQL(tt.filter) + require.NoError(t, err) + p, err := g.WalkExpr(fs.Filter) + require.NoError(t, err) + assertJSONEqual(t, tt.want, p.Filter) + }) + } +} + +func assertJSONEqual(t *testing.T, want string, got any) { + t.Helper() + gotBytes, err := json.Marshal(got) + require.NoError(t, err) + var gotNorm, wantNorm any + require.NoError(t, json.Unmarshal(gotBytes, &gotNorm)) + require.NoError(t, json.Unmarshal([]byte(want), &wantNorm)) + assert.Equal(t, wantNorm, gotNorm, "generated ES filter mismatch\nwant: %s\ngot: %s", want, string(gotBytes)) +}