chore: some refactoring of postgres code (#1166)

Move a few functions around for consistency, renamed files. No actual
functionality changes. Some logging output would produce slightly
different results because of the module name changes.

martin/src/pg/configurator.rs → martin/src/pg/builder.rs

@@ -9,12 +9,10 @@ use crate::args::BoundsCalcType;
 use crate::pg::config::{PgConfig, PgInfo};
 use crate::pg::config_function::{FuncInfoSources, FunctionInfo};
 use crate::pg::config_table::{TableInfo, TableInfoSources};
-use crate::pg::function_source::{merge_func_info, query_available_function};
 use crate::pg::pg_source::{PgSource, PgSqlInfo};
 use crate::pg::pool::PgPool;
-use crate::pg::table_source::{
-    calc_srid, merge_table_info, query_available_tables, table_to_query,
-};
+use crate::pg::query_functions::query_available_function;
+use crate::pg::query_tables::{query_available_tables, table_to_query};
 use crate::pg::utils::{find_info, find_kv_ignore_case, normalize_key, InfoMap};
 use crate::pg::PgError::InvalidTableExtent;
 use crate::pg::{PgCfgPublish, PgCfgPublishFuncs, PgResult};
@@ -26,6 +24,20 @@ use crate::OptBoolObj::{Bool, NoValue, Object};
 pub type SqlFuncInfoMapMap = InfoMap<InfoMap<(PgSqlInfo, FunctionInfo)>>;
 pub type SqlTableInfoMapMapMap = InfoMap<InfoMap<InfoMap<TableInfo>>>;
 
+/// A builder for creating a set of sources from a Postgres database
+#[derive(Debug)]
+pub struct PgBuilder {
+    pool: PgPool,
+    default_srid: Option<i32>,
+    auto_bounds: BoundsCalcType,
+    max_feature_count: Option<usize>,
+    auto_functions: Option<PgBuilderFuncs>,
+    auto_tables: Option<PgBuilderTables>,
+    id_resolver: IdResolver,
+    tables: TableInfoSources,
+    functions: FuncInfoSources,
+}
+
 #[derive(Debug, PartialEq)]
 #[cfg_attr(test, serde_with::skip_serializing_none, derive(serde::Serialize))]
 pub struct PgBuilderFuncs {
@@ -45,19 +57,6 @@ pub struct PgBuilderTables {
     extent: Option<u32>,
 }
 
-#[derive(Debug)]
-pub struct PgBuilder {
-    pool: PgPool,
-    default_srid: Option<i32>,
-    auto_bounds: BoundsCalcType,
-    max_feature_count: Option<usize>,
-    auto_functions: Option<PgBuilderFuncs>,
-    auto_tables: Option<PgBuilderTables>,
-    id_resolver: IdResolver,
-    tables: TableInfoSources,
-    functions: FuncInfoSources,
-}
-
 /// Combine `from_schema` field from the `config.auto_publish` and `config.auto_publish.tables/functions`
 macro_rules! get_auto_schemas {
     ($config:expr, $typ:ident) => {
@@ -140,8 +139,7 @@ impl PgBuilder {
             let dup = if dup { "duplicate " } else { "" };
 
             let id2 = self.resolve_id(id, cfg_inf);
-            let Some(merged_inf) = merge_table_info(self.default_srid, &id2, cfg_inf, db_inf)
-            else {
+            let Some(merged_inf) = db_inf.append_cfg_info(cfg_inf, &id2, self.default_srid) else {
                 continue;
             };
             warn_on_rename(id, &id2, "Table");
@@ -184,9 +182,7 @@ impl PgBuilder {
                             .replace("{table}", &table)
                             .replace("{column}", &geom_column);
                         let id2 = self.resolve_id(&source_id, &db_inf);
-                        let Some(srid) =
-                            calc_srid(&db_inf.format_id(), &id2, db_inf.srid, 0, self.default_srid)
-                        else {
+                        let Some(srid) = db_inf.calc_srid(&id2, 0, self.default_srid) else {
                             continue;
                         };
                         db_inf.srid = srid;
@@ -243,7 +239,7 @@ impl PgBuilder {
                 continue;
             };
 
-            let merged_inf = merge_func_info(cfg_inf, db_inf);
+            let merged_inf = db_inf.append_cfg_info(cfg_inf);
 
             let dup = !used.insert((&cfg_inf.schema, func_name));
             let dup = if dup { "duplicate " } else { "" };

martin/src/pg/config.rs

@@ -8,9 +8,9 @@ use tilejson::TileJSON;
 
 use crate::args::{BoundsCalcType, DEFAULT_BOUNDS_TIMEOUT};
 use crate::config::{copy_unrecognized_config, UnrecognizedValues};
+use crate::pg::builder::PgBuilder;
 use crate::pg::config_function::FuncInfoSources;
 use crate::pg::config_table::TableInfoSources;
-use crate::pg::configurator::PgBuilder;
 use crate::pg::utils::on_slow;
 use crate::pg::PgResult;
 use crate::source::TileInfoSources;

martin/src/pg/config_function.rs

@@ -83,3 +83,15 @@ impl PgInfo for FunctionInfo {
         patch_json(tilejson, self.tilejson.as_ref())
     }
 }
+
+impl FunctionInfo {
+    /// For a given function info discovered from the database, append the configuration info provided by the user
+    #[must_use]
+    pub fn append_cfg_info(&self, cfg_inf: &FunctionInfo) -> FunctionInfo {
+        FunctionInfo {
+            // TileJson does not need to be merged because it cannot be de-serialized from config
+            tilejson: self.tilejson.clone(),
+            ..cfg_inf.clone()
+        }
+    }
+}

martin/src/pg/config_table.rs

@@ -1,11 +1,12 @@
 use std::collections::{BTreeMap, HashMap};
 
+use log::{info, warn};
 use serde::{Deserialize, Serialize};
 use tilejson::{Bounds, TileJSON, VectorLayer};
 
 use crate::config::UnrecognizedValues;
 use crate::pg::config::PgInfo;
-use crate::pg::utils::{patch_json, InfoMap};
+use crate::pg::utils::{normalize_key, patch_json, InfoMap};
 
 pub type TableInfoSources = InfoMap<TableInfo>;
 
@@ -103,3 +104,86 @@ impl PgInfo for TableInfo {
         patch_json(tilejson, self.tilejson.as_ref())
     }
 }
+
+impl TableInfo {
+    /// For a given table info discovered from the database, append the configuration info provided by the user
+    #[must_use]
+    pub fn append_cfg_info(
+        &self,
+        cfg_inf: &TableInfo,
+        new_id: &String,
+        default_srid: Option<i32>,
+    ) -> Option<Self> {
+        // Assume cfg_inf and self have the same schema/table/geometry_column
+        let mut inf = TableInfo {
+            // These values must match the database exactly
+            schema: self.schema.clone(),
+            table: self.table.clone(),
+            geometry_column: self.geometry_column.clone(),
+            // These values are not serialized, so copy auto-detected values from the database
+            geometry_index: self.geometry_index,
+            is_view: self.is_view,
+            tilejson: self.tilejson.clone(),
+            // Srid requires some logic
+            srid: self.calc_srid(new_id, cfg_inf.srid, default_srid)?,
+            prop_mapping: HashMap::new(),
+            ..cfg_inf.clone()
+        };
+
+        match (&self.geometry_type, &cfg_inf.geometry_type) {
+            (Some(src), Some(cfg)) if src != cfg => {
+                warn!(
+                    r#"Table {} has geometry type={src}, but source {new_id} has {cfg}"#,
+                    self.format_id()
+                );
+            }
+            _ => {}
+        }
+
+        let empty = BTreeMap::new();
+        let props = self.properties.as_ref().unwrap_or(&empty);
+
+        if let Some(id_column) = &cfg_inf.id_column {
+            let prop = normalize_key(props, id_column.as_str(), "id_column", new_id)?;
+            inf.prop_mapping.insert(id_column.clone(), prop);
+        }
+
+        if let Some(p) = &cfg_inf.properties {
+            for key in p.keys() {
+                let prop = normalize_key(props, key.as_str(), "property", new_id)?;
+                inf.prop_mapping.insert(key.clone(), prop);
+            }
+        }
+
+        Some(inf)
+    }
+
+    /// Determine the SRID value to use for a table, or None if unknown, assuming self is a table info from the database
+    #[must_use]
+    pub fn calc_srid(&self, new_id: &str, cfg_srid: i32, default_srid: Option<i32>) -> Option<i32> {
+        match (self.srid, cfg_srid, default_srid) {
+            (0, 0, Some(default_srid)) => {
+                info!(
+                    "Table {} has SRID=0, using provided default SRID={default_srid}",
+                    self.format_id()
+                );
+                Some(default_srid)
+            }
+            (0, 0, None) => {
+                let info = "To use this table source, set default or specify this table SRID in the config file, or set the default SRID with  --default-srid=...";
+                warn!("Table {} has SRID=0, skipping. {info}", self.format_id());
+                None
+            }
+            (0, cfg, _) => Some(cfg), // Use the configured SRID
+            (src, 0, _) => Some(src), // Use the source SRID
+            (src, cfg, _) if src != cfg => {
+                warn!(
+                    "Table {} has SRID={src}, but source {new_id} has SRID={cfg}",
+                    self.format_id()
+                );
+                None
+            }
+            (_, cfg, _) => Some(cfg),
+        }
+    }
+}

martin/src/pg/mod.rs

@@ -1,18 +1,18 @@
+mod builder;
 mod config;
 mod config_function;
 mod config_table;
-mod configurator;
 mod errors;
-mod function_source;
 mod pg_source;
 mod pool;
-mod table_source;
+mod query_functions;
+mod query_tables;
 mod tls;
 mod utils;
 
 pub use config::{PgCfgPublish, PgCfgPublishFuncs, PgCfgPublishTables, PgConfig, PgSslCerts};
 pub use config_function::FunctionInfo;
 pub use config_table::TableInfo;
 pub use errors::{PgError, PgResult};
-pub use function_source::query_available_function;
 pub use pool::{PgPool, POOL_SIZE_DEFAULT};
+pub use query_functions::query_available_function;

martin/src/pg/function_source.rs → martin/src/pg/query_functions.rs

@@ -5,8 +5,8 @@ use log::{debug, warn};
 use postgres_protocol::escape::escape_identifier;
 use serde_json::Value;
 
+use crate::pg::builder::SqlFuncInfoMapMap;
 use crate::pg::config_function::FunctionInfo;
-use crate::pg::configurator::SqlFuncInfoMapMap;
 use crate::pg::pg_source::PgSqlInfo;
 use crate::pg::pool::PgPool;
 use crate::pg::PgError::PostgresError;
@@ -62,7 +62,7 @@ pub async fn query_available_function(pool: &PgPool) -> PgResult<SqlFuncInfoMapM
 
             // Query preparation: the schema and function can't be part of a prepared query, so they
             // need to be escaped by hand.
-            // However schema and function comes from database introspection so they should be safe.
+            // However, schema and function comes from database introspection, so they should be safe.
             let mut query = String::new();
             query.push_str(&escape_identifier(&schema));
             query.push('.');
@@ -118,14 +118,6 @@ pub async fn query_available_function(pool: &PgPool) -> PgResult<SqlFuncInfoMapM
     Ok(res)
 }
 
-pub fn merge_func_info(cfg_inf: &FunctionInfo, db_inf: &FunctionInfo) -> FunctionInfo {
-    FunctionInfo {
-        // TileJson does not need to be merged because it cannot be de-serialized from config
-        tilejson: db_inf.tilejson.clone(),
-        ..cfg_inf.clone()
-    }
-}
-
 fn jsonb_to_vec(jsonb: Option<Value>) -> Option<Vec<String>> {
     jsonb.map(|json| {
         json.as_array()