Skip to content

Commit

Permalink
Fixed v3 string filter
Browse files Browse the repository at this point in the history
  • Loading branch information
@alpinskiy
alpinskiy committed Nov 20, 2024
1 parent 6db9602 commit 954dbe3
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 43 deletions.
8 changes: 5 additions & 3 deletions internal/api/sql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,13 @@ func writeTagCond(sb *strings.Builder, f data_model.TagFilters, in bool) {
}
tag := format.TagID(i)
for _, valPair := range values {
if len(valPair.Value) > 0 {
if valPair.HasValue() {
strValues = append(strValues, valPair.Value)
}
// we allow 0 here because it is a valid value for raw tags
if tag != format.StringTopTagID {
// zero mapped value is not valid filter when string value specified,
// because string filter value automatically excludes RAW tags
// and zero tag value might be only valid for RAW tags
if tag != format.StringTopTagID && (!valPair.HasValue() || valPair.Mapped != 0) {
intValues = append(intValues, valPair.Mapped)
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/api/sql_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestTagValuesQueryV3(t *testing.T) {
tagID: "2",
numResults: 5,
}
pq.filterIn.Append(1, data_model.TagValue{Value: "one", Mapped: 1}, data_model.TagValue{Value: "two", Mapped: 2})
pq.filterIn.Append(1, data_model.NewTagValue("one", 1), data_model.NewTagValue("two", 2))
pq.filterNotIn.AppendValue(0, "staging")
lod := getLod(t, Version3)

Expand Down Expand Up @@ -155,7 +155,7 @@ func TestLoadPointsQueryV3(t *testing.T) {
isStringTop: false,
kind: data_model.DigestCountSec.Kind(false),
}
pq.filterIn.Append(1, data_model.TagValue{Value: "one", Mapped: 1}, data_model.TagValue{Value: "two", Mapped: 2})
pq.filterIn.Append(1, data_model.NewTagValue("one", 1), data_model.NewTagValue("two", 2))
pq.filterNotIn.AppendValue(0, "staging")
lod := getLod(t, Version3)

Expand All @@ -179,7 +179,7 @@ func TestLoadPointsQueryV3_maxHost(t *testing.T) {
isStringTop: false,
kind: data_model.DigestCountSec.Kind(true),
}
pq.filterIn.Append(1, data_model.TagValue{Value: "one", Mapped: 1}, data_model.TagValue{Value: "two", Mapped: 2})
pq.filterIn.Append(1, data_model.NewTagValue("one", 1), data_model.NewTagValue("two", 2))
pq.filterNotIn.AppendValue(0, "staging")
lod := getLod(t, Version3)

Expand Down
46 changes: 42 additions & 4 deletions internal/data_model/query_filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,18 @@ type TagFilters struct {
type TagValues []TagValue

type TagValue struct {
f TagValueFlags
Value string
Mapped int32
}

type TagValueFlags int

const (
Value TagValueFlags = 1 << iota
Mapped
)

func MetricIDFilter(metricID int32) TagFilters {
return MetricFilter(&format.MetricMetaValue{MetricID: metricID})
}
Expand Down Expand Up @@ -90,15 +98,15 @@ func (f *QueryFilter) MatchMetrics(m map[string]*format.MetricMetaValue) {
func (f *TagFilters) AppendValue(tag int, val ...string) {
s := make(TagValues, len(val))
for i := 0; i < len(val); i++ {
s[i].Value = val[i]
s[i] = NewTagValueS(val[i])
}
f.Append(tag, s...)
}

func (f *TagFilters) AppendMapped(tag int, val ...int32) {
s := make(TagValues, len(val))
for i := 0; i < len(val); i++ {
s[i].Mapped = val[i]
s[i] = NewTagValueM(val[i])
}
f.Append(tag, s...)
}
Expand All @@ -120,11 +128,41 @@ func (v TagValues) Sort() {
})
}

func NewTagValue(s string, n int32) TagValue {
return TagValue{
f: Value | Mapped,
Value: s,
Mapped: n,
}
}

func NewTagValueS(s string) TagValue {
return TagValue{
f: Value,
Value: s,
}
}

func NewTagValueM(n int32) TagValue {
return TagValue{
f: Mapped,
Mapped: n,
}
}

func (v TagValue) HasValue() bool {
return v.f&Value != 0
}

func (v TagValue) IsMapped() bool {
return v.f&Mapped != 0
}

func (v TagValue) String() string {
if v.Value != "" {
if v.HasValue() {
return v.Value
}
if v.Mapped != 0 {
if v.IsMapped() {
return strconv.Itoa(int(v.Mapped))
}
return ""
Expand Down
70 changes: 37 additions & 33 deletions internal/promql/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1177,37 +1177,33 @@ func (ev *evaluator) buildSeriesQuery(ctx context.Context, sel *parser.VectorSel
i := tag.Index
switch matcher.Type {
case labels.MatchEqual:
id, err := ev.getTagValueID(metric, i, matcher.Value)
if err != nil {
if errors.Is(err, ErrNotFound) {
if ev.opt.Version == data_model.Version3 {
// we allow unmapped values for v3 requests
sel.FilterIn.AppendValue(i, matcher.Value)
} else {
// string is not mapped, result is guaranteed to be empty
emptyCount[i]++
continue
}
if v, err := ev.getTagValue(metric, i, matcher.Value); err == nil {
sel.FilterIn.Append(i, v)
} else if errors.Is(err, ErrNotFound) {
if ev.opt.Version == data_model.Version3 {
// we allow unmapped values for v3 requests
sel.FilterIn.AppendValue(i, matcher.Value)
} else {
return SeriesQuery{}, fmt.Errorf("failed to map string %q: %v", matcher.Value, err)
// string is not mapped, result is guaranteed to be empty
emptyCount[i]++
continue
}
} else {
return SeriesQuery{}, fmt.Errorf("failed to map string %q: %v", matcher.Value, err)
}
sel.FilterIn.Append(i, data_model.TagValue{Value: matcher.Value, Mapped: id})
case labels.MatchNotEqual:
id, err := ev.getTagValueID(metric, i, matcher.Value)
if err != nil {
if errors.Is(err, ErrNotFound) {
// we allow unmapped values for v3 requests
if ev.opt.Version == data_model.Version3 {
sel.FilterNotIn.AppendValue(i, matcher.Value)
} else {
continue // ignore values with no mapping
}
if v, err := ev.getTagValue(metric, i, matcher.Value); err == nil {
sel.FilterNotIn.Append(i, v)
} else if errors.Is(err, ErrNotFound) {
// we allow unmapped values for v3 requests
if ev.opt.Version == data_model.Version3 {
sel.FilterNotIn.AppendValue(i, matcher.Value)
} else {
return SeriesQuery{}, err
continue // ignore values with no mapping
}
} else {
return SeriesQuery{}, fmt.Errorf("failed to map string %q: %v", matcher.Value, err)
}
sel.FilterNotIn.Append(i, data_model.TagValue{Value: matcher.Value, Mapped: id})
case labels.MatchRegexp:
m, err := ev.getTagValues(ctx, metric, i, offset)
if err != nil {
Expand Down Expand Up @@ -1333,50 +1329,58 @@ func (ev *evaluator) getTagValues(ctx context.Context, metric *format.MetricMeta
if s == " 0" {
s = ""
}
res = append(res, data_model.TagValue{Value: s, Mapped: id})
res = append(res, data_model.NewTagValue(s, id))
}
m2[offset] = res
return res, nil
}

func (ev *evaluator) getTagValueID(metric *format.MetricMetaValue, tagX int, tagV string) (int32, error) {
func (ev *evaluator) getTagValue(metric *format.MetricMetaValue, tagX int, tagV string) (data_model.TagValue, error) {
if tagV == "" {
return 0, nil
return data_model.NewTagValue("", 0), nil
}
if format.HasRawValuePrefix(tagV) {
return format.ParseCodeTagValue(tagV)
return data_model.NewTagValue("", 0), nil
}
if tagX < 0 || len(metric.Tags) <= tagX {
return 0, ErrNotFound
return data_model.TagValue{}, ErrNotFound
}
t := metric.Tags[tagX]
if t.Raw {
// histogram bucket label
if t.Name == labels.BucketLabel {
if v, err := strconv.ParseFloat(tagV, 32); err == nil {
return statshouse.LexEncode(float32(v)), nil
return data_model.NewTagValueM(statshouse.LexEncode(float32(v))), nil
}
}
// mapping from raw value comments
var s string
for k, v := range t.ValueComments {
if v == tagV {
if s != "" {
return 0, fmt.Errorf("ambiguous comment to value mapping")
return data_model.TagValue{}, fmt.Errorf("ambiguous comment to value mapping")
}
s = k
}
}
if s != "" {
return format.ParseCodeTagValue(s)
v, err := format.ParseCodeTagValue(s)
if err != nil {
return data_model.TagValue{}, err
}
return data_model.NewTagValueM(v), nil
}
}
return ev.GetTagValueID(TagValueIDQuery{
v, err := ev.GetTagValueID(TagValueIDQuery{
Version: ev.opt.Version,
Metric: metric,
TagIndex: tagX,
TagValue: tagV,
})
if err != nil {
return data_model.TagValue{}, err
}
return data_model.NewTagValue(tagV, v), nil
}

func (ev *evaluator) getStringTop(ctx context.Context, metric *format.MetricMetaValue, offset int64) ([]string, error) {
Expand Down

0 comments on commit 954dbe3

Please sign in to comment.