feat: Support shared columns in junction tables for composite foreign keys

[nlenepveu]

feat: Support shared columns in junction tables for composite foreign keys

EDIT: Implementation Updated Based on Feedback

The implementation has been updated following @sgarner @alumni review to treat this as a bug fix rather than a feature. The original configuration-based approach has been replaced with a simpler solution that respects explicitly named columns.

Key Changes:

• Removed preserveSharedColumns and preserveSharedColumn configuration options
• Added isExplicitlyNamed property to distinguish user-provided vs auto-generated column names
• Modified column renaming logic to preserve explicitly named columns while maintaining backward compatibility for implicit names
• Refactored shared column handling into separate configureSharedColumns method

Impact: Only affects users who explicitly set duplicate column names and expected TypeORM to automatically rename them. The vast majority of users will see no change in behavior.

---

Description of change

This pull request implements shared column preservation for junction tables in composite foreign key scenarios, addressing issue #11682.

Problem Solved: Currently, TypeORM automatically renames columns that appear in both joinColumns and inverseJoinColumns to avoid duplicates (e.g., tenant_id → tenant_id_1, tenant_id_2). This prevents composite foreign key constraints in partitioned many-to-many relationships.

Solution Implemented: ~~Adds configuration options to preserve shared columns~~ Updated approach based on maintainer feedback:

The implementation now respects explicitly named columns instead of adding new configuration options. This is treated as a bug fix rather than a feature:

@JoinTable({
    name: "user_groups_shared",
    joinColumns: [
        { name: "tenant_id", referencedColumnName: "tenantId" }, // Explicit name - preserved
        { name: "user_id", referencedColumnName: "id" },
    ],
    inverseJoinColumns: [
        { name: "tenant_id", referencedColumnName: "tenantId" }, // Explicit name - preserved (shared)
        { name: "group_id", referencedColumnName: "id" },
    ],
})

Technical Implementation:

• ~~Pre-calculates shared columns during metadata building~~ Removed configuration-based approach
• ~~Skips renaming for marked shared columns~~ Now skips renaming for explicitly named columns
• Added isExplicitlyNamed property to track user-provided vs auto-generated column names
• Maintains backward compatibility: implicit column names still get renamed as before
• Refactored shared column handling into separate configureSharedColumns method
• Marks inverse-side shared columns as non-insertable/updatable to prevent duplicate column SQL errors

Behavior Changes:

Scenario	Before	After
Implicit column names	tenant_id_1, tenant_id_2	tenant_id_1, tenant_id_2 (unchanged)
Explicit column names	tenant_id_1, tenant_id_2	tenant_id, tenant_id (preserved)

Generated DML/DQL:

-- With explicit shared column names (new behavior)
INSERT INTO "user_groups_shared"("tenant_id", "user_id", "group_id") VALUES ($1, $2, $3);

-- With implicit column names (existing behavior preserved)
INSERT INTO "user_groups"("tenant_id_1", "user_id", "tenant_id_2", "group_id") VALUES ($1, $2, $3, $4);

Current Scope - DML/DQL Focus: This implementation primarily addresses the immediate pain point described in issue #11682: the inability to use TypeORM's DML/DQL operations with user-created partitioned junction tables.

The main problem users face today is that when they manually create partitioned junction tables with shared columns (which TypeORM currently cannot create), TypeORM's column renaming logic generates incorrect INSERT/UPDATE/DELETE statements that don't match the actual table schema.

This feature solves that core issue by allowing TypeORM to generate correct DML/DQL statements for existing tables with shared columns when column names are explicitly provided.

DDL Considerations: While DDL operations (table creation) are not addressed in this implementation, they remain an important consideration for future enhancements. For now, users must create junction tables manually when using partitioned tables or composite foreign key constraints.

The junction table must be created manually like:

CREATE TABLE "user_groups" (
    "tenant_id" varchar NOT NULL,  -- Shared column for both FKs
    "user_id" varchar NOT NULL,
    "group_id" varchar NOT NULL,
    PRIMARY KEY ("tenant_id", "user_id", "group_id"),
    FOREIGN KEY ("tenant_id", "user_id") REFERENCES "user"("tenant_id", "id"),
    FOREIGN KEY ("tenant_id", "group_id") REFERENCES "group"("tenant_id", "id")
) PARTITION BY HASH ("tenant_id");

This feature ensures TypeORM can then properly interact with such manually-created tables through correct DML/DQL generation when explicit column names are used.

How this change was verified

Comprehensive Test Suite (20 tests passing):

• Regression tests for existing column renaming behavior (implicit names)
• New tests for explicit column name preservation
• SQL generation validation for INSERT/DELETE/UPDATE operations
• Functional relationship management from both owner and inverse sides
• Cross-driver testing (SQLite and PostgreSQL)

Breaking Changes

Minimal breaking change exposure: Only affects users who:

1. Explicitly set the same column name for both joinColumns and inverseJoinColumns
2. AND expected TypeORM to automatically rename them with suffixes

This is considered a bug fix since the original behavior of renaming explicitly named columns was unintended and prevented valid use cases.

Pull-Request Checklist

• [x] Code is up-to-date with the master branch
• [x] This pull request links relevant issues as Fixes #11682
• [x] There are new or updated unit tests validating the change (20 comprehensive tests)
• [x] Documentation has been updated to reflect this change (JSDoc comments on new functionality)
• [x] Updated implementation based on maintainer feedback to respect explicit column names instead of adding configuration options

Summary by CodeRabbit

• New Features
  • Improved junction table handling: preserves explicitly named join columns and supports shared columns in many-to-many relations.
  • Smarter column naming: only renames implicit duplicates, keeping user-specified names intact.
• Bug Fixes
  • Prevents duplicate bindings by marking shared inverse columns as non-insertable/non-updatable.
  • More reliable SQL generation for junction tables with explicit or shared columns.
• Tests
  • Added comprehensive functional tests for implicit, partially explicit, and fully explicit junction table column naming.
  • Introduced an in-memory query logger for test assertions.

[coderabbitai[bot]]

Walkthrough

Adds explicit-name awareness to junction column generation, preserves shared columns when explicitly named, and marks inverse shared columns non-insertable/non-updatable. Introduces ColumnMetadata.isExplicitlyNamed, a protected configureSharedColumns() step in JunctionEntityMetadataBuilder, and new functional tests (implicit, partial explicit, explicit shared) plus a MemoryLogger for assertions.

Changes

Cohort / File(s)	Summary
Junction metadata builder src/metadata-builder/JunctionEntityMetadataBuilder.ts	Computes and propagates isExplicitlyNamed for join columns; conditionally preserves explicitly named duplicates; adds protected configureSharedColumns(junctionColumns, inverseJunctionColumns) to mark inverse shared columns non-insertable/non-updatable; updates control flow to apply shared-column handling during construction.
Column metadata flag src/metadata/ColumnMetadata.ts	Adds public boolean isExplicitlyNamed; wires constructor option and assignment; no other behavior changes.
Test entities (implicit/partial/explicit) test/functional/relations/junction-table-column-naming/entity/*Group.ts	Introduces three self-referential Group entities to cover fully implicit, partial explicit, and explicit shared-column join table configurations.
Functional test suite and logger test/functional/relations/junction-table-column-naming/junction-table-column-naming.ts, test/functional/relations/junction-table-column-naming/memory-logger.ts	New functional tests validating column naming, SQL generation, and behavior for shared columns; adds MemoryLogger to capture queries for assertions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant ORM as JunctionEntityMetadataBuilder
  participant CM as ColumnMetadata
  participant QB as Query Builder

  Dev->>ORM: Build junction metadata (joinColumns, inverseJoinColumns)
  ORM->>ORM: Detect duplicates & explicit names
  ORM->>CM: Create ColumnMetadata (isExplicitlyNamed)
  ORM->>ORM: configureSharedColumns(junction, inverse)
  alt Shared column detected
    ORM->>CM: Mark inverse.shared isInsert=false, isUpdate=false
  else No shared columns or implicit duplicates
    ORM->>ORM: Rename implicit duplicates
  end
  QB->>CM: Read flags
  QB-->>Dev: Generate INSERT/UPDATE excluding non-insertable updates

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• typeorm/typeorm#11400 — Addresses relation column metadata cloning to avoid cross-relation overwrites; related to this PR’s handling of duplicated/shared relation columns.

Suggested labels

comp: relations, need review help, size-m

Suggested reviewers

• sgarner
• alumni
• gioboa
• mguida22

Poem

I hop between columns, neat and named,
Tenant tails shared, yet never blamed.
One paw inserts, the other stays—
Inverse hush, no updates’ haze.
Junctions tidy, queries sing,
Thump-thump! I ship this spring. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title accurately and concisely describes the primary change by emphasizing support for shared columns in junction tables to handle composite foreign keys, matching the implemented feature without extraneous details.
Linked Issues Check	✅ Passed	The implementation fulfills the core objectives of linked issue #11682 by detecting and preserving shared columns through explicit naming, marking inverse-side columns non-insertable and non-updatable to prevent duplicate SQL errors, maintaining backward compatibility for implicit names, and providing comprehensive DML/DQL test coverage across drivers.
Out of Scope Changes Check	✅ Passed	All modifications, including the addition of the isExplicitlyNamed flag, the new configureSharedColumns method, and supporting tests with the MemoryLogger utility, directly relate to enabling shared-column support in junction tables and do not introduce functionality outside the scope of the linked issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• [ ] 📝 Generate Docstrings

🧪 Generate unit tests

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

[!TIP]

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new[bot]]

• typeorm-sql-js-example

  npm i https://pkg.pr.new/typeorm/typeorm@11686
  

commit: b2f0c43

[nlenepveu]

I feel like the preserveSharedColumn option is unnecessary and overcomplicates the implementation.

Couldn't we just make TypeORM respect manually configured column names? i.e. the renaming of columns for deduplication should only apply when we are automatically generating a column name.

So if the user said both fields should be named tenant_id then that should be respected. But if they didn't give a column name then we'd generate tenant_id_1 and tenant_id_2. (I'm actually surprised it doesn't already work that way.)

@sgarner Thanks for the feedback!

The design decision behind preserveSharedColumn was mainly to avoid changing the existing behavior and potentially breaking backwards compatibility. I’m very much in favor of a simpler API, and indeed we were also disappointed by the current behavior where manually configured column names get silently renamed.

If you think it’s acceptable to remove the automatic renaming in favor of always respecting user-provided names, and that the backward-compatibility risk is manageable, I’m fully supportive of going in that direction.

[sgarner]

Yes, without the option it would mean this change could be breaking for some users.

I think that may still be preferable to adding an option that produces two different behaviors, one of which is undesirable, which we then have to maintain.

Since the previous behavior was never documented, and this scenario probably doesn't affect many people, including such a break in a 0.3.x point release might be acceptable anyway.

Otherwise this could wait until 1.x.

WDYT @alumni ?

[nlenepveu]

I agree the breaking change shouldn’t be a big deal to fix. Users can always explicitly rename the column themselves in the join table definition. So this feels manageable even in a 0.3.x release.

That said, if we want to be more compliant with semver and only introduce BC breaks in a major (or maybe a 0.4 given we haven’t had a major yet), we could also take a middle path:

• Release preserveSharedColumn now, but mark it as deprecated right away.
• Add a console.warn whenever a column gets renamed automatically, informing users that this behavior will change in the next major.

This way, nothing breaks immediately, we can ship the fix in the next release, and users get clear warning that they need to adjust. Then in the next major we drop the option entirely and default to the simpler, correct behavior.

[coveralls]

coverage: 80.76% (+0.002%) from 80.758% when pulling b2f0c43ca40daea21f87896988105b4be240184d on digital-value-apps:feat/junction-table-preserve-shared-columns into c4f5d12f3ffb70ffcf3a1c255947d8bd5fe76e67 on typeorm:master.

[alumni]

I think if the properties (columns) are explicitly named, TypeORM should always respect the user's choice and not rename anything (basically I see the current behavior as a bug), i.e.:

joinColumns: [
    { name: "tenant_id", referencedColumnName: "tenantId" },
    { name: "user_id", referencedColumnName: "id" },
],
inverseJoinColumns: [
    { name: "tenant_id", referencedColumnName: "tenantId" }, // Same column - will be shared
    { name: "group_id", referencedColumnName: "id" },
],

should produce an association (join, junction, etc) table with 3 columns (tenant_id, user_id, group_id).

Side note: it has always bugged me that name in JoinColumnOptions must be a column name, but referencedColumnName is a property name. I am starting to think it could makes sense (except for referencedColumnName) :)

[sgarner]

Side note: it has always bugged me that name in JoinColumnOptions must be a column name, but referencedColumnName is a property name.

Yes! There are several places where the ambiguity between "column names" and "property names" is confusing in TypeORM, but this example is particularly egregious. We should improve this sometime.

[sgarner]

@nlenepveu Could you please update the implementation to remove the preserveSharedColumn option and respect explicitly set column names instead?

We will treat this as a bug fix rather than a feature.

Note that we aren't adding specific tests for GitHub issues any more, so the tests in this PR should also be moved to the appropriate test suite(s) for the functionality they are testing.

[alumni]

I'll try tomorrow to take a look at the history of this JunctionEntityMetadataBuilder.changeDuplicatedColumnNames - I'd like to see why it was added. The current behavior is intentional, but I'd like to understand the context.

Maybe it's not needed to keep this behavior.

Also: the methods of these metadata builder classes are protected, even thought the classes are not extended. Maybe it's also some leftover code from a different time.

Later edit: https://github.com/typeorm/typeorm/commit/8934e27dc412a90f8ef5b80aac19ebc86d59c054 introduced changeDuplicatedColumnNames. But why?

[nlenepveu]

I’m fine to drop the preserveSharedColumn option and rework the PR accordingly.

I’ll update the PR to:

• Remove the preserveSharedColumn option.
• Adjust the implementation so explicitly set column names are always respected, and only implicit names are subject to renaming via changeDuplicatedColumnNames.
• Move the tests into the appropriate existing test suites rather than keeping them tied to the GitHub issue.

On the side note: totally agree about the name vs referencedColumnName inconsistency, that’s confusing.

[nlenepveu]

Later edit: 8934e27 introduced changeDuplicatedColumnNames. But why?

Looking at the example, it seems this was added to support using @JoinTable() without options in self-referencing entities (looks like a graph-like structure). In that context, it makes sense. The developer doesn’t care about the exact table design and just wants the ORM to generate something that works out of the box. This supports keeping the current behavior when join columns are implicit, while removing it when column names are set explicitly.

[sgarner]

That said, if we want to be more compliant with semver and only introduce BC breaks in a major (or maybe a 0.4 given we haven’t had a major yet), we could also take a middle path

Since TypeORM is still on the 0.x track we are technically able to make breaking changes in any release and be compliant with semver. But of course we try not to do that since many thousands of people are using TypeORM in production already.

Either way, a change in behavior is not considered breaking if the original behavior was a bug and/or undocumented. I think that applies in this case.

Under the proposed change plan the exposure is low. Only if a user explicitly set the same name for two columns in a junction table and expected TypeORM to suffix them will they see a break. If they didn't set names, the current behavior continues.

We'll discuss with other maintainers to make a final decision but my personal view is this is acceptable for a 0.3.x release.

[nlenepveu]

@sgarner @alumni

I’ve updated the PR based on the discussion to ensure explicitly set column names are always respected. This should now align with the intended direction. Let me know if you see anything blocking, otherwise I think it’s ready to move forward.

[qodo-free-for-open-source-projects[bot]]

PR Code Suggestions ✨

Category	Suggestion	Impact
General	Add explicit naming check condition In configureSharedColumns, only mark inverse columns as read-only if the column names were explicitly provided by the user to avoid unintended side effects with implicitly named columns. src/metadata-builder/JunctionEntityMetadataBuilder.ts [423-440] protected configureSharedColumns( junctionColumns: ColumnMetadata[], inverseJunctionColumns: ColumnMetadata[], ) { junctionColumns.forEach((junctionColumn) => { inverseJunctionColumns.forEach((inverseJunctionColumn) => { if ( junctionColumn.givenDatabaseName === - inverseJunctionColumn.givenDatabaseName + inverseJunctionColumn.givenDatabaseName && + (junctionColumn.isExplicitlyNamed || inverseJunctionColumn.isExplicitlyNamed) ) { // Mark inverse side column as read-only to prevent duplicate bindings // The owner side (with @JoinTable) retains control over shared column values inverseJunctionColumn.isInsert = false inverseJunctionColumn.isUpdate = false } }) }) } [ ] Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that implicitly duplicated columns should be renamed, not made read-only, and proposes a valid check to ensure read-only logic only applies to intentionally shared (explicitly named) columns.	Medium
[ ] More

[nlenepveu]

Can you rebase this PR with the current master branch please?

Done!

[gioboa]

Done!

Thanks