DataFrame: Handle nanosecond-precision timestamp fields #64529
Conversation
| @@ -136,6 +136,7 @@ export interface Field<T = any, V = Vector<T>> { | |||
| */ | |||
| config: FieldConfig; | |||
| values: V; // The raw field values | |||
| additionalValues?: Record<string, V>; | |||
There was a problem hiding this comment.
Given that this is a pattern we know applies to time, and struggle to think of good cases for other things should we do something liket:
| additionalValues?: Record<string, V>; | |
| /** When type === FieldType.time, we can support nanosecond time bla bla bla... 8? | |
| nanos?: number[]; |
There was a problem hiding this comment.
can do 👍 .. just to doublecheck, we can go directly to a javascript-array, no need to have an ArrayVector ?
gabor
force-pushed
the
gabor/nanosec-time
branch
from
04e3028
to
472c038
Compare
|
(Open the links below in a new tab to go to the correct steps)
|
grafanabot
added
the
levitate breaking change
gabor
force-pushed
the
gabor/nanosec-time
branch
from
472c038
to
36e1f0a
Compare
gabor
requested review from
tskarhed,
JoaoSilvaGrafana and
andresmgot
and removed request for
a team
| // TODO: expand arrays further using bases,factors | ||
|
|
||
| return { | ||
| const dataFrameField: Field & { entities: FieldValueEntityLookup } = { | ||
| ...f, | ||
| type: type ?? guessFieldType(f.name, buffer), | ||
| config: f.config ?? {}, | ||
| values: new ArrayVector(buffer), | ||
| // the presence of this prop is an optimization signal & lookup for consumers | ||
| entities: entities ?? {}, |
There was a problem hiding this comment.
nit: since you are not using nanos elsewhere it can be defined here inline: nanos: data?.nanos?.[index],
There was a problem hiding this comment.
yes, i agree. the question is this... the nanos field will always be there that way, but it's value will be undefined. in other words, with current code:
{ a: 1, b: 2}inline nanos code:
{ a: 1, b: 2, nanos: undefined }what i do not know, do we care about such details of not having a field "be there but value will be undefined"?
| delete (sfield as any).entities; | ||
| data.values.push(values.toArray()); | ||
|
|
||
| if (nanos != null) { |
There was a problem hiding this comment.
the type of nanos is number[] | undefined, either the type or this condition is wrong
There was a problem hiding this comment.
!= null covers both null and undefined with fewer chars :)
There was a problem hiding this comment.
TIL javascript magic
> undefined != null
false
> undefined === null
false
> undefined == null
true
Anyway, it's just a nit but if nanos can be null I would define the type as nanos?: number[] | null
There was a problem hiding this comment.
sorry @andresmgot , it's an annoying trick (the != null) and i in general really dislike such tricks, but this one is so good at basically isNullish(value) 😿
There was a problem hiding this comment.
Anyway, it's just a nit but if nanos can be null I would define the type as nanos?: number[] | null
it's a little tricky, but: field.nanos can never be null.
(dataframejson.nanos is a different thing, and that one is an array-of-arrays-or-nulls 😄 , like: [null, null, [1,2,3,4], null])
and the way we extract a value from that strange array causes the null to appear in our life, and that's why we need to check for both null and undefined later.
gabor
changed the title
gabor
changed the title
gabor
added
add to changelog
no-backport
requires grafana/grafana-plugin-sdk-go#647
nanosnanosfield(NOTE: an example usage is in #64687)