add statement table and add script categorise challenges and solutions - #14
add statement table and add script categorise challenges and solutions#14rohxnn wants to merge 2 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| matched_id = await conn.fetchval( | ||
| """ | ||
| SELECT id | ||
| FROM statements | ||
| WHERE raw_statement = $1 | ||
| AND id != $2 | ||
| LIMIT 1 | ||
| """, | ||
| cleaned, new_id | ||
| ) |
There was a problem hiding this comment.
@rohxnn could you please check this :
1. The query is actually Case-Sensitive
Your docstring mentions: checks whether an identical statement (case-insensitive) already exists.
However, the = operator in PostgreSQL is case-sensitive.
Fix: You should use ILIKE or LOWER() to ensure it's truly case-insensitive.
2. Missing ORDER BY (Non-Deterministic)
You are using LIMIT 1 without an ORDER BY clause. If there are multiple identical statements already in the database, Postgres will return an arbitrary row.
Fix: To maintain a proper hierarchy, you usually want to link to the original (oldest) statement. You should add an ORDER BY created_at ASC (or whatever your timestamp column is named).
3. Parent-Child Chaining
If you match against a statement that is already a duplicate (it has a parent_id), your new statement will point to a child instead of the root parent. This creates deep chains of duplicates instead of a flat hierarchy.
Fix: It's usually best to ensure you are linking to a root statement by adding AND parent_id IS NULL.
4. Performance (Missing Index)
If the statements table grows large, scanning the raw_statement text column will become very slow.
Fix: You should ensure you have an index on this column. Since we want a case-insensitive match, an expression index is best:
CREATE INDEX idx_statements_raw_lower ON statements (LOWER(raw_statement));
Recommended Approach
Here is the improved version of your query incorporating these best practices:
# Look for the original existing statement with exactly the same text (case-insensitive)
matched_id = await conn.fetchval(
"""
SELECT id
FROM statements
WHERE LOWER(raw_statement) = LOWER($1)
AND id != $2
AND parent_id IS NULL -- Only match against root statements to prevent chaining
ORDER BY created_at ASC -- Guarantee we get the oldest/original statement
LIMIT 1
""",
cleaned, new_id
)Alternative (Check before insert):
Currently, you insert the statement and then look for duplicates. If performance is a strict concern, you could run the SELECT first. If a match is found, you immediately insert the new statement with the parent_id already set, rather than doing an INSERT, then a SELECT, then an UPDATE.
| ) | ||
|
|
||
| # Extract challenges for story submission into statements table | ||
| raw_story_challenges = data.get("challenges") or data.get("challenge") |
There was a problem hiding this comment.
Could you please check the exact key name? Is it challenges or challenge? Please keep it consistent and use only one.
| CREATE TABLE statements ( | ||
| id UUID PRIMARY KEY DEFAULT gen_random_uuid(), | ||
|
|
||
| submission_id TEXT NOT NULL, | ||
| tenant_code TEXT NOT NULL, | ||
|
|
||
| submission_type TEXT NOT NULL, | ||
| statement_type TEXT NOT NULL, | ||
| raw_statement TEXT NOT NULL, | ||
|
|
||
| parent_id UUID, | ||
|
|
||
| created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, | ||
| updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, | ||
|
|
||
| FOREIGN KEY (submission_id, tenant_code) | ||
| REFERENCES submissions(submission_id, tenant_code) | ||
| ON DELETE CASCADE, | ||
|
|
||
| FOREIGN KEY (parent_id) | ||
| REFERENCES statements(id) | ||
| ON DELETE SET NULL | ||
| ); | ||
|
|
||
| -- Index for fast exact-match deduplication. | ||
| CREATE INDEX idx_statements_raw_statement | ||
| ON statements (raw_statement); |
There was a problem hiding this comment.
@rohxnn could you please check this logic :
-- Automatically promote a duplicate child to be the new parent if a parent is deleted
CREATE OR REPLACE FUNCTION promote_statement_child()
RETURNS TRIGGER AS $$
DECLARE
new_parent_id UUID;
BEGIN
-- Only act if the statement being deleted is a root (parent)
IF OLD.parent_id IS NULL THEN
-- Find one child to become the new parent (the oldest duplicate)
SELECT id INTO new_parent_id
FROM statements
WHERE parent_id = OLD.id
ORDER BY created_at ASC
LIMIT 1;
IF new_parent_id IS NOT NULL THEN
-- Make the chosen child a root (new parent)
UPDATE statements
SET parent_id = NULL
WHERE id = new_parent_id;
-- Repoint all remaining children to the new parent
UPDATE statements
SET parent_id = new_parent_id
WHERE parent_id = OLD.id AND id != new_parent_id;
END IF;
END IF;
RETURN OLD;
END;
$$ LANGUAGE plpgsql;
DROP TRIGGER IF EXISTS trg_promote_statement_child ON statements;
CREATE TRIGGER trg_promote_statement_child
BEFORE DELETE ON statements
FOR EACH ROW
EXECUTE FUNCTION promote_statement_child();it's a BEFORE DELETE trigger, it fires instantly whenever a deletion is attempted on a statement.
If the statement being deleted is the original parent (i.e. OLD.parent_id IS NULL), it automatically finds the oldest duplicate, promotes it to the new parent by stripping its parent_id, updates all other siblings to point to this new parent, and finally proceeds with deleting the original statement.
This means you don't have to write any manual cleanup logic in your python codebase, PostgreSQL will securely handle this on every single DELETE statement.
No description provided.