Skip to content

Commit

Permalink
fix - queryspec Orders equivalency (#366)
Browse files Browse the repository at this point in the history
Reported via Pollinators
([link](https://honeycombpollinators.slack.com/archives/C017T9FFT0D/p1692690089887929)).

Prior to this change a `honeycombio_query` as below would cause an
infinite diff:

```hcl
resource "honeycombio_query" "test" {
  dataset    = var.dataset
  query_json = <<-EOF
{
  "breakdowns": ["app.tenant"],
  "calculations": [{ "op": "COUNT" }],
  "orders": [
    {
      "column": "app.tenant",
      "order": "ascending"
    }
  ],
  "time_range": 7200
}
EOF
}
```

Plan output:

```
  # honeycombio_query.test must be replaced
-/+ resource "honeycombio_query" "test" {
      ~ id         = "AD1Ue8TJCfC" -> (known after apply)
      ~ query_json = jsonencode(
          ~ {
              ~ orders             = [
                  ~ {
                      + order  = "ascending"
                        # (1 unchanged attribute hidden)
                    },
                ]
                # (3 unchanged attributes hidden)
            } # forces replacement
        )
        # (1 unchanged attribute hidden)
    }
```
  • Loading branch information
jharley authored Sep 18, 2023
1 parent 56589b9 commit 54bd888
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 3 deletions.
19 changes: 16 additions & 3 deletions client/query_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,22 @@ func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
}
}

// Orders have a default ascending order, so we need to check equivalence
if len(qs.Orders) != len(other.Orders) {
return false
}
for i := range qs.Orders {
if PtrValueOrDefault(qs.Orders[i].Order, SortOrderAsc) != PtrValueOrDefault(other.Orders[i].Order, SortOrderAsc) {
return false
}
if !reflect.DeepEqual(qs.Orders[i].Column, other.Orders[i].Column) {
return false
}
if !reflect.DeepEqual(qs.Orders[i].Op, other.Orders[i].Op) {
return false
}
}

// the exact order of filters does not matter, but their equvalence does
if !Equivalent(qs.Filters, other.Filters) {
return false
Expand All @@ -91,9 +107,6 @@ func (qs *QuerySpec) EquivalentTo(other QuerySpec) bool {
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
Expand Down
72 changes: 72 additions & 0 deletions client/query_spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,78 @@ func TestQuerySpec_EquivalentTo(t *testing.T) {
},
false,
},
{
"Equivalent column orders",
QuerySpec{
Orders: []OrderSpec{
{Column: ToPtr("column_1")},
},
},
QuerySpec{
Orders: []OrderSpec{
{
Column: ToPtr("column_1"),
Order: ToPtr(SortOrderAsc),
},
},
},
true,
},
{
"Not equivalent column orders",
QuerySpec{
Orders: []OrderSpec{
{Column: ToPtr("column_2")},
},
},
QuerySpec{
Orders: []OrderSpec{
{
Column: ToPtr("column_1"),
Order: ToPtr(SortOrderAsc),
},
},
},
false,
},
{
"Equivalent Op orders with unspecified default",
QuerySpec{
Orders: []OrderSpec{
{
Op: ToPtr(CalculationOpCount),
Order: ToPtr(SortOrderAsc),
},
},
},
QuerySpec{
Orders: []OrderSpec{
{Op: ToPtr(CalculationOpCount)},
},
},
true,
},
{
"Not equivalent Op orders",
QuerySpec{
Orders: []OrderSpec{
{
Op: ToPtr(CalculationOpCountDistinct),
Column: ToPtr("column_1"),
Order: ToPtr(SortOrderAsc),
},
},
},
QuerySpec{
Orders: []OrderSpec{
{
Op: ToPtr(CalculationOpCount),
Order: ToPtr(SortOrderDesc),
},
},
},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 54bd888

Please sign in to comment.