Skip to content

Fix Versionable referrer columns with table schemas - #2072

Open
surekap wants to merge 1 commit into
propelorm:masterfrom
surekap:agent/fix-versionable-schema-qualified-referrers
Open

surekap wants to merge 1 commit into
propelorm:masterfrom
surekap:agent/fix-versionable-schema-qualified-referrers

Conversation

@surekap

@surekap surekap commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • prevent Versionable referrer column names from containing a table schema
  • use Table::getCommonName() consistently when creating and resolving generated referrer columns
  • add a MySQL regression test with explicit database, table, and foreign-key schemas

Problem

For schema-qualified tables, Table::getName() returns a value such as:

grapevine.responsibility_matrix_item

Versionable uses that value as a column-name prefix in three places. It consequently attempts to create and resolve columns named:

grapevine.responsibility_matrix_item_ids
grapevine.responsibility_matrix_item_versions

The schema is table-location metadata and must not become part of a column identifier. This can produce invalid generated SQL and inconsistent behavior between schemas with and without explicit qualification.

Fix

Use Table::getCommonName() in:

  • addForeignKeyVersionColumns()
  • getReferrerIdsColumn()
  • getReferrerVersionsColumn()

The generated names remain responsibility_matrix_item_ids and responsibility_matrix_item_versions, independent of the table schema.

Verification

  • The new test fails on current master before the production change.
  • vendor/bin/phpunit tests/Propel/Tests/Generator/Behavior/Versionable/VersionableBehaviorTest.php passes: 16 tests, 23 assertions.
  • vendor/bin/phpcs src/Propel/Generator/Behavior/Versionable/VersionableBehavior.php passes.
  • git diff --check passes.

The complete legacy test file has existing PHPCS findings unrelated to this change; the newly added method introduces no new finding.

Scope

This PR intentionally does not rename Versionable-generated foreign keys or indexes. Changing those names would create migration churn for existing applications and should be evaluated separately.

The related schema-qualified Archivable lookup fix is already isolated in #2070.

@surekap
surekap marked this pull request as ready for review August 6, 2026 05:38
@mringler

Copy link
Copy Markdown
Contributor

Nice!

Added to Perpl and released as patch, along with your other two PRs.

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