Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

refactor: optimize identity related indices #4173

Merged
merged 11 commits into from
Oct 29, 2024
Merged

Conversation

aeneasr
Copy link
Member

@aeneasr aeneasr commented Oct 24, 2024

Related issue(s)

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

@@ -956,11 +956,11 @@ func (p *IdentityPersister) ListIdentities(ctx context.Context, params identity.
// important to normalize the identifier before querying the database.

joins = params.TransformStatement(`
INNER JOIN identity_credentials ic ON ic.identity_id = identities.id
INNER JOIN identity_credential_identifiers ici ON ici.identity_credential_id = ic.id`)
INNER JOIN identity_credentials ic ON ic.identity_id = identities.id AND ic.nid = identities.nid
Copy link
Member Author

Choose a reason for hiding this comment

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

  • This will need an update in our overrides @arekkas

@aeneasr aeneasr marked this pull request as ready for review October 28, 2024 15:56
@aeneasr aeneasr requested a review from alnr October 28, 2024 15:56
@aeneasr aeneasr changed the title Remove unused indices nidid refactor: optimize identity related indices Oct 28, 2024
This patch changes sorting to improve performance on list session endpoints. It also removes the `x-total-count` header from list responses.

BREAKING CHANGE: The total count header `x-total-count` will no longer be sent in response to `GET /admin/sessions` requests.

Closes ory-corp/cloud#7177
@aeneasr aeneasr force-pushed the remove-unused-indices-nidid branch from 1b8c016 to f6ece1a Compare October 29, 2024 06:04
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Project coverage is 78.56%. Comparing base (b4c453b) to head (14369fc).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
persistence/sql/persister_session.go 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4173      +/-   ##
==========================================
- Coverage   78.57%   78.56%   -0.02%     
==========================================
  Files         379      379              
  Lines       27042    27033       -9     
==========================================
- Hits        21249    21239      -10     
+ Misses       4181     4180       -1     
- Partials     1612     1614       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aeneasr aeneasr merged commit e24f993 into master Oct 29, 2024
29 checks passed
@aeneasr aeneasr deleted the remove-unused-indices-nidid branch October 29, 2024 12:02
malosayli pushed a commit to nayla-finance/kratos that referenced this pull request Nov 6, 2024
…indices (ory#4173)

This patch changes sorting to improve performance on list session endpoints. It also removes the `x-total-count` header from list responses.

BREAKING CHANGE: The total count header `x-total-count` will no longer be sent in response to `GET /admin/sessions` requests.

Closes https://github.com/ory-corp/cloud/issues/7177
Closes https://github.com/ory-corp/cloud/issues/7175
Closes https://github.com/ory-corp/cloud/issues/7176
malosayli pushed a commit to nayla-finance/kratos that referenced this pull request Nov 6, 2024
…indices (ory#4173)

This patch changes sorting to improve performance on list session endpoints. It also removes the `x-total-count` header from list responses.

BREAKING CHANGE: The total count header `x-total-count` will no longer be sent in response to `GET /admin/sessions` requests.

Closes https://github.com/ory-corp/cloud/issues/7177
Closes https://github.com/ory-corp/cloud/issues/7175
Closes https://github.com/ory-corp/cloud/issues/7176
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