Repository: supabase/cli
Pull request #1201 (2023-06-12 08:34:59)

fix: dump data with batch inserts
## What kind of change does this PR introduce?

Bug fix https://github.com/supabase/cli/issues/1183

## What is the current behavior?

Data only dump only generates single column inserts which is extremely slow to restore

## What is the new behavior?

- uses `--rows-per-insert` to batch dump 100k records

## Additional context

Tested locally with a million records

```bash
$ psql --single-transaction --variable ON_ERROR_STOP=1  -f supabase/seed.sql
2.02s user 0.63s system 15% cpu 16.957 total
```

using copy instead of insert
```bash
psql --single-transaction --variable ON_ERROR_STOP=1  -f supabase/copy.sql 
0.11s user 0.11s system 4% cpu 5.076 total
```

Although `copy ... from stdin` is faster, pgx support is limited to its [programmatic interface](https://pkg.go.dev/github.com/jackc/pgx#hdr-Copy_Protocol). We might consider using psql instead since parsing the copy statement client side is too complex.


coveralls: ## Pull Request Test Coverage Report for [Build 5241358969](https://coveralls.io/builds/60662757)

* **0** of **7**   **(0.0%)**  changed or added relevant lines in **1** file are covered.
* **7** unchanged lines in **1** file lost coverage.
* Overall coverage decreased (**-0.1%**) to **62.145%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [internal/db/dump/dump.go](https://coveralls.io/builds/60662757/source?filename=internal%2Fdb%2Fdump%2Fdump.go#L47) | 0 | 7 | 0.0%
<!-- | **Total:** | **0** | **7** | **0.0%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [internal/gen/keys/keys.go](https://coveralls.io/builds/60662757/source?filename=internal%2Fgen%2Fkeys%2Fkeys.go#L94) | 7 | 7.94% |
<!-- | **Total:** | **7** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/60662757/badge)](https://coveralls.io/builds/60662757) |
| :-- | --: |
| Change from base [Build 5239275362](https://coveralls.io/builds/60658339): |  -0.1% |
| Covered Lines: | 4590 |
| Relevant Lines: | 7386 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/cmd/db.go b/cmd/db.go
index 0d481358..a22ac879 100644
--- a/cmd/db.go
+++ b/cmd/db.go
@@ -97,6 +97,7 @@ var (
 	}
 
 	dataOnly     bool
+	useCopy      bool
 	roleOnly     bool
 	keepComments bool
 
@@ -109,7 +110,7 @@ var (
 				return err
 			}
 			ctx, _ := signal.NotifyContext(cmd.Context(), os.Interrupt)
-			return dump.Run(ctx, file, dbConfig, dataOnly, roleOnly, keepComments, fsys)
+			return dump.Run(ctx, file, dbConfig, dataOnly, roleOnly, keepComments, useCopy, fsys)
 		},
 	}
 
@@ -238,6 +239,7 @@ func init() {
 	// Build dump command
 	dumpFlags := dbDumpCmd.Flags()
 	dumpFlags.BoolVar(&dataOnly, "data-only", false, "Dumps only data records.")
+	dumpFlags.BoolVar(&useCopy, "use-copy", false, "Uses copy statements in place of inserts.")
 	dumpFlags.BoolVar(&roleOnly, "role-only", false, "Dumps only cluster roles.")
 	dumpFlags.BoolVar(&keepComments, "keep-comments", false, "Keeps commented lines from pg_dump output.")
 	dbDumpCmd.MarkFlagsMutuallyExclusive("data-only", "role-only")
diff --git a/internal/db/dump/dump.go b/internal/db/dump/dump.go
index a9f14104..a9228224 100644
--- a/internal/db/dump/dump.go
+++ b/internal/db/dump/dump.go
@@ -23,7 +23,7 @@ var (
 	dumpRoleScript string
 )
 
-func Run(ctx context.Context, path string, config pgconn.Config, dataOnly, roleOnly, keepComments bool, fsys afero.Fs) error {
+func Run(ctx context.Context, path string, config pgconn.Config, dataOnly, roleOnly, keepComments, useCopy bool, fsys afero.Fs) error {
 	// Initialise output stream
 	var outStream afero.File
 	if len(path) > 0 {
@@ -44,12 +44,16 @@ func Run(ctx context.Context, path string, config pgconn.Config, dataOnly, roleO
 		script = dumpDataScript
 		// We want to dump user data in auth, storage, etc. for migrating to new project
 		excludedSchemas = append([]string{
+			// "auth",
 			"extensions",
 			"pgbouncer",
 			"realtime",
-			"_realtime"},
-			utils.SystemSchemas...,
-		)
+			"_realtime",
+			// "storage",
+			"_analytics",
+			// "supabase_functions",
+			"supabase_migrations",
+		}, utils.SystemSchemas...)
 	} else if roleOnly {
 		fmt.Fprintln(os.Stderr, "Dumping roles from remote database...")
 		script = dumpRoleScript
@@ -60,24 +64,27 @@ func Run(ctx context.Context, path string, config pgconn.Config, dataOnly, roleO
 	}
 	// Run script in docker
 	cmd := []string{"bash", "-c", script, "--"}
+	env := []string{}
+	if !useCopy {
+		env = append(env, "COLUMN_INSERTS=1")
+	}
 	if !keepComments {
-		// Delete comments when arg1 is set
-		cmd = append(cmd, "1")
+		env = append(env, "DELETE_COMMENTS=1")
 	}
 	if err := utils.DockerRunOnceWithConfig(
 		ctx,
 		container.Config{
 			Image: utils.Pg15Image,
-			Env: []string{
-				"PGHOST=" + config.Host,
+			Env: append(env,
+				"PGHOST="+config.Host,
 				fmt.Sprintf("PGPORT=%d", config.Port),
-				"PGUSER=" + config.User,
-				"PGPASSWORD=" + config.Password,
-				"EXCLUDED_SCHEMAS=" + strings.Join(excludedSchemas, "|"),
-				"RESERVED_ROLES=" + strings.Join(utils.ReservedRoles, "|"),
-				"ALLOWED_CONFIGS=" + strings.Join(utils.AllowedConfigs, "|"),
-				"DB_URL=" + config.Database,
-			},
+				"PGUSER="+config.User,
+				"PGPASSWORD="+config.Password,
+				"EXCLUDED_SCHEMAS="+strings.Join(excludedSchemas, "|"),
+				"RESERVED_ROLES="+strings.Join(utils.ReservedRoles, "|"),
+				"ALLOWED_CONFIGS="+strings.Join(utils.AllowedConfigs, "|"),
+				"DB_URL="+config.Database,
+			),
 			Cmd: cmd,
 		},
 		container.HostConfig{
diff --git a/internal/db/dump/templates/dump_data.sh b/internal/db/dump/templates/dump_data.sh
index 09e931a4..37300226 100755
--- a/internal/db/dump/templates/dump_data.sh
+++ b/internal/db/dump/templates/dump_data.sh
@@ -7,18 +7,18 @@ set -euo pipefail
 #   --exclude-table  omit data from migration history tables as they are managed by platform
 #   --column-inserts only column insert syntax is supported, ie. no copy from stdin
 #   --schema '*'     include all other schemas by default
+#
+# Never delete SQL comments because multiline records may begin with them.
 pg_dump \
     --data-only \
-    --column-inserts \
     --quote-all-identifier \
+    ${COLUMN_INSERTS:+--rows-per-insert 100000} \
     --exclude-schema "$EXCLUDED_SCHEMAS" \
     --exclude-table "auth.schema_migrations" \
     --exclude-table "storage.migrations" \
     --exclude-table "supabase_functions.migrations" \
     --schema '*' \
-    --dbname "$DB_URL" \
-| sed -E "${1:+/^--/d}" \
-| uniq
+    --dbname "$DB_URL"
 
 # Reset session config generated by pg_dump
 echo "RESET ALL;"
diff --git a/internal/db/dump/templates/dump_role.sh b/internal/db/dump/templates/dump_role.sh
index 3d9b275e..c6bd51df 100755
--- a/internal/db/dump/templates/dump_role.sh
+++ b/internal/db/dump/templates/dump_role.sh
@@ -22,7 +22,7 @@ pg_dumpall \
 | sed -E "s/ (NOSUPERUSER|NOREPLICATION)//g" \
 | sed -E "s/^-- (.* SET \"($ALLOWED_CONFIGS)\" .*)/\1/" \
 | sed -E "s/GRANT \".*\" TO \"($RESERVED_ROLES)\"/-- &/" \
-| sed -E "${1:+/^--/d}" \
+| sed -E "${DELETE_COMMENTS:+/^--/d}" \
 | uniq
 
 # Reset session config generated by pg_dump
diff --git a/internal/db/dump/templates/dump_schema.sh b/internal/db/dump/templates/dump_schema.sh
index d528800f..b1d122f2 100755
--- a/internal/db/dump/templates/dump_schema.sh
+++ b/internal/db/dump/templates/dump_schema.sh
@@ -25,7 +25,7 @@ pg_dump \
 | sed -E "s/^REVOKE (.+) ON (.+) \"($EXCLUDED_SCHEMAS)\"/-- &/" \
 | sed -E 's/^CREATE POLICY "cron_job_/-- &/' \
 | sed -E 's/^ALTER TABLE "cron"/-- &/' \
-| sed -E "${1:+/^--/d}" \
+| sed -E "${DELETE_COMMENTS:+/^--/d}" \
 | uniq
 
 # Reset session config generated by pg_dump
diff --git a/internal/db/remote/commit/commit.go b/internal/db/remote/commit/commit.go
index 1893ef70..ec2b820a 100644
--- a/internal/db/remote/commit/commit.go
+++ b/internal/db/remote/commit/commit.go
@@ -74,7 +74,7 @@ func fetchRemote(p utils.Program, ctx context.Context, schema []string, timestam
 		return err
 	} else if len(migrations) == 0 {
 		p.Send(utils.StatusMsg("Committing initial migration on remote database..."))
-		return dump.Run(ctx, path, config, false, false, false, fsys)
+		return dump.Run(ctx, path, config, false, false, false, false, fsys)
 	}
 
 	w := utils.StatusWriter{Program: p}


Test patch:
diff --git a/internal/db/dump/dump_test.go b/internal/db/dump/dump_test.go
index 8826795f..c22c369e 100644
--- a/internal/db/dump/dump_test.go
+++ b/internal/db/dump/dump_test.go
@@ -35,7 +35,7 @@ func TestPullCommand(t *testing.T) {
 		apitest.MockDockerStart(utils.Docker, imageUrl, containerId)
 		require.NoError(t, apitest.MockDockerLogs(utils.Docker, containerId, "hello world"))
 		// Run test
-		err := Run(context.Background(), "schema.sql", dbConfig, false, false, false, fsys)
+		err := Run(context.Background(), "schema.sql", dbConfig, false, false, false, false, fsys)
 		// Check error
 		assert.NoError(t, err)
 		assert.Empty(t, apitest.ListUnmatchedRequests())
@@ -54,7 +54,7 @@ func TestPullCommand(t *testing.T) {
 		apitest.MockDockerStart(utils.Docker, imageUrl, containerId)
 		require.NoError(t, apitest.MockDockerLogs(utils.Docker, containerId, "hello world"))
 		// Run test
-		err := Run(context.Background(), "", dbConfig, false, false, false, fsys)
+		err := Run(context.Background(), "", dbConfig, false, false, false, false, fsys)
 		// Check error
 		assert.NoError(t, err)
 		assert.Empty(t, apitest.ListUnmatchedRequests())
@@ -70,7 +70,7 @@ func TestPullCommand(t *testing.T) {
 			Get("/v" + utils.Docker.ClientVersion() + "/images").
 			Reply(http.StatusServiceUnavailable)
 		// Run test
-		err := Run(context.Background(), "", dbConfig, false, false, false, fsys)
+		err := Run(context.Background(), "", dbConfig, false, false, false, false, fsys)
 		// Check error
 		assert.ErrorContains(t, err, "Error running pg_dump on remote database: request returned Service Unavailable")
 		assert.Empty(t, apitest.ListUnmatchedRequests())
@@ -85,7 +85,7 @@ func TestPullCommand(t *testing.T) {
 		apitest.MockDockerStart(utils.Docker, imageUrl, containerId)
 		require.NoError(t, apitest.MockDockerLogs(utils.Docker, containerId, "hello world"))
 		// Run test
-		err := Run(context.Background(), "schema.sql", dbConfig, false, false, false, fsys)
+		err := Run(context.Background(), "schema.sql", dbConfig, false, false, false, false, fsys)
 		// Check error
 		assert.ErrorContains(t, err, "operation not permitted")
 		assert.Empty(t, apitest.ListUnmatchedRequests())