fix: remove unnecessary index #418
Conversation
WalkthroughA new Alembic migration script was added to remove the unique constraint "unique_text_version_user" from the "prompts" table, which covers the columns "text," "version," and "created_by." The script also defines how to restore this constraint if the migration is downgraded. Changes
Estimated code review effort1 (~2 minutes) Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| def upgrade() -> None: | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.drop_constraint("unique_text_version_user", "prompts", type_="unique") | ||
| # ### end Alembic commands ### | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add schema= kwarg to ensure cross-schema safety
op.drop_constraint() defaults to the current search path.
If the prompts table lives in a dedicated schema (e.g., core.prompts), the call will silently fail at runtime. Pass the explicit schema to make the migration deterministic across all envs.
- op.drop_constraint("unique_text_version_user", "prompts", type_="unique")
+ op.drop_constraint(
+ "unique_text_version_user",
+ "prompts",
+ type_="unique",
+ schema="public", # adjust if another schema is used
+ )🏁 Script executed:
#!/bin/bash
set -e
# List all op.drop_constraint calls in migrations
echo "=== op.drop_constraint calls ==="
rg "op\.drop_constraint" -n app/alembic/versions
# List any explicit schema= usage in migrations
echo "=== schema= in migrations ==="
rg "schema\s*=" -n app/alembic/versions
# Inspect env.py for default schema or version_table_schema
echo "=== version_table_schema in env.py ==="
rg "version_table_schema" -n app/alembic/env.py || echo "No version_table_schema setting found"
echo "=== search_path in env.py ==="
rg "search_path" -n app/alembic/env.py || echo "No search_path manipulation found"Length of output: 1972
🏁 Script executed:
#!/bin/bash
# Allow greps to fail without exiting
set +e
echo "=== version_table_schema in env.py ==="
rg "version_table_schema" -n app/alembic/env.py
echo "=== search_path in env.py ==="
rg "search_path" -n app/alembic/env.pyLength of output: 251
🏁 Script executed:
#!/bin/bash
# Search for any table definitions with explicit schema usage across the repo
rg "schema\s*=" -n .Length of output: 3728
Add explicit schema= argument to op.drop_constraint for deterministic migrations
Without an explicit schema, op.drop_constraint() relies on the current search path and can silently fail if prompts isn’t in the default schema.
Locations to update:
- app/alembic/versions/20250621130556_bcbe001128d1_remove_unique_text_version_user_.py:22
Suggested diff:
- op.drop_constraint("unique_text_version_user", "prompts", type_="unique")
+ op.drop_constraint(
+ "unique_text_version_user",
+ "prompts",
+ type_="unique",
+ schema="public", # adjust if your prompts table lives in another schema
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def upgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.drop_constraint("unique_text_version_user", "prompts", type_="unique") | |
| # ### end Alembic commands ### | |
| def upgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.drop_constraint( | |
| "unique_text_version_user", | |
| "prompts", | |
| type_="unique", | |
| schema="public", # adjust if your prompts table lives in another schema | |
| ) | |
| # ### end Alembic commands ### |
🤖 Prompt for AI Agents
In
app/alembic/versions/20250621130556_bcbe001128d1_remove_unique_text_version_user_.py
at line 22, the call to op.drop_constraint lacks an explicit schema argument,
which can cause the migration to fail if the prompts table is not in the default
schema. Fix this by adding the schema parameter with the correct schema name to
the op.drop_constraint call to ensure deterministic behavior regardless of the
search path.
| def downgrade() -> None: | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.create_unique_constraint( | ||
| "unique_text_version_user", "prompts", ["text", "version", "created_by"] | ||
| ) | ||
| # ### end Alembic commands ### |
There was a problem hiding this comment.
Downgrade can hard-fail once duplicates exist
After the unique constraint is removed, rows with the same (text, version, created_by) tuple can be inserted.
If such duplicates exist, op.create_unique_constraint() will raise and the downgrade becomes impossible.
Mitigate by:
- Either adding a pre-check that aborts with a clear message, or
- Deduplicating rows (delete/merge) prior to recreating the constraint.
def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
+ # Fail early if duplicates would break the unique constraint.
+ # (Example for Postgres; adapt to other engines.)
+ op.execute(
+ """
+ DO $$
+ BEGIN
+ IF EXISTS (
+ SELECT (text, version, created_by)
+ FROM prompts
+ GROUP BY text, version, created_by
+ HAVING COUNT(*) > 1
+ ) THEN
+ RAISE EXCEPTION
+ 'Downgrade aborted – duplicates would violate unique_text_version_user';
+ END IF;
+ END;
+ $$;
+ """
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def downgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| op.create_unique_constraint( | |
| "unique_text_version_user", "prompts", ["text", "version", "created_by"] | |
| ) | |
| # ### end Alembic commands ### | |
| def downgrade() -> None: | |
| # ### commands auto generated by Alembic - please adjust! ### | |
| # Fail early if duplicates would break the unique constraint. | |
| # (Example for Postgres; adapt to other engines.) | |
| op.execute( | |
| """ | |
| DO $$ | |
| BEGIN | |
| IF EXISTS ( | |
| SELECT (text, version, created_by) | |
| FROM prompts | |
| GROUP BY text, version, created_by | |
| HAVING COUNT(*) > 1 | |
| ) THEN | |
| RAISE EXCEPTION | |
| 'Downgrade aborted – duplicates would violate unique_text_version_user'; | |
| END IF; | |
| END; | |
| $$; | |
| """ | |
| ) | |
| op.create_unique_constraint( | |
| "unique_text_version_user", "prompts", ["text", "version", "created_by"] | |
| ) | |
| # ### end Alembic commands ### |
🤖 Prompt for AI Agents
In
app/alembic/versions/20250621130556_bcbe001128d1_remove_unique_text_version_user_.py
around lines 26 to 31, the downgrade function recreates a unique constraint
without handling potential duplicates, which can cause the operation to fail. To
fix this, add a pre-check before recreating the constraint to detect duplicates
on (text, version, created_by) and either abort with a clear error message or
implement logic to deduplicate the rows by deleting or merging duplicates before
calling op.create_unique_constraint.
``
Summary by CodeRabbit