diff --git a/CHANGELOG.md b/CHANGELOG.md index d73bd5a..e618bf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased - Foreign keys + +- Reference DDL: add the missing foreign keys (conversation tree, usage, data audit, + budget accounting, profile skills) with orphan quarantine and their indexes; make + `assistant_model.provider_id` `ON DELETE CASCADE`. A Postgres guardrail test fails + any declared relation without a foreign key. + ## Unreleased - Durable turns - Renew user-owned run leases with heartbeats, poll cross-replica Stop, enforce diff --git a/UPGRADE.md b/UPGRADE.md index 939bc94..6fbdc09 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,33 @@ # Upgrading qbit-assistant +## Foreign keys (2026-10-14) + +Recommended: apply the "Foreign keys" block at the end of +`docs/schema-postgres.sql` (after the index audit). It is guarded and safe to re-run. +- Adds the missing foreign keys across the conversation tables, usage, data audit, + budget accounting and profile skills, with the indexes they need. +- Detail rows of a conversation `CASCADE`; the message/turn cycle, usage and the data + audit use `SET NULL`, so usage keeps its cost rows when a conversation is deleted. +- Usage, budget accounting and the data audit hold their provider (`NO ACTION`): + disable a provider with history instead of deleting it. +- `assistant_model.provider_id` changes from `NO ACTION` to `CASCADE` + (constraint renamed `fk_assistant_model_provider`). +- Rows that point at a missing parent are first copied to `qqq_fk_quarantine` + (created if absent), then nulled, or deleted for the `NOT NULL` profile-skill + links. Check that table after the upgrade. + +No code change. + +## Index audit (2026-10-14) + +Recommended, not required: apply the "Index audit" block at the end of +`docs/schema-postgres.sql`. It adds six indexes, every one `IF NOT EXISTS`: +- usage by turn (read on each scheduled run) +- the three retention date filters (turn events, tool calls, inactive conversations) +- the two `ON DELETE SET NULL` foreign keys on `assistant_scheduled_run` + +No code change. + ## Durable turns (Unreleased) Apply the **Durable turns** block at the end of `docs/schema-postgres.sql` before diff --git a/docs/schema-postgres.sql b/docs/schema-postgres.sql index 46a3ab7..c6bccf1 100644 --- a/docs/schema-postgres.sql +++ b/docs/schema-postgres.sql @@ -592,3 +592,112 @@ CREATE INDEX assistant_turn_user_status ON assistant_turn (user_id, status); -- Check for pre-existing duplicate message.end rows before applying this index. CREATE UNIQUE INDEX assistant_turn_event_terminal ON assistant_turn_event (turn_id) WHERE event_type = 'message.end'; + +-- Index audit (2026-10-14): lookup, retention and FK paths with no leading index. +-- ScheduledTurn.cost reads usage by turn on every scheduled run. +CREATE INDEX IF NOT EXISTS assistant_usage_turn ON assistant_usage (turn_id); +-- Retention: deleteTurnEventsBefore, clearToolResultsBefore and listConversationsInactiveSince filter by date. +CREATE INDEX IF NOT EXISTS assistant_turn_event_create_date ON assistant_turn_event (create_date); +CREATE INDEX IF NOT EXISTS assistant_tool_call_create_date ON assistant_tool_call (create_date); +CREATE INDEX IF NOT EXISTS assistant_conversation_modify_date ON assistant_conversation (modify_date); +-- ON DELETE SET NULL children: deleting a conversation or turn would scan scheduled runs without these. +CREATE INDEX IF NOT EXISTS assistant_scheduled_run_conversation ON assistant_scheduled_run (conversation_id); +CREATE INDEX IF NOT EXISTS assistant_scheduled_run_turn ON assistant_scheduled_run (turn_id); + +-- Foreign keys (2026-10-14). Run on new and existing databases; every step is guarded +-- and re-runnable. The rules follow how the qbit deletes: +-- * AssistantStore.deleteConversation (also the retention purge) deletes turn events, +-- tool calls, feedback, messages, turns, data audit and pending actions, then the +-- conversation. The conversation's own detail rows CASCADE, so an admin delete of a +-- conversation cleans up the same way. +-- * The message <-> turn cycle (turn.user_message_id / assistant_message_id and +-- message.turn_id) and the other optional pointers are SET NULL. Rows are inserted +-- parent first, so nothing is deferred. +-- * assistant_usage keeps its cost rows when a conversation goes (SET NULL), and the +-- data audit trail is never cascaded into (SET NULL). +-- * Provider references from usage, budget accounting and the data audit are NO ACTION +-- (like assistant_turn.provider_id): disable a provider with history, do not delete +-- it. A provider's model rows are its own (CASCADE). +-- * assistant_budget_usage.usage_id has no foreign key on purpose: the projection +-- outlives its usage row so budgets keep counting the spend. +-- Rows that already point at a missing parent are recorded in qqq_fk_quarantine first, +-- then nulled (nullable columns) or deleted with the full row recorded (NOT NULL). +CREATE INDEX IF NOT EXISTS assistant_message_parent_message ON assistant_message (parent_message_id); +CREATE INDEX IF NOT EXISTS assistant_message_turn ON assistant_message (turn_id); +CREATE INDEX IF NOT EXISTS assistant_turn_user_message ON assistant_turn (user_message_id); +CREATE INDEX IF NOT EXISTS assistant_turn_assistant_message ON assistant_turn (assistant_message_id); +CREATE INDEX IF NOT EXISTS assistant_turn_provider ON assistant_turn (provider_id); +CREATE INDEX IF NOT EXISTS assistant_tool_call_pending_action ON assistant_tool_call (pending_action_id); +CREATE INDEX IF NOT EXISTS assistant_feedback_message ON assistant_feedback (message_id); +CREATE INDEX IF NOT EXISTS assistant_pending_action_turn ON assistant_pending_action (turn_id); +CREATE INDEX IF NOT EXISTS assistant_data_audit_provider ON assistant_data_audit (provider_id); +CREATE INDEX IF NOT EXISTS assistant_budget_usage_provider ON assistant_budget_usage (provider_id); +CREATE INDEX IF NOT EXISTS assistant_profile_skill_skill ON assistant_profile_skill (skill_id); +CREATE INDEX IF NOT EXISTS assistant_budget_downgrade_model ON assistant_budget (downgrade_model_id); +CREATE INDEX IF NOT EXISTS assistant_conversation_scheduled_task ON assistant_conversation (scheduled_task_id); +CREATE INDEX IF NOT EXISTS assistant_model_fallback_model ON assistant_model (fallback_model_id); +CREATE INDEX IF NOT EXISTS assistant_profile_simple_model ON assistant_profile (simple_model_id); +CREATE INDEX IF NOT EXISTS assistant_profile_complex_model ON assistant_profile (complex_model_id); +CREATE INDEX IF NOT EXISTS assistant_profile_classifier_model ON assistant_profile (classifier_model_id); +CREATE INDEX IF NOT EXISTS assistant_rag_source_embedding_model ON assistant_rag_source (embedding_model_id); +CREATE INDEX IF NOT EXISTS assistant_scheduled_task_profile ON assistant_scheduled_task (profile_id); + +CREATE TABLE IF NOT EXISTS qqq_fk_quarantine (id BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, table_name VARCHAR(100) NOT NULL, column_name VARCHAR(100) NOT NULL, row_id VARCHAR(100), missing_parent_id VARCHAR(100), action VARCHAR(20) NOT NULL, row_data TEXT, changeset VARCHAR(200), quarantined_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP); + +DO $$ +DECLARE + r RECORD; + old TEXT; +BEGIN + FOR r IN SELECT * FROM (VALUES + ('assistant_message', 'conversation_id', 'assistant_conversation', 'CASCADE', TRUE), + ('assistant_message', 'parent_message_id', 'assistant_message', 'SET NULL', TRUE), + ('assistant_message', 'turn_id', 'assistant_turn', 'SET NULL', TRUE), + ('assistant_turn', 'conversation_id', 'assistant_conversation', 'CASCADE', TRUE), + ('assistant_turn', 'user_message_id', 'assistant_message', 'SET NULL', TRUE), + ('assistant_turn', 'assistant_message_id', 'assistant_message', 'SET NULL', TRUE), + ('assistant_turn_event', 'turn_id', 'assistant_turn', 'CASCADE', TRUE), + ('assistant_tool_call', 'conversation_id', 'assistant_conversation', 'CASCADE', TRUE), + ('assistant_tool_call', 'turn_id', 'assistant_turn', 'CASCADE', TRUE), + ('assistant_tool_call', 'pending_action_id', 'assistant_pending_action', 'SET NULL', TRUE), + ('assistant_feedback', 'conversation_id', 'assistant_conversation', 'CASCADE', TRUE), + ('assistant_feedback', 'message_id', 'assistant_message', 'CASCADE', TRUE), + ('assistant_pending_action', 'conversation_id', 'assistant_conversation', 'CASCADE', TRUE), + ('assistant_pending_action', 'turn_id', 'assistant_turn', 'SET NULL', TRUE), + ('assistant_data_audit', 'conversation_id', 'assistant_conversation', 'SET NULL', TRUE), + ('assistant_data_audit', 'turn_id', 'assistant_turn', 'SET NULL', TRUE), + ('assistant_usage', 'conversation_id', 'assistant_conversation', 'SET NULL', TRUE), + ('assistant_usage', 'turn_id', 'assistant_turn', 'SET NULL', TRUE), + ('assistant_usage', 'provider_id', 'assistant_provider', 'NO ACTION', TRUE), + ('assistant_data_audit', 'provider_id', 'assistant_provider', 'NO ACTION', TRUE), + ('assistant_budget_usage', 'provider_id', 'assistant_provider', 'NO ACTION', TRUE), + ('assistant_profile_skill', 'profile_id', 'assistant_profile', 'CASCADE', FALSE), + ('assistant_profile_skill', 'skill_id', 'assistant_skill', 'CASCADE', FALSE) + ) AS v(child, col, parent, rule, nullable) + LOOP + IF NOT EXISTS (SELECT 1 FROM information_schema.table_constraints WHERE constraint_schema = current_schema() AND LOWER(constraint_name) = 'fk_' || r.child || '_' || r.col) THEN + IF r.nullable THEN + EXECUTE format('INSERT INTO qqq_fk_quarantine (table_name, column_name, row_id, missing_parent_id, action, changeset) ' + || 'SELECT %L, %L, c.id::text, c.%I::text, ''NULLED'', ''qbit-assistant 2026-10-14 foreign keys'' FROM %I c ' + || 'WHERE c.%I IS NOT NULL AND NOT EXISTS (SELECT 1 FROM %I p WHERE p.id = c.%I)', r.child, r.col, r.col, r.child, r.col, r.parent, r.col); + EXECUTE format('UPDATE %I c SET %I = NULL WHERE c.%I IS NOT NULL AND NOT EXISTS (SELECT 1 FROM %I p WHERE p.id = c.%I)', r.child, r.col, r.col, r.parent, r.col); + ELSE + EXECUTE format('INSERT INTO qqq_fk_quarantine (table_name, column_name, row_id, missing_parent_id, action, row_data, changeset) ' + || 'SELECT %L, %L, c.id::text, c.%I::text, ''DELETED'', row_to_json(c)::text, ''qbit-assistant 2026-10-14 foreign keys'' FROM %I c ' + || 'WHERE NOT EXISTS (SELECT 1 FROM %I p WHERE p.id = c.%I)', r.child, r.col, r.col, r.child, r.parent, r.col); + EXECUTE format('DELETE FROM %I c WHERE NOT EXISTS (SELECT 1 FROM %I p WHERE p.id = c.%I)', r.child, r.parent, r.col); + END IF; + EXECUTE format('ALTER TABLE %I ADD CONSTRAINT %I FOREIGN KEY (%I) REFERENCES %I (id) ON DELETE %s', r.child, 'fk_' || r.child || '_' || r.col, r.col, r.parent, r.rule); + END IF; + END LOOP; + + -- assistant_model.provider_id: the inline NO ACTION key becomes CASCADE, named as on kof22. + IF NOT EXISTS (SELECT 1 FROM information_schema.table_constraints WHERE constraint_schema = current_schema() AND LOWER(constraint_name) = 'fk_assistant_model_provider') THEN + FOR old IN SELECT conname FROM pg_constraint + WHERE contype = 'f' AND conrelid = 'assistant_model'::regclass AND confrelid = 'assistant_provider'::regclass + LOOP + EXECUTE format('ALTER TABLE assistant_model DROP CONSTRAINT %I', old); + END LOOP; + ALTER TABLE assistant_model ADD CONSTRAINT fk_assistant_model_provider FOREIGN KEY (provider_id) REFERENCES assistant_provider (id) ON DELETE CASCADE; + END IF; +END $$; diff --git a/qbit-assistant/pom.xml b/qbit-assistant/pom.xml index 45eb1d0..b74292e 100644 --- a/qbit-assistant/pom.xml +++ b/qbit-assistant/pom.xml @@ -87,6 +87,25 @@ 4.33.0 test + + + org.testcontainers + junit-jupiter + 1.21.4 + test + + + org.testcontainers + postgresql + 1.21.4 + test + + + org.postgresql + postgresql + 42.7.14 + test + diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java index 081c77e..f5bcea4 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/AssistantDefaultsChangelogTest.java @@ -345,6 +345,11 @@ class AssistantDefaultsChangelogTest extends BaseAssistantTest { for(String sql : statements(ddl)) { + if(sql.startsWith("DO $$")) + { + // the foreign-key block is PL/pgSQL (Postgres only); SchemaForeignKeyPostgresTest runs it on real Postgres + continue; + } Matcher m = Pattern.compile("^(CREATE TABLE|ALTER TABLE) (assistant_[a-z_]+)\\b(.*)", Pattern.DOTALL).matcher(sql); if(m.find() && DDL_TABLES.contains(m.group(2)) && (m.group(1).startsWith("CREATE") || m.group(3).strip().startsWith("ADD COLUMN"))) { @@ -420,7 +425,8 @@ class AssistantDefaultsChangelogTest extends BaseAssistantTest /******************************************************************************* - ** SQL statements, without comment lines, split at a ';' that ends a line. + ** SQL statements, without comment lines, split at a ';' that ends a line; + ** a DO $$ ... $$ block stays one statement. *******************************************************************************/ static List statements(String sql) { @@ -433,14 +439,36 @@ class AssistantDefaultsChangelogTest extends BaseAssistantTest text.append(stripped).append("\n"); } } - List rs = new ArrayList<>(); + List rs = new ArrayList<>(); + StringBuilder block = null; for(String statement : text.toString().split(";\\s*\\n")) { + String stripped = statement.strip(); + if(block == null && stripped.startsWith("DO $$") && !stripped.endsWith("$$")) + { + // a dollar-quoted block has its own semicolons: keep it whole up to its closing $$ + block = new StringBuilder(stripped); + continue; + } + if(block != null) + { + block.append(";\n").append(stripped); + if(stripped.endsWith("$$")) + { + rs.add(block.toString()); + block = null; + } + continue; + } if(!statement.isBlank()) { - rs.add(statement.strip().replaceAll(";$", "")); + rs.add(stripped.replaceAll(";$", "")); } } + if(block != null) + { + rs.add(block.toString()); + } return (rs); } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/persistence/SchemaForeignKeyPostgresTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/persistence/SchemaForeignKeyPostgresTest.java new file mode 100644 index 0000000..a830c86 --- /dev/null +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/persistence/SchemaForeignKeyPostgresTest.java @@ -0,0 +1,387 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.persistence; + + +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import com.kingsrook.qbits.assistant.testing.BaseAssistantTest; +import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; +import com.kingsrook.qqq.backend.core.model.metadata.joins.JoinOn; +import com.kingsrook.qqq.backend.core.model.metadata.joins.QJoinMetaData; +import com.kingsrook.qqq.backend.core.model.metadata.possiblevalues.QPossibleValueSource; +import com.kingsrook.qqq.backend.core.model.metadata.possiblevalues.QPossibleValueSourceType; +import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; +import static org.assertj.core.api.Assertions.assertThat; + + +/******************************************************************************* + ** Guardrail for the reference DDL (docs/schema-postgres.sql) on a real + ** PostgreSQL: every relation the qbit declares (a table possible-value source + ** on an assistant_ field, or a join between assistant_ tables), plus the + ** relations only the code knows, has a foreign key with the intended ON DELETE + ** rule, and every foreign key column leads an index. A new relation without a + ** foreign key fails here unless it is added to EXEMPT with a reason. + *******************************************************************************/ +@Testcontainers(disabledWithoutDocker = true) +class SchemaForeignKeyPostgresTest extends BaseAssistantTest +{ + @Container + static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg16").asCompatibleSubstituteFor("postgres")); + + ///////////////////////////////////////////////////////////////////////// + // relations the meta-data does not declare (no possible-value source) // + ///////////////////////////////////////////////////////////////////////// + private static final List CODE_ONLY = List.of( + new Relation("assistant_data_audit", "turn_id", "assistant_turn"), + new Relation("assistant_data_audit", "provider_id", "assistant_provider"), + new Relation("assistant_budget_usage", "provider_id", "assistant_provider")); + + ////////////////////////////////////////////////////////////////// + // relations that deliberately have no foreign key, with reason // + ////////////////////////////////////////////////////////////////// + private static final Map EXEMPT = Map.of( + new Relation("assistant_budget_usage", "usage_id", "assistant_usage"), + "budget accounting projection: it must outlive its usage row so budgets keep counting the spend"); + + /////////////////////////////////////////////////////////////////////////// + // the intended ON DELETE rule (pg confdeltype: a NO ACTION, c CASCADE, // + // n SET NULL) - see the foreign key block in docs/schema-postgres.sql // + /////////////////////////////////////////////////////////////////////////// + private static final Map RULES = new LinkedHashMap<>(); + + static + { + rule("assistant_message", "conversation_id", "assistant_conversation", "c"); + rule("assistant_message", "parent_message_id", "assistant_message", "n"); + rule("assistant_message", "turn_id", "assistant_turn", "n"); + rule("assistant_turn", "conversation_id", "assistant_conversation", "c"); + rule("assistant_turn", "user_message_id", "assistant_message", "n"); + rule("assistant_turn", "assistant_message_id", "assistant_message", "n"); + rule("assistant_turn_event", "turn_id", "assistant_turn", "c"); + rule("assistant_tool_call", "conversation_id", "assistant_conversation", "c"); + rule("assistant_tool_call", "turn_id", "assistant_turn", "c"); + rule("assistant_tool_call", "pending_action_id", "assistant_pending_action", "n"); + rule("assistant_feedback", "conversation_id", "assistant_conversation", "c"); + rule("assistant_feedback", "message_id", "assistant_message", "c"); + rule("assistant_pending_action", "conversation_id", "assistant_conversation", "c"); + rule("assistant_pending_action", "turn_id", "assistant_turn", "n"); + rule("assistant_data_audit", "conversation_id", "assistant_conversation", "n"); + rule("assistant_data_audit", "turn_id", "assistant_turn", "n"); + rule("assistant_data_audit", "provider_id", "assistant_provider", "a"); + rule("assistant_usage", "conversation_id", "assistant_conversation", "n"); + rule("assistant_usage", "turn_id", "assistant_turn", "n"); + rule("assistant_usage", "provider_id", "assistant_provider", "a"); + rule("assistant_budget_usage", "provider_id", "assistant_provider", "a"); + rule("assistant_profile_skill", "profile_id", "assistant_profile", "c"); + rule("assistant_profile_skill", "skill_id", "assistant_skill", "c"); + rule("assistant_model", "provider_id", "assistant_provider", "c"); + rule("assistant_scheduled_run", "conversation_id", "assistant_conversation", "n"); + rule("assistant_scheduled_run", "turn_id", "assistant_turn", "n"); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + record Relation(String child, String column, String parent) + { + @Override + public String toString() + { + return (child + "." + column + " -> " + parent); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private static void rule(String child, String column, String parent, String rule) + { + RULES.put(new Relation(child, column, parent), rule); + } + + + + /******************************************************************************* + ** Apply the whole reference DDL, then its foreign key block a second time + ** (it must be re-runnable on an existing database). + *******************************************************************************/ + @BeforeAll + static void applyDdl() throws Exception + { + String ddl = Files.readString(Path.of("..", "docs", "schema-postgres.sql")); + try(Connection connection = connect(); Statement statement = connection.createStatement()) + { + statement.execute(ddl); + int start = ddl.indexOf("-- Foreign keys (2026-10-14)"); + if(start >= 0) + { + statement.execute(ddl.substring(start)); + } + } + } + + + + /******************************************************************************* + ** Every declared or code-only relation has a foreign key, unless exempt. + *******************************************************************************/ + @Test + void everyRelationHasAForeignKey() throws Exception + { + Set relations = new LinkedHashSet<>(declaredRelations()); + assertThat(relations).as("declared relations found in the meta-data").hasSizeGreaterThan(20); + relations.addAll(CODE_ONLY); + + Map foreignKeys = foreignKeys(); + List missing = new ArrayList<>(); + for(Relation relation : relations) + { + if(!foreignKeys.containsKey(relation) && !EXEMPT.containsKey(relation)) + { + missing.add(relation.toString()); + } + } + assertThat(missing).as("relations without a foreign key (add one to docs/schema-postgres.sql, or to EXEMPT with a reason)").isEmpty(); + + for(Relation exempt : EXEMPT.keySet()) + { + assertThat(foreignKeys).as("exempt relation " + exempt + " must stay without a foreign key").doesNotContainKey(exempt); + } + } + + + + /******************************************************************************* + ** The ON DELETE rules match how the qbit deletes. + *******************************************************************************/ + @Test + void deleteRulesMatchTheDeletePaths() throws Exception + { + Map foreignKeys = foreignKeys(); + for(Map.Entry entry : RULES.entrySet()) + { + assertThat(foreignKeys.get(entry.getKey())).as(entry.getKey().toString()).isEqualTo(entry.getValue()); + } + } + + + + /******************************************************************************* + ** Every foreign key column is the first column of some index. + *******************************************************************************/ + @Test + void everyForeignKeyColumnLeadsAnIndex() throws Exception + { + List unindexed = new ArrayList<>(); + try(Connection connection = connect(); Statement statement = connection.createStatement(); + ResultSet rs = statement.executeQuery(""" + SELECT c.conrelid::regclass::text, a.attname + FROM pg_constraint c + JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = c.conkey[1] + WHERE c.contype = 'f' AND c.connamespace = 'public'::regnamespace + AND NOT EXISTS (SELECT 1 FROM pg_index i WHERE i.indrelid = c.conrelid AND i.indkey[0] = c.conkey[1]) + """)) + { + while(rs.next()) + { + unindexed.add(rs.getString(1) + "." + rs.getString(2)); + } + } + assertThat(unindexed).isEmpty(); + } + + + + /******************************************************************************* + ** AssistantStore.deleteConversation's delete order works under the foreign + ** keys; usage (cost accounting) survives with its pointers nulled. A plain + ** delete of a conversation (admin screen) cascades through the + ** message <-> turn cycle and keeps usage and the data audit. + *******************************************************************************/ + @Test + void conversationDeletesWorkUnderTheForeignKeys() throws Exception + { + try(Connection connection = connect(); Statement statement = connection.createStatement()) + { + connection.setAutoCommit(false); + for(int c : new int[] { 1, 2 }) + { + int m = c * 10; + statement.execute("INSERT INTO assistant_provider (id, name, provider_type) VALUES (" + c + ", 'p" + c + "', 'anthropic')"); + statement.execute("INSERT INTO assistant_conversation (id, user_id) VALUES (" + c + ", 'u')"); + statement.execute("INSERT INTO assistant_message (id, conversation_id, user_id, role) VALUES (" + m + ", " + c + ", 'u', 'USER')"); + statement.execute("INSERT INTO assistant_turn (id, conversation_id, user_id, user_message_id, provider_id) VALUES (" + c + ", " + c + ", 'u', " + m + ", NULL)"); + statement.execute("INSERT INTO assistant_message (id, conversation_id, user_id, role, turn_id, parent_message_id) VALUES (" + (m + 1) + ", " + c + ", 'u', 'ASSISTANT', " + c + ", " + m + ")"); + statement.execute("UPDATE assistant_message SET turn_id = " + c + " WHERE id = " + m); + statement.execute("UPDATE assistant_turn SET assistant_message_id = " + (m + 1) + " WHERE id = " + c); + statement.execute("INSERT INTO assistant_turn_event (turn_id, user_id) VALUES (" + c + ", 'u')"); + statement.execute("INSERT INTO assistant_pending_action (id, conversation_id, turn_id, user_id) VALUES (" + c + ", " + c + ", " + c + ", 'u')"); + statement.execute("INSERT INTO assistant_tool_call (conversation_id, turn_id, pending_action_id, user_id) VALUES (" + c + ", " + c + ", " + c + ", 'u')"); + statement.execute("INSERT INTO assistant_feedback (conversation_id, message_id, user_id) VALUES (" + c + ", " + (m + 1) + ", 'u')"); + statement.execute("INSERT INTO assistant_data_audit (id, conversation_id, turn_id, provider_id, user_id) VALUES (" + c + ", " + c + ", " + c + ", " + c + ", 'u')"); + statement.execute("INSERT INTO assistant_usage (id, conversation_id, turn_id, provider_id, user_id) VALUES (" + c + ", " + c + ", " + c + ", " + c + ", 'u')"); + } + + //////////////////////////////////////////////////////////// + // conversation 1: AssistantStore.deleteConversation order // + //////////////////////////////////////////////////////////// + statement.execute("DELETE FROM assistant_turn_event WHERE turn_id IN (SELECT id FROM assistant_turn WHERE conversation_id = 1)"); + for(String table : List.of("assistant_tool_call", "assistant_feedback", "assistant_message", "assistant_turn", "assistant_data_audit", "assistant_pending_action")) + { + statement.execute("DELETE FROM " + table + " WHERE conversation_id = 1"); + } + statement.execute("DELETE FROM assistant_conversation WHERE id = 1"); + assertThat(queryString(statement, "SELECT conversation_id || '/' || turn_id FROM assistant_usage WHERE id = 1")).as("usage kept, pointers nulled").isNull(); + assertThat(queryString(statement, "SELECT provider_id::text FROM assistant_usage WHERE id = 1")).isEqualTo("1"); + + ////////////////////////////////////////////// + // conversation 2: a plain conversation delete // + ////////////////////////////////////////////// + statement.execute("DELETE FROM assistant_conversation WHERE id = 2"); + for(String table : List.of("assistant_message", "assistant_turn", "assistant_tool_call", "assistant_feedback", "assistant_pending_action")) + { + assertThat(queryString(statement, "SELECT COUNT(*)::text FROM " + table + " WHERE conversation_id = 2")).as(table).isEqualTo("0"); + } + assertThat(queryString(statement, "SELECT COUNT(*)::text FROM assistant_turn_event")).isEqualTo("0"); + assertThat(queryString(statement, "SELECT (conversation_id IS NULL AND turn_id IS NULL)::text FROM assistant_data_audit WHERE id = 2")).as("data audit kept").isEqualTo("true"); + assertThat(queryString(statement, "SELECT (conversation_id IS NULL AND turn_id IS NULL)::text FROM assistant_usage WHERE id = 2")).as("usage kept").isEqualTo("true"); + connection.rollback(); + } + } + + + + /******************************************************************************* + ** Possible-value-source fields and joins between assistant_ tables. + *******************************************************************************/ + private List declaredRelations() + { + List relations = new ArrayList<>(); + for(QTableMetaData table : qInstance.getTables().values()) + { + if(!table.getName().startsWith("assistant_")) + { + continue; + } + for(QFieldMetaData field : table.getFields().values()) + { + QPossibleValueSource pvs = field.getPossibleValueSourceName() == null ? null : qInstance.getPossibleValueSource(field.getPossibleValueSourceName()); + if(pvs != null && pvs.getType() == QPossibleValueSourceType.TABLE && pvs.getTableName().startsWith("assistant_")) + { + relations.add(new Relation(table.getName(), snake(field.getName()), pvs.getTableName())); + } + } + } + for(QJoinMetaData join : qInstance.getJoins().values()) + { + if(!join.getLeftTable().startsWith("assistant_") || !join.getRightTable().startsWith("assistant_")) + { + continue; + } + for(JoinOn on : join.getJoinOns()) + { + if("id".equals(on.getLeftField())) + { + relations.add(new Relation(join.getRightTable(), snake(on.getRightField()), join.getLeftTable())); + } + else if("id".equals(on.getRightField())) + { + relations.add(new Relation(join.getLeftTable(), snake(on.getLeftField()), join.getRightTable())); + } + } + } + return (relations); + } + + + + /******************************************************************************* + ** Single-column foreign keys in the database, with their delete rule. + *******************************************************************************/ + private static Map foreignKeys() throws Exception + { + Map foreignKeys = new LinkedHashMap<>(); + try(Connection connection = connect(); Statement statement = connection.createStatement(); + ResultSet rs = statement.executeQuery(""" + SELECT c.conrelid::regclass::text, a.attname, c.confrelid::regclass::text, c.confdeltype + FROM pg_constraint c + JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = c.conkey[1] + WHERE c.contype = 'f' AND array_length(c.conkey, 1) = 1 AND c.connamespace = 'public'::regnamespace + """)) + { + while(rs.next()) + { + foreignKeys.put(new Relation(rs.getString(1), rs.getString(2), rs.getString(3)), rs.getString(4)); + } + } + return (foreignKeys); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private static String queryString(Statement statement, String sql) throws Exception + { + try(ResultSet rs = statement.executeQuery(sql)) + { + return (rs.next() ? rs.getString(1) : null); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private static String snake(String camel) + { + return (camel.replaceAll("([A-Z])", "_$1").toLowerCase()); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private static Connection connect() throws Exception + { + return (DriverManager.getConnection(POSTGRES.getJdbcUrl(), POSTGRES.getUsername(), POSTGRES.getPassword())); + } +}