From 945e2513879dff21912a414ccb8fd83cd2616c30 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 3 Apr 2024 21:28:40 -0700 Subject: [PATCH 1/3] read only --- internal/endtoend/endtoend_test.go | 9 +-- internal/sqltest/local/postgres.go | 88 ++++++++++++++++++------------ 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/internal/endtoend/endtoend_test.go b/internal/endtoend/endtoend_test.go index 8420340d80..124c7c936e 100644 --- a/internal/endtoend/endtoend_test.go +++ b/internal/endtoend/endtoend_test.go @@ -120,7 +120,6 @@ func TestReplay(t *testing.T) { "managed-db": { Mutate: func(t *testing.T, path string) func(*config.Config) { return func(c *config.Config) { - c.Cloud.Project = "01HAQMMECEYQYKFJN8MP16QC41" // TODO: Read from environment for i := range c.SQL { files := []string{} for _, s := range c.SQL[i].Schema { @@ -138,18 +137,12 @@ func TestReplay(t *testing.T) { // URI: uri, // } default: - c.SQL[i].Database = &config.Database{ - Managed: true, - } + // pass } } } }, Enabled: func() bool { - // Return false if no auth token exists - if len(os.Getenv("SQLC_AUTH_TOKEN")) == 0 { - return false - } if len(os.Getenv("POSTGRESQL_SERVER_URI")) == 0 { return false } diff --git a/internal/sqltest/local/postgres.go b/internal/sqltest/local/postgres.go index a283273ac5..fae4187fbb 100644 --- a/internal/sqltest/local/postgres.go +++ b/internal/sqltest/local/postgres.go @@ -3,21 +3,21 @@ package local import ( "context" "fmt" + "hash/fnv" "net/url" "os" "strings" - "sync" "testing" "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgxpool" + "golang.org/x/sync/singleflight" migrate "github.com/sqlc-dev/sqlc/internal/migrations" + "github.com/sqlc-dev/sqlc/internal/pgx/poolcache" "github.com/sqlc-dev/sqlc/internal/sql/sqlpath" ) -var postgresPool *pgxpool.Pool -var postgresSync sync.Once +var flight singleflight.Group func PostgreSQL(t *testing.T, migrations []string) string { ctx := context.Background() @@ -28,16 +28,9 @@ func PostgreSQL(t *testing.T, migrations []string) string { t.Skip("POSTGRESQL_SERVER_URI is empty") } - postgresSync.Do(func() { - pool, err := pgxpool.New(ctx, dburi) - if err != nil { - t.Fatal(err) - } - postgresPool = pool - }) - - if postgresPool == nil { - t.Fatalf("PostgreSQL pool creation failed") + postgresPool, err := poolcache.New(ctx, dburi) + if err != nil { + t.Fatalf("PostgreSQL pool creation failed: %s", err) } var seed []string @@ -45,48 +38,71 @@ func PostgreSQL(t *testing.T, migrations []string) string { if err != nil { t.Fatal(err) } + + h := fnv.New64() for _, f := range files { blob, err := os.ReadFile(f) if err != nil { t.Fatal(err) } + h.Write(blob) seed = append(seed, migrate.RemoveRollbackStatements(string(blob))) } + name := fmt.Sprintf("sqlc_test_%x", h.Sum(nil)) + uri, err := url.Parse(dburi) if err != nil { t.Fatal(err) } + uri.Path = name - name := fmt.Sprintf("sqlc_test_%s", id()) + key := uri.String() - if _, err := postgresPool.Exec(ctx, fmt.Sprintf(`CREATE DATABASE "%s"`, name)); err != nil { - t.Fatal(err) - } + _, err, _ = flight.Do(key, func() (interface{}, error) { + row := postgresPool.QueryRow(ctx, + fmt.Sprintf(`SELECT datname FROM pg_database WHERE datname = '%s'`, name)) - uri.Path = name - dropQuery := fmt.Sprintf(`DROP DATABASE IF EXISTS "%s" WITH (FORCE)`, name) + var datname string + if err := row.Scan(&datname); err == nil { + fmt.Println("database already exists", name) + // Database already exists + return nil, nil + } - t.Cleanup(func() { - if _, err := postgresPool.Exec(ctx, dropQuery); err != nil { - t.Fatal(err) + fmt.Println("creating database name", name) + if _, err := postgresPool.Exec(ctx, fmt.Sprintf(`CREATE DATABASE "%s"`, name)); err != nil { + return nil, err } - }) - conn, err := pgx.Connect(ctx, uri.String()) - if err != nil { - t.Fatalf("connect %s: %s", name, err) - } - defer conn.Close(ctx) + dropQuery := fmt.Sprintf(`DROP DATABASE IF EXISTS "%s" WITH (FORCE)`, name) - for _, q := range seed { - if len(strings.TrimSpace(q)) == 0 { - continue + cleanup := func() { + if _, err := postgresPool.Exec(ctx, dropQuery); err != nil { + t.Logf("failed cleaning up: %s", err) + } } - if _, err := conn.Exec(ctx, q); err != nil { - t.Fatalf("%s: %s", q, err) + + conn, err := pgx.Connect(ctx, uri.String()) + if err != nil { + cleanup() + return nil, fmt.Errorf("connect %s: %s", name, err) + } + defer conn.Close(ctx) + + for _, q := range seed { + if len(strings.TrimSpace(q)) == 0 { + continue + } + if _, err := conn.Exec(ctx, q); err != nil { + cleanup() + return nil, fmt.Errorf("%s: %s", q, err) + } } + return nil, nil + }) + if err != nil { + t.Fatalf("create db: %s", err) } - - return uri.String() + return key } From 4c88da75fa49164a725f88441ef3678ade6a6a87 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 3 Apr 2024 21:37:00 -0700 Subject: [PATCH 2/3] test: Run end-to-end tests against a databse --- internal/pgx/poolcache/poolcache.go | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 internal/pgx/poolcache/poolcache.go diff --git a/internal/pgx/poolcache/poolcache.go b/internal/pgx/poolcache/poolcache.go new file mode 100644 index 0000000000..93401ec936 --- /dev/null +++ b/internal/pgx/poolcache/poolcache.go @@ -0,0 +1,32 @@ +package poolcache + +import ( + "context" + "sync" + + "github.com/jackc/pgx/v5/pgxpool" +) + +var lock sync.RWMutex +var pools = map[string]*pgxpool.Pool{} + +func New(ctx context.Context, uri string) (*pgxpool.Pool, error) { + lock.RLock() + existing, found := pools[uri] + lock.RUnlock() + + if found { + return existing, nil + } + + pool, err := pgxpool.New(ctx, uri) + if err != nil { + return nil, err + } + + lock.Lock() + pools[uri] = pool + lock.Unlock() + + return pool, nil +} From f634f227ae6b8188a70ac69b6ac9a071b365f837 Mon Sep 17 00:00:00 2001 From: Kyle Conroy Date: Wed, 3 Apr 2024 23:13:19 -0700 Subject: [PATCH 3/3] Fix some invalid SQL --- internal/endtoend/endtoend_test.go | 2 +- .../golang_initialisms_url/schema.sql | 2 +- .../golang_invalid_sql_package/schema.sql | 2 +- .../schema_scoped_filter/mysql/schema.sql | 1 - internal/sqltest/local/postgres.go | 38 ++++++++----- scripts/cleanup-test-dbs/main.go | 55 +++++++++++++++++++ 6 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 scripts/cleanup-test-dbs/main.go diff --git a/internal/endtoend/endtoend_test.go b/internal/endtoend/endtoend_test.go index 124c7c936e..c2792ee1d3 100644 --- a/internal/endtoend/endtoend_test.go +++ b/internal/endtoend/endtoend_test.go @@ -127,7 +127,7 @@ func TestReplay(t *testing.T) { } switch c.SQL[i].Engine { case config.EnginePostgreSQL: - uri := local.PostgreSQL(t, files) + uri := local.ReadOnlyPostgreSQL(t, files) c.SQL[i].Database = &config.Database{ URI: uri, } diff --git a/internal/endtoend/testdata/golang_initialisms_url/schema.sql b/internal/endtoend/testdata/golang_initialisms_url/schema.sql index 83ab160129..d0b2506904 100644 --- a/internal/endtoend/testdata/golang_initialisms_url/schema.sql +++ b/internal/endtoend/testdata/golang_initialisms_url/schema.sql @@ -1,4 +1,4 @@ -CREATE TABLE foo( +CREATE TABLE foo ( bar_id text, site_url text ); diff --git a/internal/endtoend/testdata/golang_invalid_sql_package/schema.sql b/internal/endtoend/testdata/golang_invalid_sql_package/schema.sql index 1bd72529f8..e29621be57 100644 --- a/internal/endtoend/testdata/golang_invalid_sql_package/schema.sql +++ b/internal/endtoend/testdata/golang_invalid_sql_package/schema.sql @@ -1,3 +1,3 @@ -CREATE TABLE foo( +CREATE TABLE foo ( bar text ); diff --git a/internal/endtoend/testdata/schema_scoped_filter/mysql/schema.sql b/internal/endtoend/testdata/schema_scoped_filter/mysql/schema.sql index a22ab0907d..e5cb43f4bf 100644 --- a/internal/endtoend/testdata/schema_scoped_filter/mysql/schema.sql +++ b/internal/endtoend/testdata/schema_scoped_filter/mysql/schema.sql @@ -1,3 +1,2 @@ CREATE SCHEMA foo; CREATE TABLE foo.bar (id serial not null); - diff --git a/internal/sqltest/local/postgres.go b/internal/sqltest/local/postgres.go index fae4187fbb..3520d42b82 100644 --- a/internal/sqltest/local/postgres.go +++ b/internal/sqltest/local/postgres.go @@ -20,6 +20,14 @@ import ( var flight singleflight.Group func PostgreSQL(t *testing.T, migrations []string) string { + return postgreSQL(t, migrations, true) +} + +func ReadOnlyPostgreSQL(t *testing.T, migrations []string) string { + return postgreSQL(t, migrations, false) +} + +func postgreSQL(t *testing.T, migrations []string, rw bool) string { ctx := context.Background() t.Helper() @@ -49,13 +57,19 @@ func PostgreSQL(t *testing.T, migrations []string) string { seed = append(seed, migrate.RemoveRollbackStatements(string(blob))) } - name := fmt.Sprintf("sqlc_test_%x", h.Sum(nil)) + var name string + if rw { + name = fmt.Sprintf("sqlc_test_%s", id()) + } else { + name = fmt.Sprintf("sqlc_test_%x", h.Sum(nil)) + } uri, err := url.Parse(dburi) if err != nil { t.Fatal(err) } uri.Path = name + dropQuery := fmt.Sprintf(`DROP DATABASE IF EXISTS "%s" WITH (FORCE)`, name) key := uri.String() @@ -65,27 +79,17 @@ func PostgreSQL(t *testing.T, migrations []string) string { var datname string if err := row.Scan(&datname); err == nil { - fmt.Println("database already exists", name) - // Database already exists + t.Logf("database exists: %s", name) return nil, nil } - fmt.Println("creating database name", name) + t.Logf("creating database: %s", name) if _, err := postgresPool.Exec(ctx, fmt.Sprintf(`CREATE DATABASE "%s"`, name)); err != nil { return nil, err } - dropQuery := fmt.Sprintf(`DROP DATABASE IF EXISTS "%s" WITH (FORCE)`, name) - - cleanup := func() { - if _, err := postgresPool.Exec(ctx, dropQuery); err != nil { - t.Logf("failed cleaning up: %s", err) - } - } - conn, err := pgx.Connect(ctx, uri.String()) if err != nil { - cleanup() return nil, fmt.Errorf("connect %s: %s", name, err) } defer conn.Close(ctx) @@ -95,12 +99,18 @@ func PostgreSQL(t *testing.T, migrations []string) string { continue } if _, err := conn.Exec(ctx, q); err != nil { - cleanup() return nil, fmt.Errorf("%s: %s", q, err) } } return nil, nil }) + if rw || err != nil { + t.Cleanup(func() { + if _, err := postgresPool.Exec(ctx, dropQuery); err != nil { + t.Fatalf("failed cleaning up: %s", err) + } + }) + } if err != nil { t.Fatalf("create db: %s", err) } diff --git a/scripts/cleanup-test-dbs/main.go b/scripts/cleanup-test-dbs/main.go new file mode 100644 index 0000000000..51093db307 --- /dev/null +++ b/scripts/cleanup-test-dbs/main.go @@ -0,0 +1,55 @@ +package main + +import ( + "context" + "fmt" + "log" + "os" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" +) + +func main() { + if err := run(); err != nil { + log.Fatal(err) + } +} + +const query = ` +SELECT datname +FROM pg_database +WHERE datname LIKE 'sqlc_test_%' +` + +func run() error { + ctx := context.Background() + dburi := os.Getenv("POSTGRESQL_SERVER_URI") + if dburi == "" { + return fmt.Errorf("POSTGRESQL_SERVER_URI is empty") + } + pool, err := pgxpool.New(ctx, dburi) + if err != nil { + return err + } + + rows, err := pool.Query(ctx, query) + if err != nil { + return err + } + + names, err := pgx.CollectRows(rows, pgx.RowTo[string]) + if err != nil { + return err + } + + for _, name := range names { + drop := fmt.Sprintf(`DROP DATABASE IF EXISTS "%s" WITH (FORCE)`, name) + if _, err := pool.Exec(ctx, drop); err != nil { + return err + } + log.Println("dropping database", name) + } + + return nil +}