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

Upgrade to Postgres 17 #264

Merged
merged 37 commits into from
Oct 31, 2024
Merged

Upgrade to Postgres 17 #264

merged 37 commits into from
Oct 31, 2024

Conversation

msepga
Copy link
Contributor

@msepga msepga commented Sep 26, 2024

Noteworthy commits that change the build/test system:

These are no longer built automatically.
If the helper `spcachekey_hash` and `spcachekey_equal` static functions
are not included in the source, this fails to build. We instead just
avoid generating the hashtable function implementations entirely.
The parser now adds a single entry in the `keys` array, with the string
`value`.
With PostgreSQL 17, we need to need to support `JsonTablePlan`, an
abstract node type that itself has no fields. In the fingerprint and
protobuf function generators, we now add support to cast these nodes to
plain `(Node*)`, fixing compiler warnings where pointer types mismatch.
@msepga msepga force-pushed the 17-latest-dev branch 3 times, most recently from ce9e2ee to 97ce3a3 Compare September 27, 2024 18:57
@msepga msepga marked this pull request as ready for review September 27, 2024 19:49
@msepga msepga requested a review from lfittl September 30, 2024 23:15
@lelit
Copy link
Contributor

lelit commented Oct 13, 2024

Hi, I'm quite slowly trying to upgrade my pglast tool to this branch, having some problem here and there with the new abstract nodes, but I'm confident I'll get there.

One of the first failing tests is a worrisome segmentation fault: the test simply exercises the protobuf parse/deparse entry points (ie, it just calls pg_query_parse_protobuf() and then pg_query_deparse_protobuf(), with very little massaging on the pglast side), passing a trivial query such as SELECT 1.

So I morphed your examples/simple.c into the following deparse.c:

// Welcome to the easiest way to parse an SQL query :-)
// Compile the file like this:
//
// cc -I../ -L../ deparse.c -lpg_query

#include <pg_query.h>
#include <stdio.h>
#include <stdlib.h>

size_t testCount = 6;
const char* tests[] = {
  "SELECT 1",
  "SELECT * FROM x WHERE z = 2",
  "SELECT 5.41414",
  "SELECT $1",
  "SELECT 999999999999999999999::numeric/1000000000000000000000",
  "SELECT 4790999999999999999999999999999999999999999999999999999999999999999999999999999999999999 * 9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999"
};

int main() {
  PgQueryProtobufParseResult result;
  size_t i;

  for (i = 0; i < testCount; i++) {
    result = pg_query_parse_protobuf(tests[i]);

    if (result.error) {
      printf("error: %s at %d of query %s\n",
             result.error->message,
             result.error->cursorpos,
             tests[i]);
    } else {
      PgQueryDeparseResult deparse_result = pg_query_deparse_protobuf(result.parse_tree);
      if (deparse_result.error) {
        printf("\nERROR for \"%s\"\n  %s\n",
               tests[i],
               deparse_result.error->message);
        pg_query_free_deparse_result(deparse_result);
      } else {
        printf("\n%s --> %s\n",
               tests[i],
               deparse_result.query);
        pg_query_free_deparse_result(deparse_result);
      }
    }

    pg_query_free_protobuf_parse_result(result);
  }

  pg_query_exit();

  return 0;
}

And it fails in the same way:

Program received signal SIGSEGV, Segmentation fault.
0x000055555559f4ab in pg_query_protobuf_to_nodes (protobuf=...) at src/pg_query_readfuncs_protobuf.c:169
169		if (result->n_stmts > 0)
(gdb) bt
#0  0x000055555559f4ab in pg_query_protobuf_to_nodes (protobuf=...) at src/pg_query_readfuncs_protobuf.c:169
#1  0x000055555558e532 in pg_query_deparse_protobuf (parse_tree=...) at src/pg_query_deparse.c:19
#2  0x000055555558e171 in main ()

What am I missing here?

Thanks&bye.

@msepga
Copy link
Contributor Author

msepga commented Oct 15, 2024

Hi @lelit, thanks for the test!

I can't seem to reproduce this here. I've tried the example on macOS (M1 ARM chip) with both DEBUG=1 and regular optimized builds, as well is in an x86_64 ubuntu VM.

Can you try running make clean && make libpg_query.a just to be sure it's not some kind leftover compiled object from a previous release, interfering with the test?

@lelit
Copy link
Contributor

lelit commented Oct 16, 2024

Can you try running make clean && make libpg_query.a just to be sure it's not some kind leftover compiled object from a previous release, interfering with the test?

That fixed it indeed, thank you: didn't come to mind given that I was operating in a (evidently not enterely) fresh clone of pglast, of which libpg_query is a submodule!

Copy link
Member

@lfittl lfittl left a comment

Choose a reason for hiding this comment

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

👍

Outstanding TODO:

  • Add PL/pgSQL test for array types
  • Update Makefile to copy PL/pgSQL regression tests automatically

@msepga msepga merged commit 7fb9821 into 17-latest Oct 31, 2024
28 checks passed
@msepga msepga deleted the 17-latest-dev branch October 31, 2024 01:41
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