mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-21 15:04:25 +00:00
perf(query): use UNION of index seeks for ListUsers login equality (#12701)
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> # Which Problems Are Solved Login V2 `ListUsers` lookups of the form username OR email (and email OR phone) hash-join the full `users14` / `users14_humans` tables, then filter. That cost grows with instance size (~40ms at 10k users, ~650ms at 200k). Top-level login-name equals already used an indexed join, but the same login-name filter inside an `OrQuery` fell back to the slow view-based exists query. # How the Problems Are Solved **TL;DR:** equality lookups on username, email, phone, or login name become `INNER JOIN (UNION of indexed ID seeks) AS matches`. Mixed / contains / name searches keep the old WHERE shape. ### Reviewer map | Read first | Why | | --- | --- | | `internal/query/user_login_equality.go` | Typed filters + extractor + UNION join | | `internal/query/user.go` (`prepareUsersQuery`, constructors) | Wiring; username/email/phone equals return `lowerEqualsFilter` | | `cmd/setup/76.go` + `cmd/setup/76/*.sql` | Concurrent indexes for existing deployments; drop invalid leftovers | | `internal/query/projection/user.go` | Same indexes in `Init()` for new databases | | `internal/query/user_login_name_exists_test.go` | SQL-shape coverage | | `internal/api/grpc/user/v2/integration_test/query_test.go` | Nested username AND/OR user results | ### Query rewrite - Extract one equals leaf, a flat `OrQuery` of those leaves, or the same nested under `AndQuery`. Remaining AND conjuncts stay as WHERE. Equality nested under `OrQuery` / `NotQuery` is left as WHERE. - Login-name equals is a UNION arm that reuses `login_names3_users` SQL, so it works inside OR as well as at the top level. - EQUALS seeks use `LOWER(col)` for the index, then recheck `col = value` so case-sensitive searches do not match extra rows. EQUALS_IGNORE_CASE is unchanged. - Empty OR arms are not skipped (that would change OR meaning); mixed empty-OR falls back to WHERE. Login UI already omits empty arms. ### Indexes - Setup 76: `CREATE INDEX CONCURRENTLY IF NOT EXISTS` on `users14_username_lower_idx` `(instance_id, LOWER(username))` and `users14_humans_phone_lower_idx` `(instance_id, LOWER(phone))`. Not wrapped in a transaction. - Before create, drop any leftover `pg_index.indisvalid = false` indexes of those names (failed/cancelled `CREATE INDEX CONCURRENTLY`). `IF NOT EXISTS` would otherwise skip rebuild. - `Init()` creates the same indexes without `CONCURRENTLY` (empty tables). Existing `users14_username` and email lower indexes are unchanged. # Additional Changes - Unit tests assert UNION SQL for username OR email, email OR phone, login name OR email, username OR username, nested `AndQuery(loginName, org)`, and nested `AndQuery(Or(...), org)`; that contains / display-name / mixed OR / nested `OrQuery(AndQuery(...), ...)` do not UNION; that email EQUALS rechecks the raw column; and that empty-OR is not extracted. - Setup 76 sqlmock coverage: missing `users14` skips work; valid/absent indexes only create; invalid leftover username index is dropped then both creates run. - `TestServer_ListUsers_UsernameAndOr` checks live results: org AND (alice OR bob) returns both users, sibling username AND is empty, nested `AndQuery(Or(...), org)` returns both. # Additional Context - Base branch: `main` - Extends the login-name equals rewrite from #12460 to username/email/phone OR trees used by Login V2 search. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-1f043b1c-6168-45ca-9e9e-c18451994207?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-1f043b1c-6168-45ca-9e9e-c18451994207&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --------- Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: Silvan <[email protected]>
This commit is contained in:
Silvan
co-authored by
Cursor Agent
Silvan
Cursor Agent
Silvan
parent
95cbe6a796
commit
684de2e691
@@ -0,0 +1,93 @@
|
||||
package setup
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"embed"
|
||||
"fmt"
|
||||
|
||||
"github.com/zitadel/zitadel/backend/v3/instrumentation/logging"
|
||||
"github.com/zitadel/zitadel/internal/database"
|
||||
"github.com/zitadel/zitadel/internal/eventstore"
|
||||
)
|
||||
|
||||
var (
|
||||
//go:embed 76/*.sql
|
||||
users14LoginEqualityIndexes embed.FS
|
||||
)
|
||||
|
||||
const (
|
||||
users14TableExistsQuery = "SELECT exists(SELECT 1 FROM information_schema.tables WHERE table_schema = 'projections' AND table_name = 'users14')"
|
||||
users14InvalidIndexQuery = `SELECT EXISTS (
|
||||
SELECT 1
|
||||
FROM pg_index i
|
||||
JOIN pg_class c ON c.oid = i.indexrelid
|
||||
JOIN pg_namespace n ON n.oid = c.relnamespace
|
||||
WHERE n.nspname = 'projections'
|
||||
AND c.relname = $1
|
||||
AND NOT i.indisvalid
|
||||
)`
|
||||
users14UsernameLowerIdx = "users14_username_lower_idx"
|
||||
users14HumansPhoneLowerIdx = "users14_humans_phone_lower_idx"
|
||||
dropInvalidIndexConcurrently = "DROP INDEX CONCURRENTLY IF EXISTS projections."
|
||||
)
|
||||
|
||||
var users14LoginEqualityIndexNames = []string{
|
||||
users14UsernameLowerIdx,
|
||||
users14HumansPhoneLowerIdx,
|
||||
}
|
||||
|
||||
type Users14LoginEqualityIndexes struct {
|
||||
dbClient *database.DB
|
||||
}
|
||||
|
||||
func (mig *Users14LoginEqualityIndexes) Execute(ctx context.Context, _ eventstore.Event) error {
|
||||
var exists bool
|
||||
err := mig.dbClient.QueryRowContext(ctx, func(r *sql.Row) error {
|
||||
return r.Scan(&exists)
|
||||
}, users14TableExistsQuery)
|
||||
if err != nil || !exists {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := mig.dropInvalidIndexes(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
statements, err := readStatements(users14LoginEqualityIndexes, "76")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, stmt := range statements {
|
||||
logging.Info(ctx, "execute statement", "file", stmt.file, "migration", mig.String())
|
||||
if _, err := mig.dbClient.ExecContext(ctx, stmt.query); err != nil {
|
||||
return fmt.Errorf("%s %s: %w", mig.String(), stmt.file, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (mig *Users14LoginEqualityIndexes) dropInvalidIndexes(ctx context.Context) error {
|
||||
for _, name := range users14LoginEqualityIndexNames {
|
||||
var invalid bool
|
||||
err := mig.dbClient.QueryRowContext(ctx, func(r *sql.Row) error {
|
||||
return r.Scan(&invalid)
|
||||
}, users14InvalidIndexQuery, name)
|
||||
if err != nil {
|
||||
return fmt.Errorf("%s check invalid index %s: %w", mig.String(), name, err)
|
||||
}
|
||||
if !invalid {
|
||||
continue
|
||||
}
|
||||
drop := dropInvalidIndexConcurrently + name
|
||||
logging.Info(ctx, "drop invalid leftover index", "index", name, "migration", mig.String())
|
||||
if _, err := mig.dbClient.ExecContext(ctx, drop); err != nil {
|
||||
return fmt.Errorf("%s drop invalid index %s: %w", mig.String(), name, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (mig *Users14LoginEqualityIndexes) String() string {
|
||||
return "76_users14_login_equality_indexes"
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_username_lower_idx ON projections.users14 (instance_id, LOWER(username));
|
||||
@@ -0,0 +1 @@
|
||||
CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_humans_phone_lower_idx ON projections.users14_humans (instance_id, LOWER(phone));
|
||||
@@ -0,0 +1,92 @@
|
||||
package setup
|
||||
|
||||
import (
|
||||
"context"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/DATA-DOG/go-sqlmock"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/database"
|
||||
)
|
||||
|
||||
const (
|
||||
users14UsernameLowerCreate = "CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_username_lower_idx ON projections.users14 (instance_id, LOWER(username));\n"
|
||||
users14PhoneLowerCreate = "CREATE INDEX CONCURRENTLY IF NOT EXISTS users14_humans_phone_lower_idx ON projections.users14_humans (instance_id, LOWER(phone));\n"
|
||||
)
|
||||
|
||||
func TestUsers14LoginEqualityIndexes_Execute(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
expects func(sqlmock.Sqlmock)
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "table missing, no drop or create",
|
||||
expects: func(mock sqlmock.Sqlmock) {
|
||||
mock.ExpectQuery(regexp.QuoteMeta(users14TableExistsQuery)).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "table exists, indexes valid, creates both concurrently",
|
||||
expects: func(mock sqlmock.Sqlmock) {
|
||||
expectUsers14TableExists(mock, true)
|
||||
expectInvalidIndex(mock, users14UsernameLowerIdx, false)
|
||||
expectInvalidIndex(mock, users14HumansPhoneLowerIdx, false)
|
||||
mock.ExpectExec(regexp.QuoteMeta(users14UsernameLowerCreate)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
mock.ExpectExec(regexp.QuoteMeta(users14PhoneLowerCreate)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "username index invalid, drop then create both",
|
||||
expects: func(mock sqlmock.Sqlmock) {
|
||||
expectUsers14TableExists(mock, true)
|
||||
expectInvalidIndex(mock, users14UsernameLowerIdx, true)
|
||||
mock.ExpectExec(regexp.QuoteMeta(dropInvalidIndexConcurrently + users14UsernameLowerIdx)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
expectInvalidIndex(mock, users14HumansPhoneLowerIdx, false)
|
||||
mock.ExpectExec(regexp.QuoteMeta(users14UsernameLowerCreate)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
mock.ExpectExec(regexp.QuoteMeta(users14PhoneLowerCreate)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 0))
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
db, mock, err := sqlmock.New()
|
||||
require.NoError(t, err)
|
||||
defer func() {
|
||||
require.NoError(t, mock.ExpectationsWereMet())
|
||||
}()
|
||||
defer db.Close()
|
||||
tt.expects(mock)
|
||||
|
||||
mig := &Users14LoginEqualityIndexes{
|
||||
dbClient: &database.DB{DB: db},
|
||||
}
|
||||
err = mig.Execute(context.Background(), nil)
|
||||
if tt.wantErr {
|
||||
assert.Error(t, err)
|
||||
return
|
||||
}
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func expectUsers14TableExists(mock sqlmock.Sqlmock, exists bool) {
|
||||
mock.ExpectQuery(regexp.QuoteMeta(users14TableExistsQuery)).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(exists))
|
||||
}
|
||||
|
||||
func expectInvalidIndex(mock sqlmock.Sqlmock, name string, invalid bool) {
|
||||
mock.ExpectQuery(regexp.QuoteMeta(users14InvalidIndexQuery)).
|
||||
WithArgs(name).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(invalid))
|
||||
}
|
||||
@@ -195,6 +195,7 @@ type Steps struct {
|
||||
s73FixUserGrantRoles *FixUserGrantRoles
|
||||
s74Apps7OIDCConfigsAddRegistrationToken *Apps7OIDCConfigsAddRegistrationToken
|
||||
s75Apps7OIDCConfigsAddAppLinkConfig *Apps7OIDCConfigsAddAppLinkConfig
|
||||
s76Users14LoginEqualityIndexes *Users14LoginEqualityIndexes
|
||||
RelationalTables *TransactionalTables
|
||||
}
|
||||
|
||||
|
||||
@@ -258,6 +258,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string)
|
||||
steps.s73FixUserGrantRoles = &FixUserGrantRoles{eventstore: eventstoreClient}
|
||||
steps.s74Apps7OIDCConfigsAddRegistrationToken = &Apps7OIDCConfigsAddRegistrationToken{dbClient: dbClient}
|
||||
steps.s75Apps7OIDCConfigsAddAppLinkConfig = &Apps7OIDCConfigsAddAppLinkConfig{dbClient: dbClient}
|
||||
steps.s76Users14LoginEqualityIndexes = &Users14LoginEqualityIndexes{dbClient: dbClient}
|
||||
|
||||
err = projection.Create(ctx, dbClient, eventstoreClient, config.Projections, nil, nil, nil)
|
||||
if err != nil {
|
||||
@@ -316,6 +317,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string)
|
||||
steps.s67SyncMemberRoleFields,
|
||||
steps.s69CacheTablesLogged,
|
||||
steps.s70AddEventStoreCommandEnforceOwner,
|
||||
steps.s76Users14LoginEqualityIndexes,
|
||||
} {
|
||||
setupErr = executeMigration(ctx, eventstoreClient, step, "migration failed")
|
||||
if setupErr != nil {
|
||||
|
||||
Reference in New Issue
Block a user