Aggregation comparison API #4793
Conversation
| if err != nil { | ||
| return fmt.Errorf("error building query: %w", err) | ||
| } | ||
| fmt.Println(sqlString, args) |
There was a problem hiding this comment.
Leftover from debugging?
proto/rill/runtime/v1/queries.proto
Outdated
| @@ -323,9 +323,11 @@ message MetricsViewAggregationRequest { | |||
| // Required | |||
| repeated MetricsViewAggregationMeasure measures = 4; | |||
| // Optional. Defaults to unsorted | |||
| repeated MetricsViewAggregationSort sort = 5; | |||
| repeated MetricsViewComparisonSort sort = 5; | |||
There was a problem hiding this comment.
Is there a reason you didn't add configuration for the comparison in the MetricsViewAggregationMeasure type as discussed here? https://www.notion.so/rilldata/Pivot-APIs-improvements-8ee3db49882e4093af85de68198f5984
What should the frontend put in measures to request a comparison value?
Also, if keeping this, we should rename the type to MetricsViewAggregationSort to align with the API.
There was a problem hiding this comment.
+1 to this. There is no way to add having filter on comparison otherwise. So might need to build and pass aliases to buildExpression for having clause.
Because this is missing, i get this error when i use comparison in having,
error building query: unknown column filter: total_records__delta_abs
Request object,
{
"metricsView": "AdBids_dimension_expression",
"measures": [{
"name": "total_records"
}],
"dimensions": [{
"name": "publisher"
}],
"having": {
"cond": {
"op": "OPERATION_AND",
"exprs": [{
"cond": {
"op": "OPERATION_GT",
"exprs": [{
"ident": "total_records__delta_abs"
}, {
"val": 0
}]
}
}]
}
},
"timeRange": {
"isoDuration": "P7D"
},
"comparisonTimeRange": {
"isoDuration": "P7D",
"isoOffset": "rill-PW"
}
}
There was a problem hiding this comment.
yes, it's the next step to adapt it to the new proposal
| for _, sf := range q.Result.Schema.Fields { | ||
| fmt.Printf("%v ", sf.Name) | ||
| } | ||
| fmt.Printf("\n") | ||
|
|
||
| for i, row := range q.Result.Data { | ||
| for _, sf := range q.Result.Schema.Fields { | ||
| fmt.Printf("%v ", row.Fields[sf.Name].AsInterface()) | ||
| } | ||
| fmt.Printf(" %d \n", i) | ||
|
|
||
| } |
There was a problem hiding this comment.
Avoid prints if possible
| for _, sf := range q.Result.Schema.Fields { | ||
| fmt.Printf("%v ", sf.Name) | ||
| } | ||
| fmt.Printf("\n") | ||
|
|
||
| for i, row := range q.Result.Data { | ||
| for _, sf := range q.Result.Schema.Fields { | ||
| fmt.Printf("%v ", row.Fields[sf.Name].AsInterface()) | ||
| } | ||
| fmt.Printf(" %d \n", i) | ||
|
|
||
| } |
|
@egor-ryashin Can you fix the merge conflicts here? |
No description provided.