Skip to content

fix: normalize segfault - #53

Merged
levkk merged 1 commit into
mainfrom
levkk-fix-normalize
Sep 14, 2026
Merged

levkk merged 1 commit into
mainfrom
levkk-fix-normalize

Conversation

@levkk

@levkk levkk commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Our normalize generates a bunch of invalid SQL nodes. Fixed the issues + running normalize on hasura and Postgres regression tests.

@levkk
levkk merged commit 6a6e167 into main Sep 14, 2026
1 check passed
@levkk
levkk deleted the levkk-fix-normalize branch September 14, 2026 17:18
levkk added a commit to pgdogdev/pgdog that referenced this pull request Sep 14, 2026
### Problem

`pg_parse_raw::normalize` generated invalid SQL AST nodes, which caused
`pg_parse_raw::deparse` to crash with a SEGSEV. Additionally, we were
not running `normalize` on Postgres regression tests, which would of
immediately caught this.

### Solution

pgdogdev/pg_raw_parse#53

Handle the special AST nodes and run `normalize` and `deparse` on the
entire Postgres regression test suite + hasura e2e test suite.
@sgrif

sgrif commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

I'd rather go back to using libpg_parse's impl if the custom one has to grow to this size

@levkk

levkk commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

That's fine, maybe that's the move going forward. My main concern is stability, although performance is a close second. Is our implementation considerably faster than pg_query's?

@sgrif

sgrif commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

I don't remember the degree to which it's faster, but I know ours is faster if we're starting with an already parsed AST, slower if we're starting with a string.

@levkk

levkk commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

This would be the hot path, so maybe it's worth keeping around. Now that we run it against postgres regression and a bunch of weird ones generated by hasura, i'm pretty confident it won't at least segfault again.

@sgrif

sgrif commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

sg

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.

2 participants