Repeating panel queries don't handle the "All" selection correctly

[delroth]

Describe the bug

When using a multi-valued dashboard variable and a repeating panel, the wrong value for the variable is substituted in each instance of the panel when the "All" value is selected. Or, more precisely, no value is substituted and the condition using the repeating variable is just dropped.

I suspect this isn't a bug with this particular datasource, maybe instead a bug with the sqlds library or somewhere else in the stack. I unfortunately don't know enough about Grafana to really track it down to the right place.

Versions

Grafana v8.4.1 Plugin built from revision fd5cdcf5c054fe8eef40915dae1145cfdbc7a0a7 aka. current latest.

Query

Dashboard showing this issue: https://mon.dolphin-emu.org/d/rvvg02qGk/analytics-quirks-report

Base query set for the repeating panel:

SELECT
  gameid AS `Game ID`,
  COUNT(*) AS `Count`
FROM default.event
WHERE
  type = 'quirk' AND
  quirk = '$quirk' AND
  gameid IS NOT NULL AND
  $__timeFilter(ts)
GROUP BY gameid
ORDER BY `Count` DESC

When selecting one or multiple value, but not "All", a sample substituted query (correct, note that it has the AND quirk = ...):

SELECT gameid AS `Game ID` , COUNT(*) AS `Count` FROM default.event WHERE type = 'quirk' AND quirk = 'uses-DVDLowOffset' AND gameid IS NOT NULL AND ts >= '1645024345' AND ts <= '1645629145' GROUP BY gameid ORDER BY `Count` DESC

When selecting the "All" value, every single panel has the same substituted query (broken, it doesn't have any filter on quirk, should be same as the previous case):

SELECT gameid AS `Game ID` , COUNT(*) AS `Count` FROM default.event WHERE gameid IS NOT NULL AND ts >= '1645024431' AND ts <= '1645629231' GROUP BY gameid ORDER BY `Count` DESC

To Reproduce

Steps to reproduce the behavior:

1. Add a multi-valued variable to a dashboard.
2. Create a repeating panel using the variable as a query filter.

Expected behavior

Every panel properly substitutes the right value for the repetition variable in the "All" selection case, instead of stripping out the filter.

Screenshots

Dashboard link (same as linked above): https://mon.dolphin-emu.org/d/rvvg02qGk/analytics-quirks-report

Additional context

n/a

[scottlepp]

@delroth #88 should fix this.

NOTE: If you select ALL, it will optimize your query and just remove that line from your where clause, since it's not really filtering anything.

Also, when using multi value variables you'll want to use IN

SELECT
  gameid AS `Game ID`,
  COUNT(*) AS `Count`
FROM default.event
WHERE
  type = 'quirk' AND
  quirk IN ($quirk) AND
  gameid IS NOT NULL AND
  $__timeFilter(ts)
GROUP BY gameid
ORDER BY `Count` DESC

[delroth]

Updated to 0.12.2 and unfortunately the bug seems to still occur. Any chance you could reopen this?

NOTE: If you select ALL, it will optimize your query and just remove that line from your where clause, since it's not really filtering anything.

This is incorrect behavior in the case of repeated panels though. The way I understand it (and the way it works with other data sources, as far as I can tell), using a multi-valued variable with repeated panels should have each panel "see" only a single value for the variable.

To give an example, let's say there is a multi-valued variable with values "a", "b", "c". If I select "All" and have a repeated panel setup, I should have 3 panels each being filtered to the single values "a", "b" and "c" respectively. Instead, the ClickHouse data source right now shows 3 panels but each with no filter (the 3 queries are thus identical), which is not how repeated panels should operate. Note that selecting several values does work with the ClickHouse data source right now, and does the right thing (e.g. if I select "a" + "b" I do get 2 panels, one filtering for "a", one filtering for "b"), it's just "All" that doesn't work.

Similarly, your previous comment on using "IN" instead of "WHERE" shouldn't apply to repeated panels. When repeating panels, there is no way the query should see more than one value for the variable, because each value maps to a different instance/repeat of the panel.

Happy to clarify further with some examples from other data sources in case my description isn't clear enough and/or you disagree with what I described as the "correct" behavior!

[delroth]

My suspicion, again, knowing pretty much nothing about Grafana: CHDatasource.ts:61

rawQuery = removeConditionalAlls(rawQuery, templateSrv.getVariables());
...
rawSql: this.replace(rawQuery, scoped)

I think this templateSrv.getVariables might be looking at the variable value at the dashboard level, vs. scoped which would be the value at the panel level. This would match the symptoms: the variable interpolation looks at the scoped variables and so does the interpolation correctly for the repeated panels when a set of values are selected. But when "All" is selected at the dashboard level, removeConditionalAlls deletes the filter even though the scoped variable would have a different value from "All".

If this is true, I think the correct fix would be to have removeConditionalAlls act on scoped instead of templateSrv.getVariables(), or possibly a combination of both. WDYT?

[delroth]

To support this theory, I added some logging in the relevant code path. First logged value is scoped, second is templateSrv.getVariables:

For the repeated panels, even though the value in templateSrv.getVariables is $__all, the value in scoped is the "instantiated" value for the panel (in this case, mismatched-gpu-colors-between-xf-and-bp).

[delroth]

89 is my attempt at a fix, confirmed to work on my dashboard. I've also checked that a non-repeating panel still has its "All" filter stripped as intended.

[scottlepp]

Fixed by #89