launchql / libpg-query-node

libpg_query PG port for node.js
MIT License
49 stars 23 forks source link

Missing constant values in AST result output. #65

Open julianjensen opened 4 months ago

julianjensen commented 4 months ago

Issue

It appears that some constants are not correct in the AST output from the parser. I made a minimal example to illustrate the issues. No errors are thrown when running the example. In essence, integers are not present in the AST when the integer is 0 or negative. Boolean values are only present when the value is true but not when false.

This example is a blank project that has only a single dependency:

{
  "dependencies": {
    "libpg-query": "^16.2.0"
  }
}

It contains a single JavaScript file (index.js):

const p = require( 'libpg-query' );

const tests = {
    failBool: {
        sql: 'create table test_table ( yes_no boolean default false, num_val integer default 1 );',
        txt: `Bad boolean, correct integer`
    },
    okayBool: {
        sql: 'create table test_table ( yes_no boolean default true, num_val integer default 1 );',
        txt: `Correct boolean, correct integer`
    },
    failInt: {
        sql: 'create table test_table ( yes_no boolean default true, num_val integer default 0 );',
        txt: `Correct boolean, bad integer`
    },
    failIntAgain: {
        sql: 'create table test_table ( yes_no boolean default true, num_val integer default -1 );',
        txt: `Correct boolean, bad integer`
    }
}

function displayProblem( { sql, ast, txt } )
{
    const table = ast.stmts[ 0 ].stmt.CreateStmt.tableElts;

    const bool = table[ 0 ].ColumnDef.constraints[ 0 ].Constraint.raw_expr.A_Const.boolval;
    const int  = table[ 1 ].ColumnDef.constraints[ 0 ].Constraint.raw_expr.A_Const.ival;

    console.log( `${txt}\n${'-'.repeat( txt.length )}\nsql: ${sql}` );
    console.log( `bool (okay? ${typeof bool.boolval === 'boolean'}):`, bool );
    console.log( `int (okay? ${typeof int.ival === 'number'}):`, int );
    console.log();
}

Promise
    .all( Object.values( tests ).map( ({ sql, txt }) => p.parseQuery( sql ).then( ast => ( { sql, ast, txt } ) ) ) )
    .then( results => results.forEach( displayProblem ) )
    .then( () => p.parseQuery( 'select 0;' ) )
    .then( result => console.log( JSON.stringify( result, null, 4 ) ) );
;

Running this example, displays the following output:

Bad boolean, correct integer
----------------------------
sql: create table test_table ( yes_no boolean default false, num_val integer default 1 );
bool (okay? false): {}
int (okay? true): { ival: 1 }

Correct boolean, correct integer
--------------------------------
sql: create table test_table ( yes_no boolean default true, num_val integer default 1 );
bool (okay? true): { boolval: true }
int (okay? true): { ival: 1 }

Correct boolean, bad integer
----------------------------
sql: create table test_table ( yes_no boolean default true, num_val integer default 0 );
bool (okay? true): { boolval: true }
int (okay? false): {}

Correct boolean, bad integer
----------------------------
sql: create table test_table ( yes_no boolean default true, num_val integer default -1 );
bool (okay? true): { boolval: true }
int (okay? false): {}

{
    "version": 160001,
    "stmts": [
        {
            "stmt": {
                "SelectStmt": {
                    "targetList": [
                        {
                            "ResTarget": {
                                "val": {
                                    "A_Const": {
                                        "ival": {},
                                        "location": 7
                                    }
                                },
                                "location": 7
                            }
                        }
                    ],
                    "limitOption": "LIMIT_OPTION_DEFAULT",
                    "op": "SETOP_NONE"
                }
            },
            "stmt_len": 8
        }
    ]
}

As can be seen, true and positive integers look okay but other values show up as an empty object. I included the example from the README.md to illustrate the issue in another simple context. It can be seen to fail here, as well.

Expected

Expected ival and boolval to hold correct values consistent with the values in the SQL source.

Reproduction

mkdir test-pg
cd test-pg
pnpm init
pnpm add libpg-query

# Create index.js with the contents above

node index.js

My system is as follows:

> more /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=23.10
DISTRIB_CODENAME=mantic
DISTRIB_DESCRIPTION="Ubuntu 23.10"

> uname -a
Linux main-desktop 6.5.0-28-generic #29-Ubuntu SMP PREEMPT_DYNAMIC Thu Mar 28 23:46:48 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

> pnpm version
{
  'test-pg': '1.0.0',
  npm: '10.7.0',
  node: '22.1.0',
  acorn: '8.11.3',
  ada: '2.7.8',
  ares: '1.28.1',
  base64: '0.5.2',
  brotli: '1.1.0',
  cjs_module_lexer: '1.2.2',
  cldr: '45.0',
  icu: '75.1',
  llhttp: '9.2.1',
  modules: '127',
  napi: '9',
  nghttp2: '1.61.0',
  nghttp3: '0.7.0',
  ngtcp2: '1.3.0',
  openssl: '3.0.13+quic',
  simdjson: '3.8.0',
  simdutf: '5.2.4',
  tz: '2024a',
  undici: '6.13.0',
  unicode: '15.1',
  uv: '1.48.0',
  uvwasi: '0.0.20',
  v8: '12.4.254.14-node.11',
  zlib: '1.3.0.1-motley-7d77fb7'
}
aleclarson commented 3 weeks ago

Looks like it's intentional to match protobuf behavior.

aleclarson commented 2 weeks ago

Omission of negative integers was a bug, now fixed by https://github.com/pganalyze/libpg_query/pull/249, which was just merged

pyramation commented 1 week ago

Thanks @aleclarson !

Makes sense that it was a part of upstream, will get this merged ASAP!