JackieTien97 commented on code in PR #122:
URL: https://github.com/apache/iotdb-extras/pull/122#discussion_r3763471834
##########
connectors/grafana-plugin/pkg/plugin/table_query.go:
##########
@@ -247,6 +376,19 @@ func (d *IoTDBDataSource) queryTableModel(ctx
context.Context, qp *queryParam) b
timeout = ms
}
}
+ timestampPrecision := "ms"
+ if hasStandaloneMacro(qp.Sql, intervalMSRe) {
+ timestampPrecision, err = readTimestampPrecision(session,
&timeout)
Review 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.
##########
connectors/grafana-plugin/pkg/plugin/table_query.go:
##########
@@ -84,16 +91,51 @@ func formatTimeLiteral(ms int64) string {
return time.UnixMilli(ms).UTC().Format("2006-01-02T15:04:05.000") +
"+00:00"
}
-// expandTableMacros rewrites the Grafana time macros a dashboard author can
put
-// in table-model SQL into concrete bounds for the panel's range:
+// expandTableMacros rewrites the Grafana time and interval macros a dashboard
+// author can put in table-model SQL. The precision-aware RPC path calls the
+// internal helper with the server's timestamp precision.
//
// $__timeFilter(col) -> (col >= <from> AND col <= <to>)
// $__timeFrom[()] -> <from>
// $__timeTo[()] -> <to>
+// $__interval -> a fixed-width IoTDB duration literal
+// $__interval_ms -> the interval in server timestamp units
//
// Bounds are ISO 8601 UTC timestamp literals, which IoTDB compares against
// TIMESTAMP columns independently of the server's timestamp precision.
-func expandTableMacros(sql string, startMs int64, endMs int64) string {
+func expandTableMacros(sql string, startMs int64, endMs int64, intervalMS
int64) (string, error) {
+ return expandTableMacrosWithPrecision(sql, startMs, endMs, intervalMS,
"ms")
+}
+
+// expandTableMacrosWithPrecision expands the two Grafana interval macros in
+// addition to the existing time macros. intervalMS is Grafana's runtime
+// suggested step in milliseconds; it is never read from Dashboard JSON.
+// timestampPrecision controls the unit of integer TIMESTAMP arithmetic used by
+// $__interval_ms and must be ms, us, or ns.
+func expandTableMacrosWithPrecision(sql string, startMs int64, endMs int64,
intervalMS int64, timestampPrecision string) (string, error) {
+ hasInterval := hasStandaloneMacro(sql, intervalRe)
+ hasIntervalMS := hasStandaloneMacro(sql, intervalMSRe)
+ if (hasInterval || hasIntervalMS) && intervalMS <= 0 {
+ // Defensive validation for direct callers. queryTableModel
performs the
+ // authoritative request-path check before acquiring an RPC
session.
+ return "", errors.New(invalidIntervalMacroMessage)
+ }
+
+ if hasIntervalMS {
+ scaled, err := scaleIntervalMS(intervalMS, timestampPrecision)
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
