better timegroup-macro handling for sub-second intervals

[gabor]

(fixes #257)

the $__timeGroup(col, '5s') macro creates roughly this: TIMESTAMP_SECONDS(DIV(UNIX_SECONDS(col), 5) * 5)

this works fine, but if the interval is smaller than 1second, for example 50 milliseconds, it becomes this:
TIMESTAMP_SECONDS(DIV(UNIX_SECONDS(col), 0.05) * 0.05).

the problem is the DIV(.... , 0.05) part. it does not work with non-integer values.

the solution is to switch from seconds to milliseconds. it's what this PR does. (thanks @MatinF for looking into this!)

[zoltanbedi]

lgtm

[yesoreyeram]

won't this break existing queries? ( If I understand correctly, this new one return milliseconds instead of seconds?? 🤔 )

[MatinF]

I guess it is a valid point that if users are today parsing it as below, it would be a problem to update this directly:

  $__timeGroup(t,$__interval) 

Basically, users would then have to shift to $__interval_ms for it to work properly.

A softer transition may be to create a new macro and explain it in the documentation, e.g. call it as below:

  $__timeGroupMs(t,$__interval_ms)  

It is of course more clean to just have one option, but it will be an abrupt transition for existing users.

[gabor]

@yesoreyeram thanks for raising this point.. but, after some more thinking, it should be still fine. the reason is this:
let's say $__interval is 2s (2 seconds).

• in the current (seconds-based approach), this will become: TIMESTAMP_SECONDS(DIV(UNIX_SECONDS(col), 2) * 2), because the code:
  • parses 2s into a time.Duration:

    google-bigquery-datasource/pkg/bigquery/macros.go

    Line 42 in 5d94894

    interval, err := gtime.ParseInterval(intervalVar)

  • then calls .Seconds() on the duration:

    google-bigquery-datasource/pkg/bigquery/macros.go

    Line 49 in 5d94894

    return fmt.Sprintf("TIMESTAMP_SECONDS(DIV(UNIX_SECONDS(%s), %v) * %v)", timeVar, interval.Seconds(), interval.Seconds()), nil

• in the PR, this will become: TIMESTAMP_MILLIS(DIV(UNIX_MILLIS(f1), 2000) * 2000), because the code:
  • parses 2s into a time.Duration (no change here)
  • then calls.Milliseconds() on the time.Duration:

    google-bigquery-datasource/pkg/bigquery/macros.go

    Line 49 in 3e80cfe

    return fmt.Sprintf("TIMESTAMP_MILLIS(DIV(UNIX_MILLIS(%s), %v) * %v)", timeVar, interval.Milliseconds(), interval.Milliseconds()), nil

so i think it should be ok. WDYT?

[yesoreyeram]

Makes sense Gabor. Thanks for claryfying.