Repository: supabase/cli
Pull request #472 (2022-09-24 12:21:23)

fix: allow for long lines in SQL migrations
## What kind of change does this PR introduce?

Bug fix

## What is the current behavior?

When lines that are too long are encountered, the scanner in `internal/utils/parser/token.go#Split` fails, but the `.Err()` is not checked, causing the error to be suppressed and the `internal/utils/parser/token.go#Split` func to return partial data.

See #470 

## What is the new behavior?

* Errors produced by the scanner used in `internal/utils/parser/token.go#Split` are propagated to the caller.
* Lines up to 256K can now be read by `internal/utils/parser/token.go#Split`.

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

* **13** of **17**   **(76.47%)**  changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.08%**) to **39.832%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [internal/db/diff/migra.go](https://coveralls.io/builds/52731213/source?filename=internal%2Fdb%2Fdiff%2Fmigra.go#L133) | 3 | 5 | 60.0%
| [internal/db/reset/reset.go](https://coveralls.io/builds/52731213/source?filename=internal%2Fdb%2Freset%2Freset.go#L103) | 3 | 5 | 60.0%
<!-- | **Total:** | **13** | **17** | **76.47%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/52731213/badge)](https://coveralls.io/builds/52731213) |
| :-- | --: |
| Change from base [Build 3110861124](https://coveralls.io/builds/52704952): |  0.08% |
| Covered Lines: | 1992 |
| Relevant Lines: | 5001 |

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


Patch:
diff --git a/internal/db/diff/migra.go b/internal/db/diff/migra.go
index 8744b2be..a680fbcd 100644
--- a/internal/db/diff/migra.go
+++ b/internal/db/diff/migra.go
@@ -128,7 +128,11 @@ func ApplyMigrations(ctx context.Context, url string, fsys afero.Fs, options ...
 			defer sql.Close()
 			// Batch migration commands, without using statement cache
 			batch := pgconn.Batch{}
-			for _, line := range parser.Split(sql) {
+			lines, err := parser.Split(sql)
+			if err != nil {
+				return err
+			}
+			for _, line := range lines {
 				trim := strings.TrimSpace(strings.TrimRight(line, ";"))
 				if len(trim) > 0 {
 					batch.ExecParams(trim, nil, nil, nil, nil)
diff --git a/internal/db/reset/reset.go b/internal/db/reset/reset.go
index c7875d8b..18eb7cdc 100644
--- a/internal/db/reset/reset.go
+++ b/internal/db/reset/reset.go
@@ -98,7 +98,11 @@ func SeedDatabase(ctx context.Context, url string, fsys afero.Fs, options ...fun
 	defer conn.Close(ctx)
 	// Batch seed commands, safe to use statement cache
 	batch := pgx.Batch{}
-	for _, line := range parser.Split(sql) {
+	lines, err := parser.Split(sql)
+	if err != nil {
+		return err
+	}
+	for _, line := range lines {
 		trim := strings.TrimSpace(strings.TrimRight(line, ";"))
 		if len(trim) > 0 {
 			batch.Queue(trim)
diff --git a/internal/utils/parser/token.go b/internal/utils/parser/token.go
index e868a7b5..05910097 100644
--- a/internal/utils/parser/token.go
+++ b/internal/utils/parser/token.go
@@ -6,6 +6,15 @@ import (
 	"unicode/utf8"
 )
 
+const (
+	// Default max capacity is 64 * 1024 which is not enough for certain lines
+	// containing e.g. geographical data.
+	// 256K ought to be enough for anybody...
+	maxScannerCapacity = 256 * 1024
+	// Equal to `startBufSize` from `bufio/scan.go`
+	startBufSize = 4096
+)
+
 // State transition table for tokenizer:
 //
 //   Ready -> Ready (default)
@@ -67,13 +76,18 @@ func (t *tokenizer) ScanToken(data []byte, atEOF bool) (advance int, token []byt
 // token can be parsed as statement separator.
 //
 // Each statement is split as it is, without removing comments or white spaces.
-func Split(sql io.Reader) (stats []string) {
+func Split(sql io.Reader) (stats []string, err error) {
 	t := tokenizer{state: &ReadyState{}}
 	scanner := bufio.NewScanner(sql)
+
+	// Increase scanner capacity to support very long lines containing e.g. geodata
+	buf := make([]byte, startBufSize)
+	scanner.Buffer(buf, maxScannerCapacity)
+
 	scanner.Split(t.ScanToken)
 	for scanner.Scan() {
 		token := scanner.Text()
 		stats = append(stats, token)
 	}
-	return stats
+	return stats, scanner.Err()
 }


Test patch:
diff --git a/internal/utils/parser/state_test.go b/internal/utils/parser/state_test.go
index a89ff172..e8005003 100644
--- a/internal/utils/parser/state_test.go
+++ b/internal/utils/parser/state_test.go
@@ -5,30 +5,35 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLineComment(t *testing.T) {
 	t.Run("after separator", func(t *testing.T) {
 		sql := "END;-- comment"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"END;", "-- comment"}, stats)
 	})
 
 	t.Run("before separator", func(t *testing.T) {
 		sql := "SELECT --; 1"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT --; 1"}, stats)
 	})
 
 	t.Run("not started", func(t *testing.T) {
 		sql := "- ;END"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"- ;", "END"}, stats)
 	})
 
 	t.Run("between lines", func(t *testing.T) {
 		sql := "-- /* \n; */ END"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"-- /* \n;", " */ END"}, stats)
 	})
 }
@@ -36,19 +41,22 @@ func TestLineComment(t *testing.T) {
 func TestBlockComment(t *testing.T) {
 	t.Run("contains separator", func(t *testing.T) {
 		sql := "SELECT /* ; */ 1;"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT /* ; */ 1;"}, stats)
 	})
 
 	t.Run("nested block", func(t *testing.T) {
 		sql := "SELECT /*; /*;*/ ;*/ 1"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT /*; /*;*/ ;*/ 1"}, stats)
 	})
 
 	t.Run("not started", func(t *testing.T) {
 		sql := "/ * ; */ END"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"/ * ;", " */ END"}, stats)
 	})
 }
@@ -56,19 +64,22 @@ func TestBlockComment(t *testing.T) {
 func TestSeparator(t *testing.T) {
 	t.Run("no spaces", func(t *testing.T) {
 		sql := ";END;;"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{";", "END;", ";"}, stats)
 	})
 
 	t.Run("between spaces", func(t *testing.T) {
 		sql := "BEGIN   ;  END"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"BEGIN   ;", "  END"}, stats)
 	})
 
 	t.Run("backslash escaped", func(t *testing.T) {
 		sql := "\\;;\\;"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"\\;;", "\\;"}, stats)
 	})
 }
@@ -76,19 +87,22 @@ func TestSeparator(t *testing.T) {
 func TestDollarQuote(t *testing.T) {
 	t.Run("named tag", func(t *testing.T) {
 		sql := "$tag$ any ; string$tag$"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"$tag$ any ; string$tag$"}, stats)
 	})
 
 	t.Run("anonymous tag", func(t *testing.T) {
 		sql := "$$\"Dane's horse\"$$"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"$$\"Dane's horse\"$$"}, stats)
 	})
 
 	t.Run("not started", func(t *testing.T) {
 		sql := "SELECT \"$\"; $$"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT \"$\";", " $$"}, stats)
 	})
 }
@@ -96,19 +110,22 @@ func TestDollarQuote(t *testing.T) {
 func TestSingleQuote(t *testing.T) {
 	t.Run("escapes separator", func(t *testing.T) {
 		sql := "SELECT ';' 1"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT ';' 1"}, stats)
 	})
 
 	t.Run("preserves single quote", func(t *testing.T) {
 		sql := "SELECT ';'';' 1"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT ';'';' 1"}, stats)
 	})
 
 	t.Run("literal backslash", func(t *testing.T) {
 		sql := "SELECT '\\'; 1'"
-		stats := Split(strings.NewReader(sql))
+		stats, err := Split(strings.NewReader(sql))
+		require.NoError(t, err)
 		assert.ElementsMatch(t, []string{"SELECT '\\';", " 1'"}, stats)
 	})
 }
diff --git a/internal/utils/parser/token_test.go b/internal/utils/parser/token_test.go
index 5e6559e5..32aa1d88 100644
--- a/internal/utils/parser/token_test.go
+++ b/internal/utils/parser/token_test.go
@@ -34,7 +34,8 @@ func TestSplit(t *testing.T) {
 
 	sql, err := os.Open(filepath.Join(testdata, "all.sql"))
 	require.NoError(t, err)
-	stats := Split(sql)
+	stats, err := Split(sql)
+	require.NoError(t, err)
 
 	assert.ElementsMatch(t, fixture, stats[:len(fixture)])
 }