From f2053b789f326b746168953b94e1fd53c0e2a151 Mon Sep 17 00:00:00 2001 From: lejy Date: Mon, 9 Nov 2026 12:27:20 +0100 Subject: [PATCH] feat: support column comments through driver capabilities --- .tabularium | 1 + README.md | 3 + src/handlers/column_comments.rs | 17 ++++++ src/handlers/column_comments_tests.rs | 19 +++++++ src/handlers/ddl.rs | 25 +++++++-- src/handlers/ddl_tests.rs | 71 ++++++++++++++++++++++++ src/models.rs | 2 + tests/live_db.rs | 80 +++++++++++++++++++++++++++ 8 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 src/handlers/column_comments.rs create mode 100644 src/handlers/column_comments_tests.rs diff --git a/.tabularium b/.tabularium index 8159789..7fa8814 100644 --- a/.tabularium +++ b/.tabularium @@ -74,6 +74,7 @@ "sql_dialect": "postgres", "alter_primary_key": true, "alter_column": true, + "column_comments": true, "create_foreign_keys": true, "manage_tables": true, "supports_ssl": true, diff --git a/README.md b/README.md index 8082b86..b3d618e 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,9 @@ both drivers against the same live database and compares every response. - **DDL Generation** — `CREATE TABLE`, `ADD COLUMN`, `ALTER COLUMN` (including implicit-cast-compatible `TYPE` changes), `CREATE INDEX`, `ADD CONSTRAINT FOREIGN KEY`, plus the corresponding drops. +- **Column Comments** — The `column_comments` capability enables descriptions in + Add/Modify Column. Comment edits generate `COMMENT ON COLUMN`; clearing a + description removes it with `IS NULL`, without restating the column definition. - **View & Trigger Lifecycle** — Create/alter/drop views, create/drop triggers, refresh materialized views. - **BLOB Support** — Export a `bytea` column to a file or preview it as a diff --git a/src/handlers/column_comments.rs b/src/handlers/column_comments.rs new file mode 100644 index 0000000..10f2eae --- /dev/null +++ b/src/handlers/column_comments.rs @@ -0,0 +1,17 @@ +pub(super) fn comment_sql(table_ref: &str, column: &str, comment: Option<&str>) -> String { + let value = match comment.filter(|value| !value.is_empty()) { + // Explicit escape strings also work when standard_conforming_strings is off. + Some(value) => format!("E'{}'", value.replace('\\', "\\\\").replace('\'', "''")), + None => "NULL".to_string(), + }; + format!( + "COMMENT ON COLUMN {}.\"{}\" IS {}", + table_ref, + column.replace('"', "\"\""), + value + ) +} + +#[cfg(test)] +#[path = "column_comments_tests.rs"] +mod tests; diff --git a/src/handlers/column_comments_tests.rs b/src/handlers/column_comments_tests.rs new file mode 100644 index 0000000..4550a87 --- /dev/null +++ b/src/handlers/column_comments_tests.rs @@ -0,0 +1,19 @@ +use super::comment_sql; + +#[test] +fn quotes_identifiers_and_preserves_comment_text() { + assert_eq!( + comment_sql("\"s\".\"t\"", "col\"umn", Some(" O'Brien\\path\n日本語 ")), + "COMMENT ON COLUMN \"s\".\"t\".\"col\"\"umn\" IS E' O''Brien\\\\path\n日本語 '" + ); +} + +#[test] +fn clearing_a_comment_uses_null() { + for comment in [None, Some("")] { + assert_eq!( + comment_sql("\"public\".\"t\"", "c", comment), + "COMMENT ON COLUMN \"public\".\"t\".\"c\" IS NULL" + ); + } +} diff --git a/src/handlers/ddl.rs b/src/handlers/ddl.rs index 27d4a3d..61acca5 100644 --- a/src/handlers/ddl.rs +++ b/src/handlers/ddl.rs @@ -10,6 +10,9 @@ use serde_json::Value; +#[path = "column_comments.rs"] +mod column_comments; + use crate::client; use crate::models::{inner_params, ColumnDefinition, ConnectionParams}; use crate::rpc::{error_response, ok_response}; @@ -46,10 +49,17 @@ pub async fn get_add_column_sql(id: Value, params: &Value) -> Value { .and_then(|v| serde_json::from_value(v.clone()).ok()); match column { - Some(column) => ok_response( - id, - Value::from(vec![build_add_column_sql(table, &column, schema)]), - ), + Some(column) => { + let mut statements = vec![build_add_column_sql(table, &column, schema)]; + if let Some(comment) = column.comment.as_deref().filter(|value| !value.is_empty()) { + statements.push(column_comments::comment_sql( + &qualified(schema, table), + &column.name, + Some(comment), + )); + } + ok_response(id, Value::from(statements)) + } None => error_response(id, -32602, "Invalid params: missing or malformed 'column'"), } } @@ -352,6 +362,13 @@ fn build_alter_column_sql( } } + if old_column.comment.as_deref().unwrap_or("") != new_column.comment.as_deref().unwrap_or("") { + stmts.push(column_comments::comment_sql( + &tbl, + &new_column.name, + new_column.comment.as_deref(), + )); + } if stmts.is_empty() { return Err("No changes detected".to_string()); } diff --git a/src/handlers/ddl_tests.rs b/src/handlers/ddl_tests.rs index 5549b78..b5ff2e0 100644 --- a/src/handlers/ddl_tests.rs +++ b/src/handlers/ddl_tests.rs @@ -8,6 +8,70 @@ use super::{ }; use crate::models::ColumnDefinition; +#[tokio::test] +async fn column_comments_are_supported_by_the_rpc_contract() { + let manifest: serde_json::Value = + serde_json::from_str(include_str!("../../.tabularium")).unwrap(); + assert_eq!(manifest["capabilities"]["column_comments"], true); + let definition = serde_json::json!({ + "name": "email", "data_type": "TEXT", "is_nullable": false, + "is_pk": false, "is_auto_increment": false, "default_value": "'unknown'" + }); + for (old, new, literal) in [ + ( + None, + Some("O'Brien\\path\n日本語"), + "E'O''Brien\\\\path\n日本語'", + ), + (Some("old"), Some("new"), "E'new'"), + (Some("old"), None, "NULL"), + ] { + let mut old_column = definition.clone(); + old_column["comment"] = serde_json::json!(old); + let mut new_column = definition.clone(); + new_column["comment"] = serde_json::json!(new); + let result = super::get_alter_column_sql(serde_json::json!(1), &serde_json::json!({ + "table": "customers", "schema": "sales", "old_column": old_column, "new_column": new_column + })).await; + assert_eq!( + result["result"], + serde_json::json!([format!( + "COMMENT ON COLUMN \"sales\".\"customers\".\"email\" IS {literal}" + )]) + ); + } + let mut added = definition; + added["comment"] = serde_json::json!("Contact"); + let result = super::get_add_column_sql( + serde_json::json!(1), + &serde_json::json!({ + "table": "customers", "column": added + }), + ) + .await; + assert_eq!(result["result"].as_array().unwrap().len(), 2); + assert_eq!( + result["result"][1], + "COMMENT ON COLUMN \"public\".\"customers\".\"email\" IS E'Contact'" + ); +} + +#[test] +fn unchanged_comments_are_not_rewritten_and_renames_use_the_new_name() { + let mut old = column("email", "TEXT"); + old.comment = Some("existing".into()); + let mut new = old.clone(); + new.name = "contact\"address".into(); + let renamed = build_alter_column_sql("customers", &old, &new, "sales").unwrap(); + assert_eq!(renamed.len(), 1); + new.comment = Some("new".into()); + let updated = build_alter_column_sql("customers", &old, &new, "sales").unwrap(); + assert_eq!( + updated[1], + "COMMENT ON COLUMN \"sales\".\"customers\".\"contact\"\"address\" IS E'new'" + ); +} + fn column(name: &str, data_type: &str) -> ColumnDefinition { ColumnDefinition { name: name.to_string(), @@ -16,6 +80,7 @@ fn column(name: &str, data_type: &str) -> ColumnDefinition { is_pk: false, is_auto_increment: false, default_value: None, + comment: None, } } @@ -32,6 +97,7 @@ mod create_table { is_pk: true, is_auto_increment: true, default_value: None, + comment: None, }, column("name", "TEXT"), ]; @@ -50,6 +116,7 @@ mod create_table { is_pk: true, is_auto_increment: true, default_value: Some("1".to_string()), + comment: None, }]; let sql = build_create_table_sql("t", &columns, "public"); // is_auto_increment suppresses both NOT NULL and DEFAULT even though @@ -67,6 +134,7 @@ mod create_table { is_pk: true, is_auto_increment: true, default_value: None, + comment: None, }]; let sql = build_create_table_sql("t", &columns, "public"); assert!(sql.contains("\"id\" BIGSERIAL")); @@ -81,6 +149,7 @@ mod create_table { is_pk: false, is_auto_increment: false, default_value: None, + comment: None, }]; let sql = build_create_table_sql("t", &columns, "public"); assert!(sql.contains("\"name\" TEXT NOT NULL")); @@ -95,6 +164,7 @@ mod create_table { is_pk: false, is_auto_increment: false, default_value: Some("'unknown@example.com'".to_string()), + comment: None, }]; let sql = build_create_table_sql("t", &columns, "public"); assert!(sql.contains("DEFAULT 'unknown@example.com'")); @@ -120,6 +190,7 @@ mod add_column { is_pk: false, is_auto_increment: false, default_value: Some("0".to_string()), + comment: None, }; let sql = build_add_column_sql("all_types", &col, "test_schema"); assert!(sql diff --git a/src/models.rs b/src/models.rs index c9aae50..e2422fe 100644 --- a/src/models.rs +++ b/src/models.rs @@ -90,6 +90,8 @@ pub struct ColumnDefinition { pub is_pk: bool, pub is_auto_increment: bool, pub default_value: Option, + #[serde(default)] + pub comment: Option, } /// Mirrors `crate::models::RoutineCallArg` on the host — one argument of a diff --git a/tests/live_db.rs b/tests/live_db.rs index a9f7a3b..a050507 100644 --- a/tests/live_db.rs +++ b/tests/live_db.rs @@ -30,6 +30,86 @@ use std::process::{Child, ChildStdin, Command, Stdio}; use serde_json::{json, Value}; +#[test] +fn column_comments_round_trip_through_the_plugin() { + let mut plugin = Plugin::spawn(); + let params = conn_params(); + let table = format!("live_comments_{}", uuid::Uuid::new_v4().simple()); + plugin.call_ok("execute_query", json!({"params": params, "query": format!("CREATE TABLE public.\"{table}\" (id SERIAL PRIMARY KEY, email TEXT NOT NULL DEFAULT 'unknown' UNIQUE)")})); + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let baseline = plugin.call_ok( + "get_columns", + json!({"params": params, "table": table, "schema": "public"}), + ); + let original = json!({"name": "email", "data_type": "TEXT", "is_nullable": false, "is_pk": false, + "is_auto_increment": false, "default_value": "'unknown'::text"}); + let mut old = original.clone(); + for text in [Some("Customer's email\\path\n日本語"), Some("new"), None] { + let mut new = original.clone(); + new["comment"] = json!(text); + let sql = plugin.call_ok( + "get_alter_column_sql", + json!({"table": table, "schema": "public", "old_column": old, "new_column": new}), + ); + assert_eq!(sql.as_array().unwrap().len(), 1); + for statement in sql.as_array().unwrap() { + plugin.call_ok( + "execute_query", + json!({"params": params, "query": statement}), + ); + } + let mut columns = plugin.call_ok( + "get_columns", + json!({"params": params, "table": table, "schema": "public"}), + ); + let email = columns + .as_array_mut() + .unwrap() + .iter_mut() + .find(|c| c["name"] == "email") + .unwrap(); + assert_eq!(email.get("comment").unwrap_or(&Value::Null), &json!(text)); + email.as_object_mut().unwrap().remove("comment"); + assert_eq!(columns, baseline); + old = new; + } + let mut added = original; + added["name"] = json!("notes"); + added["comment"] = json!("New column's note"); + let sql = plugin.call_ok( + "get_add_column_sql", + json!({"table": table, "column": added}), + ); + assert_eq!(sql.as_array().unwrap().len(), 2); + for statement in sql.as_array().unwrap() { + plugin.call_ok( + "execute_query", + json!({"params": params, "query": statement}), + ); + } + let columns = plugin.call_ok( + "get_columns", + json!({"params": params, "table": table, "schema": "public"}), + ); + assert_eq!( + columns + .as_array() + .unwrap() + .iter() + .find(|c| c["name"] == "notes") + .unwrap()["comment"], + "New column's note" + ); + })); + plugin.call_ok( + "execute_query", + json!({"params": params, "query": format!("DROP TABLE public.\"{table}\"")}), + ); + if let Err(panic) = outcome { + std::panic::resume_unwind(panic); + } +} + fn env_or(key: &str, default: &str) -> String { std::env::var(key).unwrap_or_else(|_| default.to_string()) }