Repository: honeycombio/terraform-provider-honeycombio (go)

Issue:
Add HAVING support to Query Specification
Currently, the provider client does not have support for `havings` and will return an error if you try to craft a query making use of them.

See: https://docs.honeycomb.io/api/query-specification/#fields-on-a-query-specification

Pull request description:
HAVING support
Adds support for "havings". Snuck in a few linter fixes as well 🙃 

Resolves #97 

Patch:
diff --git a/client/query_spec.go b/client/query_spec.go
index 45187a5..3a8da79 100644
--- a/client/query_spec.go
+++ b/client/query_spec.go
@@ -26,6 +26,11 @@ type QuerySpec struct {
 	// results. Each term must appear in either the breakdowns field or the
 	// calculations field.
 	Orders []OrderSpec `json:"orders,omitempty"`
+	// A list of objects describing filters with which to restrict returned
+	// groups. Each column/calculate_op pair must appear in the calculations
+	// field. There can be multiple havings for the same column/calculate_op
+	// pair.
+	Havings []HavingSpec `json:"havings,omitempty"`
 	// The maximum number of query results, must be between 1 and 1000.
 	Limit *int `json:"limit,omitempty"`
 	// The time range of query in seconds. Defaults to two hours. If combined
@@ -192,3 +197,36 @@ const (
 func SortOrders() []SortOrder {
 	return []SortOrder{SortOrderAsc, SortOrderDesc}
 }
+
+// HavingSpec describes filters in which to restrict returned groups.
+type HavingSpec struct {
+	CalculateOp *CalculationOp `json:"calculate_op,omitempty"`
+	Column      *string        `json:"column,omitempty"`
+	Op          *HavingOp      `json:"op,omitempty"`
+	Value       interface{}    `json:"value,omitempty"`
+}
+
+// HavingOp represents the operator of a having clause
+type HavingOp string
+
+// Declaration of having operations
+const (
+	HavingOpEquals             HavingOp = "="
+	HavingOpNotEquals          HavingOp = "!="
+	HavingOpGreaterThan        HavingOp = ">"
+	HavingOpGreaterThanOrEqual HavingOp = ">="
+	HavingOpLessThan           HavingOp = "<"
+	HavingOpLessThanOrEqual    HavingOp = "<="
+)
+
+// HavingOps returns an exhaustive list of all having operations.
+func HavingOps() []HavingOp {
+	return []HavingOp{
+		HavingOpEquals,
+		HavingOpNotEquals,
+		HavingOpGreaterThan,
+		HavingOpGreaterThanOrEqual,
+		HavingOpLessThan,
+		HavingOpLessThanOrEqual,
+	}
+}
diff --git a/client/type_helpers.go b/client/type_helpers.go
index a02dcc6..d8e88a5 100644
--- a/client/type_helpers.go
+++ b/client/type_helpers.go
@@ -10,6 +10,11 @@ func CalculationOpPtr(v CalculationOp) *CalculationOp {
 	return &v
 }
 
+// HavingOpPtr returns a pointer to the given HavingOp.
+func HavingOpPtr(v HavingOp) *HavingOp {
+	return &v
+}
+
 // ColumnTypePtr returns a pointer to the given ColumnType.
 func ColumnTypePtr(v ColumnType) *ColumnType {
 	return &v
diff --git a/docs/data-sources/query_specification.md b/docs/data-sources/query_specification.md
index 14357bd..7c18e42 100644
--- a/docs/data-sources/query_specification.md
+++ b/docs/data-sources/query_specification.md
@@ -1,8 +1,8 @@
 # Data Source: honeycombio_query_specification
 
-Construct a query that can be used in triggers and boards. For more information about the query specification, check out [Query Specification](https://docs.honeycomb.io/api/query-specification/).
+Generates a [Query Specificaiton](https://docs.honeycomb.io/api/query-specification/) in JSON format.
 
-The `json` attribute contains a serialized JSON representation which can be passed to the `query_json` field of [`honeycombio_trigger`](../resources/trigger.md) and [`honeycombio_board`](../resources/board.md).
+This is a data source which can be used to construct a JSON representation of a Honeycomb [Query Specification](https://docs.honeycomb.io/api/query-specification/). The `json` attribute contains a serialized JSON representation which can be passed to the `query_json` field of the [`honeycombio_query`](../resources/query.md) resource for use in boards and triggers.
 
 ## Example Usage
 
@@ -47,6 +47,7 @@ The following arguments are supported:
 * `filter_combination` - (Optional) How to combine multiple filters, either `AND` (default) or `OR`.
 * `breakdowns` - (Optional) A list of fields to group by.
 * `order` - (Optional) Zero or more configuration blocks (described below) describing how to order the query results. Each term must appear in either `calculation` or `breakdowns`.
+* `having` - (Optional) Zero or more filters used to restrict returned groups in the query result.
 * `limit` - (Optional)  The maximum number of query results, must be between 1 and 1000.
 * `time_range` - (Optional) The time range of the query in seconds, defaults to two hours.
 * `start_time` - (Optional) The absolute start time of the query in Unix Time (= seconds since epoch).
@@ -80,6 +81,15 @@ Each query configuration may have zero or more `order` blocks, which each accept
 * `column` - (Optional) Either a column present in `breakdown` or a column to `op` applies to.
 * `order` - (Optional) The sort direction, if set must be `ascending` or `descending`.
 
+Each query configuration may have zero or more `having` blocks, which each accept the following arguments.
+
+* `op` - The operator to apply to filter the query results. One of `=`, `!=`, `>`, `>=`, `<`, or `<=`.
+* `calculate_op` - The calculation operator to apply, supports all of the [Calculation Operators](https://docs.honeycomb.io/api/query-specification/#calculation-operators) with the exception of `HEATMAP`.
+* `column` - The column to apply the `calculate_op` to, not needed with `COUNT`.
+* `value` - The value used with `op`. Currently assumed to be a number.
+
+~> **NOTE** A having term's `column`/`calculate_op` pair must have a corresponding `calculation`. There can be multiple `having` blocks for the same `column`/`calculate_op` pair.
+
 ## Attribute Reference
 
 In addition to all arguments above, the following attributes are exported:
diff --git a/honeycombio/data_source_query_specification.go b/honeycombio/data_source_query_specification.go
index c76a6e3..a23c515 100644
--- a/honeycombio/data_source_query_specification.go
+++ b/honeycombio/data_source_query_specification.go
@@ -3,6 +3,7 @@ package honeycombio
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"strconv"
 	"strings"
 
@@ -78,6 +79,34 @@ func dataSourceHoneycombioQuerySpec() *schema.Resource {
 					},
 				},
 			},
+			"having": {
+				Type:     schema.TypeList,
+				Optional: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"calculate_op": {
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringInSlice(havingCalculateOpStrings(), false),
+						},
+						"column": {
+							Type: schema.TypeString,
+							// not required for COUNT
+							Optional: true,
+						},
+						"op": {
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringInSlice(havingOpStrings(), false),
+						},
+						"value": {
+							// API currently assumes this is a number
+							Type:     schema.TypeFloat,
+							Required: true,
+						},
+					},
+				},
+			},
 			"filter_combination": {
 				Type:     schema.TypeString,
 				Optional: true,
@@ -159,9 +188,30 @@ func dataSourceHoneycombioQuerySpecRead(ctx context.Context, d *schema.ResourceD
 		return diag.FromErr(err)
 	}
 
+	havings, err := extractHavings(d)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	// Ensure all havings have a matching calculate_op/column pair
+	for i, having := range havings {
+		found := false
+		for _, calc := range calculations {
+			if reflect.DeepEqual(having.Column, calc.Column) &&
+				*having.CalculateOp == calc.Op {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return diag.Errorf("having %d without matching column in query", i)
+		}
+	}
+
 	query := &honeycombio.QuerySpec{
 		Calculations:      calculations,
 		Filters:           filters,
+		Havings:           havings,
 		FilterCombination: honeycombio.FilterCombination(d.Get("filter_combination").(string)),
 		Breakdowns:        extractBreakdowns(d),
 		Orders:            extractOrders(d),
@@ -234,6 +284,44 @@ func extractFilters(d *schema.ResourceData) ([]honeycombio.FilterSpec, error) {
 	return filters, nil
 }
 
+func extractHavings(d *schema.ResourceData) ([]honeycombio.HavingSpec, error) {
+	havingSchemas := d.Get("having").([]interface{})
+	havings := make([]honeycombio.HavingSpec, len(havingSchemas))
+
+	for i := range havingSchemas {
+		having := &havings[i]
+
+		co, ok := d.GetOk(fmt.Sprintf("having.%d.calculate_op", i))
+		if ok {
+			having.CalculateOp = honeycombio.CalculationOpPtr(honeycombio.CalculationOp(co.(string)))
+		}
+
+		c, ok := d.GetOk(fmt.Sprintf("having.%d.column", i))
+		if ok {
+			having.Column = honeycombio.StringPtr(c.(string))
+		}
+
+		op, ok := d.GetOk(fmt.Sprintf("having.%d.op", i))
+		if ok {
+			having.Op = honeycombio.HavingOpPtr(honeycombio.HavingOp(op.(string)))
+		}
+
+		v, ok := d.GetOk(fmt.Sprintf("having.%d.value", i))
+		if ok {
+			having.Value = v
+		}
+
+		if *having.CalculateOp == honeycombio.CalculationOpCount && having.Column != nil {
+			return nil, fmt.Errorf("calculate_op %s should not have an accompanying column", *having.CalculateOp)
+		}
+		if *having.CalculateOp != honeycombio.CalculationOpCount && having.Column == nil {
+			return nil, fmt.Errorf("calculate_op %s requires a column", *having.CalculateOp)
+		}
+	}
+
+	return havings, nil
+}
+
 const multipleValuesError = "must choose one of 'value', 'value_string', 'value_integer', 'value_float', 'value_boolean'"
 
 func extractFilter(d *schema.ResourceData, index int) (honeycombio.FilterSpec, error) {
@@ -272,7 +360,7 @@ func extractFilter(d *schema.ResourceData, index int) (honeycombio.FilterSpec, e
 		filter.Value = vf
 		valueSet = true
 	}
-	vb, vbOk := d.GetOkExists(fmt.Sprintf("filter.%d.value_boolean", index))
+	vb, vbOk := d.GetOk(fmt.Sprintf("filter.%d.value_boolean", index))
 	if vbOk {
 		if valueSet {
 			return filter, fmt.Errorf(multipleValuesError)
diff --git a/honeycombio/type_helpers.go b/honeycombio/type_helpers.go
index 6019ec1..7ea25ca 100644
--- a/honeycombio/type_helpers.go
+++ b/honeycombio/type_helpers.go
@@ -57,6 +57,31 @@ func filterOpStrings() []string {
 	return out
 }
 
+func havingOpStrings() []string {
+	in := honeycombio.HavingOps()
+	out := make([]string, len(in))
+
+	for i := range in {
+		out[i] = string(in[i])
+	}
+
+	return out
+}
+
+func havingCalculateOpStrings() []string {
+	in := honeycombio.CalculationOps()
+	out := make([]string, len(in))
+
+	for i := range in {
+		// havings cannot use HEATMAP
+		if in[i] != honeycombio.CalculationOpHeatmap {
+			out[i] = string(in[i])
+		}
+	}
+
+	return out
+}
+
 func sortOrderStrings() []string {
 	in := honeycombio.SortOrders()
 	out := make([]string, len(in))


Test patch:
diff --git a/client/query_spec_test.go b/client/query_spec_test.go
index 71eeb69..620ef52 100644
--- a/client/query_spec_test.go
+++ b/client/query_spec_test.go
@@ -25,6 +25,10 @@ func TestQuerySpec(t *testing.T) {
 				Op:     CalculationOpHeatmap,
 				Column: StringPtr("duration_ms"),
 			},
+			{
+				Op:     CalculationOpP99,
+				Column: StringPtr("duration_ms"),
+			},
 		},
 		Filters: []FilterSpec{
 			{
@@ -48,6 +52,14 @@ func TestQuerySpec(t *testing.T) {
 				Order: SortOrderPtr(SortOrderDesc),
 			},
 		},
+		Havings: []HavingSpec{
+			{
+				Column:      StringPtr("duration_ms"),
+				Op:          HavingOpPtr(HavingOpGreaterThan),
+				CalculateOp: CalculationOpPtr(CalculationOpP99),
+				Value:       1000.0,
+			},
+		},
 		Limit:       IntPtr(100),
 		TimeRange:   IntPtr(3600), // 1 hour
 		Granularity: IntPtr(60),   // 1 minute
@@ -153,3 +165,40 @@ func TestFilterOps(t *testing.T) {
 		assert.NoError(t, err, fmt.Sprintf("Failed to create board that contains filter with op \"%v\"", filterOp))
 	}
 }
+
+func TestHavingOps(t *testing.T) {
+	ctx := context.Background()
+	c := newTestClient(t)
+	dataset := testDataset(t)
+
+	b := &Board{
+		Name: "go-honeycombio: TestHavingOps",
+	}
+	b, err := c.Boards.Create(ctx, b)
+	assert.NoError(t, err)
+
+	defer c.Boards.Delete(ctx, b.ID)
+
+	for _, havingOp := range HavingOps() {
+		q := QuerySpec{
+			Calculations: []CalculationSpec{
+				{
+					Op:     CalculationOpP99,
+					Column: StringPtr("duration_ms"),
+				},
+			},
+			Havings: []HavingSpec{
+				{
+					Column:      StringPtr("duration_ms"),
+					Op:          &havingOp,
+					CalculateOp: CalculationOpPtr(CalculationOpP99),
+					Value:       1000,
+				},
+			},
+		}
+		b.Queries = []BoardQuery{{Dataset: dataset, Query: &q}}
+
+		_, err = c.Boards.Update(ctx, b)
+		assert.NoError(t, err, fmt.Sprintf("Failed to create board that contains having with op \"%v\"", havingOp))
+	}
+}
diff --git a/honeycombio/data_source_query_specification_test.go b/honeycombio/data_source_query_specification_test.go
index 5ae39fe..0226863 100644
--- a/honeycombio/data_source_query_specification_test.go
+++ b/honeycombio/data_source_query_specification_test.go
@@ -29,6 +29,10 @@ data "honeycombio_query_specification" "test" {
         op     = "AVG"
         column = "duration_ms"
     }
+    calculation {
+        op     = "P99"
+        column = "duration_ms"
+    }
 
     filter {
         column = "trace.parent_id"
@@ -56,6 +60,13 @@ data "honeycombio_query_specification" "test" {
         order  = "descending"
     }
 
+    having {
+        calculate_op = "P99"
+        column       = "duration_ms"
+        op           = ">"
+        value        = 1000
+    }
+
     limit 	    = 250
     time_range  = 7200
     start_time  = 1577836800
@@ -72,6 +83,10 @@ const expectedJSON string = `{
     {
       "op": "AVG",
       "column": "duration_ms"
+    },
+    {
+      "op": "P99",
+      "column": "duration_ms"
     }
   ],
   "filters": [
@@ -103,6 +118,14 @@ const expectedJSON string = `{
       "order": "descending"
     }
   ],
+  "havings": [
+    {
+      "calculate_op": "P99",
+      "column": "duration_ms",
+      "op": "\u003e",
+      "value": 1000
+    }
+  ],
   "limit": 250,
   "time_range": 7200,
   "start_time": 1577836800,
@@ -118,6 +141,7 @@ func TestAccDataSourceHoneycombioQuery_validationChecks(t *testing.T) {
 			testStepsQueryValidationChecks_filter,
 			testStepsQueryValidationChecks_limit(),
 			testStepsQueryValidationChecks_time,
+			testStepsQueryValidationChecks_having,
 		),
 	})
 }
@@ -205,15 +229,15 @@ data "honeycombio_query_specification" "test" {
 	return []resource.TestStep{
 		{
 			Config:      fmt.Sprintf(queryLimitFmt, 0),
-			ExpectError: regexp.MustCompile("expected limit to be in the range \\(1 - 1000\\)"),
+			ExpectError: regexp.MustCompile(`expected limit to be in the range \(1 - 1000\)`),
 		},
 		{
 			Config:      fmt.Sprintf(queryLimitFmt, -5),
-			ExpectError: regexp.MustCompile("expected limit to be in the range \\(1 - 1000\\)"),
+			ExpectError: regexp.MustCompile(`expected limit to be in the range \(1 - 1000\)`),
 		},
 		{
 			Config:      fmt.Sprintf(queryLimitFmt, 1200),
-			ExpectError: regexp.MustCompile("expected limit to be in the range \\(1 - 1000\\)"),
+			ExpectError: regexp.MustCompile(`expected limit to be in the range \(1 - 1000\)`),
 		},
 	}
 }
@@ -249,6 +273,47 @@ data "honeycombio_query_specification" "test" {
 	},
 }
 
+var testStepsQueryValidationChecks_having = []resource.TestStep{
+	{
+		Config: `
+data "honeycombio_query_specification" "test" {
+  having {
+    calculate_op = "COUNT"
+    column       = "we-should-not-specify-a-column-with-COUNT"
+    op           = ">"
+    value        = 1
+  }
+}
+`,
+		ExpectError: regexp.MustCompile("calculate_op COUNT should not have an accompanying column"),
+	},
+	{
+		Config: `
+data "honeycombio_query_specification" "test" {
+  having {
+    calculate_op = "P99"
+    op           = ">="
+    value        = 1000
+  }
+}
+`,
+		ExpectError: regexp.MustCompile("calculate_op P99 requires a column"),
+	},
+	{
+		Config: `
+data "honeycombio_query_specification" "test" {
+  having {
+    calculate_op = "P95"
+    op           = ">"
+    column       = "duration_ms"
+    value        = "850"
+  }
+}
+`,
+		ExpectError: regexp.MustCompile("having 0 without matching column in query"),
+	},
+}
+
 func appendAllTestSteps(steps ...[]resource.TestStep) []resource.TestStep {
 	var allSteps []resource.TestStep
 	for _, s := range steps {
diff --git a/honeycombio/resource_column_test.go b/honeycombio/resource_column_test.go
index 9b4a290..dcead16 100644
--- a/honeycombio/resource_column_test.go
+++ b/honeycombio/resource_column_test.go
@@ -58,7 +58,7 @@ func TestAccHoneycombioColumn_validationErrors(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config:      testAccColumnConfigWithType(dataset, `String`),
-				ExpectError: regexp.MustCompile("expected type to be one of \\[string float integer boolean\\], got String"),
+				ExpectError: regexp.MustCompile(`expected type to be one of \[string float integer boolean\], got String`),
 			},
 		},
 	})