EURODEO / e-soh

Monorepo for the 3 connected services of e-soh
4 stars 0 forks source link

[DATASTORE] Support the value 0 for reflectable int64 metadata #177

Closed jo-asplin-met-no closed 1 day ago

jo-asplin-met-no commented 2 days ago

If you use PutObservations to insert observations for time series associated with levels -1, 0, and 1, then the level field will not appear in the response for the time series level 0. This is essentially related to how the reflect package in Go considers default values as unset even if you explicitly set it (using reflect.Value.SetInt()). One way to fix this is to represent the metadata field as *int instead of int (i.e. effectively getting a "nullable" type). Another approach is to handle default type values as a special case, e.g. by making use of reflect.Value.IsZero() etc.

How to reproduce:

First insert observations for the three time series (level -1, 0, and 1):

grpcurl -d '{"observations": [{"ts_mdata": {"timeseries_id": "timeseries_id_dummy", "version": "version_dummy", "type": "type_dummy", "standard_name": "wind_speed", "unit": "celsius", "level": -1}, "obs_mdata": {"id": "id_dummy", "geo_point": {"lat": 59.91, "lon": 10.75}, "pubtime": "2023-01-01T00:00:10Z", "data_id": "data_id_dummy", "obstime_instant": "2023-01-01T00:00:00Z", "value": "123.456"}}]}' -plaintext -proto protobuf/datastore.proto 127.0.0.1:50050 datastore.Datastore.PutObservations

grpcurl -d '{"observations": [{"ts_mdata": {"timeseries_id": "timeseries_id_dummy", "version": "version_dummy", "type": "type_dummy", "standard_name": "wind_speed", "unit": "celsius", "level": 0}, "obs_mdata": {"id": "id_dummy", "geo_point": {"lat": 59.91, "lon": 10.75}, "pubtime": "2023-01-01T00:00:10Z", "data_id": "data_id_dummy", "obstime_instant": "2023-01-01T00:00:00Z", "value": "123.456"}}]}' -plaintext -proto protobuf/datastore.proto 127.0.0.1:50050 datastore.Datastore.PutObservations

grpcurl -d '{"observations": [{"ts_mdata": {"timeseries_id": "timeseries_id_dummy", "version": "version_dummy", "type": "type_dummy", "standard_name": "wind_speed", "unit": "celsius", "level": 1}, "obs_mdata": {"id": "id_dummy", "geo_point": {"lat": 59.91, "lon": 10.75}, "pubtime": "2023-01-01T00:00:10Z", "data_id": "data_id_dummy", "obstime_instant": "2023-01-01T00:00:00Z", "value": "123.456"}}]}' -plaintext -proto protobuf/datastore.proto 127.0.0.1:50050 datastore.Datastore.PutObservations

Then try to get data for levels -1, 0, and 1, and see that we only get data for -1 and 1:

$ grpcurl -d '{"filter": {"standard_name": {"values": ["wind_speed"]}, "level": {"values": ["-1/1"]}}}' -plaintext -proto protobuf/datastore.proto 127.0.0.1:50050 datastore.Datastore.GetObservations | grep \"level\"
        "level": "-1",
        "level": "1",

And indeed that we get nothing at all when asking for just level 0:

$ grpcurl -d '{"filter": {"standard_name": {"values": ["wind_speed"]}, "level": {"values": ["0"]}}}' -plaintext -proto protobuf/datastore.proto 127.0.0.1:50050 datastore.Datastore.GetObservations | grep \"level\"
        "level": "-1",
        "level": "1",

Also observe that in the database the level 0 is there as expected:

$ PGPASSWORD=mysecretpassword psql -h localhost -p 5433 -U postgres -d data -c 'select level from time_series'
 level 
-------
     0
     1
    -1
(3 rows)
jo-asplin-met-no commented 1 day ago

As long as all int64 metadata fields are required, we don't need to worry about this. In particular, a client using GetObservations() doesn't need to distinguish between unset and explicitly set to zero value. Once we add an optional int64 field, we need to support this distinction.