Skip to content

Add bindings for pg_query_deparse_protobuf - #32

Merged
pyramation merged 2 commits into
constructive-io:tom/deparse-prfrom
tomquist:tom/deparse
May 2, 2024
Merged

Add bindings for pg_query_deparse_protobuf#32
pyramation merged 2 commits into
constructive-io:tom/deparse-prfrom
tomquist:tom/deparse

Conversation

@tomquist

Copy link
Copy Markdown
Contributor

This exposes deparse functionality to node.js. As of this issue the recommended approach is to generate a protobuf message from the JSON parse tree. One blocking issue is that protobufjs doesn't honor the json_name attribute so it isn't able to parse the existing structure into a protobuf message. There's a PR to add this, however, this seems to be stale atm. In this PR I worked around this by directly pointing to the fork and generating a static JS model from it that I checked in. By checking in the model we could also point back to the original library, however, this makes updating the static model more cumbersome.

This pull request adds bindings for pg_query_deparse_protobuf to expose deparse functionality for Node.js. Currently, the recommended approach is to generate a protobuf message from the JSON parse tree, but the protobufjs library doesn't honor the json_name attribute, which blocks parsing of the existing structure into a protobuf message. Although a PR to address this issue is available here, it seems stale at the moment.

In this PR, a workaround has been implemented by directly pointing to the fork and generating a static JS model from it that is checked in. While checking in the model allows for pointing back to the original library, it also makes updating the static model more cumbersome.

@tomquist tomquist mentioned this pull request Mar 26, 2023
@jgoux

jgoux commented Mar 30, 2023

Copy link
Copy Markdown

This is awesome, thanks for your contribution! 🥳

@benasher44

Copy link
Copy Markdown
Contributor

@pyramation pyramation mentioned this pull request Feb 20, 2024
@pyramation
pyramation deleted the branch constructive-io:tom/deparse-pr February 21, 2024 03:08
@pyramation pyramation closed this Feb 21, 2024
@pyramation

Copy link
Copy Markdown
Collaborator

oops, I deleted master and a few others in an attempt to have more clarity around branches, keep similar naming convention as libqg-query and it automatically closed this PR — re-opening :)

@pyramation pyramation reopened this Feb 21, 2024
@pyramation
pyramation changed the base branch from master to pg_query_deparse_protobuf February 21, 2024 03:53
@pyramation
pyramation changed the base branch from pg_query_deparse_protobuf to tom/deparse-pr May 2, 2024 07:04
@pyramation

Copy link
Copy Markdown
Collaborator

Would love to breathe life back into this! I've actually written protobuf parsers from scratch (see https://github.com/cosmology-tech/telescope)

@tomquist was this PR working for you? If so, I can look into the details of the option json_name and handle this

I could also make a nice pipeline for updating the proto.js that gets generated. I'll take a deeper look :)

@pyramation

Copy link
Copy Markdown
Collaborator
$ npm run test

> libpg-query@13.3.0 test
> mocha --timeout 5000



  Queries
    Sync Parsing
      ✓ should return a single-item parse result for common queries
      ✓ should support parsing multiple queries
      ✓ should not parse a bogus query
    Async parsing
      ✓ should return a promise resolving to same result
      ✓ should reject on bogus queries
    Deparsing
      ✓ async function should return a promise resolving to same SQL
      ✓ sync function should return a same SQL
      ✓ should reject on bogus input
    Fingerprint
      sync
        ✓ should not fingerprint a bogus query
        ✓ should fingerprint a query
      async
        ✓ should not fingerprint a bogus query
        ✓ should fingerprint a query

  PlPgSQL (async)
    ✓ should parse a function


  13 passing (12ms)

NOTE: yarn doesn't work with Tom's branch, but npm install does :)

I think this is a great start, I can work with this!

@pyramation
pyramation marked this pull request as ready for review May 2, 2024 20:49
@pyramation

Copy link
Copy Markdown
Collaborator

I'm gonna merge this PR into a dev branch, and work on this!

@pyramation
pyramation merged commit 9b6a671 into constructive-io:tom/deparse-pr May 2, 2024
@pyramation pyramation mentioned this pull request May 2, 2024
@pyramation

Copy link
Copy Markdown
Collaborator

new PR #63

benasher44 added a commit to ashbyhq/libpg-query-node that referenced this pull request Aug 12, 2026
libpg-query exposed only the parse half of libpg_query. Getting SQL back out
of an AST meant reaching for pgsql-deparser, a hand-written TypeScript
reimplementation of Postgres' deparseRawStmt that has to track a C file
changing every major release.

libpg_query has shipped pg_query_deparse_protobuf() since 2.x — it's present
at every tag this repo builds. The blocker was the one PR constructive-io#32 hit in 2023: it
takes a protobuf-encoded parse tree, and protobufjs ignores the json_name
option. pg_query.proto leans on 1,683 of those annotations to map JSON keys
like SelectStmt and targetList onto snake_case proto fields, so the tree
parse() returns can't be re-encoded. PR constructive-io#32 worked around it by vendoring a
96k-line static model generated from a protobufjs fork.

@bufbuild/protobuf honours json_name, so the fix is now just: generate
schemas, fromJson -> toBinary, hand the bytes to a new WASM entrypoint.

  import { parse, deparse } from 'libpg-query';
  await deparse(await parse('select a,b   from   t'));  // SELECT a, b FROM t

New @ashbyhq/pgsql-proto package (proto/) holds the generated codecs, one per
major version behind ./v13../v18 subpaths. All protobuf-es usage stays behind
an encodeParseTree(tree) -> Uint8Array facade so consumers don't inherit its
moduleResolution: node16 requirement. Generated schemas are committed, so
neither CI nor consumers need a protobuf toolchain.

The C wrapper returns the PgQueryDeparseResult struct rather than a bare
string, so a deparse failure is distinguishable from a query starting with the
word "error" and surfaces real PgQueryError details. Encoding is strict: a
misspelled field or bogus enum value throws instead of being dropped and
deparsed into quietly wrong SQL.

libpg_query grew pg_query_deparse_protobuf_opts and
pg_query_deparse_comments_for_query in its 18 line — present in both pganalyze
18.0.0 and the constructive fork we build v18 from, absent from every 13-17
tag. So v18 additionally gets pretty-printing and comment preservation via
extractComments(); on 13-17 those options are accepted and ignored, keeping
one call signature across versions.

Three pre-existing problems had to be fixed to land this:

- copy:templates was silently reverting v15/v17/v18 to upstream libpg_query.
  Their Makefiles carried hand-edited fork overrides the template didn't
  model, so regenerating pointed v18 at a branch that doesn't exist upstream.
  Repo and ref now live in x-publish.

- v13's WASM build was broken; CI worked around it by downloading a prebuilt
  tarball, which would have shipped a v13 without the new exports. The
  spinlock patch adds a configure flag that never runs, because 13-2.2.0 ships
  pre-extracted Postgres sources. Undefining HAVE_SPINLOCKS directly fixes it —
  slock_t is only typedef'd on that path. CI now builds v13 for real.

- protos/17 was stale relative to the branch we build. Purely additive
  (SummaryResult), so encoding was never at risk, but syncing it regenerated
  types/17 and enums/17. fetch:protos now follows the same repo and ref the
  WASM links against, so the schema can't drift from the library again.

Linking the deparser grows the WASM by ~285-434 KB (v17: +368 KB / +32%).
Isolated from toolchain drift by rebuilding v17 without deparse: 1,150,974
bytes vs 1,150,984 published, a 10-byte delta. It's linked unconditionally, so
parse-only users pay for it too; documented in every README.

538 tests pass: 7 proto, 72 on each of v13-v17, 133 on v18, 38 parser.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

4 participants