-
-
Notifications
You must be signed in to change notification settings - Fork 964
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
feat: remove duplicate queries during settings flow and use better index hint for credentials lookup #4193
Conversation
dfdc9bb
to
8a3266e
Compare
@@ -806,11 +806,11 @@ func identifiersTableNameWithIndexHint(con *pop.Connection) string { | |||
ici := "identity_credential_identifiers" | |||
switch con.Dialect.Name() { | |||
case "cockroach": | |||
ici += "@identity_credential_identifiers_nid_i_ici_idx" | |||
ici += "@identity_credential_identifiers_nid_ici_i_idx" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alnr the old index is not dropped yet (identity_credential_identifiers_nid_i_ici_idx
), will be done in the next release
@@ -0,0 +1,4 @@ | |||
CREATE INDEX IF NOT EXISTS identity_credential_identifiers_ici_nid_i_idx | |||
ON identity_credential_identifiers (identity_credential_id ASC, nid ASC, identifier ASC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alnr i changed the index structure to use identity_credential_id
as the prefix (it is also the highest cardinality) and then we can use it both for FK and the query credentials
@@ -199,23 +205,20 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request, | |||
case "oidc": | |||
group = node.OpenIDConnectGroup | |||
} | |||
var traits identity.Traits | |||
if i != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i is never nil
This patch reduces duplicate GetIdentity queries as part of submitting the settings flow, and improves an index to significantly reduce credential lookup.
For better debugging, more tracing ha been added to the settings module.
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
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.
works.
Further Comments