PromQL: Fix string and parentheses handling in engine (#6612)

* WIP: PromQL: Allow engine to return strings

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

* Add test suggested by @roidelapluie

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

* Fix typo in React UI

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

* Fix parenthesis handling for functions and aggregator params

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

* Add more tests

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

* Fix React UI test

Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>

promql/engine.go

@@ -535,6 +535,7 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *EvalStmt) (
 			defaultEvalInterval: GetDefaultEvaluationInterval(),
 			logger:              ng.logger,
 		}
+
 		val, err := evaluator.Eval(s.Expr)
 		if err != nil {
 			return nil, warnings, err
@@ -542,10 +543,17 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *EvalStmt) (
 
 		evalSpanTimer.Finish()
 
-		mat, ok := val.(Matrix)
+		var mat Matrix
-		if !ok {
+
+		switch result := val.(type) {
+		case Matrix:
+			mat = result
+		case String:
+			return result, warnings, nil
+		default:
 			panic(errors.Errorf("promql.Engine.exec: invalid expression type %q", val.Type()))
 		}
+
 		query.matrix = mat
 		switch s.Expr.Type() {
 		case ValueTypeVector:
@@ -1020,6 +1028,7 @@ func (ev *evaluator) eval(expr Expr) Value {
 
 	switch e := expr.(type) {
 	case *AggregateExpr:
+		unwrapParenExpr(&e.Param)
 		if s, ok := e.Param.(*StringLiteral); ok {
 			return ev.rangeEval(func(v []Value, enh *EvalNodeHelper) Vector {
 				return ev.aggregation(e.Op, e.Grouping, e.Without, s.Val, v[0].(Vector), enh)
@@ -1049,7 +1058,9 @@ func (ev *evaluator) eval(expr Expr) Value {
 		// Check if the function has a matrix argument.
 		var matrixArgIndex int
 		var matrixArg bool
-		for i, a := range e.Args {
+		for i := range e.Args {
+			unwrapParenExpr(&e.Args[i])
+			a := e.Args[i]
 			if _, ok := a.(*MatrixSelector); ok {
 				matrixArgIndex = i
 				matrixArg = true
@@ -1324,6 +1335,8 @@ func (ev *evaluator) eval(expr Expr) Value {
 		res := newEv.eval(e.Expr)
 		ev.currentSamples = newEv.currentSamples
 		return res
+	case *StringLiteral:
+		return String{V: e.Val, T: ev.startTimestamp}
 	}
 
 	panic(errors.Errorf("unhandled expression of type: %T", expr))
@@ -2100,3 +2113,14 @@ func documentedType(t ValueType) string {
 func formatDate(t time.Time) string {
 	return t.UTC().Format("2006-01-02T15:04:05.000Z07:00")
 }
+
+// unwrapParenExpr does the AST equivalent of removing parentheses around a expression.
+func unwrapParenExpr(e *Expr) {
+	for {
+		if p, ok := (*e).(*ParenExpr); ok {
+			*e = p.Expr
+		} else {
+			break
+		}
+	}
+}

promql/testdata/aggregators.test

@@ -17,6 +17,10 @@ eval instant at 50m SUM BY (group) (http_requests{job="api-server"})
   {group="canary"} 700
   {group="production"} 300
 
+eval instant at 50m SUM BY (group) (((http_requests{job="api-server"})))
+  {group="canary"} 700
+  {group="production"} 300
+
 # Test alternative "by"-clause order.
 eval instant at 50m sum by (group) (http_requests{job="api-server"})
   {group="canary"} 700
@@ -160,6 +164,11 @@ eval_ordered instant at 50m topk(3, http_requests)
 	http_requests{group="canary", instance="0", job="app-server"} 700
 	http_requests{group="production", instance="1", job="app-server"} 600
 
+eval_ordered instant at 50m topk((3), (http_requests))
+	http_requests{group="canary", instance="1", job="app-server"} 800
+	http_requests{group="canary", instance="0", job="app-server"} 700
+	http_requests{group="production", instance="1", job="app-server"} 600
+
 eval_ordered instant at 50m topk(5, http_requests{group="canary",job="app-server"})
 	http_requests{group="canary", instance="1", job="app-server"} 800
 	http_requests{group="canary", instance="0", job="app-server"} 700
@@ -233,6 +242,13 @@ eval instant at 5m count_values("version", version)
 	{version="7"} 2
 	{version="8"} 2
 
+
+eval instant at 5m count_values(((("version"))), version)
+       {version="6"} 5
+       {version="7"} 2
+       {version="8"} 2
+
+
 eval instant at 5m count_values without (instance)("version", version)
 	{job="api-server", group="production", version="6"} 3
 	{job="api-server", group="canary", version="8"} 2
@@ -276,3 +292,9 @@ eval instant at 1m quantile without(point)(scalar(foo), data)
 	{test="two samples"} 0.8
 	{test="three samples"} 1.6
 	{test="uneven samples"} 2.8
+
+
+eval instant at 1m quantile without(point)((scalar(foo)), data)
+	{test="two samples"} 0.8
+	{test="three samples"} 1.6
+	{test="uneven samples"} 2.8

promql/testdata/functions.test

@@ -48,6 +48,11 @@ eval instant at 50m changes(http_requests[50m])
 	{path="/bar"} 9
 	{path="/biz"} 1
 
+eval instant at 50m changes((http_requests[50m]))
+	{path="/foo"} 8
+	{path="/bar"} 9
+	{path="/biz"} 1
+
 eval instant at 50m changes(nonexistent_metric[50m])
 
 clear
@@ -207,6 +212,10 @@ eval instant at 0m label_replace(testmetric, "dst", "value-$1", "src", "non-matc
   testmetric{src="source-value-10",dst="original-destination-value"} 0
   testmetric{src="source-value-20",dst="original-destination-value"} 1
 
+eval instant at 0m label_replace((((testmetric))), (("dst")), (("value-$1")), (("src")), (("non-matching-regex")))
+  testmetric{src="source-value-10",dst="original-destination-value"} 0
+  testmetric{src="source-value-20",dst="original-destination-value"} 1
+
 # label_replace drops labels that are set to empty values.
 eval instant at 0m label_replace(testmetric, "dst", "", "dst", ".*")
   testmetric{src="source-value-10"} 0
@@ -236,6 +245,9 @@ eval instant at 5s timestamp(metric)
 eval instant at 10s timestamp(metric)
   {} 10
 
+eval instant at 10s timestamp(((metric)))
+  {} 10
+
 # Tests for label_join.
 load 5m
   testmetric{src="a",src1="b",src2="c",dst="original-destination-value"} 0
@@ -308,6 +320,11 @@ eval instant at 0m clamp_max(clamp_min(test_clamp, -20), 70)
 	{src="clamp-b"}	0
 	{src="clamp-c"}	70
 
+eval instant at 0m clamp_max((clamp_min(test_clamp, (-20))), (70))
+	{src="clamp-a"}	-20
+	{src="clamp-b"}	0
+	{src="clamp-c"}	70
+
 
 # Tests for sort/sort_desc.
 clear
@@ -393,6 +410,9 @@ eval instant at 1m stdvar_over_time(metric[1m])
 eval instant at 1m stddev_over_time(metric[1m])
   {} 3.249615
 
+eval instant at 1m stddev_over_time((metric[1m]))
+  {} 3.249615
+
 # Tests for stddev_over_time and stdvar_over_time #4927.
 clear
 load 10s
@@ -447,6 +467,11 @@ eval instant at 1m quantile_over_time(2, data[1m])
 	{test="three samples"} +Inf
 	{test="uneven samples"} +Inf
 
+eval instant at 1m (quantile_over_time(2, (data[1m])))
+	{test="two samples"} +Inf
+	{test="three samples"} +Inf
+	{test="uneven samples"} +Inf
+
 clear
 
 # Test time-related functions.

promql/testdata/literals.test

@@ -46,6 +46,9 @@ eval instant at 50m 2.
 eval instant at 50m 1 / 0
 	+Inf
 
+eval instant at 50m ((1) / (0))
+	+Inf
+
 eval instant at 50m -1 / 0
 	-Inf
 

promql/testdata/operators.test

@@ -93,6 +93,9 @@ eval instant at 50m SUM(http_requests) BY (job) + SUM(http_requests) BY (job)
 	{job="api-server"} 2000
 	{job="app-server"} 5200
 
+eval instant at 50m (SUM((http_requests)) BY (job)) + SUM(http_requests) BY (job)
+	{job="api-server"} 2000
+	{job="app-server"} 5200
 
 eval instant at 50m http_requests{job="api-server", group="canary"}
 	http_requests{group="canary", instance="0", job="api-server"} 300
@@ -112,6 +115,16 @@ eval instant at 50m rate(http_requests[25m]) * 25 * 60
   {group="production", instance="1", job="api-server"} 100
   {group="production", instance="1", job="app-server"} 300
 
+eval instant at 50m (rate((http_requests[25m])) * 25) * 60
+  {group="canary", instance="0", job="api-server"} 150
+  {group="canary", instance="0", job="app-server"} 350
+  {group="canary", instance="1", job="api-server"} 200
+  {group="canary", instance="1", job="app-server"} 400
+  {group="production", instance="0", job="api-server"} 50
+  {group="production", instance="0", job="app-server"} 249.99999999999997
+  {group="production", instance="1", job="api-server"} 100
+  {group="production", instance="1", job="app-server"} 300
+
 
 eval instant at 50m http_requests{group="canary"} and http_requests{instance="0"}
 	http_requests{group="canary", instance="0", job="api-server"} 300

promql/testdata/subquery.test

@@ -73,6 +73,9 @@ eval instant at 1010s sum_over_time(metric1[30s:10s] offset 5s)
 eval instant at 1010s sum_over_time(metric1[30s:10s] offset 3s)
   {} 297
 
+eval instant at 1010s sum_over_time((metric1)[30s:10s] offset 3s)
+  {} 297
+
 # Nested subqueries
 eval instant at 1000s rate(sum_over_time(metric1[30s:10s])[50s:10s])
   {} 0.4
@@ -110,4 +113,5 @@ eval instant at 20s min_over_time(metric[10s:10s])
   {} 1
 
 eval instant at 20m min_over_time(rate(metric[5m])[20m:1m])
-  {} 0.12119047619047618
+  {} 0.12119047619047618
+

web/ui/react-app/src/pages/graph/DataTable.test.tsx

@@ -261,7 +261,7 @@ describe('DataTable', () => {
     it('renders a string row', () => {
       const table = dataTable.find(Table);
       const rows = table.find('tr');
-      expect(rows.text()).toEqual('scalart');
+      expect(rows.text()).toEqual('stringt');
     });
   });
 });

web/ui/react-app/src/pages/graph/DataTable.tsx

@@ -103,7 +103,7 @@ const DataTable: FC<QueryResult> = ({ data }) => {
     case 'string':
       rows.push(
         <tr key="0">
-          <td>scalar</td>
+          <td>string</td>
           <td>{data.result[1]}</td>
         </tr>
       );