small fixes and tweaks

README.md

@@ -4,7 +4,7 @@ Valve - A lightweight validation engine written in rust.
 
 ## Table of contents
 
-_Generated using [markdown-toc](https://github.com/jonschlinkert/markdown-toc)_
+_ToC Generated using [markdown-toc](https://github.com/jonschlinkert/markdown-toc)_
 
 * [Design and concepts](#design-and-concepts)
   + [The Valve database](#the-valve-database)
@@ -723,11 +723,11 @@ The columns of the rule table have the following significance:
 
 In some cases it is useful to be able to try and guess what the table table and column table configuration should be, using information about the current state of the Valve instance, for a given data table not currently managed by Valve. To do this one may use Valve's command line interface to run the **guess** subcommand as follows:
 
-    ontodev_valve guess [OPTIONS] SOURCE DESTINATION TABLE_TSV
+    ontodev_valve guess [OPTIONS] SOURCE DATABASE TABLE_TSV
 
 where:
 - `SOURCE` is the location of the '.tsv' representing the table table.
-- `DESTINATION` is the path to a PostgreSQL or SQLite database.
+- `DATABASE` is the path to a PostgreSQL or SQLite database.
 - `TABLE_TSV` is the '.tsv' file representing the data table whose column configuration is to be guessed.
 
 For the list of possible options, and for general information on Valve's command line interface, see [command line usage](#command-line-usage).

scripts/export_messages.py

@@ -88,7 +88,7 @@ def get_column_order_and_info_for_sqlite(cursor, table):
         for row in pragma_rows:
             if row["pk"] != 0:
                 primary_keys[row["pk"]] = row["name"]
-            elif not row["name"] not in ("row_number", "row_order"):
+            else:
                 non_pk_columns.append(row["name"])
         primary_keys = dict(sorted(primary_keys.items()))
         sorted_columns = [primary_keys[key] for key in primary_keys] + non_pk_columns

src/guess.rs

@@ -1,7 +1,7 @@
 //! Implementation of the column configuration guesser
 
 use crate::{
-    toolkit::{get_query_params, local_sql_syntax, QueryParam},
+    toolkit::{get_query_param, local_sql_syntax, QueryParam},
     valve::{Valve, ValveConfig, ValveDatatypeConfig},
     SQL_PARAM,
 };
@@ -668,12 +668,7 @@ pub fn annotate(
                         foreign.table, foreign.column, SQL_PARAM
                     ),
                 );
-                let param = get_query_params(&vec![json!(value)], &foreign.sql_type)
-                    .pop()
-                    .expect(&format!(
-                        "Could not determine query parameter for '{}'",
-                        value
-                    ));
+                let param = get_query_param(&json!(value), &foreign.sql_type);
                 let mut query = sqlx_query(&sql);
                 match param {
                     QueryParam::Integer(p) => query = query.bind(p),

src/toolkit.rs

@@ -211,7 +211,9 @@ pub fn get_column_for_label(
         .into())
     } else {
         // If we cannot find the column corresponding to the label, then we conclude that the label
-        // name we received was fact the column name, for a column that has no label:
+        // name we received was the column name of a column with an empty label. This is because an
+        // an empty label signifies, not that the column has no label, but that the column's label
+        // is the same as it's column name.
         Ok(label.to_string())
     }
 }
@@ -1185,9 +1187,7 @@ pub fn read_config_files(
                     for label_name in &defined_labels {
                         let column_name =
                             get_column_for_label(&this_column_config, label_name, &table_name)?;
-                        if !column_name.is_empty() {
-                            column_order.push(column_name);
-                        }
+                        column_order.push(column_name);
                     }
                 } else {
                     return Err(ValveError::ConfigError(format!("'{}' is empty", path)).into());
@@ -1661,7 +1661,7 @@ pub fn get_sql_for_text_view(
 /// reformatted (for instance using the function [local_sql_syntax()]) in accordance with the syntax
 /// accepted by the underlying database, and then bound before being executed by sqlx.
 /// These placeholders represent: (1) the column name and (2) the table name.
-pub fn query_column_with_message_value(column: &str, pool: &AnyPool) -> String {
+pub fn generic_select_with_message_value(column: &str, pool: &AnyPool) -> String {
     let is_clause = if pool.any_kind() == AnyKind::Sqlite {
         "IS"
     } else {
@@ -1715,7 +1715,7 @@ pub fn generic_select_with_message_values(
         .iter()
         .map(|column| {
             sql_params.append(&mut vec![column.to_string(), table.to_string()]);
-            query_column_with_message_value(column, pool)
+            generic_select_with_message_value(column, pool)
         })
         .collect::<Vec<_>>();
 
@@ -1781,11 +1781,11 @@ pub async fn get_affected_rows(
         ),
     );
     sql_params.push(value.to_string());
+
     let mut query = sqlx_query(&sql);
     for param in &sql_params {
         query = query.bind(param);
     }
-
     let mut valve_rows = vec![];
     for row in query.fetch_all(tx.acquire().await?).await? {
         let mut contents = IndexMap::new();
@@ -1934,9 +1934,6 @@ pub async fn get_db_value(
                  END AS "{column}"
                FROM "{table}_view" WHERE "row_number" = {row_number}
             "#,
-            column = column,
-            is_clause = is_clause,
-            row_number = row_number,
             casted_column = if pool.any_kind() == AnyKind::Sqlite {
                 cast_column_sql_to_text(column, "non-text")
             } else {
@@ -2353,17 +2350,10 @@ pub async fn record_row_change(
         .into());
     }
 
-    fn to_text(row: Option<&SerdeMap>, quoted: bool) -> String {
+    fn to_text(row: Option<&SerdeMap>) -> String {
         match row {
             None => "NULL".to_string(),
-            Some(r) => {
-                let inner = format!("{}", json!(r));
-                if !quoted {
-                    inner
-                } else {
-                    format!("'{}'", inner)
-                }
-            }
+            Some(r) => format!("{}", json!(r)),
         }
     }
 
@@ -2460,7 +2450,7 @@ pub async fn record_row_change(
                                 format_value(&new_value.to_string(), &numeric_re),
                             )),
                         );
-                        let column_summary = to_text(Some(&column_summary), false);
+                        let column_summary = to_text(Some(&column_summary));
                         summary.push(column_summary);
                     }
                 }
@@ -2470,27 +2460,27 @@ pub async fn record_row_change(
     }
 
     let summary = summarize(from, to)?;
-    let (from, to) = (to_text(from, false), to_text(to, false));
+    let (from, to) = (to_text(from), to_text(to));
     let mut params = vec![table];
     let from_param = {
         if from == "NULL" {
-            from.to_string()
+            from
         } else {
             params.push(&from);
             SQL_PARAM.to_string()
         }
     };
     let to_param = {
         if to == "NULL" {
-            to.to_string()
+            to
         } else {
             params.push(&to);
             SQL_PARAM.to_string()
         }
     };
     let summary_param = {
         if summary == "NULL" {
-            summary.to_string()
+            summary
         } else {
             params.push(&summary);
             SQL_PARAM.to_string()
@@ -3224,7 +3214,7 @@ pub async fn insert_new_row_tx(
             ),
         );
         let mut query = sqlx_query(&message_sql);
-        for param in vec![table, column, &value, level, rule, &message] {
+        for param in [table, column, &value, level, rule, &message] {
             query = query.bind(param);
         }
         query.execute(tx.acquire().await?).await?;
@@ -3751,36 +3741,34 @@ pub fn compile_condition(
     }
 }
 
-/// Given a list of [SerdeValue]s and the SQL type that they should all conform to, return
-/// a list of [QueryParam]s corresponding to each value, which can then be bound to an SQL string.
-pub fn get_query_params(values: &Vec<SerdeValue>, sql_type: &str) -> Vec<QueryParam> {
-    let mut param_values = vec![];
-
-    for value in values {
-        let param_value = value
-            .as_str()
-            .expect(&format!("'{}' is not a string", value));
-        if sql_type == "numeric" {
+/// Given a [SerdeValue] of the type [SerdeValue::String], as well as the SQL type that it will
+/// be converted to when it is inserted to the database (which can be a non-string), return a
+/// [QueryParam] corresponding to the value.
+pub fn get_query_param(value: &SerdeValue, sql_type: &str) -> QueryParam {
+    let param_value = value
+        .as_str()
+        .expect(&format!("'{}' is not a string", value));
+    match sql_type {
+        "numeric" => {
             let numeric_value: f64 = param_value
                 .parse()
                 .expect(&format!("{param_value} is not numeric"));
-            param_values.push(QueryParam::Numeric(numeric_value));
-        } else if sql_type == "integer" {
+            QueryParam::Numeric(numeric_value)
+        }
+        "integer" => {
             let integer_value: i32 = param_value
                 .parse()
                 .expect(&format!("{param_value} is not an integer"));
-            param_values.push(QueryParam::Integer(integer_value));
-        } else if sql_type == "real" {
+            QueryParam::Integer(integer_value)
+        }
+        "real" => {
             let real_value: f64 = param_value
                 .parse()
                 .expect(&format!("{param_value} is not a real"));
-            param_values.push(QueryParam::Real(real_value));
-        } else {
-            param_values.push(QueryParam::String(param_value.to_string()));
+            QueryParam::Real(real_value)
         }
+        _ => QueryParam::String(param_value.to_string()),
     }
-
-    param_values
 }
 
 /// Given the config map, the name of a datatype, and a database connection pool used to determine

src/validate.rs

@@ -4,7 +4,7 @@ use crate::{
     ast::Expression,
     toolkit::{
         cast_sql_param_from_text, get_column_value, get_column_value_as_string,
-        get_datatype_ancestors, get_query_params, get_sql_type_from_global_config,
+        get_datatype_ancestors, get_query_param, get_sql_type_from_global_config,
         get_table_options_from_config, get_value_type, is_sql_type_error, local_sql_syntax,
         ColumnRule, CompiledCondition, QueryAsIf, QueryAsIfKind, QueryParam, ValueType,
     },
@@ -328,7 +328,10 @@ pub async fn validate_rows_constraints(
                     .map(|_| SQL_PARAM.to_string())
                     .collect::<Vec<_>>()
                     .join(", ");
-                let param_values = get_query_params(&values, &sql_type);
+                let param_values = values
+                    .iter()
+                    .map(|value| get_query_param(value, &sql_type))
+                    .collect::<Vec<_>>();
 
                 // Foreign keys always correspond to columns with unique constraints so we do not
                 // need to use the keyword 'DISTINCT' when querying the normal version of the table:
@@ -457,7 +460,10 @@ pub async fn validate_rows_constraints(
                 .map(|_| SQL_PARAM.to_string())
                 .collect::<Vec<_>>()
                 .join(", ");
-            let param_values = get_query_params(&values, &sql_type);
+            let param_values = values
+                .iter()
+                .map(|value| get_query_param(value, &sql_type))
+                .collect::<Vec<_>>();
 
             let sql = local_sql_syntax(
                 pool,
@@ -1284,15 +1290,9 @@ pub async fn validate_cell_foreign_constraints(
                         as_if_clause, table, column, SQL_PARAM,
                     ),
                 );
-                let param = get_query_params(&vec![value.into()], &sql_type)
-                    .pop()
-                    .expect(&format!(
-                        "Could not determine query parameter for '{}'",
-                        value
-                    ));
                 let frows = {
                     let mut query = sqlx_query(&fsql);
-                    match param {
+                    match get_query_param(&json!(value), &sql_type) {
                         QueryParam::Integer(p) => query = query.bind(p),
                         QueryParam::Numeric(p) => query = query.bind(p),
                         QueryParam::Real(p) => query = query.bind(p),

src/valve.rs

@@ -1955,11 +1955,9 @@ impl Valve {
         if self.verbose {
             println!("Saving tables: {} ...", tables.join(", "));
         }
-        // Collect the paths and possibly the options of all of the tables that were requested to be
-        // saved:
+        // Build the query to get the path and possibly the options info from the table table for
+        // all of the tables that were requested to be saved:
         let options_enabled = self.column_enabled_in_db("table", "options").await?;
-
-        // Build the query to get the path and options info from the table table:
         let mut params = vec![];
         let sql_param_str = tables
             .iter()
@@ -2008,7 +2006,7 @@ impl Valve {
             let path = row.try_get::<&str, &str>("path").ok().unwrap_or_default();
             let path = match save_dir {
                 Some(save_dir) => {
-                    // If the table is not saveable it can still be saved to the saved_dir if it
+                    // If the table is not saveable it can still be saved to the saved_dir if one
                     // has been specified and if it is not identical to the table's configured path:
                     if !options.contains("save") {
                         let path_dir = Path::new(path)