Skip to content

Commit e1cffc7

Browse files
tianzhouclaude
andauthored
fix: show error location with SQL context when plan fails (#394)
* 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) <noreply@anthropic.com> * test: add multi-byte UTF-8 test and use identity comparison for passthrough - 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e5673da commit e1cffc7

File tree

4 files changed

+119
-2
lines changed

4 files changed

+119
-2
lines changed

internal/postgres/desired_state.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@ import (
66
"context"
77
"crypto/rand"
88
"encoding/hex"
9+
"errors"
910
"fmt"
1011
"regexp"
1112
"strings"
1213
"sync"
1314
"time"
15+
16+
"github.com/jackc/pgx/v5/pgconn"
1417
)
1518

1619
// DesiredStateProvider is an interface that abstracts the desired state database provider.
@@ -440,3 +443,46 @@ func replaceSchemaInDefaultPrivileges(sql string, targetSchema, tempSchema strin
440443

441444
return result
442445
}
446+
447+
// enhanceApplyError extracts the surrounding SQL context from a PostgreSQL error's
448+
// position field to help the user locate the problematic statement in their schema files.
449+
func enhanceApplyError(err error, sql string) error {
450+
var pgErr *pgconn.PgError
451+
if !errors.As(err, &pgErr) || pgErr.Position == 0 {
452+
return err
453+
}
454+
455+
// PostgreSQL Position is 1-based character (not byte) offset
456+
runes := []rune(sql)
457+
pos := int(pgErr.Position) - 1
458+
if pos < 0 || pos >= len(runes) {
459+
return err
460+
}
461+
462+
line := 1
463+
lineStart := 0
464+
for i := 0; i < pos; i++ {
465+
if runes[i] == '\n' {
466+
line++
467+
lineStart = i + 1
468+
}
469+
}
470+
col := pos - lineStart + 1
471+
472+
const contextLines = 3
473+
lines := strings.Split(sql, "\n")
474+
475+
startLine := max(line-contextLines, 1)
476+
endLine := min(line+contextLines, len(lines))
477+
478+
var snippet strings.Builder
479+
for i := startLine; i <= endLine; i++ {
480+
prefix := " "
481+
if i == line {
482+
prefix = "> "
483+
}
484+
snippet.WriteString(fmt.Sprintf("%s%5d | %s\n", prefix, i, lines[i-1]))
485+
}
486+
487+
return fmt.Errorf("%w\n\nError location (line %d, column %d):\n%s", err, line, col, snippet.String())
488+
}

internal/postgres/desired_state_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package postgres
22

33
import (
4+
"fmt"
45
"reflect"
6+
"strings"
57
"testing"
8+
9+
"github.com/jackc/pgx/v5/pgconn"
610
)
711

812
func TestSplitDollarQuotedSegments(t *testing.T) {
@@ -291,3 +295,70 @@ func TestStripSchemaQualifications_PreservesStringLiterals(t *testing.T) {
291295
})
292296
}
293297
}
298+
299+
func TestEnhanceApplyError(t *testing.T) {
300+
sql := "CREATE TABLE foo (id int);\nCREATE TABLE bar (\n name text\n);\nSELECT 1;\nCREATE TABLE baz (id int);"
301+
302+
t.Run("pgError with position", func(t *testing.T) {
303+
// Position points to "SELECT" on line 5
304+
pos := int32(strings.Index(sql, "SELECT 1") + 1) // 1-based
305+
pgErr := &pgconn.PgError{
306+
Message: "syntax error at or near \"SELECT\"",
307+
Code: "42601",
308+
Position: pos,
309+
}
310+
enhanced := enhanceApplyError(pgErr, sql)
311+
errMsg := enhanced.Error()
312+
313+
if !strings.Contains(errMsg, "line 5") {
314+
t.Errorf("expected error to mention line 5, got: %s", errMsg)
315+
}
316+
if !strings.Contains(errMsg, "SELECT 1") {
317+
t.Errorf("expected error to contain the offending line, got: %s", errMsg)
318+
}
319+
// Should still contain original error
320+
if !strings.Contains(errMsg, "syntax error") {
321+
t.Errorf("expected error to contain original message, got: %s", errMsg)
322+
}
323+
})
324+
325+
t.Run("multi-byte UTF-8 position", func(t *testing.T) {
326+
// PostgreSQL Position counts characters, not bytes.
327+
// "café" is 4 characters but 5 bytes (é is 2 bytes in UTF-8).
328+
mbSQL := "-- café\nSELECT 1;"
329+
// "SELECT" starts at character position 9 (1-based): "-- café\n" = 8 chars
330+
pgErr := &pgconn.PgError{
331+
Message: "syntax error",
332+
Code: "42601",
333+
Position: 9,
334+
}
335+
enhanced := enhanceApplyError(pgErr, mbSQL)
336+
errMsg := enhanced.Error()
337+
338+
if !strings.Contains(errMsg, "line 2, column 1") {
339+
t.Errorf("expected line 2, column 1 for multi-byte SQL, got: %s", errMsg)
340+
}
341+
if !strings.Contains(errMsg, "SELECT 1") {
342+
t.Errorf("expected snippet to contain the error line, got: %s", errMsg)
343+
}
344+
})
345+
346+
t.Run("non-pg error passes through", func(t *testing.T) {
347+
origErr := fmt.Errorf("some other error")
348+
result := enhanceApplyError(origErr, sql)
349+
if result != origErr {
350+
t.Errorf("expected same error instance, got: %s", result.Error())
351+
}
352+
})
353+
354+
t.Run("pgError without position passes through", func(t *testing.T) {
355+
pgErr := &pgconn.PgError{
356+
Message: "some error",
357+
Code: "42601",
358+
}
359+
result := enhanceApplyError(pgErr, sql)
360+
if result != pgErr {
361+
t.Errorf("expected same error instance, got: %s", result.Error())
362+
}
363+
})
364+
}

internal/postgres/embedded.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ func (ep *EmbeddedPostgres) ApplySchema(ctx context.Context, schema string, sql
238238
// Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY
239239
// that cannot run in transactions. Those are migration details, not state declarations.
240240
if _, err := util.ExecContextWithLogging(ctx, conn, schemaAgnosticSQL, "apply desired state SQL to temporary schema"); err != nil {
241-
return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ep.tempSchema, err)
241+
return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ep.tempSchema, enhanceApplyError(err, schemaAgnosticSQL))
242242
}
243243

244244
return nil

internal/postgres/external.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql
149149
// Note: Desired state SQL should never contain operations like CREATE INDEX CONCURRENTLY
150150
// that cannot run in transactions. Those are migration details, not state declarations.
151151
if _, err := util.ExecContextWithLogging(ctx, conn, schemaAgnosticSQL, "apply desired state SQL to temporary schema"); err != nil {
152-
return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ed.tempSchema, err)
152+
return fmt.Errorf("failed to apply schema SQL to temporary schema %s: %w", ed.tempSchema, enhanceApplyError(err, schemaAgnosticSQL))
153153
}
154154

155155
return nil

0 commit comments

Comments
 (0)