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
1199c74
Compare
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