Skip to content

Commit

Permalink
query_spec - add 'equivalence' to suppress diffs (#236)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

- Closes #233 

## Short description of the changes

Adds the concept of equivalent (but not strictly equal) QuerySpecs to be
used to suppress diffs (e.g. filters or havings in a different order).

Also, sneaks in a `ToPtr` typed function and removes all the specific
`Ptr` functions (e.g. `StringPtr`, `BoolPtr`).
  • Loading branch information
jharley authored Nov 7, 2022
1 parent a2b404d commit 7530a64
Show file tree
Hide file tree
Showing 15 changed files with 330 additions and 83 deletions.
6 changes: 3 additions & 3 deletions client/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ func TestBoards(t *testing.T) {
Calculations: []CalculationSpec{
{
Op: CalculationOpAvg,
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
},
TimeRange: IntPtr(3600), // 1 hour
TimeRange: ToPtr(3600), // 1 hour
})
assert.NoError(t, err)

Expand Down Expand Up @@ -78,7 +78,7 @@ func TestBoards(t *testing.T) {
Op: CalculationOpCount,
},
},
TimeRange: IntPtr(7200), // 2 hours
TimeRange: ToPtr(7200), // 2 hours
})
assert.NoError(t, err)
b.ColumnLayout = BoardColumnStyleMulti
Expand Down
8 changes: 4 additions & 4 deletions client/column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ func TestColumns(t *testing.T) {
t.Run("Create", func(t *testing.T) {
data := &Column{
KeyName: "column_test",
Hidden: BoolPtr(false),
Hidden: ToPtr(false),
Description: "This column is created by a test",
Type: ColumnTypePtr(ColumnTypeFloat),
Type: ToPtr(ColumnTypeFloat),
}
column, err = c.Columns.Create(ctx, dataset, data)

Expand Down Expand Up @@ -65,9 +65,9 @@ func TestColumns(t *testing.T) {
data := &Column{
ID: column.ID,
KeyName: "a-new-keyname",
Hidden: BoolPtr(true),
Hidden: ToPtr(true),
Description: "This is a new description",
Type: ColumnTypePtr(ColumnTypeBoolean),
Type: ToPtr(ColumnTypeBoolean),
}
column, err = c.Columns.Update(ctx, dataset, data)

Expand Down
2 changes: 1 addition & 1 deletion client/query_result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestQueryResults(t *testing.T) {
Op: "COUNT",
},
},
TimeRange: IntPtr(60 * 60 * 24),
TimeRange: ToPtr(60 * 60 * 24),
})

t.Run("Create", func(t *testing.T) {
Expand Down
67 changes: 67 additions & 0 deletions client/query_spec.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package client

import "reflect"

// QuerySpec represents a Honeycomb query.
//
// API docs: https://docs.honeycomb.io/api/query-specification/
Expand Down Expand Up @@ -48,6 +50,71 @@ type QuerySpec struct {
Granularity *int `json:"granularity,omitempty"`
}

// Determines if two QuerySpecs are equivalent
func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
// The order of Calculations is important for visualization rendering, so we're looking for equality
calcMatch := true
if len(qs.Calculations) != len(other.Calculations) {
calcMatch = false
} else {
for i := range qs.Calculations {
if !reflect.DeepEqual(qs.Calculations[i], other.Calculations[i]) {
calcMatch = false
break
}
}
}
if !calcMatch {
// 'COUNT' is the default Calculation and equivalent to an empty Calculations -- check that before we give up
defaultCalc := []CalculationSpec{{Op: CalculationOpCount}}
if (other.Calculations != nil && reflect.DeepEqual(qs.Calculations, defaultCalc)) ||
(qs.Calculations != nil && reflect.DeepEqual(other.Calculations, defaultCalc)) ||
(len(qs.Calculations) > 1 || len(other.Calculations) > 1) {
return false
}
}

// the exact order of filters does not matter, but their equvalence does
if !Equivalent(qs.Filters, other.Filters) {
return false
}
if qs.FilterCombination != other.FilterCombination {
// 'AND' is the default and equivalent to an empty Filter Combination
if (qs.FilterCombination == FilterCombinationAnd && other.FilterCombination != "") ||
(other.FilterCombination == FilterCombinationAnd && qs.FilterCombination != "") {
return false
}
}
if !reflect.DeepEqual(qs.Breakdowns, other.Breakdowns) {
return false
}
if !reflect.DeepEqual(qs.Orders, other.Orders) {
return false
}
// the exact order of havings does not matter, but their equvalence does
if !Equivalent(qs.Havings, other.Havings) {
return false
}
if !reflect.DeepEqual(qs.Limit, other.Limit) {
return false
}
if !reflect.DeepEqual(qs.TimeRange, other.TimeRange) {
// 7200 is the default, so an unset value is equivalent to 7200
if (reflect.DeepEqual(qs.TimeRange, 7200) && other.TimeRange != nil) ||
(reflect.DeepEqual(other.TimeRange, 7200) && qs.TimeRange != nil) {
return false
}
}
if !reflect.DeepEqual(qs.StartTime, other.StartTime) || !reflect.DeepEqual(qs.EndTime, other.EndTime) {
return false
}
if !reflect.DeepEqual(qs.Granularity, other.Granularity) {
return false
}

return true
}

// CalculationSpec represents a calculation within a query.
type CalculationSpec struct {
Op CalculationOp `json:"op"`
Expand Down
143 changes: 132 additions & 11 deletions client/query_spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ func TestQuerySpec(t *testing.T) {
},
{
Op: CalculationOpHeatmap,
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
{
Op: CalculationOpP99,
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
},
Filters: []FilterSpec{
Expand All @@ -48,26 +48,147 @@ func TestQuerySpec(t *testing.T) {
Breakdowns: []string{"column_1", "column_2"},
Orders: []OrderSpec{
{
Column: StringPtr("column_1"),
Column: ToPtr("column_1"),
},
{
Op: CalculationOpPtr(CalculationOpCount),
Order: SortOrderPtr(SortOrderDesc),
Op: ToPtr(CalculationOpCount),
Order: ToPtr(SortOrderDesc),
},
},
Havings: []HavingSpec{
{
Column: StringPtr("duration_ms"),
Op: HavingOpPtr(HavingOpGreaterThan),
CalculateOp: CalculationOpPtr(CalculationOpP99),
Column: ToPtr("duration_ms"),
Op: ToPtr(HavingOpGreaterThan),
CalculateOp: ToPtr(CalculationOpP99),
Value: 1000.0,
},
},
Limit: IntPtr(100),
TimeRange: IntPtr(3600), // 1 hour
Granularity: IntPtr(60), // 1 minute
Limit: ToPtr(100),
TimeRange: ToPtr(3600), // 1 hour
Granularity: ToPtr(60), // 1 minute
}

_, err := c.Queries.Create(ctx, dataset, &query)
assert.NoError(t, err)
}

func TestQuerySpec_EquivalentTo(t *testing.T) {
tests := []struct {
name string
a, b QuerySpec
want bool
}{
{"Empty", QuerySpec{}, QuerySpec{}, true},
{
"Empty Defaults",
QuerySpec{
Calculations: []CalculationSpec{
{
Op: "COUNT",
},
},
FilterCombination: "AND",
TimeRange: ToPtr(7200),
},
QuerySpec{},
true,
},
{
"Equivalent but shuffled",
QuerySpec{
Calculations: []CalculationSpec{
{
Op: "HEATMAP",
Column: ToPtr("duration_ms"),
},
{
Op: "COUNT",
},
},
Filters: []FilterSpec{
{
Column: "colA",
Op: "=",
Value: "a",
},
{
Column: "colC",
Op: "=",
Value: "c",
},
{
Column: "colB",
Op: "=",
Value: "b",
},
},
Breakdowns: []string{"colB", "colA"},
TimeRange: ToPtr(7200),
},
QuerySpec{
Calculations: []CalculationSpec{
{
Op: "HEATMAP",
Column: ToPtr("duration_ms"),
},
{
Op: "COUNT",
},
},
Filters: []FilterSpec{
{
Column: "colC",
Op: "=",
Value: "c",
},
{
Column: "colB",
Op: "=",
Value: "b",
},
{
Column: "colA",
Op: "=",
Value: "a",
},
},
Breakdowns: []string{"colB", "colA"},
FilterCombination: "AND",
},
true,
},
{
"Calculation order matters",
QuerySpec{
Calculations: []CalculationSpec{
{
Op: "HEATMAP",
Column: ToPtr("duration_ms"),
},
{
Op: "COUNT",
},
},
},
QuerySpec{
Calculations: []CalculationSpec{
{
Op: "COUNT",
},
{
Op: "HEATMAP",
Column: ToPtr("duration_ms"),
},
},
},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := tt.a.EquivalentTo(tt.b); got != tt.want {
t.Errorf("QuerySpec.EquivalentTo() = %v, want %v", got, tt.want)
}
})
}
}
14 changes: 7 additions & 7 deletions client/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestQueries(t *testing.T) {
},
{
Op: CalculationOpHeatmap,
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
},
Filters: []FilterSpec{
Expand All @@ -42,16 +42,16 @@ func TestQueries(t *testing.T) {
Breakdowns: []string{"column_1", "column_2"},
Orders: []OrderSpec{
{
Column: StringPtr("column_1"),
Column: ToPtr("column_1"),
},
{
Op: CalculationOpPtr(CalculationOpCount),
Order: SortOrderPtr(SortOrderDesc),
Op: ToPtr(CalculationOpCount),
Order: ToPtr(SortOrderDesc),
},
},
Limit: IntPtr(100),
TimeRange: IntPtr(3600), // 1 hour
Granularity: IntPtr(60), // 1 minute
Limit: ToPtr(100),
TimeRange: ToPtr(3600), // 1 hour
Granularity: ToPtr(60), // 1 minute
}

query, err = c.Queries.Create(ctx, dataset, data)
Expand Down
8 changes: 4 additions & 4 deletions client/trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestTriggers(t *testing.T) {
Calculations: []CalculationSpec{
{
Op: CalculationOpP99,
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
},
Filters: []FilterSpec{
Expand Down Expand Up @@ -70,7 +70,7 @@ func TestTriggers(t *testing.T) {
data.AlertType = trigger.AlertType
data.Recipients[0].ID = trigger.Recipients[0].ID
// set default time range
data.Query.TimeRange = IntPtr(300)
data.Query.TimeRange = ToPtr(300)

// set the default alert type
data.AlertType = TriggerAlertTypeValueOnChange
Expand Down Expand Up @@ -158,7 +158,7 @@ func TestMatchesTriggerSubset(t *testing.T) {
Op: CalculationOpCount,
},
},
Limit: IntPtr(100),
Limit: ToPtr(100),
},
expectedErr: errors.New("limit is not allowed in a trigger query"),
},
Expand All @@ -171,7 +171,7 @@ func TestMatchesTriggerSubset(t *testing.T) {
},
Orders: []OrderSpec{
{
Column: StringPtr("duration_ms"),
Column: ToPtr("duration_ms"),
},
},
},
Expand Down
Loading

0 comments on commit 7530a64

Please sign in to comment.