fix(schema-builder): prevent column drop on rename when column counts differ (#3357) - #12798

Description

renameColumns() still only renames when the unmatched sets are 1:1 or have equal lengths; if a
column is renamed while another column is added/removed, the lengths differ and no rename happens.
Because renameColumns() runs before dropRemovedColumns()/addNewColumns(), the old column will
be dropped and the “renamed” column re-added empty, causing data loss.

Code

src/schema-builder/RdbmsSchemaBuilder.ts[R377-379]

+            } else if (
+                renamedTableColumns.length === renamedMetadataColumns.length
+            ) {

Evidence

The new code only renames in the 1:1 case or when the unmatched lists have equal length; otherwise
it does nothing, and schema sync then drops columns present in the table but not in metadata and
adds columns present in metadata but not the table. The operation ordering shows rename runs before
drop/add, so a skipped rename leads directly to drop+add behavior.

src/schema-builder/RdbmsSchemaBuilder.ts[320-405]
src/schema-builder/RdbmsSchemaBuilder.ts[227-248]
src/schema-builder/RdbmsSchemaBuilder.ts[792-864]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`renameColumns()` only performs renames when (a) exactly one metadata and one table column are unmatched, or (b) the counts of unmatched columns are equal. This means a rename occurring alongside a column add/remove (or any other situation that makes the unmatched sets different sizes) will not be renamed, and will fall through to `dropRemovedColumns()` + `addNewColumns()`.

### Issue Context
This method runs before `dropRemovedColumns()` / `addNewColumns()`, so missed renames are particularly dangerous because they can turn into drop+add behavior.

### Fix Focus Areas
- src/schema-builder/RdbmsSchemaBuilder.ts[320-406]
- src/schema-builder/RdbmsSchemaBuilder.ts[227-248]
- src/schema-builder/RdbmsSchemaBuilder.ts[792-864]

### Suggested fix approach
1. Build rename candidates based on *name presence*, not on the full (name+type+nullable+unique) match:
  - `missingInTableByName`: metadata columns (non-virtual) whose `databaseName` does **not** exist in `table.columns` by name.
  - `missingInMetadataByName`: table columns whose `name` does **not** exist in metadata `databaseName`s.
  This excludes columns that merely changed nullability/unique/etc but still have the same name.
2. Attempt to match candidates across these two sets by a signature (at least: normalized type, nullable, normalized unique; optionally include length/precision/scale if you want to reduce false positives).
3. Perform **one-to-one** matching (don’t require the sets to be same size). Only rename pairs that can be matched uniquely; leave unmatched items for drop/add.
4. Add/adjust tests to cover: rename + add column, rename + drop column.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

In the multi-column branch, each old table column uses renamedMetadataColumns.find(...) without
removing already-matched metadata columns, so the same metadata column can be matched multiple
times. This can rename multiple DB columns to the same new name or mis-rename a column that merely
changed nullability/unique (not its name), leading to schema errors or unintended drops/recreates
later in sync.

Code

src/schema-builder/RdbmsSchemaBuilder.ts[R383-390]

+                    const matchMeta = renamedMetadataColumns.find(
+                        (meta) =>
+                            this.dataSource.driver.normalizeType(meta) ===
+                                oldCol.type &&
+                            meta.isNullable === oldCol.isNullable &&
+                            this.dataSource.driver.normalizeIsUnique(meta) ===
+                                oldCol.isUnique,
+                    )

Evidence

The pairing logic uses renamedMetadataColumns.find(...) inside a loop and never removes the
matched metadata column, so multiple old columns can resolve to the same matchMeta. The code also
doesn’t constrain matches by name presence (it matches solely on type/nullability/unique),
increasing the chance of mis-pairing when multiple columns are “unmatched” for reasons other than
rename.

src/schema-builder/RdbmsSchemaBuilder.ts[361-405]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The multi-column rename loop matches metadata columns via `.find(...)` but does not mark a metadata column as consumed. When multiple unmatched columns share the same (type, nullable, unique) signature, the first matching metadata column can be reused for multiple renames.

### Issue Context
This is especially likely when one column rename is combined with another column’s nullability/unique change (those columns also become “unmatched” under the current filters), or when multiple columns share the same basic type/nullability.

### Fix Focus Areas
- src/schema-builder/RdbmsSchemaBuilder.ts[320-405]

### Suggested fix approach
1. Use a working list/set of remaining metadata candidates (e.g., `const remaining = [...missingInTableByName]`).
2. For each old column, search *remaining* for matches; when a match is selected, remove it from `remaining`.
3. If multiple metadata candidates match the same old column (ambiguous), skip renaming that old column to avoid incorrect pairing.
4. Consider tightening the match signature (e.g., include length/precision/scale) to reduce accidental matches.
5. Add a regression test: rename one column while changing another column’s nullability/unique; ensure only the renamed column is renamed and the other column is altered (not renamed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new regression test claims to cover column-count changes or multiple column changes, but it only
renames a single column and duplicates the existing “change column name” test’s behavior. As a
result, the new multi-column matching logic and the “counts differ” scenario remain untested.

Code

test/functional/schema-builder/column/change/change-column/change-column.test.ts[R50-57]

+    it("should rename column even when column count changes or multiple columns change (issue #3357)", () =>
+        Promise.all(
+            dataSources.map(async (dataSource) => {
+                const postMetadata = dataSource.getMetadata(Post)
+                const nameColumn =
+                    postMetadata.findColumnWithPropertyName("name")!
+                nameColumn.propertyName = "headline"
+                nameColumn.build(dataSource)

Evidence

The new test only mutates nameColumn.propertyName and then synchronizes, which is the same shape
as the existing rename test above it; no other column is modified and no column is added/removed, so
the added code paths are not exercised.

test/functional/schema-builder/column/change/change-column/change-column.test.ts[26-72]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The added test name asserts coverage for column count changes / multiple column changes, but the body only changes `propertyName` for a single column and calls `synchronize()`. This does not exercise the newly-added multi-column matching branch nor the scenario where a rename occurs alongside add/remove.

### Issue Context
There is already an earlier test that renames `name -> title` with the same pattern.

### Fix Focus Areas
- test/functional/schema-builder/column/change/change-column/change-column.test.ts[26-72]

### Suggested fix approach
1. Extend the new test to actually trigger multiple “unmatched” columns, e.g.:
  - Rename `name -> headline` AND change another column’s `isNullable` / `unique` (in a way that affects the rename detection filters), then assert:
    - the renamed column exists under the new name,
    - the old name is gone,
    - the other column’s property change is applied (and it was not renamed).
2. Add a separate test for rename + add/remove column (if feasible in this test setup), verifying the renamed column is not dropped/recreated.
3. If the scenario is not feasible in this suite, rename the test to reflect what it actually tests to avoid misleading future maintainers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The renameColumns() JSDoc says it only works when one column changed, but this PR adds a
multi-column matching branch; the comment is now misleading and may cause future refactors to
accidentally break supported behavior. Keeping docs aligned is important here because rename
detection bugs can cause destructive drop+add outcomes.

Code

src/schema-builder/RdbmsSchemaBuilder.ts[R361-365]

+            // If exactly one table column and one metadata column unmatched, or matching pairs
            if (
-                renamedTableColumns.length === 0 ||
-                renamedTableColumns.length > 1
-            )
-                continue
+                renamedTableColumns.length === 1 &&
+                renamedMetadataColumns.length === 1
+            ) {

Evidence

The JSDoc explicitly states a single-column limitation, but the implementation now contains logic to
handle multiple unmatched columns, so the documentation no longer describes the actual behavior.

src/schema-builder/RdbmsSchemaBuilder.ts[315-319]
src/schema-builder/RdbmsSchemaBuilder.ts[361-405]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`renameColumns()` has a JSDoc comment stating it only works when one column per table changed, but the implementation now attempts to rename multiple columns.

### Issue Context
Rename detection is safety-critical because missed renames can fall through to `dropRemovedColumns()` / `addNewColumns()`.

### Fix Focus Areas
- src/schema-builder/RdbmsSchemaBuilder.ts[315-319]
- src/schema-builder/RdbmsSchemaBuilder.ts[320-405]

### Suggested fix approach
Update the JSDoc to describe the current behavior and limitations (e.g., what constitutes a rename candidate, how ambiguity is handled, and what happens when multiple changes occur).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗