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

Expose deparseSync #31

Closed
wants to merge 2 commits into from
Closed

Conversation

jgoux
Copy link

@jgoux jgoux commented Feb 7, 2023

Hello 👋,

I'm attempting to expose the pg_query_deparse_protobuf function.

I tried to follow the existing code, but I don't have any experience with C++ or NAPI so I'm stuck on the DeparseQuerySync function in sync.cc. 😅

I understand that pg_query_deparse_protobuf is expecting the struct PgQueryProtobuf as its 1st argument, but I'm not sure how to declare and pass it.

Right now it generates this error:

../src/sync.cc:8:33: error: no matching function for call to 'pg_query_deparse_protobuf'
  PgQueryDeparseResult result = pg_query_deparse_protobuf(parse_tree.c_str());
                                ^~~~~~~~~~~~~~~~~~~~~~~~~
../libpg_query/include/pg_query.h:95:22: note: candidate function not viable: no known conversion from 'const std::basic_string<char>::value_type *' (aka 'const char *') to 'PgQueryProtobuf' for 1st argument
PgQueryDeparseResult pg_query_deparse_protobuf(PgQueryProtobuf parse_tree);

Any help is appreciated. 🙏

EDIT:

In fact, it seems like pg_query_deparse_protobuf will only be able to accept the result of pg_query_parse_protobuf, so it won't work on the result of pg_query_parse if I understand correctly. 🤔
We would need to add a pg_query_deparse to libpg_query which would accept the result of pg_query_parse and skip the protobuf layer. I opened an issue about it: pganalyze/libpg_query#178.

I would love to have your feedback @pyramation on this matter, I saw you opened an issue about deparsing in libpg_query and you were interested in bringing it into this lib. 😄

@wesselvdv
Copy link

Might be interesting to just expose all the protobuf stuff. It’s a bit more efficient than json in terms of size.

@tomquist
Copy link

Opened #32 to address the protobuf issue by generating a protobuf message from the json parse tree, which gets sent over to the native library.

@jgoux jgoux closed this Mar 30, 2023
@pyramation pyramation reopened this Feb 21, 2024
@pyramation pyramation changed the base branch from v15 to expose-deparse February 21, 2024 03:51
@pyramation pyramation deleted the branch launchql:expose-deparse May 2, 2024 08:00
@pyramation pyramation closed this May 2, 2024
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