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

Add index on publicKeyCredentialId in Doctrine mapping #670

Merged
merged 2 commits into from
Jan 18, 2025

Conversation

Spomky
Copy link
Contributor

@Spomky Spomky commented Jan 4, 2025

This commit introduces a database index on the publicKeyCredentialId field in the PublicKeyCredentialSource mapping. The added index aims to optimize query performance for operations involving this column.

Target branch: 5.1.x
Resolves issue #665

  • It is a Bug fix
  • It is a New feature
  • Breaks BC
  • Includes Deprecations

@Spomky Spomky added the enhancement New feature or request label Jan 4, 2025
@Spomky Spomky added this to the 5.1.0 milestone Jan 4, 2025
@Spomky Spomky self-assigned this Jan 4, 2025
Comment on lines 8 to 10
<indexes>
<index name="pkcid_idx" columns="publicKeyCredentialId"/>
</indexes>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about making it a unique column instead? That already gives it an index, plus it guarantees it's unique.

Marked the publicKeyCredentialId field as unique in the PublicKeyCredentialSource mapping configuration. This ensures database-level enforcement of uniqueness for this field, preventing duplicate entries.
@Spomky Spomky force-pushed the features/pkcid-index branch from eb36430 to 8640216 Compare January 18, 2025 10:39
Marked the publicKeyCredentialId field as unique in the PublicKeyCredentialSource mapping configuration. This ensures database-level enforcement of uniqueness for this field, preventing duplicate entries.
@Spomky Spomky merged commit 281025e into 5.1.x Jan 18, 2025
15 checks passed
@Spomky Spomky deleted the features/pkcid-index branch January 18, 2025 15:30
@nicodemuz
Copy link

nicodemuz commented Jan 18, 2025

I just deployed this to production via php bin/console doctrine:schema:update --force --dump-sql, and got the following error:

CREATE UNIQUE INDEX UNIQ_6AD0B4DF72A8BD77 ON public_key_credential_sources (public_key_credential_id);

 Updating database schema...


In ExceptionConverter.php line 118:
                                                                                                                                                                                             
  An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1170 BLOB/TEXT column 'public_key_credential_id' used in key specification without a key length

I'm using MySQL.

@Spomky
Copy link
Contributor Author

Spomky commented Jan 19, 2025

Hello @nicodemuz,

This should be fixed in 5.1.1

@nicodemuz
Copy link

Hello @nicodemuz,

This should be fixed in 5.1.1

@Spomky The MySQL query becomes now:

ALTER TABLE public_key_credential_sources CHANGE public_key_credential_id public_key_credential_id TINYTEXT NOT NULL;

With an error:

An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1170 BLOB/TEXT column 'public_key_credential_id' used in key specification without a key length

I just tested that it (doctrine:schema:update) works with:

<field name="publicKeyCredentialId" type="string" unique="true" length="250"/>

But I'm not sure if this will break anything.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants