Repository navigation
Conversation
|
@psteinroe Thanks for the contribution, and appreciate the proactive disclosure of having used Claude Code - I think that's not a deal breaker as long as you have tested/reviewed the result. I've been looking at this already as part of the Postgres 18 PR - I will take your work here into account (and may use parts of it), and will try to take this into the Postgres 18 update that I'm actively working on completing. |
|
@psteinroe I ended up running out of time to get this into the initial 18 release, but I think this still makes sense to do. In case you have time available to work on this in this PR after all, two things that'd be helpful:
|
80a3249 to
dc0914f
Compare
|
@psteinroe Thank you for addressing the review! |
fair question! nearly all of the 15k is generated code that needs to be checked in, like for the regular parse tree. beside the generated code, there is only about 600 new lines plus 300 for tests. but to make it easier, i split the commits now so the generated part is clearly separated. |
Adds plpgsql.h, and parser/parser.h for RawParseMode, to the headers extract_headers.rb reads, so that the PL/pgSQL output can be generated from them like the parse tree output. Two small fixes to the extraction, which don't change the data for the existing headers: * Double pointers like "char **fieldnames" kept only one "*" in their type. * Anonymous enums (the PLPGSQL_RC_* return codes) were stored without a name. The data for the other headers is kept as is: re-extracting gram.h with a newer Bison and the Postgres 18.6 headers changes unrelated entries (token values, CreateStatsStmt.owner).
Generated by scripts/extract_headers.rb, no manual changes.
The PL/pgSQL output is now generated from plpgsql.h by generate_protobuf_and_funcs.rb, like the parse tree output, instead of being written by hand in pg_query_json_plpgsql.c. The generator adds Protobuf messages for the PL/pgSQL structs to pg_query.proto, and output functions using the same WRITE_* macros to pg_query_outfuncs_plpgsql_defs.c, so the JSON output follows these messages (the protobuf-cpp build produces it with MessageToJsonString). What can't be told from the struct definitions is listed in the generator: fields that are not output (state set at execution time, catalog lookups, back-pointers), the struct type of Lists and arrays, and the types like PLpgSQL_stmt that point to one of several structs, which are output as a oneof of these, like Node. This changes the JSON output in a few ways: * All fields the parser sets are output, which adds retvarno (pganalyze#333), dno for all datums, initvarnos, the exception block varnos and some function settings * Fields that point to a specific struct (e.g. expr, datatype) are output like specific node fields, without the struct name around them * Enums are output by name instead of as numbers * PLpgSQL_row fields are output as fieldnames and varnos, and exception conditions as a linked list, following the struct definitions The output functions now check the stack depth, so pg_query_parse_plpgsql returns an error instead of crashing for overly nested functions. Also adds sample functions for the statement types the samples didn't cover yet.
Generated by scripts/generate_protobuf_and_funcs.rb (pg_query.proto, src/include/pg_query_outfuncs_plpgsql_defs.c) and protoc 36.2 with protoc-gen-upb (protobuf/pg_query.upb*), no manual changes.
Returns the PL/pgSQL functions in the input as a Protobuf message (PlpgsqlParseResult), like pg_query_parse_protobuf does for the parse tree. The result is freed with pg_query_free_plpgsql_protobuf_parse_result. The functions are output from the same generated definitions as the JSON output of pg_query_parse_plpgsql, so test/parse_plpgsql_protobuf checks that the output can be decoded, has the same functions as the JSON output, and returns errors the same way.
dc0914f to
59331b5
Compare
|
@lfittl i think it's ready for another review. same disclosure as last time: Generated with AI, and I have no experience with ruby or c. validated it on the language server project by pointing the binding at this branch. |
Adds support for parsing plpgsql into protobuf, replacing the experimental json output. I do not have much experience in C or Ruby but tried to follow existing patterns as close as possible. I verified it by pointing the Rust binding in the Postgres Language Server project at my fork. Tests pass, and it seems to work just fine (supabase-community/postgres-language-server#639).
This would enable much better plpgsql support in the Language Server! :)
Summary of changes:
Update: Rebased on
18-latestand reworked following the review:extract_headers.rbandgenerate_protobuf_and_funcs.rbinstead of separate scripts. They generate both the JSON output ofpg_query_parse_plpgsql, which replaces the hand-writtenpg_query_json_plpgsql.cand fixes fix: serializeretvarnofield in PLpgSQL RETURN/RETURN NEXT JSON output #333, and the newpg_query_parse_plpgsql_protobuf.