how to upgrade pg_query?

[pyramation]

the version of pg_query is old and cannot parse quite a few statements now that are valid.

I'm happy to help and make a PR, are can somebody provide some insight on where to start?

[pyramation]

I was able to download https://github.com/lfittl/libpg_query and compile a new archive file. I then copy the .a file into this repo (and also copy header file):

cp ../libpg_query/libpg_query.a libpg_query/osx/   # I would definitely do linux once I figure out osx
cp ../libpg_query/pg_query.h libpg_query/include/

Then install, build and test

yarn install
yarn build
yarn test

test fails - any clue what I need to do in this process?

1) pg-query should parse a query:

      AssertionError [ERR_ASSERTION]: 'undefined' == 'object'
      + expected - actual

      -undefined
      +object

      at Context.<anonymous> (test/index.js:6:12)

  2) pg-query should parse a null:
     TypeError: Cannot read property 'targetList' of undefined
      at Context.<anonymous> (test/index.js:10:58)

  3) pg-query should parse an empty string:
     TypeError: Cannot read property 'targetList' of undefined
      at Context.<anonymous> (test/index.js:14:56)

[pyramation]

@zhm @lfittl any thoughts here?

[pyramation]

update: I got tests to pass! However, this would seemingly be major breaking changes. The update introduces RawStmt.stmt to each node

var query = require('../');
var assert = require('assert');

describe('pg-query', function() {
  it('should parse a query', function() {
    assert.equal(typeof query.parse('select 1').query[0].RawStmt.stmt.SelectStmt, 'object');
  });

  it('should parse a null', function() {
    assert(query.parse("select null").query[0].RawStmt.stmt.SelectStmt.targetList[0].ResTarget.val.A_Const.val.Null);
  });

  it('should parse an empty string', function() {
    assert(query.parse("select ''").query[0].RawStmt.stmt.SelectStmt.targetList[0].ResTarget.val.A_Const.val.String.str === '');
  });

  it('should not parse a bogus query', function() {
    assert.ok(query.parse('NOT A QUERY').error instanceof Error);
  });
});

[lfittl]

@pyramation Yeah, unfortunately the RawStmt change was one in upstream Postgres with 10, so its not really something that can be avoided.

For better or worse, Postgres parse trees are not supposed to be consistent across versions, which is why this would happen again in the next release (i.e. 11).

[pyramation]

Thanks @lfittl, I see. I suppose people should just tag versions for this repo... what's a good way to deal with this?

fwiw, the tests are passing: https://travis-ci.org/pyramation/node-pg-query-native/builds/414224051

Should I still make a PR?

[pyramation]

here's my PR: #10

[lfittl]

@pyramation Yeah, in general I would consider upgrading the parser version to be a major version increment on this library, and expect people to upgrade to that version whenever they are ready.

Unfortunately no better way to deal with this that I have found :)

[beeing]

@pyramation Since there's no response from @zhm, will you do the honour to publish a new npm package for this - with different name, since this is breaking change anyway.

[beeing]

And BTW, this module will break when npm install in node 10.x - need to update the nan package as well.

[pyramation]

I already did!

npm install pgsql-parser

I have two repos, one is the new name: https://github.com/pyramation/pgsql-parser
The other is the fork to stay in the community: https://github.com/pyramation/pg-query-parser

[pyramation]

@beeing I upgraded pg_query, overhauled the testing system, and added a TON of new functionality. I think well over 100 commits from master.

[pyramation]

Underneath, I also had to fork pg-query-native, https://github.com/pyramation/node-pg-query-native

for now I just called it pg-query-native-latest: https://github.com/pyramation/pg-query-parser/blob/master/package.json#L59

[beeing]

Ah... that's nice. Must have missed this out. Thanks!

[beeing]

I just npm i pg-query-native-latest on node 10, it works.
However noticed many V8 deprecation warnings during the build.

[beeing]

Just out of curiosity, how about removing the RawStmt.stmt after parsing it to maintain the backward compatibility?

And BTW, do you able to add Issues in your forked repo as well? We can use your repo to file new issues going forward.

[lfittl]

@beeing Just one thought re: removing RawStmt.stmt for compatibility - I wouldn’t recommend this for two reasons:

1. That parse node sometimes has benefits (when parsing multiple statements it tells you where each statement ends)
2. This is not guaranteed to be the only incompatible change across versions - its better that the consumer is explicitly aware of the version of the parser (9.5 vs 10), as in a few months there will be a parser release for Postgres 12, and that one will be incompatible in different ways

Unfortunately its not preventable to have incompatible parse trees across versions - thats the same way that Postgres internally handles these structures.

[beeing]

Point noted. Thanks for sharing @lfittl