[V2] Replace GORM with sqlx for database operations#7160
Merged
Conversation
…ache services Replace GORM-based AutoMigrate with explicit SQL schema files and embed.FS-based migration wrappers that use the new database.Migrate() function from flytestdlib. Signed-off-by: Kevin Su <pingsutw@apache.org>
Remove all gorm:"..." tag portions and TableName() methods from model structs. The db:"..." tags (used by sqlx) are retained. For project.go, add db tags that were previously missing. Table names will now live in SQL queries rather than on the model. Signed-off-by: Kevin Su <pingsutw@apache.org>
Replace gorm.DB with sqlx.DB throughout the database foundation layer.
This removes the GORM dependency from the core database package in favor
of the lighter-weight sqlx + pgx driver combination.
- Rewrite db.go: GetDB/GetReadOnlyDB return *sqlx.DB, remove logConfig param
- Rewrite postgres.go: use sqlx.Open("pgx", dsn) instead of gorm.Open
- Create migrate.go: custom SQL migration runner using schema_migrations table
- Rewrite embedded_postgres_test_helper.go: use sqlx instead of gorm
- Delete gorm.go: GORM logger adapter no longer needed
- Add github.com/jmoiron/sqlx dependency
Signed-off-by: Kevin Su <pingsutw@apache.org>
Replace GORM-based cache service repository implementations with raw sqlx queries. Update error handling to use database/sql.ErrNoRows and a new ErrAlreadyExists sentinel instead of GORM-specific errors. Signed-off-by: Kevin Su <pingsutw@apache.org>
Replace all GORM-based database operations in the runs repository layer with raw SQL via sqlx. This removes the GORM runtime dependency from the runs data access layer while preserving identical behavior. - Rename GormQueryExpr/GormQueryExpression/GetGormOrderExpr to QueryExpr/QueryExpression/GetOrderExpr across interfaces, impls, mocks, and tests - Rewrite project.go, task.go, action.go to use sqlx.DB with $N placeholders and raw SQL (INSERT, SELECT, UPDATE with RETURNING) - Add rewritePlaceholders helper to convert ? to $N for PostgreSQL - Store DSN in actionRepo for pq.Listener instead of extracting from GORM dialector - Add RunSqlxTestMain helper for embedded postgres test infrastructure - Add db tags to Project model for sqlx compatibility - Update all test files to use *sqlx.DB Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Replace all remaining GORM references with sqlx across the app wiring, service layer, setup files, and test files. Remove gorm, gorm-postgres-driver, and gormigrate dependencies from go.mod. Changes: - app/db.go: InitDB returns *sqlx.DB, calls GetDB without logConfig - app/context.go: SetupContext.DB field is now *sqlx.DB - runs/setup.go: Use migrations.RunMigrations(), simplify ready check - cache_service/setup.go: Same migration and ready check updates - cache_service/service/service.go: NewCacheService takes *sqlx.DB - runs/service/run_logs_service.go: Use sql.ErrNoRows instead of gorm.ErrRecordNotFound - runs/service/run_logs_service_test.go: Match sql.ErrNoRows - All test main_test.go files: Use database.RunTestMain with sqlx callbacks - runs/test/api/setup_test.go: Use sqlx.DB, GetDB without logConfig, RunMigrations - go.mod: Remove gorm.io/gorm, gorm.io/driver/postgres, go-gormigrate/gormigrate Signed-off-by: Kevin Su <pingsutw@apache.org>
PostgreSQL requires `= ANY($1)` with pq.Array() for parameterized IN clauses, not `IN $1`. Also regenerate mocks to match renamed interfaces. Signed-off-by: Kevin Su <pingsutw@apache.org>
Update TestBasicFilter_ValueIn and TestParseStringFilters_ValueInState to expect the PostgreSQL-compatible `= ANY(?)` syntax with pq.Array instead of `IN ?`. Signed-off-by: Kevin Su <pingsutw@apache.org>
1. Add prefix parameter to Migrate/Rollback so each service records versions as "runs/001_init_schema" and "cache_service/001_init_schema" instead of bare "001_init_schema". This prevents collisions when multiple services share the same database (unified manager mode). 2. Change all TIMESTAMP columns to TIMESTAMPTZ in migration SQL files. pgx round-trips Go time.Time values through PostgreSQL, and bare TIMESTAMP (without time zone) silently strips the timezone on write then assumes UTC on read, causing offsets in non-UTC environments. TIMESTAMPTZ preserves the timezone correctly. Signed-off-by: Kevin Su <pingsutw@apache.org>
The previous replace-all of TIMESTAMP→TIMESTAMPTZ accidentally also converted CURRENT_TIMESTAMP→CURRENT_TIMESTAMPTZ which doesn't exist in PostgreSQL. CURRENT_TIMESTAMP works correctly with TIMESTAMPTZ columns. Signed-off-by: Kevin Su <pingsutw@apache.org>
- Accept v2's move of app/ -> flytestdlib/app/ and update imports - Remove unused app/db.go (InitDB was dead code) - Fix database.GetDB calls in cmd/ binaries (remove logger.GetConfig arg) Signed-off-by: Kevin Su <pingsutw@apache.org>
machichima
reviewed
Apr 8, 2026
machichima
reviewed
Apr 8, 2026
Signed-off-by: Kevin Su <pingsutw@apache.org>
Member
|
Should we add |
machichima
reviewed
Apr 8, 2026
| return fmt.Errorf("failed to read migration file %s: %w", f, err) | ||
| } | ||
|
|
||
| tx, err := db.BeginTxx(ctx, nil) |
Member
There was a problem hiding this comment.
Coud we add defer tx.Rollback() here to ensure the transaction is closed when Commit failed
Member
Author
There was a problem hiding this comment.
If BeginTxx returns an error, no transaction was created
|
|
||
| var countWhere []string | ||
| if input.Filter != nil { | ||
| expr, _ := input.Filter.QueryExpression("") |
Member
There was a problem hiding this comment.
Should we capture the error here and return it? Like how we did in other function
Member
Author
There was a problem hiding this comment.
we skipped the error before, so Claude skipped it too. Update it now
popojk
reviewed
Apr 8, 2026
popojk
reviewed
Apr 8, 2026
Signed-off-by: Kevin Su <pingsutw@apache.org>
popojk
reviewed
Apr 8, 2026
AdilFayyaz
approved these changes
Apr 8, 2026
Signed-off-by: Kevin Su <pingsutw@apache.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracking issue
N/A
Why are the changes needed?
GORM adds ORM abstraction overhead and implicit query generation that makes it harder to reason about database behavior. Replacing it with sqlx gives us:
database/sqlWhat changes were proposed in this pull request?
Complete migration from
gorm.io/gormtogithub.com/jmoiron/sqlxacross the entire V2 codebase:Database Foundation (
flytestdlib/database/)GetDB()/GetReadOnlyDB()now return*sqlx.DBinstead of*gorm.DBsqlx.Open("pgx", dsn)with the pgx stdlib drivermigrate.go) that reads numbered.sqlfiles fromembed.FSgorm.go(GORM logger adapter)Models
gorm:"..."struct tags withdb:"..."tags for sqlx field mappingTableName()methods — table names are now explicit in SQL queriesRepository Implementations
sqlx.GetContext,sqlx.SelectContext,db.ExecContextgorm.ErrRecordNotFound→sql.ErrNoRowsthroughoutgorm.ErrDuplicatedKey→ customErrAlreadyExistssentinelclause.OnConflict{DoNothing: true}→INSERT ... ON CONFLICT DO NOTHINGclause.Returning{}→RETURNINGSQL clausesGormQueryExpression→QueryExpression,GetGormOrderExpr→GetOrderExprMigrations
gormigratewith plain SQL migration files undersql/directoriesruns/migrations/sql/001_init_schema.sql— full runs service schemacache_service/migrations/sql/001_init_schema.sql— cache service schemaRunMigrations(ctx, db)App & Service Layer
app/context.go:DB *gorm.DB→DB *sqlx.DBmigrations.RunMigrations()instead of gormigratesql.ErrNoRowsDependencies
github.com/jmoiron/sqlx,github.com/jackc/pgx/v5/stdlibgorm.io/gorm,gorm.io/driver/postgres,github.com/go-gormigrate/gormigrate/v2How was this patch tested?
go build ./...passes cleanlygo vet ./...passesgo test ./runs/repository/impl/...— 41 tests).gofileLabels
Check all the applicable boxes
main