DRAFT: Deferred index creation (comments-based) - do not merge#374
Draft
michal-morzywolek wants to merge 90 commits intomainfrom
Draft
DRAFT: Deferred index creation (comments-based) - do not merge#374michal-morzywolek wants to merge 90 commits intomainfrom
michal-morzywolek wants to merge 90 commits intomainfrom
Conversation
Implements 14 test methods covering: - Valid steps with @Version and package-based versioning - Sequence ordering and validation - Error detection for missing/duplicate sequences - Invalid version format detection - Package name validation - Multiple validation error accumulation Increases coverage from 56% to 94% instruction coverage. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DeferredIndexStatus enum: PENDING, IN_PROGRESS, COMPLETED, FAILED - DeferredIndexOperationType enum: ADD - DeferredIndexOperation domain class representing a row from the DeferredIndexOperation table plus ordered column names - DeferredIndexOperationDAO for all CRUD operations on the deferred index queue, following the UpgradeStatusTableServiceImpl pattern (SqlScriptExecutorProvider + SqlDialect, enum values stored via .name()) - 10 unit tests using ArgumentCaptor to verify DSL statement structure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ce split - Add DeferredAddIndex: SchemaChange for deferred index creation; apply() updates metadata only, reverse() removes index from metadata, isApplied() checks actual DB schema first then DeferredIndexOperation PENDING queue via DAO - Split DeferredIndexOperationDAO into interface (@ImplementedBy) + DAOImpl class, following UpgradeStatusTableService/Impl convention - Wire DeferredAddIndex into SchemaChangeVisitor (visit method), AbstractSchemaChangeVisitor (updates schema, no DDL), SchemaChangeSequence.InternalVisitor, and SchemaChangeAdaptor (default + Combining) - Add 11 tests for DeferredAddIndex covering apply, reverse, isApplied, and accept - Rename TestDeferredIndexOperationDAO -> TestDeferredIndexOperationDAOImpl Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add addIndexDeferred() to SchemaEditor interface and SchemaChangeSequence.Editor: creates a DeferredAddIndex carrying the step's @uuid, calls visitor.visit() only (no schemaAndDataChangeVisitor — no DDL runs on the target table during upgrade) - AbstractSchemaChangeVisitor.visit(DeferredAddIndex): write INSERT SQL into DeferredIndexOperation and DeferredIndexOperationColumn as part of the upgrade script; upgradeUUID read from DeferredAddIndex rather than stored visitor state - DeferredAddIndex: add upgradeUUID field + getter; updated toString() to include it - HumanReadableStatementProducer: implement addIndexDeferred() via generateAddIndexString - Tests: TestInlineTableUpgrader, TestSchemaChangeSequence, TestDeferredAddIndex updated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces DeferredIndexChangeService (interface + DeferredIndexChangeServiceImpl) to track pending deferred ADD INDEX operations within an upgrade session and emit the compensating SQL when subsequent schema changes interact with them: - RemoveIndex on a pending deferred ADD → cancel (DELETE) instead of DROP INDEX - RemoveTable → cancel all pending deferred indexes on that table - RemoveColumn → cancel pending deferred indexes referencing that column - RenameTable → UPDATE tableName in PENDING rows and update in-memory tracking - ChangeColumn (rename) → UPDATE columnName in PENDING column rows AbstractSchemaChangeVisitor delegates entirely to DeferredIndexChangeService, keeping SQL construction out of the visitor. The service is independently tested with 19 unit tests covering edge cases: multiple indexes on the same table, partial column matches, case-insensitivity, and single-UPDATE coverage for multi-index column renames. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Stage 7: DeferredIndexExecutor picks up PENDING operations, builds indexes via SqlDialect.deferredIndexDeploymentStatements(), and manages retry with exponential backoff. Progress logged at 30s intervals. Stage 8: awaitCompletion() polls the queue for multi-instance deployments where non-executor nodes must block until index builds finish. Stage 9: DeferredIndexRecoveryService detects stale IN_PROGRESS operations (exceeded staleThresholdSeconds) and resets or completes them based on whether the index exists in the schema. Stage 10: DeferredIndexValidator force-executes any PENDING operations before a new upgrade runs, ensuring no missing indexes. Supporting changes: DeferredIndexTimestamps utility, retryBaseDelayMs config field, DAO.hasNonTerminalOperations(), SqlDialect base method for deferred index DDL. 28 tests (10 executor, 6 recovery, 4 validator, 8 unit) all passing. Coverage: Executor 87%/81%, Recovery 100%, Validator 100%, Timestamps 100%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Override deferredIndexDeploymentStatements() in PostgreSQLDialect (CREATE INDEX CONCURRENTLY) and OracleDialect (ONLINE PARALLEL NOLOGGING) so deferred index builds avoid table-level write locks. DeferredIndexExecutor.buildIndex() now uses a dedicated autocommit connection because PostgreSQL's CONCURRENTLY cannot run inside a transaction block. This is harmless for other platforms. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
10 H2 integration tests covering: pending row creation, executor completion, auto-cancel, unique/multi-column/new-table indexes, populated table builds, multiple indexes per step, executor idempotency, and recovery-to-execution pipeline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…x/RenameIndex deferred handling - Fix DeferredIndexChangeServiceImpl to use DeferredIndexTimestamps.currentTimestamp() instead of System.currentTimeMillis() for createdTime (yyyyMMddHHmmss format) - Fix DeferredIndexOperationDAOImpl to use literal(boolean)/getBoolean() for the indexUnique column instead of literal(int)/getInt() - Add ChangeIndex/RenameIndex handling for pending deferred indexes in AbstractSchemaChangeVisitor: ChangeIndex cancels the deferred op and adds the new index immediately; RenameIndex updates the queued index name - Add updatePendingIndexName() to DeferredIndexChangeService/Impl - Add unit tests for deferred branches in both visitor test classes - Add integration tests for deferred-add-then-change and deferred-add-then-rename Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add retryMaxDelayMs config (default 5 min) to DeferredIndexConfig to cap exponential backoff and prevent overflow at high retry counts - DeferredIndexValidator now throws IllegalStateException when forced execution fails, blocking the upgrade until the issue is resolved - Update integration test to expect the exception on failed forced execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ry tracking - Add retryMaxDelayMs config (default 5 min) to cap exponential backoff and prevent overflow at high retry counts (#4) - DeferredIndexValidator throws IllegalStateException when forced execution fails, blocking the upgrade until resolved (#5) - updatePendingColumnName/updatePendingTableName now rebuild in-memory DeferredAddIndex entries so cancelPendingReferencingColumn finds indexes by renamed column/table names (#7) - Add unit and integration tests for all three fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace STRING(100) operationId PK on DeferredIndexOperation with BIG_INTEGER id column. Change DeferredIndexOperationColumn FK from STRING(100) to BIG_INTEGER to match. Update domain class, DAO interface/impl, services, executor, recovery service, and all tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
findOperationsByStatus and findStaleInProgressOperations now use a single LEFT OUTER JOIN query to fetch operations with their column names, eliminating per-operation column lookups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DeferredAddIndex.apply() now uses equalsIgnoreCase() for index name comparison, consistent with reverse() and other SchemaChange classes. DeferredIndexChangeServiceImpl SQL statements now use the original casing from stored DeferredAddIndex entries rather than the caller's casing, ensuring SQL WHERE clauses match rows on case-sensitive databases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tableName, indexName, and columnName were STRING(30) which is too narrow for the validated maximum identifier length of 60 characters. Made SchemaValidator.MAX_LENGTH public and referenced it directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove existsByUpgradeUUIDAndIndexName from DAO interface and implementation — no production code calls it. Update stale comment in SchemaChangeSequence that referenced a future stage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cted directly The DAO is never injected via Guice — all consumers construct it directly with ConnectionResources. Remove the misleading annotations to match the actual usage pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
addIndexDeferred now prefixes "Deferred: " so the output is distinguishable from a regular addIndex operation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each service now validates the config fields it consumes at construction time: threadPoolSize, maxRetries, retry delays, staleThresholdSeconds, and operationTimeoutSeconds. Also fixes stale comment in SchemaChangeSequence and distinguishes deferred index in human-readable output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…vate Introduce DeferredIndexService as the single public entry point for adopters. The facade orchestrates recovery, execution, and failure detection in one execute() call, and provides awaitCompletion() for passive nodes. Internal classes (Executor, RecoveryService, Validator, Operation, OperationType, Status) are now package-private. Config validation is consolidated in DeferredIndexServiceImpl. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update column and index name assertions to match actual table structure after previous refactoring (operationId → id, updated index names). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- TestDeferredIndexValidatorUnit (5 tests): validates empty queue shortcut, successful execution, failure exception with count - TestDeferredIndexRecoveryServiceUnit (6 tests): stale recovery for index-exists, index-absent, table-missing, multiple ops, and case-insensitive index name matching - TestDeferredIndexExecutorUnit additions (8 tests): empty queue, single success, retry-then-success, permanent failure, getStatus before/after execution, awaitCompletion true/false paths - Added test constructors to DeferredIndexValidator and DeferredIndexRecoveryService for mock injection - Made DeferredIndexValidator.createExecutor() overridable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- TestDeferredIndexOperation (2 tests): full POJO getter/setter coverage including nullable fields → 100% line coverage - TestDeferredAddIndex additions (4 tests): toString(), apply/reverse with existing other indexes, isApplied with non-matching index → 100% line coverage - TestDeferredIndexExecutorUnit additions (3 tests): unique index reconstruction, SQLException from getConnection, zero-timeout awaitCompletion → 83% line coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace internal factory methods with proper Guice @Inject/@singleton wiring so that DeferredIndexService can be injected by adopters. All services now share a single DAO instance instead of each creating its own. Bind DeferredIndexConfig in MorfModule. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deployers can now configure a set of index names that should be built immediately during upgrade even when the upgrade step uses addIndexDeferred(). This allows overriding deferral for critical indexes without modifying upgrade steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deployers can now configure a set of index names that should be deferred even when the upgrade step uses addIndex(). This enables retroactive deferred index creation on old upgrade steps without modifying them. Includes conflict validation that throws if an index name appears in both forceImmediateIndexes and forceDeferredIndexes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Always augment source schema with pending deferred indexes - Force-build only when upgrade steps exist (not on every restart) - Remove forceDeferredIndexBuildOnRestart flag from UpgradeConfigAndContext - Fix executor reuse bug: null out threadPool after shutdown so Guice singleton can be called again after forceBuildAllPending - Fix forceBuildAllPending: check FAILED ops even when no PENDING ops exist, preventing upgrades with stale failed indexes - Add per-index INFO log during schema augmentation - Rewrite lifecycle tests for unified behavior - Add test for executor reuse after completion - Add test for FAILED ops blocking force-build before upgrade Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ield tests - Replace bare @OverRide methods with @see Javadoc pattern matching existing Morf codebase convention across all files on this branch - Split TestDeferredIndexOperation.testAllGettersAndSetters into 12 individual test methods with Javadoc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nsupported platforms - Add SqlDialect.supportsDeferredIndexCreation() returning false by default - Override to true in PostgreSQLDialect, OracleDialect, H2Dialect - MySQL/SQL Server inherit false — addIndexDeferred() silently becomes addIndex() - Check in AbstractSchemaChangeVisitor.visit(DeferredAddIndex) at execution time - Update HumanReadableStatementProducer to dialect-neutral log message - Add dialect test in AbstractSqlDialectTest with per-dialect overrides - Add fallback unit test in TestInlineTableUpgrader - Add integration test verifying unsupported dialect builds index immediately Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ngeIndex test The morf-h2v2 dialect was missing the supportsDeferredIndexCreation() override, inheriting false from the base SqlDialect. This caused all integration tests using H2v2 to silently fall back to immediate AddIndex — the deferred index path was never exercised. Also fixes testDeferredAddFollowedByChangeIndex which previously asserted 0 operations after changeIndex (only correct when deferred was unsupported). With deferred support enabled, changeIndex on a pending deferred correctly cancels the old and re-tracks the new as PENDING. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…arated string Replace the separate DeferredIndexOperationColumn child table with a single indexColumns STRING(2000) column on DeferredIndexOperation. Column names are stored as comma-separated ordered values (e.g. "name,status"). This simplifies: - INSERT: 1 statement instead of N+1 (no child rows) - DELETE: 1 statement instead of 2 (no subquery for child cleanup) - DAO: no JOIN, simple single-table SELECT - Column rename UPDATE: set full indexColumns string on main table The DeferredIndexOperation POJO keeps List<String> columnNames — the DAO handles serialization to/from the comma-separated string on read/write. Also renames OPERATION_TABLE constant to DEFERRED_INDEX_OP_TABLE in the DAO. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…point of use Delete DeferredIndexExecutionConfig — move its 5 fields into UpgradeConfigAndContext with deferred-index-prefixed names: - deferredIndexThreadPoolSize (was threadPoolSize) - deferredIndexMaxRetries (was maxRetries) - deferredIndexRetryBaseDelayMs (was retryBaseDelayMs) - deferredIndexRetryMaxDelayMs (was retryMaxDelayMs) - deferredIndexForceBuildTimeoutSeconds (was executionTimeoutSeconds) Config validation moved to point of use: - DeferredIndexExecutorImpl.execute() validates thread pool and retry config - DeferredIndexReadinessCheckImpl.awaitCompletion() validates timeout DeferredIndexServiceImpl constructor simplified from 3 args to 2 (executor, dao) — no config dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When false (the default), addIndexDeferred() behaves identically to addIndex() — indexes are built immediately during the upgrade. The tracking table is unaffected. Checks at: - SchemaChangeSequence.Editor.addIndexDeferred() — primary gate - SchemaChangeSequence.Editor.addIndex() — gates forceDeferredIndexes - DeferredIndexReadinessCheckImpl — both methods are no-ops when disabled - DeferredIndexExecutorImpl.execute() — returns immediately when disabled Force-immediate and force-deferred overrides log at INFO level when triggered. Added integration test verifying disabled behavior. All deferred index config grouped under a section comment in UpgradeConfigAndContext. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ctor loop - Extract string literal constants in DeferredIndexOperationDAOImpl, DeferredIndexChangeServiceImpl, DeferredIndexExecutorImpl - Remove unused imports (LinkedHashMap, TableReference, assertEquals, contains, same-package SchemaChange) - Replace lambdas with method references (Column::getName, Index::getName, ResultSet::next) - Narrow throws Exception to throws InterruptedException - Differentiate duplicate test method in TestDeferredIndexLifecycle - Extract augmentSchemaWithOperation() from augmentSchemaWithPendingIndexes() to eliminate nested if/else and restore stale-row comment as Javadoc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…UPDATE
- Delete orphaned Javadoc block above validateExecutorConfig()
- Make logProgress() private, remove its unit test
- Only generate UPDATE for affected indexes in updatePendingColumnName()
- Add constants for all column names in DeferredIndexChangeServiceImpl
- Use LOG_ARROW constant in rename log messages
- Fix DeferredAddIndex.apply(): Arrays.asList(new Index[]{}) → List.of()
- Update stale Javadoc in DeferredIndexServiceImpl and ReadinessCheckImpl
- Break long line in sleepForBackoff()
- Remove unnecessary throws Exception from test method
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Make deferred a first-class property on Index, like unique. Add isDeferred() default method to Index interface, deferred field to IndexBean, deferred() builder method to IndexBuilder, and preserve the flag in RenameIndex. Default is false — zero behavioral change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add parseDeferredIndexesFromComment() and buildDeferredIndexCommentSegments() to DatabaseMetaDataProviderUtils for reading/writing DEFERRED:[name|cols|unique] segments in table comments. Add generateTableCommentStatements() to SqlDialect as an override point for dialects that support table comments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add indexDropStatementsIfExists() and renameIndexStatementsIfExists() to SqlDialect for safe operations on deferred indexes that may not physically exist. Base implementation uses standard IF EXISTS syntax (valid for H2). Oracle overrides with PL/SQL exception-handling wrappers (ORA-01418). PostgreSQL overrides to append COMMENT ON INDEX after rename. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extend PostgreSQL, Oracle, H2 (v1+v2) dialects with generateTableCommentStatements() that writes DEFERRED:[name|cols|unique] segments in table comments alongside existing REALNAME metadata. Extend MetaDataProviders to parse DEFERRED segments from table comments and merge virtual deferred indexes into the table's index list. Physical indexes take precedence; virtual deferred indexes are added only when no physical index with that name exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove DeferredAddIndex schema change, DeferredIndexChangeService, and the addIndexDeferred() method from SchemaEditor. Deferred index creation is now driven by the isDeferred() flag on the Index itself. SchemaChangeSequence.Editor.addIndex() resolves config overrides (kill switch, forceImmediate, forceDeferred) to set the effective deferred flag. AbstractSchemaChangeVisitor.visit(AddIndex) generates COMMENT ON TABLE for deferred indexes instead of CREATE INDEX. visit(RemoveIndex), visit(ChangeIndex), and visit(RenameIndex) use IF EXISTS for deferred indexes. visit(RemoveColumn) removes deferred indexes referencing the dropped column. visit(RenameTable) regenerates the deferred index comment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the DeferredIndexOperationDAO-based executor with a comments-based approach: the executor opens a SchemaResource, finds indexes where isDeferred()=true (virtual indexes from table comments not yet physically built), and builds them using deferredIndexDeploymentStatements(). Remove DeferredIndexOperation POJO, DAO, status enum, and the CreateDeferredIndexOperationTables upgrade step. Remove forceBuildAllPending() from the readiness check — the visitor now handles deferred indexes in-place. Simplify DeferredIndexService: add getMissingDeferredIndexStatements() to expose raw SQL for applications that want custom execution. Remove getProgress() (no tracking table to query). Simplify UpgradeConfigAndContext: remove exponential backoff config (retryBaseDelayMs, retryMaxDelayMs, forceBuildTimeoutSeconds). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add PLAN-deferred-index-comments.md (repo), integration guide and dev description (home dir). Existing docs for the tracking-table branch are untouched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite all integration tests to use the new API:
- addIndexDeferred() -> addIndex(tableName, index("name").deferred())
- Remove all DeferredIndexOperationDAO, DeferredIndexStatus references
- Update constructor calls to match new signatures
- Remove tests that relied on tracking-table status queries
- Update upgrade step classes in v1_0_0 and v2_0_0
All tests pass across all modules (unit + integration).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 8 new test scenarios covering previously untested code paths:
- Same-step: add deferred then remove/change/rename in same step
- Cross-step unbuilt: deferred index not yet built, later step
removes/changes/renames it ("change the plan" path)
- Cross-step built: deferred index physically built, later step
removes/renames it
Fix MetaDataProvider merge logic: physical indexes that match a DEFERRED
comment declaration now get isDeferred()=true. Previously, built deferred
indexes appeared as isDeferred()=false, causing the visitor to skip
comment cleanup when they were later removed — leaving ghost virtual
indexes. Fixed in all 4 MetaDataProviders (PostgreSQL, Oracle, H2, H2v2).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…verage Update integration guide: add "change the plan" section, correct MetaDataProvider description (physical+deferred indexes get isDeferred=true), detail comment lifecycle for all operations. Update dev description: correct line count (~3600 net removed), add MetaDataProvider merge behavior, integration test coverage summary. Update repo plan: add key design rule about permanent comments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The MetaDataProvider marks built deferred indexes as isDeferred()=true
(matching the comment declaration). The executor's findMissingDeferredIndexes
finds these and attempts CREATE INDEX, which fails for already-built indexes.
Add indexExistsPhysically() check after failure — if the index exists in the
catalog, treat as success and skip. This prevents noisy error logs on
idempotent executor re-runs.
Rewrite deferred-index-comments-dev.txt as a proper JIRA description
matching the format of the tracking-table branch description (Confluence
wiki markup with h2/h3 headings, ||tables||, {code} blocks, numbered
lists, risks table, full test scenario table).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Optimize executor to avoid full schema scan on large databases: each dialect provides findTablesWithDeferredIndexesSql() which queries the catalog for table comments containing /DEFERRED: segments. Only matching tables are loaded. PostgreSQL queries pg_description, Oracle queries ALL_TAB_COMMENTS, H2 queries INFORMATION_SCHEMA.TABLES. Add PostgreSQL invalid index detection: before building a deferred index, the executor checks pg_index.indisvalid. If an invalid index exists (from a crashed CREATE INDEX CONCURRENTLY), it is dropped before rebuild. Change indexExistsPhysically() to use raw JDBC DatabaseMetaData.getIndexInfo() instead of SchemaResource, bypassing the MetaDataProvider merge that marks built deferred indexes as isDeferred()=true. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Refactor PostgreSQL/Oracle deferredIndexDeploymentStatements to build CREATE INDEX directly via new SqlDialect.buildCreateIndexStatement() helper, eliminating brittle string replacement hack - Extract MetaDataProvider deferred index merge logic to shared DatabaseMetaDataProviderUtils.mergeDeferredIndexes(), removing 4x duplication across H2, H2v2, PostgreSQL, Oracle providers - Fix cross-step column rename bug: implement updateDeferredIndexColumnName to rebuild deferred indexes with renamed column before comment regen - Fix cross-step column removal bug: removeDeferredIndexesReferencingColumn now also removes indexes from in-memory schema via TableOverrideSchema - Add "giving up" ERROR log after all executor retries exhausted - Add interrupt check at top of retry loop in executeWithRetry - Remove unused two-arg DeferredIndexReadinessCheck.create() factory - Remove redundant double getIndexInfo call in indexExistsPhysically - Rename stale DeferredAddIndex test methods to DeferredIndexDeployment - Consolidate tests: merge TestDeferredIndexExecutor into Integration, delete readiness check integration tests, delete orphaned fixture - Add 7 new integration tests: cross-step column rename/removal/table rename, multi-table, getMissingDeferredIndexStatements, pre-existing indexes, unique constraint violation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The readiness check was a pass-through that returned its input unchanged. In the comments-based model, the MetaDataProvider already includes virtual deferred indexes from table comments — no schema augmentation is needed. Removes: DeferredIndexReadinessCheck interface, DeferredIndexReadinessCheckImpl, TestDeferredIndexReadinessCheckUnit, and all references in Upgrade, Upgrade.Factory, MorfModule, TestMorfModule, TestUpgrade, DeferredIndexService javadoc. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PostgreSQL's indexDeploymentStatements and deferredIndexDeploymentStatements duplicated the CREATE INDEX building logic. Extract to a shared private method that takes an optional afterIndexKeyword parameter (e.g. "CONCURRENTLY"). PostgreSQL does not schema-qualify the index name (only the table name), so this uses a dialect-specific helper rather than the base class buildCreateIndexStatement() which prefixes both. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
experimental/deferred-index-creation(DRAFT: Deferred index creation - do not merge #372)index("name").deferred(), stored as DEFERRED segments in table commentsisDeferred()=trueTest plan
mvn clean verifypasses (checkstyle + javadoc + spotbugs)~/deferred-index-comments-dev.txt,~/deferred-index-comments-integration-guide.md)🤖 Generated with Claude Code