add structs to mv query cache key (#6818)

* add structs to mv query cache key

* test refactor

* better test

* comment

* time decode hook

* Update runtime/metricsview/query.go

Co-authored-by: Benjamin Egelund-Müller <[email protected]>

* use inbuilt string to time decode hook

---------

Co-authored-by: Benjamin Egelund-Müller <[email protected]>

Author: pjain1

runtime/metricsview/query.go

@@ -2,8 +2,10 @@ package metricsview
 
 import (
 	"fmt"
+	"reflect"
 	"time"
 
+	"github.com/mitchellh/mapstructure"
 	runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
 	"github.com/rilldata/rill/runtime/pkg/timeutil"
 )
@@ -54,6 +56,33 @@ type MeasureCompute struct {
 	URI             *MeasureComputeURI             `mapstructure:"uri"`
 }
 
+func (q *Query) AsMap() (map[string]any, error) {
+	queryMap := make(map[string]any)
+	decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
+		Result: &queryMap,
+		DecodeHook: func(from reflect.Type, to reflect.Type, data any) (any, error) {
+			if from == reflect.TypeOf(&time.Time{}) {
+				t, ok := data.(*time.Time)
+				if !ok {
+					return nil, fmt.Errorf("expected *time.Time, got %T", data)
+				}
+				return map[string]any{
+					"t": t.Format(time.RFC3339Nano),
+				}, nil
+			}
+			return data, nil
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = decoder.Decode(q)
+	if err != nil {
+		return nil, err
+	}
+	return queryMap, nil
+}
+
 func (m *MeasureCompute) Validate() error {
 	n := 0
 	if m.Count {

runtime/pkg/mapstructureutil/mapstructureutil.go

@@ -1,7 +1,6 @@
 package mapstructureutil
 
 import (
-	"reflect"
 	"time"
 
 	"github.com/mitchellh/mapstructure"
@@ -10,7 +9,7 @@ import (
 // WeakDecode is similar to mapstructure.WeakDecode, but it also supports decoding RFC3339Nano-formatted timestamp strings to time.Time.
 func WeakDecode(input, output any) error {
 	config := &mapstructure.DecoderConfig{
-		DecodeHook:       stringToTimeHook,
+		DecodeHook:       mapstructure.StringToTimeHookFunc(time.RFC3339Nano),
 		Metadata:         nil,
 		Result:           output,
 		WeaklyTypedInput: true,
@@ -23,11 +22,3 @@ func WeakDecode(input, output any) error {
 
 	return decoder.Decode(input)
 }
-
-func stringToTimeHook(from, to reflect.Type, data any) (any, error) {
-	if to == reflect.TypeOf(time.Time{}) && from == reflect.TypeOf("") {
-		return time.Parse(time.RFC3339Nano, data.(string))
-	}
-
-	return data, nil
-}

runtime/resolvers/metrics.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"time"
 
-	"github.com/mitchellh/mapstructure"
 	runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
 	"github.com/rilldata/rill/runtime"
 	"github.com/rilldata/rill/runtime/metricsview"
@@ -100,15 +99,15 @@ func (r *metricsResolver) CacheKey(ctx context.Context) ([]byte, bool, error) {
 		return nil, false, nil
 	}
 
-	queryMap := make(map[string]any)
-	err = mapstructure.Decode(r.query, &queryMap)
+	queryMap, err := r.query.AsMap()
 	if err != nil {
 		return nil, false, err
 	}
 
 	queryMap["mv_cache_key"] = key
-	bytes, err := json.Marshal(queryMap)
-	return bytes, true, err
+
+	b, err := json.Marshal(queryMap)
+	return b, true, err
 }
 
 func (r *metricsResolver) Refs() []*runtimev1.ResourceName {

runtime/resolvers/testdata/metrics_comparisons.yaml

@@ -147,6 +147,34 @@ tests:
       - sum: 4
         sum_prev: 2
         time__day: "2024-01-04T00:00:00Z"
+  - name: time_as_dimension_check_cache_duckdb
+    resolver: metrics
+    properties:
+      metrics_view: duckdb_metrics
+      dimensions:
+        - name: time__day
+          compute:
+            time_floor:
+              dimension: time
+              grain: day
+      measures:
+        - name: sum
+        - name: sum_prev
+          compute:
+            comparison_value:
+              measure: sum
+      time_range:
+        end: 2024-01-04T00:00:00Z
+        start: 2024-01-03T00:00:00Z
+      comparison_time_range:
+        end: 2024-01-02T00:00:00Z
+        start: 2024-01-01T00:00:00Z
+      sort:
+        - name: time__day
+    result:
+      - sum: 3
+        sum_prev: 1
+        time__day: "2024-01-03T00:00:00Z"
   - name: iso_ranges_duckdb
     resolver: metrics
     properties:
@@ -167,9 +195,9 @@ tests:
       sort:
         - name: sum
     result:
-      - sum: 7
+      - country: US
+        sum: 7
         sum_prev: 2
-        country: "US"
   - name: iso_ranges_clickhouse
     resolver: metrics
     properties:
@@ -190,9 +218,9 @@ tests:
       sort:
         - name: sum
     result:
-      - sum: 7
+      - country: US
+        sum: 7
         sum_prev: 2
-        country: "US"
   - name: rill_time_ranges_duckdb
     resolver: metrics
     properties:
@@ -212,12 +240,12 @@ tests:
       sort:
         - name: sum
     result:
-      - sum: 5
+      - country: DK
+        sum: 5
         sum_prev: 1
-        country: "DK"
-      - sum: 7
+      - country: US
+        sum: 7
         sum_prev: 2
-        country: "US"
   - name: rill_time_ranges_clickhouse
     resolver: metrics
     properties:
@@ -237,9 +265,9 @@ tests:
       sort:
         - name: sum
     result:
-      - sum: 5
+      - country: DK
+        sum: 5
         sum_prev: 1
-        country: "DK"
-      - sum: 7
+      - country: US
+        sum: 7
         sum_prev: 2
-        country: "US"