Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Deprecate introspection of incomplete SQLite schema #6701

Merged
merged 2 commits into from
Jan 11, 2025

Conversation

morozov
Copy link
Member

@morozov morozov commented Jan 11, 2025

Q A
Type deprecation

Background

The SQL syntax allows omitting referenced column names in a foreign key constraint declaration. In this case, it's implied that the constraint references the primary key columns. Normally, databases (e.g. Postgres) present the resulting definition of the constraint in the introspection schema (i.e. the actual column names) however, SQLite reports the original SQL statement.

A foreign key constraint declaration that was declared with omitted column names is reported as not referencing any columns. And if the referenced table doesn't exist (which SQLite allows), we get a foreign key without columns.

This is a very corner case that shouldn't happen normally. However, if it does, DBAL should throw an exception instead of producing invalid results.

Problem statement

From the data model standpoint, a foreign key constraint definition without columns doesn't make sense and will be impossible to construct in 5.0.

Refactoring

Additionally, the introspection code has been refactored. The current implementation reads as "if the to column contains null, then introspect the referenced table PK and add its columns to the existing columns". While this is technically correct, the "add to the existing columns" case will never happen because there will be only one row representing a foreign key without columns. Instead, we introspect all keys and then fetch the PKs for the ones that don't have columns. This is easier to comprehend and easier to put the deprecation logic to.

@morozov morozov added this to the 4.3.0 milestone Jan 11, 2025
@morozov morozov marked this pull request as ready for review January 11, 2025 02:05
@morozov morozov requested a review from greg0ire January 11, 2025 02:05
@greg0ire
Copy link
Member

however, SQL reports

Did you mean to write "SQLite" here?

@morozov
Copy link
Member Author

morozov commented Jan 11, 2025

however, SQL reports

Did you mean to write "SQLite" here?

Yes, fixed. Thanks!

@morozov morozov merged commit 8c9efcf into doctrine:4.3.x Jan 11, 2025
64 of 65 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants