From 3c8195782167cb5f1d6ab70c59ccbab8fc567e2c Mon Sep 17 00:00:00 2001 From: tianzhou Date: Wed, 8 Apr 2026 00:02:40 -0700 Subject: [PATCH 1/2] fix: show error location with SQL context when applying desired state fails When applying schema SQL fails during plan, PostgreSQL returns an error with a character position but pgschema discarded it, producing generic messages like "syntax error at or near SELECT". Now extracts the position from pgconn.PgError and shows a context snippet with line numbers, making it easy to locate the problematic statement. Closes #389 Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/postgres/desired_state.go | 46 +++++++++++++++++++++++ internal/postgres/desired_state_test.go | 50 +++++++++++++++++++++++++ internal/postgres/embedded.go | 2 +- internal/postgres/external.go | 2 +- 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/internal/postgres/desired_state.go b/internal/postgres/desired_state.go index 6bb9fdde..393ff04d 100644 --- a/internal/postgres/desired_state.go +++ b/internal/postgres/desired_state.go @@ -6,11 +6,14 @@ import ( "context" "crypto/rand" "encoding/hex" + "errors" "fmt" "regexp" "strings" "sync" "time" + + "github.com/jackc/pgx/v5/pgconn" ) // DesiredStateProvider is an interface that abstracts the desired state database provider. @@ -440,3 +443,46 @@ func replaceSchemaInDefaultPrivileges(sql string, targetSchema, tempSchema strin return result } + +// enhanceApplyError extracts the surrounding SQL context from a PostgreSQL error's +// position field to help the user locate the problematic statement in their schema files. +func enhanceApplyError(err error, sql string) error { + var pgErr *pgconn.PgError + if !errors.As(err, &pgErr) || pgErr.Position == 0 { + return err + } + + // PostgreSQL Position is 1-based character (not byte) offset + runes := []rune(sql) + pos := int(pgErr.Position) - 1 + if pos < 0 || pos >= len(runes) { + return err + } + + line := 1 + lineStart := 0 + for i := 0; i < pos; i++ { + if runes[i] == '\n' { + line++ + lineStart = i + 1 + } + } + col := pos - lineStart + 1 + + const contextLines = 3 + lines := strings.Split(sql, "\n") + + startLine := max(line-contextLines, 1) + endLine := min(line+contextLines, len(lines)) + + var snippet strings.Builder + for i := startLine; i <= endLine; i++ { + prefix := " " + if i == line { + prefix = "> " + } + snippet.WriteString(fmt.Sprintf("%s%5d | %s\n", prefix, i, lines[i-1])) + } + + return fmt.Errorf("%w\n\nError location (line %d, column %d):\n%s", err, line, col, snippet.String()) +} diff --git a/internal/postgres/desired_state_test.go b/internal/postgres/desired_state_test.go index 09267318..5fbb02fd 100644 --- a/internal/postgres/desired_state_test.go +++ b/internal/postgres/desired_state_test.go @@ -1,8 +1,12 @@ package postgres import ( + "fmt" "reflect" + "strings" "testing" + + "github.com/jackc/pgx/v5/pgconn" ) func TestSplitDollarQuotedSegments(t *testing.T) { @@ -291,3 +295,49 @@ func TestStripSchemaQualifications_PreservesStringLiterals(t *testing.T) { }) } } + +func TestEnhanceApplyError(t *testing.T) { + sql := "CREATE TABLE foo (id int);\nCREATE TABLE bar (\n name text\n);\nSELECT 1;\nCREATE TABLE baz (id int);" + + t.Run("pgError with position", func(t *testing.T) { + // Position points to "SELECT" on line 5 + pos := int32(strings.Index(sql, "SELECT 1") + 1) // 1-based + pgErr := &pgconn.PgError{ + Message: "syntax error at or near \"SELECT\"", + Code: "42601", + Position: pos, + } + enhanced := enhanceApplyError(pgErr, sql) + errMsg := enhanced.Error() + + if !strings.Contains(errMsg, "line 5") { + t.Errorf("expected error to mention line 5, got: %s", errMsg) + } + if !strings.Contains(errMsg, "SELECT 1") { + t.Errorf("expected error to contain the offending line, got: %s", errMsg) + } + // Should still contain original error + if !strings.Contains(errMsg, "syntax error") { + t.Errorf("expected error to contain original message, got: %s", errMsg) + } + }) + + t.Run("non-pg error passes through", func(t *testing.T) { + origErr := fmt.Errorf("some other error") + result := enhanceApplyError(origErr, sql) + if result.Error() != origErr.Error() { + t.Errorf("expected passthrough, got: %s", result.Error()) + } + }) + + t.Run("pgError without position passes through", func(t *testing.T) { + pgErr := &pgconn.PgError{ + Message: "some error", + Code: "42601", + } + result := enhanceApplyError(pgErr, sql) + if result.Error() != pgErr.Error() { + t.Errorf("expected passthrough, got: %s", result.Error()) + } + }) +} diff --git a/internal/postgres/embedded.go b/internal/postgres/embedded.go index 93b8fc50..377cb219 100644 --- a/internal/postgres/embedded.go +++ b/internal/postgres/embedded.go @@ -238,7 +238,7 @@ func (ep *EmbeddedPostgres) ApplySchema(ctx context.Context, schema string, sql // Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY // that cannot run in transactions. Those are migration details, not state declarations. if _, err := util.ExecContextWithLogging(ctx, conn, schemaAgnosticSQL, "apply desired state SQL to temporary schema"); err != nil { - return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ep.tempSchema, err) + return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ep.tempSchema, enhanceApplyError(err, schemaAgnosticSQL)) } return nil diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 16ce63a3..3df5343a 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -149,7 +149,7 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql // Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY // that cannot run in transactions. Those are migration details, not state declarations. if _, err := util.ExecContextWithLogging(ctx, conn, schemaAgnosticSQL, "apply desired state SQL to temporary schema"); err != nil { - return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ed.tempSchema, err) + return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ed.tempSchema, enhanceApplyError(err, schemaAgnosticSQL)) } return nil From 9ef5525012f3f8ba8dde33820552b69b541cddff Mon Sep 17 00:00:00 2001 From: tianzhou Date: Wed, 8 Apr 2026 00:08:54 -0700 Subject: [PATCH 2/2] test: add multi-byte UTF-8 test and use identity comparison for passthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add test case with multi-byte character (café) to verify character-based position handling works correctly - Use identity comparison (result != origErr) instead of string comparison for passthrough assertions to ensure no accidental wrapping Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/postgres/desired_state_test.go | 29 +++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/internal/postgres/desired_state_test.go b/internal/postgres/desired_state_test.go index 5fbb02fd..812977c3 100644 --- a/internal/postgres/desired_state_test.go +++ b/internal/postgres/desired_state_test.go @@ -322,11 +322,32 @@ func TestEnhanceApplyError(t *testing.T) { } }) + t.Run("multi-byte UTF-8 position", func(t *testing.T) { + // PostgreSQL Position counts characters, not bytes. + // "café" is 4 characters but 5 bytes (é is 2 bytes in UTF-8). + mbSQL := "-- café\nSELECT 1;" + // "SELECT" starts at character position 9 (1-based): "-- café\n" = 8 chars + pgErr := &pgconn.PgError{ + Message: "syntax error", + Code: "42601", + Position: 9, + } + enhanced := enhanceApplyError(pgErr, mbSQL) + errMsg := enhanced.Error() + + if !strings.Contains(errMsg, "line 2, column 1") { + t.Errorf("expected line 2, column 1 for multi-byte SQL, got: %s", errMsg) + } + if !strings.Contains(errMsg, "SELECT 1") { + t.Errorf("expected snippet to contain the error line, got: %s", errMsg) + } + }) + t.Run("non-pg error passes through", func(t *testing.T) { origErr := fmt.Errorf("some other error") result := enhanceApplyError(origErr, sql) - if result.Error() != origErr.Error() { - t.Errorf("expected passthrough, got: %s", result.Error()) + if result != origErr { + t.Errorf("expected same error instance, got: %s", result.Error()) } }) @@ -336,8 +357,8 @@ func TestEnhanceApplyError(t *testing.T) { Code: "42601", } result := enhanceApplyError(pgErr, sql) - if result.Error() != pgErr.Error() { - t.Errorf("expected passthrough, got: %s", result.Error()) + if result != pgErr { + t.Errorf("expected same error instance, got: %s", result.Error()) } }) }