Repository: cloudspannerecosystem/harbourbridge (go)

Issue:
Early exit if pg_dump not parsable
If pg_dump is not parsable, we still proceed and create a DB (with empty schema) and then go on to do data conversion (which will fail since there is no schema). This all takes time and the output from the tool is very clear about what happened i.e. it says 'no schema and no data', but doesn't pinpoint the likely cause.

A better user experience would be to recognize that the pg_dump wasn't parsable, print an appropriate error and exit early.

Pull request:
fix: update to exit early when the file is not parsable.
This change fixes an issue that when the file is not parsable, we should not create the database and exit early. 

Fixes #15 

- [X] Tests pass
- [ ] Appropriate changes to README are included in PR

Patch:
diff --git a/README.md b/README.md
index 80fe147..9bce067 100644
--- a/README.md
+++ b/README.md
@@ -285,7 +285,7 @@ The HarbourBridge tool maps PostgreSQL types to Spanner types as follows:
 | `BIGINT`           | `INT64`                |                               |
 | `BIGSERIAL`        | `INT64`                | a                             |
 | `BYTEA`            | `BYTES(MAX)`           |                               |
-| `CHAR`             | `STRING(MAX)`          |                               |
+| `CHAR`             | `STRING(1)`            | CHAR defaults to length 1     |
 | `CHAR(N)`          | `STRING(N)`            | c                             |
 | `DATE`             | `DATE`                 |                               |
 | `DOUBLE PRECISION` | `FLOAT64`              |                               |
diff --git a/internal/postgres/process.go b/internal/postgres/process.go
index b776b59..586e230 100644
--- a/internal/postgres/process.go
+++ b/internal/postgres/process.go
@@ -31,11 +31,14 @@ import (
 // In schema mode, ProcessPgDump incrementally builds a schema (updating conv).
 // In data mode, ProcessPgDump uses this schema to convert PostgreSQL data
 // and writes it to Spanner, using the data sink specified in conv.
-func ProcessPgDump(conv *Conv, r *internal.Reader) {
+func ProcessPgDump(conv *Conv, r *internal.Reader) error {
 	for {
 		startLine := r.LineNumber
 		startOffset := r.Offset
-		b, stmts := readAndParseChunk(conv, r)
+		b, stmts, err := readAndParseChunk(conv, r)
+		if err != nil {
+			return err
+		}
 		ci := processStatements(conv, stmts)
 		internal.VerbosePrintf("Parsed SQL command at line=%d/fpos=%d: %d stmts (%d lines, %d bytes) ci=%v\n", startLine, startOffset, len(stmts), r.LineNumber-startLine, len(b), ci != nil)
 		if ci != nil {
@@ -53,6 +56,8 @@ func ProcessPgDump(conv *Conv, r *internal.Reader) {
 	if conv.schemaMode() {
 		conv.AddPrimaryKeys()
 	}
+
+	return nil
 }
 
 // ProcessRow converts a row of data and writes it out to Spanner.
@@ -98,7 +103,7 @@ func byteSize(r *row) int64 {
 
 // readAndParseChunk parses a chunk of pg_dump data, returning the bytes read,
 // the parsed AST (nil if nothing read), and whether we've hit end-of-file.
-func readAndParseChunk(conv *Conv, r *internal.Reader) ([]byte, []nodes.Node) {
+func readAndParseChunk(conv *Conv, r *internal.Reader) ([]byte, []nodes.Node, error) {
 	var l [][]byte
 	for {
 		b := r.ReadLine()
@@ -117,7 +122,7 @@ func readAndParseChunk(conv *Conv, r *internal.Reader) ([]byte, []nodes.Node) {
 			}
 			tree, err := pg_query.Parse(string(s))
 			if err == nil {
-				return s, tree.Statements
+				return s, tree.Statements, nil
 			}
 			// Likely causes of failing to parse:
 			// a) complex statements with embedded semicolons e.g. 'CREATE FUNCTION'
@@ -127,10 +132,7 @@ func readAndParseChunk(conv *Conv, r *internal.Reader) ([]byte, []nodes.Node) {
 			conv.stats.reparsed++
 		}
 		if r.EOF {
-			if len(l) != 0 {
-				fmt.Printf("Error parsing last %d line(s) of input\n", len(l))
-			}
-			return nil, nil
+			return nil, nil, fmt.Errorf("Error parsing last %d line(s) of input", len(l))
 		}
 	}
 }
diff --git a/main.go b/main.go
index 8104d66..e838a70 100644
--- a/main.go
+++ b/main.go
@@ -94,7 +94,10 @@ func main() {
 	if filePrefix == "" {
 		filePrefix = dbName + "."
 	}
-	firstPass(f, n, conv)
+	err = firstPass(f, n, conv)
+	if err != nil {
+		panic(fmt.Errorf("Failed to parse the data file: %v", err))
+	}
 	writeSchemaFile(conv, now, filePrefix+schemaFile)
 	db, err := createDatabase(project, instance, dbName, now, conv)
 	if err != nil {
@@ -143,13 +146,17 @@ func report(bw *spanner.BatchWriter, bytesRead int64, now time.Time, db string,
 	writeBadData(bw, conv, banner, filePrefix+badDataFile)
 }
 
-func firstPass(f *os.File, fileSize int64, conv *postgres.Conv) {
+func firstPass(f *os.File, fileSize int64, conv *postgres.Conv) error {
 	p := internal.NewProgress(fileSize, "Generating schema", internal.Verbose())
 	r := internal.NewReader(bufio.NewReader(f), p)
 	conv.SetSchemaMode() // Build schema and ignore data in pg_dump.
 	conv.SetDataSink(nil)
-	postgres.ProcessPgDump(conv, r)
+	err := postgres.ProcessPgDump(conv, r)
+	if err != nil {
+		return err
+	}
 	p.Done()
+	return nil
 }
 
 func secondPass(f *os.File, client *sp.Client, conv *postgres.Conv, totalRows int64) *spanner.BatchWriter {


Test patch:
diff --git a/internal/postgres/process_test.go b/internal/postgres/process_test.go
index 6a6d327..8e3fbf1 100644
--- a/internal/postgres/process_test.go
+++ b/internal/postgres/process_test.go
@@ -530,6 +530,20 @@ func TestProcessPgDump_AddPrimaryKeys(t *testing.T) {
 	}
 }
 
+func TestProcessPgDump_WithUnparsableContent(t *testing.T) {
+	s := "This is unparsable content"
+	conv := MakeConv()
+	conv.SetLocation(time.UTC)
+	conv.SetSchemaMode()
+	err := ProcessPgDump(conv, internal.NewReader(bufio.NewReader(strings.NewReader(s)), nil))
+	if err == nil {
+		t.Fatalf("Expect an error, but got nil")
+	}
+	if !strings.Contains(err.Error(), "Error parsing input") {
+		t.Fatalf("Expect a parsing error, but got %q", err)
+	}
+}
+
 func runProcessPgDump(s string) (*Conv, []spannerData) {
 	conv := MakeConv()
 	conv.SetLocation(time.UTC)