From 9ecd562237b10370ab0e732b17d8a0f81864e9fc Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 11 Dec 2023 14:13:30 +0100 Subject: [PATCH 01/18] Implicitly handled string attributes using reflection --- datastore/datastore/protobuf/datastore.proto | 46 ++++++++-- .../postgresql/getobservations.go | 85 ++++++++++++++----- 2 files changed, 104 insertions(+), 27 deletions(-) diff --git a/datastore/datastore/protobuf/datastore.proto b/datastore/datastore/protobuf/datastore.proto index 4fe56f0..9784705 100644 --- a/datastore/datastore/protobuf/datastore.proto +++ b/datastore/datastore/protobuf/datastore.proto @@ -6,7 +6,8 @@ import "google/protobuf/timestamp.proto"; option go_package = "./datastore"; -// Notes: +// NOTES: +// // - A _time series_ is a context defined by a set of metadata (defined in TSMetadata below) that // usually does not vary with observaion (time). // @@ -140,13 +141,46 @@ message PutObsResponse { //--------------------------------------------------------------------------- message GetObsRequest { + // --- BEGIN special handling of temporal and spatial search ----------------- TimeInterval interval = 1; // only return observations in this time range Polygon inside = 2; // if specified, only return observations in this area - repeated string platforms = 3; // if specified, only return observations matching any of these platform patterns - repeated string standard_names = 4 [json_name = "standard_names"]; // if specified, only return observations matching any of these standard names - repeated string instruments = 5; // if specified, only return observations matching any of these instruments - repeated string processing_levels = 6 [json_name = "processing_levels"]; // if specified, only return observations matching any of these processing levels - // TODO: add search filters for other metadata + // --- END special handling of temporal and spatial search ----------------- + + // --- BEGIN general handling of strings; field names must correspond exactly with string field names in TSMetadata or ObsMetadata ----- + // - if the field F is specified (where F is for example 'platform'), only observations matching at least one these values for F will be returned + // - if the field F is not specified, filtering on F is effectively disabled + repeated string version = 3; + repeated string type = 4; + repeated string title = 5; + repeated string summary = 6; + repeated string keywords = 7; + repeated string keywords_vocabulary = 8 [json_name = "keywords_vocabulary"]; + repeated string license = 9; + repeated string conventions = 10; + repeated string naming_authority = 11 [json_name = "naming_authority"]; + repeated string creator_type = 12 [json_name = "creator_type"]; + repeated string creator_name = 13 [json_name = "creator_name"]; + repeated string creator_email = 14 [json_name = "creator_email"]; + repeated string creator_url = 15 [json_name = "creator_url"]; + repeated string institution = 16; + repeated string project = 17; + repeated string source = 18; + repeated string platform = 19; + repeated string platform_vocabulary = 20 [json_name = "platform_vocabulary"]; + repeated string standard_name = 21 [json_name = "standard_name"]; + repeated string unit = 22; + repeated string instrument = 23; + repeated string instrument_vocabulary = 24 [json_name = "instrument_vocabulary"]; + repeated string id = 25; + repeated string data_id = 26 [json_name = "data_id"]; + repeated string history = 27; + repeated string metadata_id = 28 [json_name = "metadata_id"]; + repeated string processing_level = 29 [json_name = "processing_level"]; + // --- END general handling of strings ----- + + // --- BEGIN special handling of 'repeated Link' ------ + // TODO + // --- END special handling of 'repeated Link' ------ } message GetObsResponse { diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 2e01a49..78a08a5 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -5,6 +5,7 @@ import ( "datastore/common" "datastore/datastore" "fmt" + "reflect" "strings" "time" @@ -144,13 +145,14 @@ func getTimeFilter(ti *datastore.TimeInterval) string { return timeExpr } -type filterInfo struct { +type stringFilterInfo struct { colName string - patterns []string // NOTE: only []string supported for now + patterns []string } +// TODO: add filter infos for other types than string -// getMdataFilter derives from filterInfos the expression used in a WHERE clause for "match any" -// filtering on a set of attributes. +// getMdataFilter derives from stringFilterInfos the expression used in a WHERE clause for +// "match any" filtering on a set of attributes. // // The expression will be of the form // @@ -163,13 +165,13 @@ type filterInfo struct { // Values to be used for query placeholders are appended to phVals. // // Returns expression. -func getMdataFilter(filterInfos []filterInfo, phVals *[]interface{}) string { +func getMdataFilter(stringFilterInfos []stringFilterInfo, phVals *[]interface{}) string { whereExprAND := []string{} - for _, fi := range filterInfos { + for _, sfi := range stringFilterInfos { addWhereCondMatchAnyPattern( - fi.colName, fi.patterns, &whereExprAND, phVals) + sfi.colName, sfi.patterns, &whereExprAND, phVals) } whereExpr := "TRUE" // by default, don't filter @@ -219,40 +221,81 @@ func getGeoFilter(inside *datastore.Polygon, phVals *[]interface{}) (string, err return whereExpr, nil } +type stringFieldInfo struct { + field reflect.StructField + method reflect.Value + methodName string +} + // getObs gets into obs all observations that match request. // Returns nil upon success, otherwise error. func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Metadata2) error { phVals := []interface{}{} // placeholder values - timeExpr := getTimeFilter(request.GetInterval()) + // --- BEGIN get temporal and spatial search expressions ---------------- - tsMdataExpr := getMdataFilter([]filterInfo{ - {"platform", request.GetPlatforms()}, - {"standard_name", request.GetStandardNames()}, - {"instrument", request.GetInstruments()}, - // TODO: add search filters for more columns in table 'time_series' - }, &phVals) - - obsMdataExpr := getMdataFilter([]filterInfo{ - {"processing_level", request.GetProcessingLevels()}, - // TODO: add search filters for more columns in table 'observation' - }, &phVals) + timeExpr := getTimeFilter(request.GetInterval()) geoExpr, err := getGeoFilter(request.Inside, &phVals) if err != nil { return fmt.Errorf("getGeoFilter() failed: %v", err) } + // --- END get temporal and spatial search expressions ---------------- + + // --- BEGIN get search expression for string attributes ---------------- + + rv := reflect.ValueOf(request) + + stringFilterInfos := []stringFilterInfo{} + + stringFieldInfos := []stringFieldInfo{} + + addStringFields := func(s interface{}) { + for _, field := range reflect.VisibleFields(reflect.TypeOf(s)) { + mtdName := fmt.Sprintf("Get%s", field.Name) + mtd := rv.MethodByName(mtdName) + if field.IsExported() && (field.Type.Kind() == reflect.String) && (mtd.IsValid()) { + stringFieldInfos = append(stringFieldInfos, stringFieldInfo{ + field: field, + method: mtd, + methodName: mtdName, + }) + } + } + } + addStringFields(datastore.TSMetadata{}) + addStringFields(datastore.ObsMetadata{}) + + for _, sfInfo := range stringFieldInfos { + patterns, ok := sfInfo.method.Call([]reflect.Value{})[0].Interface().([]string) + if !ok { + return fmt.Errorf( + "sfInfo.method.Call() failed for method %s; failed to return []string", + sfInfo.methodName) + } + if len(patterns) > 0 { + stringFilterInfos = append(stringFilterInfos, stringFilterInfo{ + colName: common.ToSnakeCase(sfInfo.field.Name), + patterns: patterns, + }) + } + } + + mdataExpr := getMdataFilter(stringFilterInfos, &phVals) + + // --- END get search expression for string attributes ---------------- + query := fmt.Sprintf(` SELECT ts_id, observation.id, geo_point_id, pubtime, data_id, history, metadata_id, obstime_instant, processing_level, value, point FROM observation JOIN geo_point gp ON observation.geo_point_id = gp.id JOIN time_series ts on ts.id = observation.ts_id - WHERE %s AND %s AND %s AND %s + WHERE %s AND %s AND %s ORDER BY ts_id, obstime_instant - `, timeExpr, tsMdataExpr, obsMdataExpr, geoExpr) + `, timeExpr, mdataExpr, geoExpr) rows, err := db.Query(query, phVals...) if err != nil { From 1f85095fbc56b9963d50d427cb5c4c6969e4a660 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 11 Dec 2023 14:34:42 +0100 Subject: [PATCH 02/18] Prevented column name ambiguity --- .../storagebackend/postgresql/getobservations.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 78a08a5..89bcccf 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -223,6 +223,7 @@ func getGeoFilter(inside *datastore.Polygon, phVals *[]interface{}) (string, err type stringFieldInfo struct { field reflect.StructField + tableName string method reflect.Value methodName string } @@ -252,21 +253,22 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta stringFieldInfos := []stringFieldInfo{} - addStringFields := func(s interface{}) { + addStringFields := func(s interface{}, tableName string) { for _, field := range reflect.VisibleFields(reflect.TypeOf(s)) { mtdName := fmt.Sprintf("Get%s", field.Name) mtd := rv.MethodByName(mtdName) if field.IsExported() && (field.Type.Kind() == reflect.String) && (mtd.IsValid()) { stringFieldInfos = append(stringFieldInfos, stringFieldInfo{ field: field, + tableName: tableName, method: mtd, methodName: mtdName, }) } } } - addStringFields(datastore.TSMetadata{}) - addStringFields(datastore.ObsMetadata{}) + addStringFields(datastore.TSMetadata{}, "time_series") + addStringFields(datastore.ObsMetadata{}, "observation") for _, sfInfo := range stringFieldInfos { patterns, ok := sfInfo.method.Call([]reflect.Value{})[0].Interface().([]string) @@ -277,7 +279,8 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta } if len(patterns) > 0 { stringFilterInfos = append(stringFilterInfos, stringFilterInfo{ - colName: common.ToSnakeCase(sfInfo.field.Name), + colName: fmt.Sprintf( + "%s.%s", sfInfo.tableName, common.ToSnakeCase(sfInfo.field.Name)), patterns: patterns, }) } From a3122204a780922543233f553d5681281eb613d8 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 11 Dec 2023 15:39:54 +0100 Subject: [PATCH 03/18] Made constent order, avoided table alias problem --- .../storagebackend/postgresql/getobservations.go | 14 +++++++------- .../migrations/1701872471_initialise_schema.up.sql | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 89bcccf..91c43c2 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -291,14 +291,14 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta // --- END get search expression for string attributes ---------------- query := fmt.Sprintf(` - SELECT ts_id, observation.id, geo_point_id, pubtime, data_id, history, metadata_id, + SELECT ts_id, geo_point_id, observation.id, pubtime, data_id, history, metadata_id, obstime_instant, processing_level, value, point FROM observation - JOIN geo_point gp ON observation.geo_point_id = gp.id - JOIN time_series ts on ts.id = observation.ts_id + JOIN time_series on time_series.id = observation.ts_id + JOIN geo_point gp ON observation.geo_point_id = gp.id WHERE %s AND %s AND %s ORDER BY ts_id, obstime_instant - `, timeExpr, mdataExpr, geoExpr) + `, timeExpr, geoExpr, mdataExpr) rows, err := db.Query(query, phVals...) if err != nil { @@ -310,8 +310,8 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta for rows.Next() { var ( tsID int64 - id string gpID int64 + id string pubTime0 time.Time dataID string history string @@ -321,18 +321,18 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta value string point postgis.PointS ) - if err := rows.Scan(&tsID, &id, &gpID, &pubTime0, &dataID, &history, &metadataID, + if err := rows.Scan(&tsID, &gpID, &id, &pubTime0, &dataID, &history, &metadataID, &obsTimeInstant0, &processingLevel, &value, &point); err != nil { return fmt.Errorf("rows.Scan() failed: %v", err) } obsMdata := &datastore.ObsMetadata{ - Id: id, Geometry: &datastore.ObsMetadata_GeoPoint{ GeoPoint: &datastore.Point{ Lon: point.X, Lat: point.Y}, }, + Id: id, Pubtime: timestamppb.New(pubTime0), DataId: dataID, History: history, diff --git a/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql b/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql index dd7ae0c..abc1520 100644 --- a/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql +++ b/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -51,12 +51,13 @@ CREATE TABLE observation ( ts_id BIGINT NOT NULL REFERENCES time_series(id) ON DELETE CASCADE, -- --- BEGIN metadata fields that usually vary with obs time --- - id TEXT NOT NULL, -- required -- Refer to geometry via a foreign key to ensure that each distinct geometry is -- stored only once in the geo_* table, thus speeding up geo search. geo_point_id BIGINT NOT NULL REFERENCES geo_point(id) ON DELETE CASCADE, + id TEXT NOT NULL, -- required + pubtime timestamptz NOT NULL, -- required data_id TEXT NOT NULL, -- required history TEXT, From 7a8ba5343f649f186360aa684b6a49e8f350630b Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 12 Dec 2023 12:36:51 +0100 Subject: [PATCH 04/18] Organized/commented metadata fields in a clearer way --- datastore/datastore/protobuf/datastore.proto | 29 +++++--- .../1701872471_initialise_schema.up.sql | 68 +++++++++++-------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/datastore/datastore/protobuf/datastore.proto b/datastore/datastore/protobuf/datastore.proto index 9784705..98ab8e7 100644 --- a/datastore/datastore/protobuf/datastore.proto +++ b/datastore/datastore/protobuf/datastore.proto @@ -72,6 +72,7 @@ message Link { } message TSMetadata { + // --- BEGIN metadata of type 'single string' ----------------- string version = 1; string type = 2; string title = 3; @@ -94,25 +95,33 @@ message TSMetadata { string unit = 20; string instrument = 21; string instrument_vocabulary = 22 [json_name = "instrument_vocabulary"]; + // --- END metadata of type 'single string' ----------------- + + // --- BEGIN metadata of other type ----------------- repeated Link links = 23; + // --- END metadata of other type ----------------- } message ObsMetadata { - string id = 1; oneof geometry { - Point geo_point = 2 [json_name = "geo_point"]; - Polygon geo_polygon = 3 [json_name = "geo_polygon"]; + Point geo_point = 1 [json_name = "geo_point"]; + Polygon geo_polygon = 2 [json_name = "geo_polygon"]; } - google.protobuf.Timestamp pubtime = 4; - string data_id = 5 [json_name = "data_id"]; - string history = 6; - string metadata_id = 7 [json_name = "metadata_id"]; oneof obstime { - google.protobuf.Timestamp obstime_instant = 8 [json_name = "obstime_instant"]; - //TimeInterval obstime_interval = 9 [json_name = "obstime_interval"]; -- unsupported for now + google.protobuf.Timestamp obstime_instant = 3 [json_name = "obstime_instant"]; + //TimeInterval obstime_interval = 4 [json_name = "obstime_interval"]; -- unsupported for now } + google.protobuf.Timestamp pubtime = 5; + + // --- BEGIN metadata of type 'single string' ----------------- + string id = 6; + string data_id = 7 [json_name = "data_id"]; + string history = 8; + string metadata_id = 9 [json_name = "metadata_id"]; string processing_level = 10 [json_name = "processing_level"]; - string value = 11; + // --- END metadata of type 'single string' ----------------- + + string value = 11; // obs value } //--------------------------------------------------------------------------- diff --git a/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql b/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql index abc1520..0950742 100644 --- a/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql +++ b/datastore/migrate/data/migrations/1701872471_initialise_schema.up.sql @@ -4,33 +4,40 @@ CREATE TABLE time_series ( id BIGINT PRIMARY KEY GENERATED ALWAYS AS IDENTITY, -- --- BEGIN metadata fields that usually don't vary with obs time --- - version TEXT NOT NULL, -- required - type TEXT NOT NULL, -- required - title TEXT, - summary TEXT NOT NULL, -- required - keywords TEXT NOT NULL, -- required - keywords_vocabulary TEXT NOT NULL, -- required - license TEXT NOT NULL, -- required - conventions TEXT NOT NULL, -- required - naming_authority TEXT NOT NULL, -- required - creator_type TEXT, - creator_name TEXT, - creator_email TEXT, - creator_url TEXT, - institution TEXT, - project TEXT, - source TEXT, - platform TEXT NOT NULL, -- required - platform_vocabulary TEXT NOT NULL, -- required - standard_name TEXT, - unit TEXT, - instrument TEXT NOT NULL, - instrument_vocabulary TEXT NOT NULL, + + -- --- BEGIN non-string metadata ----------------- link_href TEXT[], link_rel TEXT[], link_type TEXT[], link_hreflang TEXT[], link_title TEXT[], + -- --- END non-string metadata ----------------- + + -- --- BEGIN string metadata (handleable with reflection) ----------------- + version TEXT NOT NULL, -- required + type TEXT NOT NULL, -- required + title TEXT, + summary TEXT NOT NULL, -- required + keywords TEXT NOT NULL, -- required + keywords_vocabulary TEXT NOT NULL, -- required + license TEXT NOT NULL, -- required + conventions TEXT NOT NULL, -- required + naming_authority TEXT NOT NULL, -- required + creator_type TEXT, + creator_name TEXT, + creator_email TEXT, + creator_url TEXT, + institution TEXT, + project TEXT, + source TEXT, + platform TEXT NOT NULL, -- required + platform_vocabulary TEXT NOT NULL, -- required + standard_name TEXT, + unit TEXT, + instrument TEXT NOT NULL, + instrument_vocabulary TEXT NOT NULL, + -- --- END string metadata ----------------- + -- --- END metadata fields that usually don't vary with obs time --- CONSTRAINT unique_main UNIQUE NULLS NOT DISTINCT (version, type, title, summary, keywords, @@ -52,23 +59,28 @@ CREATE TABLE observation ( -- --- BEGIN metadata fields that usually vary with obs time --- + -- --- BEGIN non-string metadata ----------------- -- Refer to geometry via a foreign key to ensure that each distinct geometry is -- stored only once in the geo_* table, thus speeding up geo search. geo_point_id BIGINT NOT NULL REFERENCES geo_point(id) ON DELETE CASCADE, - id TEXT NOT NULL, -- required + -- --- BEGIN for now support only a single instant for obs time --------- + obstime_instant timestamptz, -- NOT NULL, but implied by being part of PK; obs time variant 1: single instant + -- --- END for now support only a single instant for obs time --------- pubtime timestamptz NOT NULL, -- required + -- --- END non-string metadata ----------------- + + -- --- BEGIN string metadata (handleable with reflection) ----------------- + id TEXT NOT NULL, -- required data_id TEXT NOT NULL, -- required history TEXT, metadata_id TEXT NOT NULL, -- required + processing_level TEXT, + -- --- END string metadata ----------------- - -- --- BEGIN for now support only a single instant for obs time --------- - obstime_instant timestamptz, -- NOT NULL, but implied by being part of PK; obs time variant 1: single instant - -- --- END for now support only a single instant for obs time --------- + value TEXT NOT NULL, -- obs value (not metadata in a strict sense) - processing_level TEXT, - value TEXT NOT NULL, -- obs value -- --- END metadata fields that usually vary with obs time --- PRIMARY KEY (ts_id, obstime_instant) From 48fb8764d5b6a81d8417e0d100d6eb111c8b22ed Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 12 Dec 2023 13:01:15 +0100 Subject: [PATCH 05/18] Made consistent order --- .../postgresql/getobservations.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 91c43c2..fc36023 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -291,11 +291,11 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta // --- END get search expression for string attributes ---------------- query := fmt.Sprintf(` - SELECT ts_id, geo_point_id, observation.id, pubtime, data_id, history, metadata_id, - obstime_instant, processing_level, value, point + SELECT ts_id, geo_point_id, obstime_instant, pubtime, observation.id, data_id, history, metadata_id, + processing_level, value, geo_point.point FROM observation JOIN time_series on time_series.id = observation.ts_id - JOIN geo_point gp ON observation.geo_point_id = gp.id + JOIN geo_point ON observation.geo_point_id = geo_point.id WHERE %s AND %s AND %s ORDER BY ts_id, obstime_instant `, timeExpr, geoExpr, mdataExpr) @@ -311,18 +311,18 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta var ( tsID int64 gpID int64 - id string + obsTimeInstant0 time.Time pubTime0 time.Time + id string dataID string history string metadataID string - obsTimeInstant0 time.Time processingLevel string value string point postgis.PointS ) - if err := rows.Scan(&tsID, &gpID, &id, &pubTime0, &dataID, &history, &metadataID, - &obsTimeInstant0, &processingLevel, &value, &point); err != nil { + if err := rows.Scan(&tsID, &gpID, &obsTimeInstant0, &pubTime0, &id, &dataID, &history, &metadataID, + &processingLevel, &value, &point); err != nil { return fmt.Errorf("rows.Scan() failed: %v", err) } @@ -330,16 +330,17 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta Geometry: &datastore.ObsMetadata_GeoPoint{ GeoPoint: &datastore.Point{ Lon: point.X, - Lat: point.Y}, + Lat: point.Y, + }, + }, + Obstime: &datastore.ObsMetadata_ObstimeInstant{ + ObstimeInstant: timestamppb.New(obsTimeInstant0), }, - Id: id, Pubtime: timestamppb.New(pubTime0), + Id: id, DataId: dataID, History: history, MetadataId: metadataID, - Obstime: &datastore.ObsMetadata_ObstimeInstant{ - ObstimeInstant: timestamppb.New(obsTimeInstant0), - }, ProcessingLevel: processingLevel, Value: value, } From 47903b5e56bc1120a0fac021fde5689627bc5e45 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Wed, 13 Dec 2023 16:13:46 +0100 Subject: [PATCH 06/18] Refactorings ++ --- .../postgresql/getobservations.go | 211 +++++++++++++----- .../postgresql/gettsattrgroups.go | 6 +- 2 files changed, 155 insertions(+), 62 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index fc36023..02fd008 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -15,6 +15,37 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) +var ( + obspb2go map[string]string // association between observation specific protobuf name and + // (generated by protoc) Go name. + // NOTES: + // - Protobuf names are field names of the ObsMetadata message in datastore.proto. + // - Only fields of string type are included + // - The observation value field is not included + // - Protobuf names are identical to corresponding database column names. + // - Protobuf names are snake case (aaa_bbb) whereas Go names are camel case (aaaBbb or AaaBbb). + + obsStringMdataGoNames []string + obsStringMdataCols []string // column names qualified with table name 'observation' +) + +func init() { + obspb2go = map[string]string{} + obsStringMdataGoNames = []string{} + obsStringMdataCols = []string{} + for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.ObsMetadata{})) { + if field.IsExported() && (field.Type.Kind() == reflect.String) && + (strings.ToLower(field.Name) != "value") { // (obs value not considered metadata here) + // TODO: support non-string types, like the 'links' attribute + goName := field.Name + pbName := common.ToSnakeCase(goName) + obspb2go[pbName] = goName + obsStringMdataGoNames = append(obsStringMdataGoNames, goName) + obsStringMdataCols = append(obsStringMdataCols, fmt.Sprintf("observation.%s", pbName)) + } + } +} + // addWhereCondMatchAnyPattern appends to whereExpr an expression of the form // "(cond1 OR cond2 OR ... OR condN)" where condi tests if the ith pattern in patterns matches // colName. Matching is case-insensitive and an asterisk in a pattern matches zero or more @@ -228,24 +259,9 @@ type stringFieldInfo struct { methodName string } -// getObs gets into obs all observations that match request. -// Returns nil upon success, otherwise error. -func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Metadata2) error { - - phVals := []interface{}{} // placeholder values - - // --- BEGIN get temporal and spatial search expressions ---------------- - - timeExpr := getTimeFilter(request.GetInterval()) - - geoExpr, err := getGeoFilter(request.Inside, &phVals) - if err != nil { - return fmt.Errorf("getGeoFilter() failed: %v", err) - } - - // --- END get temporal and spatial search expressions ---------------- - - // --- BEGIN get search expression for string attributes ---------------- +// getStringMdataFilter ... (TODO: document) +func getStringMdataFilter( + request *datastore.GetObsRequest, phVals *[]interface{}) (string, error) { rv := reflect.ValueOf(request) @@ -273,7 +289,7 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta for _, sfInfo := range stringFieldInfos { patterns, ok := sfInfo.method.Call([]reflect.Value{})[0].Interface().([]string) if !ok { - return fmt.Errorf( + return "", fmt.Errorf( "sfInfo.method.Call() failed for method %s; failed to return []string", sfInfo.methodName) } @@ -286,19 +302,124 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta } } - mdataExpr := getMdataFilter(stringFilterInfos, &phVals) + return getMdataFilter(stringFilterInfos, phVals), nil +} + +// createObsQueryVals creates values used for querying observations. +// Upon success returns: +// - time filter used in 'WHERE ... AND ...' clause (possibly just 'TRUE') +// - geo filter ... ditto +// - string metadata ... ditto +// - nil +// Upon failure returns ..., ..., ..., error +func createObsQueryVals( + request *datastore.GetObsRequest, phVals *[]interface{}) (string, string, string, error) { + + timeFilter := getTimeFilter(request.GetInterval()) + + geoFilter, err := getGeoFilter(request.Inside, phVals) + if err != nil { + return "", "", "", fmt.Errorf("getGeoFilter() failed: %v", err) + } + + stringMdataFilter, err := getStringMdataFilter(request, phVals) + if err != nil { + return "", "", "", fmt.Errorf("getStringMdataFilter() failed: %v", err) + } + + return timeFilter, geoFilter, stringMdataFilter, nil +} + +// scanObsRow ... (TODO: document!) +func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { + + var ( + tsID int64 + obsTimeInstant0 time.Time + pubTime0 time.Time + value string + point postgis.PointS + ) + + // initialize colValPtrs + colValPtrs := []interface{}{ + &tsID, + &obsTimeInstant0, + &pubTime0, + &value, + &point, + } + + // complete colValPtrs with string metadata + colVals0 := make([]interface{}, len(obsStringMdataGoNames)) + for i := range obsStringMdataGoNames { + colValPtrs = append(colValPtrs, &colVals0[i]) + } + + // scan row into column value pointers + if err := rows.Scan(colValPtrs...); err != nil { + return nil, -1, fmt.Errorf("rows.Scan() failed: %v", err) + } + // initialize obsMdata with obs value and non-string metadata + obsMdata := datastore.ObsMetadata{ + Geometry: &datastore.ObsMetadata_GeoPoint{ + GeoPoint: &datastore.Point{ + Lon: point.X, + Lat: point.Y, + }, + }, + Obstime: &datastore.ObsMetadata_ObstimeInstant{ + ObstimeInstant: timestamppb.New(obsTimeInstant0), + }, + Pubtime: timestamppb.New(pubTime0), + Value: value, + } + + // complete obsMdata with string metadata + rv := reflect.ValueOf(&obsMdata) + for i, goName := range obsStringMdataGoNames { + val, ok := colVals0[i].(string) + if !ok { + return nil, -1, fmt.Errorf( + "colVals0[%d] not string: %v (type: %T)", i, colVals0[i], colVals0[i]) + } + + field := rv.Elem().FieldByName(goName) + + // NOTE: we assume the following assignemnt will never panic, hence we don't do + // any pre-validation of field + field.SetString(val) + } + + return &obsMdata, tsID, nil +} - // --- END get search expression for string attributes ---------------- +// getObs gets into obs all observations that match request. +// Returns nil upon success, otherwise error. +func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Metadata2) (retErr error) { + + // get values needed for query + phVals := []interface{}{} // placeholder values + timeFilter, geoFilter, stringMdataFilter, err := createObsQueryVals(request, &phVals) + if err != nil { + return fmt.Errorf("createQueryVals() failed: %v", err) + } + // define and execute query query := fmt.Sprintf(` - SELECT ts_id, geo_point_id, obstime_instant, pubtime, observation.id, data_id, history, metadata_id, - processing_level, value, geo_point.point + SELECT + ts_id, + obstime_instant, + pubtime, + value, + point, + %s FROM observation JOIN time_series on time_series.id = observation.ts_id JOIN geo_point ON observation.geo_point_id = geo_point.id WHERE %s AND %s AND %s ORDER BY ts_id, obstime_instant - `, timeExpr, geoExpr, mdataExpr) + `, strings.Join(obsStringMdataCols, ","), timeFilter, geoFilter, stringMdataFilter) rows, err := db.Query(query, phVals...) if err != nil { @@ -306,44 +427,15 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta } defer rows.Close() - obsMap := make(map[int64][]*datastore.ObsMetadata) + obsMap := make(map[int64][]*datastore.ObsMetadata) // observations per time series ID + + // scan rows for rows.Next() { - var ( - tsID int64 - gpID int64 - obsTimeInstant0 time.Time - pubTime0 time.Time - id string - dataID string - history string - metadataID string - processingLevel string - value string - point postgis.PointS - ) - if err := rows.Scan(&tsID, &gpID, &obsTimeInstant0, &pubTime0, &id, &dataID, &history, &metadataID, - &processingLevel, &value, &point); err != nil { - return fmt.Errorf("rows.Scan() failed: %v", err) + obsMdata, tsID, err := scanObsRow(rows) + if err != nil { + return fmt.Errorf("scanObsRow() failed: %v", err) } - obsMdata := &datastore.ObsMetadata{ - Geometry: &datastore.ObsMetadata_GeoPoint{ - GeoPoint: &datastore.Point{ - Lon: point.X, - Lat: point.Y, - }, - }, - Obstime: &datastore.ObsMetadata_ObstimeInstant{ - ObstimeInstant: timestamppb.New(obsTimeInstant0), - }, - Pubtime: timestamppb.New(pubTime0), - Id: id, - DataId: dataID, - History: history, - MetadataId: metadataID, - ProcessingLevel: processingLevel, - Value: value, - } obsMap[tsID] = append(obsMap[tsID], obsMdata) } @@ -357,6 +449,7 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta return fmt.Errorf("getTSMetadata() failed: %v", err) } + // assemble final output for tsID, obsMdata := range obsMap { *obs = append(*obs, &datastore.Metadata2{ TsMdata: tsMdata[tsID], diff --git a/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go index ae7c093..b2af085 100644 --- a/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -22,10 +22,10 @@ var ( func init() { tspb2go = map[string]string{} - for _, f := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { - if f.IsExported() && (f.Type.Kind() == reflect.String) { + for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { + if field.IsExported() && (field.Type.Kind() == reflect.String) { // TODO: support non-string types, like the 'links' attribute - goName := f.Name + goName := field.Name pbName := common.ToSnakeCase(goName) tspb2go[pbName] = goName } From 39eeca31ed64cf183799e32c8e6cd8330953ad3d Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Fri, 15 Dec 2023 16:29:49 +0100 Subject: [PATCH 07/18] Refactorings ++ --- .../postgresql/getobservations.go | 185 ++++++++++-------- .../storagebackend/postgresql/postgresql.go | 15 +- .../postgresql/putobservations.go | 60 +++--- 3 files changed, 146 insertions(+), 114 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 02fd008..1fcde71 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -16,6 +16,8 @@ import ( ) var ( + tsStringMdataGoNames []string // Go names for time series metadata of type string + obspb2go map[string]string // association between observation specific protobuf name and // (generated by protoc) Go name. // NOTES: @@ -25,18 +27,25 @@ var ( // - Protobuf names are identical to corresponding database column names. // - Protobuf names are snake case (aaa_bbb) whereas Go names are camel case (aaaBbb or AaaBbb). - obsStringMdataGoNames []string + obsStringMdataGoNames []string // Go names for observation metadata of type string obsStringMdataCols []string // column names qualified with table name 'observation' ) func init() { + tsStringMdataGoNames = []string{} + for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { + if field.IsExported() && (field.Type.Kind() == reflect.String) { + goName := field.Name + tsStringMdataGoNames = append(tsStringMdataGoNames, goName) + } + } + obspb2go = map[string]string{} obsStringMdataGoNames = []string{} obsStringMdataCols = []string{} for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.ObsMetadata{})) { if field.IsExported() && (field.Type.Kind() == reflect.String) && (strings.ToLower(field.Name) != "value") { // (obs value not considered metadata here) - // TODO: support non-string types, like the 'links' attribute goName := field.Name pbName := common.ToSnakeCase(goName) obspb2go[pbName] = goName @@ -46,6 +55,24 @@ func init() { } } +// addStringMdata ... (TODO: document!) +func addStringMdata(rv reflect.Value, stringMdataGoNames []string, colVals []interface{}) error { + for i, goName := range stringMdataGoNames { + val, ok := colVals[i].(string) + if !ok { + return fmt.Errorf("colVals[%d] not string: %v (type: %T)", i, colVals[i], colVals[i]) + } + + field := rv.Elem().FieldByName(goName) + + // NOTE: we assume the following assignemnt will never panic, hence we don't do + // any pre-validation of field + field.SetString(val) + } + + return nil +} + // addWhereCondMatchAnyPattern appends to whereExpr an expression of the form // "(cond1 OR cond2 OR ... OR condN)" where condi tests if the ith pattern in patterns matches // colName. Matching is case-insensitive and an asterisk in a pattern matches zero or more @@ -70,16 +97,74 @@ func addWhereCondMatchAnyPattern( *whereExpr = append(*whereExpr, fmt.Sprintf("(%s)", strings.Join(whereExprOR, " OR "))) } -// getTSMetadata retrieves into tsMdata metadata of time series in table time_series that match -// tsIDs. The keys of tsMdata are the time series IDs. +// scanTSRow ... (TODO: document!) +func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { + + var ( + tsID int64 + linkHref pq.StringArray + linkRel pq.StringArray + linkType pq.StringArray + linkHrefLang pq.StringArray + linkTitle pq.StringArray + ) + + // initialize colValPtrs + colValPtrs := []interface{}{ + &tsID, + &linkHref, + &linkRel, + &linkType, + &linkHrefLang, + &linkTitle, + } + + // complete colValPtrs with string metadata + colVals0 := make([]interface{}, len(tsStringMdataGoNames)) + for i := range tsStringMdataGoNames { + colValPtrs = append(colValPtrs, &colVals0[i]) + } + + // scan row into column value pointers + if err := rows.Scan(colValPtrs...); err != nil { + return nil, -1, fmt.Errorf("rows.Scan() failed: %v", err) + } + + // initialize tsMdata with non-string metadata + links := []*datastore.Link{} + for i := 0; i < len(linkHref); i++ { + links = append(links, &datastore.Link{ + Href: linkHref[i], + Rel: linkRel[i], + Type: linkType[i], + Hreflang: linkHrefLang[i], + Title: linkTitle[i], + }) + } + tsMdata := datastore.TSMetadata{ + Links: links, + } + + // complete tsMdata with string metadata + err := addStringMdata(reflect.ValueOf(&tsMdata), tsStringMdataGoNames, colVals0) + if err != nil { + return nil, -1, fmt.Errorf("addStringMdata() failed: %v", err) + } + + return &tsMdata, tsID, nil +} + +// getTSMetadata retrieves into tsMdatas metadata of time series in table time_series that match +// tsIDs. The keys of tsMdatas are the time series IDs. // Returns nil upon success, otherwise error -func getTSMetadata(db *sql.DB, tsIDs []string, tsMdata map[int64]*datastore.TSMetadata) error { +func getTSMetadata(db *sql.DB, tsIDs []string, tsMdatas map[int64]*datastore.TSMetadata) error { query := fmt.Sprintf( `SELECT id, %s FROM time_series WHERE %s`, strings.Join(getTSMdataCols(), ","), createSetFilter("id", tsIDs), ) + fmt.Printf("getTSMetadata(): query: %s\n", query) rows, err := db.Query(query) if err != nil { @@ -88,61 +173,12 @@ func getTSMetadata(db *sql.DB, tsIDs []string, tsMdata map[int64]*datastore.TSMe defer rows.Close() for rows.Next() { - var tsID int64 - var tsMdata0 datastore.TSMetadata - - linkHref := pq.StringArray{} - linkRel := pq.StringArray{} - linkType := pq.StringArray{} - linkHrefLang := pq.StringArray{} - linkTitle := pq.StringArray{} - - if err := rows.Scan( - &tsID, - &tsMdata0.Version, - &tsMdata0.Type, - &tsMdata0.Title, - &tsMdata0.Summary, - &tsMdata0.Keywords, - &tsMdata0.KeywordsVocabulary, - &tsMdata0.License, - &tsMdata0.Conventions, - &tsMdata0.NamingAuthority, - &tsMdata0.CreatorType, - &tsMdata0.CreatorName, - &tsMdata0.CreatorEmail, - &tsMdata0.CreatorUrl, - &tsMdata0.Institution, - &tsMdata0.Project, - &tsMdata0.Source, - &tsMdata0.Platform, - &tsMdata0.PlatformVocabulary, - &tsMdata0.StandardName, - &tsMdata0.Unit, - &tsMdata0.Instrument, - &tsMdata0.InstrumentVocabulary, - &linkHref, - &linkRel, - &linkType, - &linkHrefLang, - &linkTitle, - ); err != nil { - return fmt.Errorf("rows.Scan() failed: %v", err) - } - - links := []*datastore.Link{} - for i := 0; i < len(linkHref); i++ { - links = append(links, &datastore.Link{ - Href: linkHref[i], - Rel: linkRel[i], - Type: linkType[i], - Hreflang: linkHrefLang[i], - Title: linkTitle[i], - }) + tsMdata, tsID, err := scanTSRow(rows) + if err != nil { + return fmt.Errorf("scanTSRow() failed: %v", err) } - tsMdata0.Links = links - tsMdata[tsID] = &tsMdata0 + tsMdatas[tsID] = tsMdata } return nil @@ -360,6 +396,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { if err := rows.Scan(colValPtrs...); err != nil { return nil, -1, fmt.Errorf("rows.Scan() failed: %v", err) } + // initialize obsMdata with obs value and non-string metadata obsMdata := datastore.ObsMetadata{ Geometry: &datastore.ObsMetadata_GeoPoint{ @@ -376,19 +413,9 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { } // complete obsMdata with string metadata - rv := reflect.ValueOf(&obsMdata) - for i, goName := range obsStringMdataGoNames { - val, ok := colVals0[i].(string) - if !ok { - return nil, -1, fmt.Errorf( - "colVals0[%d] not string: %v (type: %T)", i, colVals0[i], colVals0[i]) - } - - field := rv.Elem().FieldByName(goName) - - // NOTE: we assume the following assignemnt will never panic, hence we don't do - // any pre-validation of field - field.SetString(val) + err := addStringMdata(reflect.ValueOf(&obsMdata), obsStringMdataGoNames, colVals0) + if err != nil { + return nil, -1, fmt.Errorf("addStringMdata() failed: %v", err) } return &obsMdata, tsID, nil @@ -427,7 +454,7 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta } defer rows.Close() - obsMap := make(map[int64][]*datastore.ObsMetadata) // observations per time series ID + obsMdatas := make(map[int64][]*datastore.ObsMetadata) // observations per time series ID // scan rows for rows.Next() { @@ -436,23 +463,23 @@ func getObs(db *sql.DB, request *datastore.GetObsRequest, obs *[]*datastore.Meta return fmt.Errorf("scanObsRow() failed: %v", err) } - obsMap[tsID] = append(obsMap[tsID], obsMdata) + obsMdatas[tsID] = append(obsMdatas[tsID], obsMdata) } // get time series - tsMdata := map[int64]*datastore.TSMetadata{} + tsMdatas := map[int64]*datastore.TSMetadata{} tsIDs := []string{} - for id := range obsMap { - tsIDs = append(tsIDs, fmt.Sprintf("%d", id)) + for tsID := range obsMdatas { + tsIDs = append(tsIDs, fmt.Sprintf("%d", tsID)) } - if err = getTSMetadata(db, tsIDs, tsMdata); err != nil { + if err = getTSMetadata(db, tsIDs, tsMdatas); err != nil { return fmt.Errorf("getTSMetadata() failed: %v", err) } // assemble final output - for tsID, obsMdata := range obsMap { + for tsID, obsMdata := range obsMdatas { *obs = append(*obs, &datastore.Metadata2{ - TsMdata: tsMdata[tsID], + TsMdata: tsMdatas[tsID], ObsMdata: obsMdata, }) } diff --git a/datastore/datastore/storagebackend/postgresql/postgresql.go b/datastore/datastore/storagebackend/postgresql/postgresql.go index 3dbb4d3..4b7bdb5 100644 --- a/datastore/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/datastore/storagebackend/postgresql/postgresql.go @@ -92,8 +92,15 @@ func NewPostgreSQL() (*PostgreSQL, error) { // getTSMdataCols returns time series metadata column names. func getTSMdataCols() []string { + // ### TODO: modify to use reflection instead of explicit field referrals return []string{ - // main section + // links section (aka. non-string metadata ...) + "link_href", + "link_rel", + "link_type", + "link_hreflang", + "link_title", + // main section (aka. string metadata ...) "version", "type", "title", @@ -116,12 +123,6 @@ func getTSMdataCols() []string { "unit", "instrument", "instrument_vocabulary", - // links section - "link_href", - "link_rel", - "link_type", - "link_hreflang", - "link_title", } } diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index 183ac79..474c315 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -44,34 +44,7 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { colVals := []interface{}{} - // main section - - colVals = []interface{}{ - tsMdata.GetVersion(), - tsMdata.GetType(), - tsMdata.GetTitle(), - tsMdata.GetSummary(), - tsMdata.GetKeywords(), - tsMdata.GetKeywordsVocabulary(), - tsMdata.GetLicense(), - tsMdata.GetConventions(), - tsMdata.GetNamingAuthority(), - tsMdata.GetCreatorType(), - tsMdata.GetCreatorName(), - tsMdata.GetCreatorEmail(), - tsMdata.GetCreatorUrl(), - tsMdata.GetInstitution(), - tsMdata.GetProject(), - tsMdata.GetSource(), - tsMdata.GetPlatform(), - tsMdata.GetPlatformVocabulary(), - tsMdata.GetStandardName(), - tsMdata.GetUnit(), - tsMdata.GetInstrument(), - tsMdata.GetInstrumentVocabulary(), - } - - // links section + // links section (aka. non-string metadata ...) getLinkVals := func(key string) ([]string, error) { linkVals := []string{} @@ -104,6 +77,35 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { } } + // main section (aka. string metadata ...) + + // ### TODO: modify to use reflection instead of explicit field referrals + + colVals = append(colVals, []interface{}{ + tsMdata.GetVersion(), + tsMdata.GetType(), + tsMdata.GetTitle(), + tsMdata.GetSummary(), + tsMdata.GetKeywords(), + tsMdata.GetKeywordsVocabulary(), + tsMdata.GetLicense(), + tsMdata.GetConventions(), + tsMdata.GetNamingAuthority(), + tsMdata.GetCreatorType(), + tsMdata.GetCreatorName(), + tsMdata.GetCreatorEmail(), + tsMdata.GetCreatorUrl(), + tsMdata.GetInstitution(), + tsMdata.GetProject(), + tsMdata.GetSource(), + tsMdata.GetPlatform(), + tsMdata.GetPlatformVocabulary(), + tsMdata.GetStandardName(), + tsMdata.GetUnit(), + tsMdata.GetInstrument(), + tsMdata.GetInstrumentVocabulary(), + }...) + return colVals, nil } @@ -155,6 +157,8 @@ func getTimeSeriesID( cols[0], cols[0], ) + fmt.Printf("insertCmd: %s; len(cols): %d; len(phs): %d\n", + insertCmd, len(cols), len(createPlaceholders(formats))) _, err = tx.Exec(insertCmd, colVals...) if err != nil { From ae569306bf8d4fa1e5d39b6d43c0c5f37775e8f8 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 12:10:42 +0100 Subject: [PATCH 08/18] Improved marking of metadata categories, made order consistent --- datastore/datastore/protobuf/datastore.proto | 64 ++++++++++--------- .../storagebackend/postgresql/postgresql.go | 7 +- .../postgresql/putobservations.go | 8 ++- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/datastore/datastore/protobuf/datastore.proto b/datastore/datastore/protobuf/datastore.proto index 98ab8e7..eb920dc 100644 --- a/datastore/datastore/protobuf/datastore.proto +++ b/datastore/datastore/protobuf/datastore.proto @@ -72,37 +72,38 @@ message Link { } message TSMetadata { - // --- BEGIN metadata of type 'single string' ----------------- - string version = 1; - string type = 2; - string title = 3; - string summary = 4; - string keywords = 5; - string keywords_vocabulary = 6 [json_name = "keywords_vocabulary"]; - string license = 7; - string conventions = 8; - string naming_authority = 9 [json_name = "naming_authority"]; - string creator_type = 10 [json_name = "creator_type"]; - string creator_name = 11 [json_name = "creator_name"]; - string creator_email = 12 [json_name = "creator_email"]; - string creator_url = 13 [json_name = "creator_url"]; - string institution = 14; - string project = 15; - string source = 16; - string platform = 17; - string platform_vocabulary = 18 [json_name = "platform_vocabulary"]; - string standard_name = 19 [json_name = "standard_name"]; - string unit = 20; - string instrument = 21; - string instrument_vocabulary = 22 [json_name = "instrument_vocabulary"]; - // --- END metadata of type 'single string' ----------------- - - // --- BEGIN metadata of other type ----------------- - repeated Link links = 23; - // --- END metadata of other type ----------------- + // --- BEGIN non-string metadata ----------------- + repeated Link links = 1; + // --- END non-string metadata ----------------- + + // --- BEGIN string metadata ----------------- + string version = 2; + string type = 3; + string title = 4; + string summary = 5; + string keywords = 6; + string keywords_vocabulary = 7 [json_name = "keywords_vocabulary"]; + string license = 8; + string conventions = 9; + string naming_authority = 10 [json_name = "naming_authority"]; + string creator_type = 101 [json_name = "creator_type"]; + string creator_name = 12 [json_name = "creator_name"]; + string creator_email = 13 [json_name = "creator_email"]; + string creator_url = 14 [json_name = "creator_url"]; + string institution = 15; + string project = 16; + string source = 17; + string platform = 18; + string platform_vocabulary = 19 [json_name = "platform_vocabulary"]; + string standard_name = 20 [json_name = "standard_name"]; + string unit = 21; + string instrument = 22; + string instrument_vocabulary = 23 [json_name = "instrument_vocabulary"]; + // --- END string metadata ----------------- } message ObsMetadata { + // --- BEGIN non-string metadata ----------------- oneof geometry { Point geo_point = 1 [json_name = "geo_point"]; Polygon geo_polygon = 2 [json_name = "geo_polygon"]; @@ -112,16 +113,17 @@ message ObsMetadata { //TimeInterval obstime_interval = 4 [json_name = "obstime_interval"]; -- unsupported for now } google.protobuf.Timestamp pubtime = 5; + // --- END non-string metadata ----------------- - // --- BEGIN metadata of type 'single string' ----------------- + // --- BEGIN string metadata ----------------- string id = 6; string data_id = 7 [json_name = "data_id"]; string history = 8; string metadata_id = 9 [json_name = "metadata_id"]; string processing_level = 10 [json_name = "processing_level"]; - // --- END metadata of type 'single string' ----------------- + // --- END string metadata ----------------- - string value = 11; // obs value + string value = 11; // obs value (not metadata in a strict sense) } //--------------------------------------------------------------------------- diff --git a/datastore/datastore/storagebackend/postgresql/postgresql.go b/datastore/datastore/storagebackend/postgresql/postgresql.go index 4b7bdb5..78f1049 100644 --- a/datastore/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/datastore/storagebackend/postgresql/postgresql.go @@ -94,13 +94,15 @@ func NewPostgreSQL() (*PostgreSQL, error) { func getTSMdataCols() []string { // ### TODO: modify to use reflection instead of explicit field referrals return []string{ - // links section (aka. non-string metadata ...) + // --- BEGIN non-string metadata --------------------------- "link_href", "link_rel", "link_type", "link_hreflang", "link_title", - // main section (aka. string metadata ...) + // --- END non-string metadata --------------------------- + + // --- BEGIN string metadata --------------------------- "version", "type", "title", @@ -123,6 +125,7 @@ func getTSMdataCols() []string { "unit", "instrument", "instrument_vocabulary", + // --- END string metadata --------------------------- } } diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index 474c315..8225a11 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -44,7 +44,7 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { colVals := []interface{}{} - // links section (aka. non-string metadata ...) + // --- BEGIN non-string metadata --------------------------- getLinkVals := func(key string) ([]string, error) { linkVals := []string{} @@ -77,7 +77,9 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { } } - // main section (aka. string metadata ...) + // --- END non-string metadata --------------------------- + + // --- BEGIN string metadata --------------------------- // ### TODO: modify to use reflection instead of explicit field referrals @@ -106,6 +108,8 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { tsMdata.GetInstrumentVocabulary(), }...) + // --- END string metadata --------------------------- + return colVals, nil } From 8ec4056707c2981fb4c6ac139632292cf7ec2d35 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 13:22:42 +0100 Subject: [PATCH 09/18] Used reflection to avoid explicit field references --- .../postgresql/getobservations.go | 3 ++ .../storagebackend/postgresql/postgresql.go | 37 ++++--------------- .../postgresql/putobservations.go | 2 +- 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 1fcde71..2357379 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -17,6 +17,7 @@ import ( var ( tsStringMdataGoNames []string // Go names for time series metadata of type string + tsStringMdataPBNames []string // protobuf names for time series metadata of type string obspb2go map[string]string // association between observation specific protobuf name and // (generated by protoc) Go name. @@ -33,10 +34,12 @@ var ( func init() { tsStringMdataGoNames = []string{} + tsStringMdataPBNames = []string{} for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { if field.IsExported() && (field.Type.Kind() == reflect.String) { goName := field.Name tsStringMdataGoNames = append(tsStringMdataGoNames, goName) + tsStringMdataPBNames = append(tsStringMdataPBNames, common.ToSnakeCase(goName)) } } diff --git a/datastore/datastore/storagebackend/postgresql/postgresql.go b/datastore/datastore/storagebackend/postgresql/postgresql.go index 78f1049..271abc5 100644 --- a/datastore/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/datastore/storagebackend/postgresql/postgresql.go @@ -92,41 +92,20 @@ func NewPostgreSQL() (*PostgreSQL, error) { // getTSMdataCols returns time series metadata column names. func getTSMdataCols() []string { - // ### TODO: modify to use reflection instead of explicit field referrals - return []string{ - // --- BEGIN non-string metadata --------------------------- + + // initialize cols with non-string metadata + cols := []string{ "link_href", "link_rel", "link_type", "link_hreflang", "link_title", - // --- END non-string metadata --------------------------- - - // --- BEGIN string metadata --------------------------- - "version", - "type", - "title", - "summary", - "keywords", - "keywords_vocabulary", - "license", - "conventions", - "naming_authority", - "creator_type", - "creator_name", - "creator_email", - "creator_url", - "institution", - "project", - "source", - "platform", - "platform_vocabulary", - "standard_name", - "unit", - "instrument", - "instrument_vocabulary", - // --- END string metadata --------------------------- } + + // complete cols with string metadata + cols = append(cols, tsStringMdataPBNames...) + + return cols } // createPlaceholders returns the list of n placeholder strings for diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index 8225a11..2ff5d31 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -81,7 +81,7 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { // --- BEGIN string metadata --------------------------- - // ### TODO: modify to use reflection instead of explicit field referrals + // ### TODO: modify to use reflection instead of explicit field referencing colVals = append(colVals, []interface{}{ tsMdata.GetVersion(), From e1139e13807d371704cc18e0996f186f98abb287 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 14:00:39 +0100 Subject: [PATCH 10/18] Used reflection to avoid explicit field references --- .../postgresql/putobservations.go | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index 2ff5d31..bad0b85 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -6,6 +6,7 @@ import ( "datastore/datastore" "fmt" "log" + "reflect" "strconv" "strings" @@ -81,32 +82,36 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { // --- BEGIN string metadata --------------------------- - // ### TODO: modify to use reflection instead of explicit field referencing - - colVals = append(colVals, []interface{}{ - tsMdata.GetVersion(), - tsMdata.GetType(), - tsMdata.GetTitle(), - tsMdata.GetSummary(), - tsMdata.GetKeywords(), - tsMdata.GetKeywordsVocabulary(), - tsMdata.GetLicense(), - tsMdata.GetConventions(), - tsMdata.GetNamingAuthority(), - tsMdata.GetCreatorType(), - tsMdata.GetCreatorName(), - tsMdata.GetCreatorEmail(), - tsMdata.GetCreatorUrl(), - tsMdata.GetInstitution(), - tsMdata.GetProject(), - tsMdata.GetSource(), - tsMdata.GetPlatform(), - tsMdata.GetPlatformVocabulary(), - tsMdata.GetStandardName(), - tsMdata.GetUnit(), - tsMdata.GetInstrument(), - tsMdata.GetInstrumentVocabulary(), - }...) + type stringFieldInfo struct { + field reflect.StructField + method reflect.Value + methodName string + } + + stringFieldInfos := []stringFieldInfo{} + + rv := reflect.ValueOf(tsMdata) + for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { + mtdName := fmt.Sprintf("Get%s", field.Name) + mtd := rv.MethodByName(mtdName) + if field.IsExported() && (field.Type.Kind() == reflect.String) && (mtd.IsValid()) { + stringFieldInfos = append(stringFieldInfos, stringFieldInfo{ + field: field, + method: mtd, + methodName: mtdName, + }) + } + } + + for _, sfInfo := range stringFieldInfos { + val, ok := sfInfo.method.Call([]reflect.Value{})[0].Interface().(string) + if !ok { + return nil, fmt.Errorf( + "sfInfo.method.Call() failed for method %s; failed to return string", + sfInfo.methodName) + } + colVals = append(colVals, val) + } // --- END string metadata --------------------------- From aae6f1ecf69e39c34c4042358de8e4182ea0c74c Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 14:27:33 +0100 Subject: [PATCH 11/18] Simplifified code --- .../postgresql/putobservations.go | 35 +++++-------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index bad0b85..7741f9a 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -82,35 +82,18 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { // --- BEGIN string metadata --------------------------- - type stringFieldInfo struct { - field reflect.StructField - method reflect.Value - methodName string - } - - stringFieldInfos := []stringFieldInfo{} - rv := reflect.ValueOf(tsMdata) for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { - mtdName := fmt.Sprintf("Get%s", field.Name) - mtd := rv.MethodByName(mtdName) - if field.IsExported() && (field.Type.Kind() == reflect.String) && (mtd.IsValid()) { - stringFieldInfos = append(stringFieldInfos, stringFieldInfo{ - field: field, - method: mtd, - methodName: mtdName, - }) - } - } - - for _, sfInfo := range stringFieldInfos { - val, ok := sfInfo.method.Call([]reflect.Value{})[0].Interface().(string) - if !ok { - return nil, fmt.Errorf( - "sfInfo.method.Call() failed for method %s; failed to return string", - sfInfo.methodName) + methodName := fmt.Sprintf("Get%s", field.Name) + method := rv.MethodByName(methodName) + if field.IsExported() && (field.Type.Kind() == reflect.String) && (method.IsValid()) { + val, ok := method.Call([]reflect.Value{})[0].Interface().(string) + if !ok { + return nil, fmt.Errorf( + "method.Call() failed for method %s; failed to return string", methodName) + } + colVals = append(colVals, val) } - colVals = append(colVals, val) } // --- END string metadata --------------------------- From 67d820375f3c2890c0e00ad949a2363ee07980eb Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 14:28:24 +0100 Subject: [PATCH 12/18] Removed debug printout --- datastore/datastore/storagebackend/postgresql/getobservations.go | 1 - 1 file changed, 1 deletion(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 2357379..3d9de4a 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -167,7 +167,6 @@ func getTSMetadata(db *sql.DB, tsIDs []string, tsMdatas map[int64]*datastore.TSM strings.Join(getTSMdataCols(), ","), createSetFilter("id", tsIDs), ) - fmt.Printf("getTSMetadata(): query: %s\n", query) rows, err := db.Query(query) if err != nil { From f4cd7e2d5dcef9e9e126509d6a4f46729ff32260 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Mon, 18 Dec 2023 15:51:57 +0100 Subject: [PATCH 13/18] Added missing documentation ++ --- .../postgresql/getobservations.go | 47 ++++++++++++------- .../postgresql/gettsattrgroups.go | 23 ++++----- .../postgresql/putobservations.go | 1 - 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 3d9de4a..0db467f 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -58,7 +58,9 @@ func init() { } } -// addStringMdata ... (TODO: document!) +// addStringMdata assigns the values of colVals to the corresponding struct fields in +// stringMdataGoNames (value i corresponds to field i ...). The struct is represented by rv. +// Returns nil upon success, otherwise error. func addStringMdata(rv reflect.Value, stringMdataGoNames []string, colVals []interface{}) error { for i, goName := range stringMdataGoNames { val, ok := colVals[i].(string) @@ -100,9 +102,10 @@ func addWhereCondMatchAnyPattern( *whereExpr = append(*whereExpr, fmt.Sprintf("(%s)", strings.Join(whereExprOR, " OR "))) } -// scanTSRow ... (TODO: document!) +// scanTSRow scans all columns from the current result row in rows and converts to a TSMetadata +// object. +// Returns (TSMetadata object, time series ID, nil) upon success, otherwise (..., ..., error). func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { - var ( tsID int64 linkHref pq.StringArray @@ -112,7 +115,7 @@ func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { linkTitle pq.StringArray ) - // initialize colValPtrs + // initialize colValPtrs with non-string metadata colValPtrs := []interface{}{ &tsID, &linkHref, @@ -161,7 +164,6 @@ func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { // tsIDs. The keys of tsMdatas are the time series IDs. // Returns nil upon success, otherwise error func getTSMetadata(db *sql.DB, tsIDs []string, tsMdatas map[int64]*datastore.TSMetadata) error { - query := fmt.Sprintf( `SELECT id, %s FROM time_series WHERE %s`, strings.Join(getTSMdataCols(), ","), @@ -218,7 +220,7 @@ type stringFilterInfo struct { colName string patterns []string } -// TODO: add filter infos for other types than string +// TODO: add filter infos for non-string types // getMdataFilter derives from stringFilterInfos the expression used in a WHERE clause for // "match any" filtering on a set of attributes. @@ -253,6 +255,9 @@ func getMdataFilter(stringFilterInfos []stringFilterInfo, phVals *[]interface{}) // getGeoFilter derives from 'inside' the expression used in a WHERE clause for keeping // observations inside this polygon. +// +// Values to be used for query placeholders are appended to phVals. +// // Returns expression. func getGeoFilter(inside *datastore.Polygon, phVals *[]interface{}) (string, error) { whereExpr := "TRUE" // by default, don't filter @@ -297,7 +302,13 @@ type stringFieldInfo struct { methodName string } -// getStringMdataFilter ... (TODO: document) +// getStringMdataFilter creates from 'request' the string metadata filter used for querying +// observations. +// +// Values to be used for query placeholders are appended to phVals. +// +// Returns upon success (string metadata filter used in a 'WHERE ... AND ...' clause (possibly +// just 'TRUE'), nil), otherwise (..., error). func getStringMdataFilter( request *datastore.GetObsRequest, phVals *[]interface{}) (string, error) { @@ -343,13 +354,16 @@ func getStringMdataFilter( return getMdataFilter(stringFilterInfos, phVals), nil } -// createObsQueryVals creates values used for querying observations. -// Upon success returns: -// - time filter used in 'WHERE ... AND ...' clause (possibly just 'TRUE') +// createObsQueryVals creates from 'request' values used for querying observations. +// +// Values to be used for query placeholders are appended to phVals. +// +// Upon success the function returns four values: +// - time filter used in a 'WHERE ... AND ...' clause (possibly just 'TRUE') // - geo filter ... ditto // - string metadata ... ditto -// - nil -// Upon failure returns ..., ..., ..., error +// - nil, +// otherwise (..., ..., ..., error). func createObsQueryVals( request *datastore.GetObsRequest, phVals *[]interface{}) (string, string, string, error) { @@ -368,9 +382,10 @@ func createObsQueryVals( return timeFilter, geoFilter, stringMdataFilter, nil } -// scanObsRow ... (TODO: document!) +// scanObsRow scans all columns from the current result row in rows and converts to an ObsMetadata +// object. +// Returns (ObsMetadata object, time series ID, nil) upon success, otherwise (..., ..., error). func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { - var ( tsID int64 obsTimeInstant0 time.Time @@ -379,7 +394,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { point postgis.PointS ) - // initialize colValPtrs + // initialize colValPtrs with non-string metadata colValPtrs := []interface{}{ &tsID, &obsTimeInstant0, @@ -399,7 +414,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { return nil, -1, fmt.Errorf("rows.Scan() failed: %v", err) } - // initialize obsMdata with obs value and non-string metadata + // initialize obsMdata with non-string metadata and obs value obsMdata := datastore.ObsMetadata{ Geometry: &datastore.ObsMetadata_GeoPoint{ GeoPoint: &datastore.Point{ diff --git a/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go b/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go index b2af085..28222b6 100644 --- a/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go +++ b/datastore/datastore/storagebackend/postgresql/gettsattrgroups.go @@ -75,9 +75,9 @@ func validateAttrs(pbNames []string) error { return nil } -// getTSMdata returns a TSMetadata object initialized from colVals. +// getTSMdata creates a TSMetadata object initialized from colVals. +// Returns (TSMetadata object, nil) upon success, otherwise (..., error). func getTSMdata(colVals map[string]interface{}) (*datastore.TSMetadata, error) { - tsMData := datastore.TSMetadata{} tp := reflect.ValueOf(&tsMData) @@ -117,12 +117,10 @@ func getTSMdata(colVals map[string]interface{}) (*datastore.TSMetadata, error) { return &tsMData, nil } -// scanTsMdata scans the current row in rows and converts the result into a TSMetadata object. -// It is assumed that cols contains exactly the columns (names, types, order) that were used for -// the query that resulted in rows. +// scanTsRow scans columns cols from the current result row in rows and converts to a TSMetadata +// object. // Returns (TSMetadata object, nil) upon success, otherwise (..., error). -func scanTsMdata(rows *sql.Rows, cols []string) (*datastore.TSMetadata, error) { - +func scanTsRow(rows *sql.Rows, cols []string) (*datastore.TSMetadata, error) { colVals0 := make([]interface{}, len(cols)) // column values colValPtrs := make([]interface{}, len(cols)) // pointers to column values for i := range colVals0 { @@ -238,9 +236,9 @@ func getTSAttrGroupsIncInstances( currInstances := []*datastore.TSMetadata{} // initial current instance set for rows.Next() { // extract tsMdata from current result row - tsMdata, err := scanTsMdata(rows, allCols) + tsMdata, err := scanTsRow(rows, allCols) if err != nil { - return nil, fmt.Errorf("scanTsMdata() failed: %v", err) + return nil, fmt.Errorf("scanTsRow() failed: %v", err) } if len(currInstances) > 0 { // check if we should create a new current instance set @@ -304,13 +302,12 @@ func getTSAttrGroupsComboOnly(db *sql.DB, cols []string) ([]*datastore.TSMdataGr // aggregate rows into groups for rows.Next() { // extract tsMdata from current result row - tsMdata, err := scanTsMdata(rows, cols) + tsMdata, err := scanTsRow(rows, cols) if err != nil { - return nil, fmt.Errorf("scanTsMdata() failed: %v", err) + return nil, fmt.Errorf("scanTsRow() failed: %v", err) } - // add new group with tsMData as the combo (and leaving the instances - // array unset) + // add new group with tsMData as the combo (and leaving the Instances array unset) groups = append(groups, &datastore.TSMdataGroup{Combo: tsMdata}) } diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index 7741f9a..bf08f5e 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -42,7 +42,6 @@ func initPutObsLimit() { // getTSColVals gets the time series metadata column values from tsMdata. // Returns (column values, nil) upon success, otherwise (..., error). func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { - colVals := []interface{}{} // --- BEGIN non-string metadata --------------------------- From e632f788dcd6de38cc8f6eb37d9aab369cc3b403 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 19 Dec 2023 09:23:08 +0100 Subject: [PATCH 14/18] Fixed typo --- .../datastore/storagebackend/postgresql/getobservations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 0db467f..52d2ac0 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -70,7 +70,7 @@ func addStringMdata(rv reflect.Value, stringMdataGoNames []string, colVals []int field := rv.Elem().FieldByName(goName) - // NOTE: we assume the following assignemnt will never panic, hence we don't do + // NOTE: we assume the following assignment will never panic, hence we don't do // any pre-validation of field field.SetString(val) } From 67705352de6b56c48fd356f96d9a591812d1f797 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 19 Dec 2023 09:59:22 +0100 Subject: [PATCH 15/18] Improved documentation --- datastore/datastore/protobuf/datastore.proto | 35 ++++++++++++------- .../postgresql/getobservations.go | 10 +++--- .../storagebackend/postgresql/postgresql.go | 2 +- .../postgresql/putobservations.go | 2 +- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/datastore/datastore/protobuf/datastore.proto b/datastore/datastore/protobuf/datastore.proto index eb920dc..5f3296d 100644 --- a/datastore/datastore/protobuf/datastore.proto +++ b/datastore/datastore/protobuf/datastore.proto @@ -76,7 +76,7 @@ message TSMetadata { repeated Link links = 1; // --- END non-string metadata ----------------- - // --- BEGIN string metadata ----------------- + // --- BEGIN string metadata (handleable with reflection) ----------------- string version = 2; string type = 3; string title = 4; @@ -115,7 +115,7 @@ message ObsMetadata { google.protobuf.Timestamp pubtime = 5; // --- END non-string metadata ----------------- - // --- BEGIN string metadata ----------------- + // --- BEGIN string metadata (handleable with reflection) ----------------- string id = 6; string data_id = 7 [json_name = "data_id"]; string history = 8; @@ -152,14 +152,29 @@ message PutObsResponse { //--------------------------------------------------------------------------- message GetObsRequest { - // --- BEGIN special handling of temporal and spatial search ----------------- + // --- BEGIN non-string metadata ------------------------- + + // temporal search TimeInterval interval = 1; // only return observations in this time range + + // spatial search Polygon inside = 2; // if specified, only return observations in this area - // --- END special handling of temporal and spatial search ----------------- - // --- BEGIN general handling of strings; field names must correspond exactly with string field names in TSMetadata or ObsMetadata ----- - // - if the field F is specified (where F is for example 'platform'), only observations matching at least one these values for F will be returned - // - if the field F is not specified, filtering on F is effectively disabled + // search wrt. TSMetadata.links + // TODO - needs special handling + + // --- END non-string metadata ------------------------- + + + // --- BEGIN string metadata (handleable with reflection) ------------------------- + // + // - field names must correspond exactly with string field names in TSMetadata or ObsMetadata + // + // - if the field F is specified (where F is for example 'platform'), only observations + // matching at least one these values for F will be returned + // + // - if the field F is not specified, filtering on F is effectively disabled + repeated string version = 3; repeated string type = 4; repeated string title = 5; @@ -187,11 +202,7 @@ message GetObsRequest { repeated string history = 27; repeated string metadata_id = 28 [json_name = "metadata_id"]; repeated string processing_level = 29 [json_name = "processing_level"]; - // --- END general handling of strings ----- - - // --- BEGIN special handling of 'repeated Link' ------ - // TODO - // --- END special handling of 'repeated Link' ------ + // --- END string metadata ------------------------------------- } message GetObsResponse { diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index 52d2ac0..a439c13 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -125,7 +125,7 @@ func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { &linkTitle, } - // complete colValPtrs with string metadata + // complete colValPtrs with string metadata (handleable with reflection) colVals0 := make([]interface{}, len(tsStringMdataGoNames)) for i := range tsStringMdataGoNames { colValPtrs = append(colValPtrs, &colVals0[i]) @@ -151,7 +151,7 @@ func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { Links: links, } - // complete tsMdata with string metadata + // complete tsMdata with string metadata (handleable with reflection) err := addStringMdata(reflect.ValueOf(&tsMdata), tsStringMdataGoNames, colVals0) if err != nil { return nil, -1, fmt.Errorf("addStringMdata() failed: %v", err) @@ -220,7 +220,7 @@ type stringFilterInfo struct { colName string patterns []string } -// TODO: add filter infos for non-string types +// TODO: add filter info for non-string types // getMdataFilter derives from stringFilterInfos the expression used in a WHERE clause for // "match any" filtering on a set of attributes. @@ -403,7 +403,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { &point, } - // complete colValPtrs with string metadata + // complete colValPtrs with string metadata (handleable with reflection) colVals0 := make([]interface{}, len(obsStringMdataGoNames)) for i := range obsStringMdataGoNames { colValPtrs = append(colValPtrs, &colVals0[i]) @@ -429,7 +429,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { Value: value, } - // complete obsMdata with string metadata + // complete obsMdata with string metadata (handleable with reflection) err := addStringMdata(reflect.ValueOf(&obsMdata), obsStringMdataGoNames, colVals0) if err != nil { return nil, -1, fmt.Errorf("addStringMdata() failed: %v", err) diff --git a/datastore/datastore/storagebackend/postgresql/postgresql.go b/datastore/datastore/storagebackend/postgresql/postgresql.go index 271abc5..7b7197f 100644 --- a/datastore/datastore/storagebackend/postgresql/postgresql.go +++ b/datastore/datastore/storagebackend/postgresql/postgresql.go @@ -102,7 +102,7 @@ func getTSMdataCols() []string { "link_title", } - // complete cols with string metadata + // complete cols with string metadata (handleable with reflection) cols = append(cols, tsStringMdataPBNames...) return cols diff --git a/datastore/datastore/storagebackend/postgresql/putobservations.go b/datastore/datastore/storagebackend/postgresql/putobservations.go index bf08f5e..9066d16 100644 --- a/datastore/datastore/storagebackend/postgresql/putobservations.go +++ b/datastore/datastore/storagebackend/postgresql/putobservations.go @@ -79,7 +79,7 @@ func getTSColVals(tsMdata *datastore.TSMetadata) ([]interface{}, error) { // --- END non-string metadata --------------------------- - // --- BEGIN string metadata --------------------------- + // --- BEGIN string metadata (handleable with reflection) --------------------------- rv := reflect.ValueOf(tsMdata) for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.TSMetadata{})) { From bb5924280ac78e360452354d20bf675c7992edb0 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 19 Dec 2023 12:18:54 +0100 Subject: [PATCH 16/18] Fixed go-fmt issues --- .../postgresql/getobservations.go | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/datastore/datastore/storagebackend/postgresql/getobservations.go b/datastore/datastore/storagebackend/postgresql/getobservations.go index a439c13..b4b0394 100644 --- a/datastore/datastore/storagebackend/postgresql/getobservations.go +++ b/datastore/datastore/storagebackend/postgresql/getobservations.go @@ -29,7 +29,7 @@ var ( // - Protobuf names are snake case (aaa_bbb) whereas Go names are camel case (aaaBbb or AaaBbb). obsStringMdataGoNames []string // Go names for observation metadata of type string - obsStringMdataCols []string // column names qualified with table name 'observation' + obsStringMdataCols []string // column names qualified with table name 'observation' ) func init() { @@ -48,7 +48,7 @@ func init() { obsStringMdataCols = []string{} for _, field := range reflect.VisibleFields(reflect.TypeOf(datastore.ObsMetadata{})) { if field.IsExported() && (field.Type.Kind() == reflect.String) && - (strings.ToLower(field.Name) != "value") { // (obs value not considered metadata here) + (strings.ToLower(field.Name) != "value") { // (obs value not considered metadata here) goName := field.Name pbName := common.ToSnakeCase(goName) obspb2go[pbName] = goName @@ -107,12 +107,12 @@ func addWhereCondMatchAnyPattern( // Returns (TSMetadata object, time series ID, nil) upon success, otherwise (..., ..., error). func scanTSRow(rows *sql.Rows) (*datastore.TSMetadata, int64, error) { var ( - tsID int64 - linkHref pq.StringArray - linkRel pq.StringArray - linkType pq.StringArray + tsID int64 + linkHref pq.StringArray + linkRel pq.StringArray + linkType pq.StringArray linkHrefLang pq.StringArray - linkTitle pq.StringArray + linkTitle pq.StringArray ) // initialize colValPtrs with non-string metadata @@ -220,6 +220,7 @@ type stringFilterInfo struct { colName string patterns []string } + // TODO: add filter info for non-string types // getMdataFilter derives from stringFilterInfos the expression used in a WHERE clause for @@ -296,9 +297,9 @@ func getGeoFilter(inside *datastore.Polygon, phVals *[]interface{}) (string, err } type stringFieldInfo struct { - field reflect.StructField - tableName string - method reflect.Value + field reflect.StructField + tableName string + method reflect.Value methodName string } @@ -324,9 +325,9 @@ func getStringMdataFilter( mtd := rv.MethodByName(mtdName) if field.IsExported() && (field.Type.Kind() == reflect.String) && (mtd.IsValid()) { stringFieldInfos = append(stringFieldInfos, stringFieldInfo{ - field: field, - tableName: tableName, - method: mtd, + field: field, + tableName: tableName, + method: mtd, methodName: mtdName, }) } @@ -426,7 +427,7 @@ func scanObsRow(rows *sql.Rows) (*datastore.ObsMetadata, int64, error) { ObstimeInstant: timestamppb.New(obsTimeInstant0), }, Pubtime: timestamppb.New(pubTime0), - Value: value, + Value: value, } // complete obsMdata with string metadata (handleable with reflection) From 707ddce0f8e2fd83d58813a4d06bd375c4e0196c Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 19 Dec 2023 12:30:39 +0100 Subject: [PATCH 17/18] Ported client code to new gRPC API Attribute fields now use singular form (instruments -> instrument) for consistency and simplification (in particular when using reflection). --- datastore/api/main.py | 6 +++--- datastore/integration-test/test_knmi.py | 12 ++++++------ datastore/load-test/locustfile.py | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/datastore/api/main.py b/datastore/api/main.py index 2313b9f..f23ce70 100644 --- a/datastore/api/main.py +++ b/datastore/api/main.py @@ -192,7 +192,7 @@ def get_locations(bbox: str = Query(..., example="5.0,52.0,6.0,52.1")) -> Featur with grpc.insecure_channel(f"{os.getenv('DSHOST', 'localhost')}:{os.getenv('DSPORT', '50050')}") as channel: grpc_stub = dstore_grpc.DatastoreStub(channel) ts_request = dstore.GetObsRequest( - instruments=["tn"], # Hack + instrument=["tn"], # Hack inside=dstore.Polygon(points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords]), ) ts_response = grpc_stub.GetObservations(ts_request) @@ -227,8 +227,8 @@ def get_data_location_id( # This is just a quick and dirty demo range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( - platforms=[location_id], - instruments=list(map(str.strip, parameter_name.split(","))), + platform=[location_id], + instrument=list(map(str.strip, parameter_name.split(","))), interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, ) return get_data_for_time_series(get_obs_request) diff --git a/datastore/integration-test/test_knmi.py b/datastore/integration-test/test_knmi.py index 31e965a..dfe500a 100644 --- a/datastore/integration-test/test_knmi.py +++ b/datastore/integration-test/test_knmi.py @@ -20,7 +20,7 @@ def grpc_stub(): def test_find_series_single_station_single_parameter(grpc_stub): - request = dstore.GetObsRequest(platforms=["06260"], instruments=["rh"]) + request = dstore.GetObsRequest(platform=["06260"], instrument=["rh"]) response = grpc_stub.GetObservations(request) assert len(response.observations) == 1 @@ -30,21 +30,21 @@ def test_find_series_single_station_single_parameter(grpc_stub): def test_find_series_all_stations_single_parameter(grpc_stub): - request = dstore.GetObsRequest(instruments=["rh"]) + request = dstore.GetObsRequest(instrument=["rh"]) response = grpc_stub.GetObservations(request) assert len(response.observations) == 46 # Not all station have RH def test_find_series_single_station_all_parameters(grpc_stub): - request = dstore.GetObsRequest(platforms=["06260"]) + request = dstore.GetObsRequest(platform=["06260"]) response = grpc_stub.GetObservations(request) assert len(response.observations) == 42 # Station 06260 doesn't have all parameters def test_get_values_single_station_single_parameter(grpc_stub): - ts_request = dstore.GetObsRequest(platforms=["06260"], instruments=["rh"]) + ts_request = dstore.GetObsRequest(platform=["06260"], instrument=["rh"]) response = grpc_stub.GetObservations(ts_request) assert len(response.observations) == 1 @@ -60,7 +60,7 @@ def test_get_values_single_station_single_parameter_one_hour(grpc_stub): end_datetime.FromDatetime(datetime(2022, 12, 31, 12)) ts_request = dstore.GetObsRequest( - platforms=["06260"], instruments=["rh"], interval=dstore.TimeInterval(start=start_datetime, end=end_datetime) + platform=["06260"], instrument=["rh"], interval=dstore.TimeInterval(start=start_datetime, end=end_datetime) ) response = grpc_stub.GetObservations(ts_request) @@ -148,7 +148,7 @@ def test_get_values_single_station_single_parameter_one_hour(grpc_stub): @pytest.mark.parametrize("coords,param_ids,expected_station_ids", input_params_polygon) def test_get_observations_with_polygon(grpc_stub, coords, param_ids, expected_station_ids): polygon = dstore.Polygon(points=[dstore.Point(lat=lat, lon=lon) for lat, lon in coords]) - get_obs_request = dstore.GetObsRequest(inside=polygon, instruments=param_ids) + get_obs_request = dstore.GetObsRequest(inside=polygon, instrument=param_ids) get_obs_response = grpc_stub.GetObservations(get_obs_request) actual_station_ids = sorted({ts.ts_mdata.platform for ts in get_obs_response.observations}) diff --git a/datastore/load-test/locustfile.py b/datastore/load-test/locustfile.py index 81e966f..3bb4892 100644 --- a/datastore/load-test/locustfile.py +++ b/datastore/load-test/locustfile.py @@ -44,8 +44,8 @@ def get_data_for_single_timeserie(self): request = dstore.GetObsRequest( interval=dstore.TimeInterval(start=from_time, end=to_time), - platforms=[random.choice(stations)], - instruments=[random.choice(parameters)], + platform=[random.choice(stations)], + instrument=[random.choice(parameters)], ) response = self.stub.GetObservations(request) assert len(response.observations) == 1 @@ -63,7 +63,7 @@ def get_data_single_station_through_bbox(self): request = dstore.GetObsRequest( interval=dstore.TimeInterval(start=from_time, end=to_time), - instruments=[random.choice(parameters)], + instrument=[random.choice(parameters)], inside=dstore.Polygon(points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords]), ) response = self.stub.GetObservations(request) From 6675f691f939d7fd9124e84e91577781896a8185 Mon Sep 17 00:00:00 2001 From: Jo Asplin Date: Tue, 19 Dec 2023 12:49:09 +0100 Subject: [PATCH 18/18] Ported client code to new gRPC API --- datastore/api/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datastore/api/main.py b/datastore/api/main.py index f23ce70..5287a0d 100644 --- a/datastore/api/main.py +++ b/datastore/api/main.py @@ -266,7 +266,7 @@ def get_data_area( assert poly.geom_type == "Polygon" range = get_datetime_range(datetime) get_obs_request = dstore.GetObsRequest( - instruments=list(map(str.strip, parameter_name.split(","))), + instrument=list(map(str.strip, parameter_name.split(","))), inside=dstore.Polygon(points=[dstore.Point(lat=coord[1], lon=coord[0]) for coord in poly.exterior.coords]), interval=dstore.TimeInterval(start=range[0], end=range[1]) if range else None, )