From 15c1765368e67d156fe08c63aea001dc978a3ae6 Mon Sep 17 00:00:00 2001 From: Suraj Nath <9503187+electron0zero@users.noreply.github.com> Date: Wed, 21 Aug 2024 23:17:58 +0530 Subject: [PATCH 1/2] fix: make nil type symmetric --- pkg/traceql/enum_statics.go | 2 +- pkg/traceql/enum_statics_test.go | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/traceql/enum_statics.go b/pkg/traceql/enum_statics.go index c1c6050720c..4c2ab7e7e83 100644 --- a/pkg/traceql/enum_statics.go +++ b/pkg/traceql/enum_statics.go @@ -39,7 +39,7 @@ func (t StaticType) isMatchingOperand(otherT StaticType) bool { return true } - if otherT == TypeNil { + if otherT == TypeNil || t == TypeNil { return true } diff --git a/pkg/traceql/enum_statics_test.go b/pkg/traceql/enum_statics_test.go index 13c1be76511..7b922b094df 100644 --- a/pkg/traceql/enum_statics_test.go +++ b/pkg/traceql/enum_statics_test.go @@ -62,17 +62,19 @@ func TestStaticType_isMatchingOperand(t *testing.T) { // both numeric {t: TypeInt, otherT: TypeFloat, want: true}, {t: TypeFloat, otherT: TypeInt, want: true}, + + // TypeNil with others {t: TypeNil, otherT: TypeIntArray, want: true}, {t: TypeNil, otherT: TypeFloatArray, want: true}, {t: TypeNil, otherT: TypeStringArray, want: true}, {t: TypeNil, otherT: TypeBooleanArray, want: true}, - {t: TypeNil, otherT: TypeInt, want: false}, - {t: TypeNil, otherT: TypeFloat, want: false}, - {t: TypeNil, otherT: TypeString, want: false}, - {t: TypeNil, otherT: TypeBoolean, want: false}, - {t: TypeNil, otherT: TypeDuration, want: false}, - {t: TypeNil, otherT: TypeStatus, want: false}, - {t: TypeNil, otherT: TypeKind, want: false}, + {t: TypeNil, otherT: TypeInt, want: true}, + {t: TypeNil, otherT: TypeFloat, want: true}, + {t: TypeNil, otherT: TypeString, want: true}, + {t: TypeNil, otherT: TypeBoolean, want: true}, + {t: TypeNil, otherT: TypeDuration, want: true}, + {t: TypeNil, otherT: TypeStatus, want: true}, + {t: TypeNil, otherT: TypeKind, want: true}, // array types {t: TypeIntArray, otherT: TypeIntArray, want: true}, @@ -96,6 +98,8 @@ func TestStaticType_isMatchingOperand(t *testing.T) { name := fmt.Sprintf("%s with %s", tt.t, tt.otherT) t.Run(name, func(t *testing.T) { assert.Equalf(t, tt.want, tt.t.isMatchingOperand(tt.otherT), "isMatchingOperand: %s", name) + // test symmetric case + assert.Equalf(t, tt.want, tt.otherT.isMatchingOperand(tt.t), "isMatchingOperand(%s) [symmetric]", name) }) } } From fb44fb9cea42490423dbee6d90345d562f84b1b3 Mon Sep 17 00:00:00 2001 From: Suraj Nath <9503187+electron0zero@users.noreply.github.com> Date: Thu, 22 Aug 2024 04:56:46 +0530 Subject: [PATCH 2/2] more tests --- pkg/traceql/ast_execute_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pkg/traceql/ast_execute_test.go b/pkg/traceql/ast_execute_test.go index 4a2345a96b2..fc3dcee08f7 100644 --- a/pkg/traceql/ast_execute_test.go +++ b/pkg/traceql/ast_execute_test.go @@ -1951,6 +1951,34 @@ func TestSpansetExistence(t *testing.T) { }, matches: true, }, + // .foo exists but flip the query - test symmetric + { + query: `{ nil != .duration }`, + span: &mockSpan{ + attributes: map[Attribute]Static{ + NewAttribute("duration"): NewStaticDuration(time.Minute), + }, + }, + matches: true, + }, + { + query: `{ nil != .bar }`, + span: &mockSpan{ + attributes: map[Attribute]Static{ + NewAttribute("bar"): NewStaticString("bzz"), + }, + }, + matches: true, + }, + { + query: `{ nil != .float }`, + span: &mockSpan{ + attributes: map[Attribute]Static{ + NewAttribute("float"): NewStaticFloat(2.0), + }, + }, + matches: true, + }, } for _, tt := range tests { // create a evalTC and use testEvaluator