Uh oh!
There was an error while loading. Please reload this page.
Add backend expansion for Grafana $__interval and $__interval_ms macros - #122
Conversation
JackieTien97
left a comment
There was a problem hiding this comment.
There are two blocking correctness and compatibility issues in the current interval macro implementation.
| } | ||
| if hasIntervalMS { | ||
| scaled, err := scaleIntervalMS(intervalMS, timestampPrecision) |
There was a problem hiding this comment.
[P1] Preserve Grafana's millisecond contract for $__interval_ms. Grafana defines this macro as the query interval in milliseconds, independent of the data source's timestamp precision; the Grafana SDK likewise expands it directly from query.Interval.Milliseconds(). Scaling it here changes a 120-second interval from 120000 to 120000000/120000000000 on us/ns servers and can silently change query results. Please expand it directly with strconv.FormatInt(intervalMS, 10) and update the precision-scaling tests accordingly. If raw IoTDB timestamp units are needed, that should be a separately named, plugin-specific macro rather than changing the standard Grafana macro.
| } | ||
| timestampPrecision := "ms" | ||
| if hasStandaloneMacro(qp.Sql, intervalMSRe) { | ||
| timestampPrecision, err = readTimestampPrecision(session, &timeout) |
There was a problem hiding this comment.
[P1] Avoid issuing privileged metadata SQL from the normal query path. In IoTDB's table dialect, SHOW VARIABLES requires the global SYSTEM privilege, so a normal Grafana service account with only database/table read access will fail before its actual query runs. The standard $__interval_ms macro does not need server precision and this lookup should be removed. If timestamp precision is still needed for a separate raw-server-unit feature before the Go client exposes the precision already returned by the session handshake, please add an explicit validated datasource setting (ms/us/ns) as a temporary solution instead of executing SHOW VARIABLES for every query.
0bd0b0f to
1199c74CompareUh oh!
There was an error while loading. Please reload this page.
This pull request adds support for Grafana's
$__intervaland$__interval_msmacros in IoTDB table-model SQL queries. It expands both macros using Grafana's runtime query interval and adds input validation, identifier-safe macro replacement, and unit tests for the new behavior.Interval Macro Expansion
$__intervalto an IoTDB fixed-width duration literal usingms,s,m,h,d, orw.$__interval_msdirectly to the query interval in milliseconds, following Grafana's standard macro semantics.backend.DataQuery.Intervalas the runtime interval source.Macro Matching
$__intervaland$__interval_msindependently when both appear in the same SQL statement.$__intervalishand$__interval_ms_extra.Query Parameter Handling
IntervalMSfield to the table query model.IntervalMSfrombackend.DataQuery.Intervalafter JSON unmarshalling.Error Handling
Tests
Unit tests cover:
$__intervaland$__interval_ms$__interval_ms