Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: PostgreSQL capture correct line and column numbers for parse error #2289

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

StevenACoffman
Copy link
Contributor

@StevenACoffman StevenACoffman commented May 28, 2023

Fixes issue #263

Recently pg_query_go began to return the full structured error info instead of just error message in pganalyze/pg_query_go#76 which has released this functionality in v4.2.1. This sqlc PR #2288 updates to use v4.2.1 for pg_query_go.

After that PR is merged, sqlc will still not correctly display the line and column numbers, as pg_query_go parser.Error type has an Error() method that only includes the message, even though that same parser.Error type has all the other fields public.

This PR attempts to normalize the pg_query_go parser.Error type to a sqlc *sqlerr.Error.

So it looks like a PgQueryError error has:

		error = malloc(sizeof(PgQueryError));
		error->message   = strdup(error_data->message);
		error->filename  = strdup(error_data->filename);
		error->funcname  = strdup(error_data->funcname);
		error->context   = NULL;
		error->lineno    = error_data->lineno;
		error->cursorpos = error_data->cursorpos;

		result.error = error;	

Which then is read by

func newPgQueryError(errC *C.PgQueryError) *Error {
	err := &Error{
		Message:   C.GoString(errC.message),
		Lineno:    int(errC.lineno),
		Cursorpos: int(errC.cursorpos),
	}
	if errC.funcname != nil {
		err.Funcname = C.GoString(errC.funcname)
	}
	if errC.filename != nil {
		err.Filename = C.GoString(errC.filename)
	}
	if errC.context != nil {
		err.Context = C.GoString(errC.context)
	}
	return err
}

So the pg_query_go *parser.Error should be fully populated:

type Error struct {
	Message   string // exception message
	Funcname  string // source function of exception (e.g. SearchSysCache)
	Filename  string // source of exception (e.g. parse.l)
	Lineno    int    // source of exception (e.g. 104)
	Cursorpos int    // char in query at which exception occurred
	Context   string // additional context (optional, can be NULL)
}

Which is not exactly the same as the sqlc *sqlerror.Error type:

type Error struct {
	Err      error
	Code     string
	Message  string
	Location int
	Line     int
	Column   int
	// Hint     string
}

I believe that:

  • pg_query_go*parser.Error.Lineno is equivalent to sqlc *sqlerror.Error.Line
  • pg_query_go *parser.Error.Cursorpos is equivalent to sqlc *sqlerror.Error.Location
  • pg_query_go *parser.Error.Message is equivalent to sqlc *sqlerror.Error.Message

dependabot bot and others added 2 commits May 26, 2023 10:57
Bumps [github.com/pganalyze/pg_query_go/v4](https://github.com/pganalyze/pg_query_go) from 4.2.0 to 4.2.1.
- [Changelog](https://github.com/pganalyze/pg_query_go/blob/main/CHANGELOG.md)
- [Commits](pganalyze/pg_query_go@v4.2.0...v4.2.1)

---
updated-dependencies:
- dependency-name: github.com/pganalyze/pg_query_go/v4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Copy link
Collaborator

@kyleconroy kyleconroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This fixes a really old annoying issue with misleading line numbers.

@Jille
Copy link
Contributor

Jille commented Jun 21, 2023

Thank you indeed! That sure was annoying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants