Skip to content

fix: remove unnecessary index #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: remove unnecessary index #418

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f7e1b8
fix: remove unnecessary index
nndn Jun 21, 2025
738447d
Merge 9f7e1b83286608ba0c4bf12f01aee778cf30c0f8 into 6deb9103c72b18562…
nndn Jun 21, 2025
cf97ae7
chore: Auto-fix pre-commit issues
github-actions[bot] Jun 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions app/alembic/versions/20250621130556_bcbe001128d1_remove_unique_text_version_user_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""Remove unique_text_version_user constraint

Revision ID: 20250621130556_bcbe001128d1
Revises: 20250310201406_97a740b07a50
Create Date: 2025-06-21 13:05:56.361347

"""

from typing import Sequence, Union

from alembic import op

# revision identifiers, used by Alembic.
revision: str = "20250621130556_bcbe001128d1"
down_revision: Union[str, None] = "20250310201406_97a740b07a50"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_constraint("unique_text_version_user", "prompts", type_="unique")
# ### end Alembic commands ###

Comment on lines +20 to +24
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.py

Length 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.

Suggested change
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 ###
Comment on lines +26 to +31
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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:

  1. Either adding a pre-check that aborts with a clear message, or
  2. 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.

Suggested change
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.