pg-nano / pg-parser

Parse your Postgres queries into a 100% type-safe AST (powered by libpg_query)
155 stars 3 forks source link

Zero and negative integers are parsed incorrectly #2

Open markandrus opened 1 day ago

markandrus commented 1 day ago

Hi, thanks for this library! The type definitions are great ✨

There appears to be an issue with parsing zero and negative integers, such as in SELECT 0 and SELECT -42. Parsing these constants produces A_Const nodes with an empty ival:

{
  "A_Const": {
    "ival": {}
  }
}

A successful parse (SELECT 9000) looks like this:

{
  "A_Const": {
    "ival": {
      "ival": 9000
    },
    "location": 7
  }
}

pgsql-parser successfully parses these, and I also notice that their A_Const nodes look different:

{
  "A_Const": {
    "val": {
      "Integer": {
        "ival": -42
      }
    },
    "location": 7
  }
}

I wonder if the @pg-nano/pg-parser types for this node are correct, or if you intentionally condensed the A_Const node? Also, why is ival optional for Integer, when the other literal types aren't?

https://github.com/pg-nano/pg-parser/blob/c6136305a43bd67aa053d3dfdac532a97e421b41/src/lib/ast.ts#L6757-L6775


❌ Parse SELECT 0 with @pg-nano/pg-parser ```js import { parseQuerySync } from '@pg-nano/pg-parser' console.log(JSON.stringify(parseQuerySync('SELECT 0', null, 2)) /* { "version": 160001, "stmts": [ { "stmt": { "SelectStmt": { "targetList": [ { "ResTarget": { "val": { "A_Const": { "ival": {}, "location": 7 } }, "location": 7 } } ], "limitOption": "LIMIT_OPTION_DEFAULT", "op": "SETOP_NONE" } } } ] } */ ```
❌ Parse SELECT -42 with @pg-nano/pg-parser ```js import { parseQuerySync } from '@pg-nano/pg-parser' console.log(JSON.stringify(parseQuerySync('SELECT -42', null, 2)) /* { "version": 160001, "stmts": [ { "stmt": { "SelectStmt": { "targetList": [ { "ResTarget": { "val": { "A_Const": { "ival": {}, "location": 7 } }, "location": 7 } } ], "limitOption": "LIMIT_OPTION_DEFAULT", "op": "SETOP_NONE" } } } ] } */ ```
✅ Parse SELECT 0 with pgsql-parser ```js import { parse } from 'pgsql-parser' console.log(JSON.stringify(parse('SELECT 0', null, 2)) /* [ { "RawStmt": { "stmt": { "SelectStmt": { "targetList": [ { "ResTarget": { "val": { "A_Const": { "val": { "Integer": { "ival": 0 } }, "location": 7 } }, "location": 7 } } ], "limitOption": "LIMIT_OPTION_DEFAULT", "op": "SETOP_NONE" } }, "stmt_location": 0 } } ] */ ```
✅ Parse SELECT -42 with pgsql-parser ```js import { parse } from 'pgsql-parser' console.log(JSON.stringify(parse('SELECT -42', null, 2)) /* [ { "RawStmt": { "stmt": { "SelectStmt": { "targetList": [ { "ResTarget": { "val": { "A_Const": { "val": { "Integer": { "ival": -42 } }, "location": 7 } }, "location": 7 } } ], "limitOption": "LIMIT_OPTION_DEFAULT", "op": "SETOP_NONE" } }, "stmt_location": 0 } } ] */ ```
markandrus commented 23 hours ago

Looks like this was fixed here: https://github.com/pganalyze/libpg_query/pull/249