Skip to content

feat: Make ix_tasks_metadata_gin index creation concurrent#234

Merged
kenairhodesScale merged 1 commit into
mainfrom
kenai/concurrentmig
May 13, 2026
Merged

feat: Make ix_tasks_metadata_gin index creation concurrent#234
kenairhodesScale merged 1 commit into
mainfrom
kenai/concurrentmig

Conversation

@kenairhodesScale
Copy link
Copy Markdown
Contributor

@kenairhodesScale kenairhodesScale commented May 13, 2026

Greptile Summary

  • Wraps CREATE INDEX CONCURRENTLY and DROP INDEX CONCURRENTLY in op.get_context().autocommit_block() for both upgrade() and downgrade(), correctly matching the pattern used in the sibling migration a9959ebcbe98 and satisfying PostgreSQL's requirement that CONCURRENTLY cannot run inside a transaction block.
  • No new logic issues identified; the change is a targeted, correct fix for the concurrent index creation pattern.

Confidence Score: 5/5

Safe to merge — the fix correctly implements autocommit_block for both upgrade and downgrade, matching the established pattern in the codebase.

No P0 or P1 issues found. The PR fully addresses the previous review comment by adding autocommit_block() wrapping and CONCURRENTLY to both operations, exactly matching the pattern in sibling migration a9959ebcbe98.

No files require special attention.

Important Files Changed

Filename Overview
agentex/database/migrations/alembic/versions/2026_05_04_1111_add_tasks_metadata_gin_index_e9c4ff9e6542.py Adds autocommit_block() wrapping and CONCURRENTLY keyword to both upgrade and downgrade, making the GIN index creation non-blocking and transaction-safe — correct pattern, no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Alembic runs migration] --> B{transaction_per_migration=True}
    B --> C[BEGIN transaction]
    C --> D[autocommit_block context manager]
    D --> E[Connection set to AUTOCOMMIT mode]
    E --> F["CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_tasks_metadata_gin\nON tasks USING GIN (task_metadata jsonb_path_ops)"]
    F --> G[Index built without table lock]
    G --> H[Connection restored to transactional mode]
    H --> I[COMMIT outer transaction]

    style F fill:#d4edda,stroke:#28a745
    style G fill:#d4edda,stroke:#28a745
Loading

Reviews (2): Last reviewed commit: "feat: Make ix_tasks_metadata_gin index c..." | Re-trigger Greptile

@kenairhodesScale kenairhodesScale requested a review from a team as a code owner May 13, 2026 23:23
Comment on lines 19 to 27
def upgrade() -> None:
op.execute(
"CREATE INDEX IF NOT EXISTS ix_tasks_metadata_gin "
"ON tasks USING GIN (task_metadata jsonb_path_ops)"
)
with op.get_context().autocommit_block():
op.execute(
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_tasks_metadata_gin "
"ON tasks USING GIN (task_metadata jsonb_path_ops)"
)


def downgrade() -> None:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0 PostgreSQL forbids CREATE INDEX CONCURRENTLY (and DROP INDEX CONCURRENTLY) inside a transaction block. env.py sets transaction_per_migration=True, which wraps every migration in its own BEGIN/COMMIT, so calling op.execute("CREATE INDEX CONCURRENTLY …") directly will immediately raise ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block and abort the migration. The newer sibling migration a9959ebcbe98 (which also has down_revision = 'e9c4ff9e6542') correctly wraps its CONCURRENTLY statements in with op.get_context().autocommit_block(): — the same pattern documented in env.py's own inline comment. Both upgrade() and downgrade() need the same fix here.

Suggested change
def upgrade() -> None:
op.execute(
"CREATE INDEX IF NOT EXISTS ix_tasks_metadata_gin "
"ON tasks USING GIN (task_metadata jsonb_path_ops)"
)
with op.get_context().autocommit_block():
op.execute(
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_tasks_metadata_gin "
"ON tasks USING GIN (task_metadata jsonb_path_ops)"
)
def downgrade() -> None:
def upgrade() -> None:
with op.get_context().autocommit_block():
op.execute(
"CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_tasks_metadata_gin "
"ON tasks USING GIN (task_metadata jsonb_path_ops)"
)
def downgrade() -> None:
with op.get_context().autocommit_block():
op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_tasks_metadata_gin")
Prompt To Fix With AI
This is a comment left during a code review.
Path: agentex/database/migrations/alembic/versions/2026_05_04_1111_add_tasks_metadata_gin_index_e9c4ff9e6542.py
Line: 19-27

Comment:
PostgreSQL forbids `CREATE INDEX CONCURRENTLY` (and `DROP INDEX CONCURRENTLY`) inside a transaction block. `env.py` sets `transaction_per_migration=True`, which wraps every migration in its own `BEGIN`/`COMMIT`, so calling `op.execute("CREATE INDEX CONCURRENTLY …")` directly will immediately raise `ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block` and abort the migration. The newer sibling migration `a9959ebcbe98` (which also has `down_revision = 'e9c4ff9e6542'`) correctly wraps its CONCURRENTLY statements in `with op.get_context().autocommit_block():` — the same pattern documented in `env.py`'s own inline comment. Both `upgrade()` and `downgrade()` need the same fix here.

```suggestion
def upgrade() -> None:
    with op.get_context().autocommit_block():
        op.execute(
            "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_tasks_metadata_gin "
            "ON tasks USING GIN (task_metadata jsonb_path_ops)"
        )

def downgrade() -> None:
    with op.get_context().autocommit_block():
        op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_tasks_metadata_gin")
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Claude Code Fix in Codex

@kenairhodesScale kenairhodesScale merged commit cb531a9 into main May 13, 2026
32 checks passed
@kenairhodesScale kenairhodesScale deleted the kenai/concurrentmig branch May 13, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants