fix(sap): respect custom constraint names in renameTable and changeColumn by dikshit-n · Pull Request #12909 · typeorm/typeorm

dikshit-n · 2026-09-28T02:48:45Z

Summary

Extend SAP HANA driver support for explicitly named primary key, foreign key, and index constraints. Previously, renameTable and changeColumn operations would regenerate constraint names using the naming strategy even when custom names were explicitly set via primaryKeyConstraintName or foreignKeyConstraintName. This caused two tests in custom-constraint-names/foreign-key to be skipped for SAP HANA (see #11348).

Changes Made

SapQueryRunner.renameTable: Skip PK constraint renaming when primaryKeyConstraintName is set. Skip FK constraint name regeneration when FK already has a user-defined name.
SapQueryRunner.changeColumn: Skip PK/index/FK constraint renaming when the constraint has a user-defined name (matching the pattern used by Cockroach and Postgres drivers).
SapQueryRunner.updatePrimaryKeys: Use primaryKeyConstraintName from the column when building DROP/CREATE PK SQL.
SapQueryRunner.dropPrimaryKeySql: Use primaryKeyConstraintName when available.
SapQueryRunner.createPrimaryKeySql: Accept optional constraintName parameter; fall back to primaryKeyConstraintName or naming strategy.
Tests: Removed two SAP HANA skip blocks in custom-constraint-names/foreign-key/test.ts.

Testing

Existing functional tests in test/functional/database-schema/custom-constraint-names/foreign-key/ now include the SAP HANA driver. A real SAP HANA instance is required to run these tests.

Related Issues

Closes #11348

Checklist

Code follows the project's style guidelines
Tests updated (skip blocks removed, no new test code required — existing tests now cover SAP)
Documentation updated if needed
PR title follows conventional commits format

…lumn

qodo-free-for-open-source-projects · 2026-09-28T02:53:28Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (5) 📜 Skill insights (0)

1. Adding a key crashes on keyless tables 📎 Requirement gap ≡ Correctness

Description

createPrimaryKeySql reads table.primaryColumns[0].primaryKeyConstraintName without checking
whether a primary column exists, while createPrimaryKey builds its query before marking columns as
primary. Creating the first key on a keyless table therefore reaches that empty-array dereference
before SQL is generated.

Code

packages/typeorm/src/driver/sap/SapQueryRunner.ts[3689]
+            table.primaryColumns[0].primaryKeyConstraintName ??

Evidence
createPrimaryKey builds its up query from the original table before changing the clone's primary
flags. Because Table.primaryColumns filters on those flags, a keyless table has an empty
primaryColumns array when createPrimaryKeySql dereferences its first element.
Honor explicitly named constraints in SAP HANA
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2131-2147]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[3687-3693]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2131-2148]
packages/typeorm/src/schema-builder/table/Table.ts[154-156]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[3682-3693]

Agent prompt

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

## Issue description
Creating the first primary key on a keyless table dereferences an empty `table.primaryColumns` array before SQL is generated.

## Fix Focus Areas
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2131-2148]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[3682-3693]

## Recommended Fix
Pass the intended constraint name when available. Otherwise, use an optional primary-column lookup and fall back to deriving the name from `columnNames` through the naming strategy when no primary column exists. Cover primary-key creation on a keyless table.

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

2. Composite key sync crashes without a key 📎 Requirement gap ≡ Correctness

Description

updatePrimaryKeys reads primaryColumns[0].primaryKeyConstraintName from the existing table even
when primaryColumns is empty. When schema synchronization adds a composite primary key to a
keyless table, the old-key block is skipped but this lookup throws before the new constraint is
queued.

Code

packages/typeorm/src/driver/sap/SapQueryRunner.ts[2269]
+            primaryColumns[0].primaryKeyConstraintName ??

Evidence
The schema builder calls updatePrimaryKeys when the existing and desired primary-column counts
differ and the desired count exceeds one, including a transition from zero primary columns to a
composite key. The method captures the existing primary columns before marking the requested columns
as primary, so primaryColumns[0] remains undefined in that case despite the later name calculation
reading it.
Honor explicitly named constraints in SAP HANA
packages/typeorm/src/schema-builder/RdbmsSchemaBuilder.ts[845-873]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2232-2269]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2233-2242]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2258-2273]
packages/typeorm/src/schema-builder/table/Table.ts[154-156]

Agent prompt

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

## Issue description
`updatePrimaryKeys` dereferences an empty list of existing primary columns when synchronizing a composite key onto a keyless table.

## Fix Focus Areas
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2233-2242]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2258-2273]

## Recommended Fix
Derive the new constraint name from the requested `columns`, falling back to the naming strategy without reading the old primary-column array. Test synchronization from a keyless table to a composite primary key.

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

3. Custom primary keys block table renames 📎 Requirement gap ≡ Correctness

Description

renameTable checks primaryKeyConstraintName to avoid regenerating a primary-key name, but SAP
loadTables sets isPrimary without populating that property from constraint metadata. Renaming a
loaded table with an explicitly named primary key therefore attempts to drop a nonexistent
generated-name constraint, and the missing name also affects the new changeColumn guard and
primary-key drop paths.

Code

packages/typeorm/src/driver/sap/SapQueryRunner.ts[R965-968]

+        if (
+            newTable.primaryColumns.length > 0 &&
+            !newTable.primaryColumns[0].primaryKeyConstraintName
+        ) {

Evidence
SAP loadTables fetches constraint rows but uses the primary-key marker only to set isPrimary,
without copying CONSTRAINT_NAME onto the loaded column. The renameTable guard then interprets
the absent primaryKeyConstraintName as a generated-name key and builds a DROP using the
naming-strategy name; the new column-rename guard and primary-key drop paths depend on that same
missing value.
Honor explicitly named constraints in SAP HANA
packages/typeorm/src/driver/sap/SapQueryRunner.ts[965-999]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1413-1427]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[3219-3228]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2997-3027]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[3237-3246]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[964-995]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1413-1450]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[3709-3719]

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 primary-key naming checks cannot recognize custom constraint names on tables loaded from SAP, causing rename and drop operations to use a generated name instead of the persisted one.

## Fix Focus Areas
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[964-995]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[1413-1416]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2997-3027]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[3219-3224]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[3237-3246]

## Recommended Fix
Populate `primaryKeyConstraintName` on loaded primary columns from their matching SAP primary-key constraint rows, then use the loaded name in rename and drop operations. Verify table and primary-column renames against an existing custom-named primary key.

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

View action required (2)

4. Composite keys lose configured names 📎 Requirement gap ≡ Correctness

Description

updatePrimaryKeys takes the replacement constraint name from the old primaryColumns instead of
the incoming columns, which carry the configured primaryKeyConstraintName. When a composite-key
update requests a different custom name, the ADD CONSTRAINT SQL uses the former name or a generated
name, and the requested name is ignored.

Code

packages/typeorm/src/driver/sap/SapQueryRunner.ts[R2268-2270]

+        const pkName =
+            primaryColumns[0].primaryKeyConstraintName ??
+            this.dataSource.namingStrategy.primaryKeyName(

Evidence
The schema builder constructs the incoming TableColumn objects with configured primary-key names,
and updatePrimaryKeys receives them as columns separately from primaryColumns, which comes
from the cloned existing table. The replacement-name expression reads primaryColumns rather than
columns, and its result supplies the ADD CONSTRAINT SQL.
Honor explicitly named constraints in SAP HANA
packages/typeorm/src/schema-builder/util/TableUtils.ts[25-38]
packages/typeorm/src/schema-builder/RdbmsSchemaBuilder.ts[854-873]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2232-2278]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2161-2172]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2233-2242]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[2258-2281]
packages/typeorm/src/schema-builder/RdbmsSchemaBuilder.ts[845-873]

Agent prompt

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

## Issue description
Updating a composite key ignores the constraint name supplied on the replacement columns and may reuse the old key's name.

## Fix Focus Areas
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2161-2172]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2233-2242]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2258-2279]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[2268-2273]

## Recommended Fix
Keep the existing columns' name for dropping and restoring the old key. Obtain the replacement key's name from the incoming columns, falling back to the naming strategy, and use that name for the new ADD and its rollback DROP.

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

5. Renamed columns leave stale references 📎 Requirement gap ≡ Correctness

Description

changeColumn returns early for custom-named indexes and foreign keys before replacing the old
column in their columnNames arrays. After the column is renamed, the cached table retains stale
constraint references, so lookups by the new name miss those constraints and later SQL built from
the cache can reference a nonexistent column.

Code

packages/typeorm/src/driver/sap/SapQueryRunner.ts[R1548-1549]

+                        // Skip renaming if foreign key has user defined constraint name
+                        if (foreignKey.name !== oldForeignKeyName) return

Evidence
Both early returns precede the only splice-and-push updates to the index and foreign-key column
arrays. The later rename changes only the table column; findColumnIndices and
findColumnForeignKeys search the unchanged arrays, which replaceCachedTable retains in the
query-runner cache.
Honor explicitly named constraints in SAP HANA
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1500-1521]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1537-1561]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1870-1875]
packages/typeorm/src/query-runner/BaseQueryRunner.ts[374-399]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1537-1563]
packages/typeorm/src/driver/sap/SapQueryRunner.ts[1609-1618]
packages/typeorm/src/schema-builder/table/Table.ts[365-386]
packages/typeorm/src/query-runner/BaseQueryRunner.ts[374-398]

Agent prompt

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

## Issue description
Skipping SQL renaming for custom-named indexes and foreign keys also skips updates to their cached column references.

## Fix Focus Areas
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[1500-1521]
- packages/typeorm/src/driver/sap/SapQueryRunner.ts[1537-1563]

## Recommended Fix
Replace the renamed column in every affected index and foreign key `columnNames` array, regardless of whether the constraint has a custom name. Skip only SQL renaming and constraint-name regeneration for custom-named constraints.

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

Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Context sources

✅ Tickets: 🎫 Allow explicitly named constraints in SAP HANA

Review mode: ⚖️ Balanced: This modifies SAP HANA schema-migration logic across rename, column-change, and primary-key paths, with several independent constraint-name handling sites and meaningful behavioral risk.

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

fix(sap): respect custom constraint names in renameTable and changeCo…

0d7fc1b

…lumn

github-actions Bot added the needs-triage PR needs issue link triage label Sep 28, 2026

Navigation Menu

Repository navigation

fix(sap): respect custom constraint names in renameTable and changeColumn - #12909

fix(sap): respect custom constraint names in renameTable and changeColumn#12909
dikshit-n wants to merge 1 commit into
typeorm:masterfrom
dikshit-n:fix/sap-hana-custom-constraint-names

dikshit-n commented Sep 28, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Conversation

dikshit-n commented Sep 28, 2026

Summary

Changes Made

Testing

Related Issues

Checklist

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 28, 2026

Code Review by Qodo

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant