Add type attribute to A_CONST nodes.

[zhm]

This fixes an issue where BitString's cannot be deparsed properly
because they cannot be distinguished from normal strings.

Example:

SELECT B'0101'

and

SELECT '0101'

This patch adds a type which outputs the integer node tag so that
callers can be certain of the type of the constant given the limitations
of the serialization format (JSON). This also fixes a bug where
BitStrings were being dumped as invalid JSON:

{ "val": b0101 }

[lfittl]

Thats the fix for #3 - correct?

[zhm]

Correct, that's a fix for #3. It was such a trivial change I just included it here ;)

[lfittl]

Looks good!

I'll do some quick testing with the Ruby library, to make sure this doesn't confuse any callers - looking to merge this later today.

[zhm]

@lfittl one possible thing to consider with the bitstrings is if the leading char should be stripped.

Current PR:

{
    "A_CONST": {
        "location": 7,
        "type": 654,
        "val": "b0001"
    }
}

The lib could either return "val": "b0001" or "val": "0001" with the leader stripped since it's implicit from the type. Not sure which is better, just thought I'd bring it up!

[lfittl]

@zhm Good point - lets maybe do the version without b, since you're right, its already in the type.

Also, looking at the output, do you think we can output the type as a readable name instead? (might make parsing more reliable across versions)

[zhm]

👍 for readable type names!

[zhm]

I updated the PR to use strings instead of enums for the type names.

We now have:

integer
float
string
bitstring
null

Also I think we need to leave the b prefix on bitstrings. It turns out there's yet another syntax for bitstrings. Both of the following are returned as bitstrings:

SELECT B'0101`

> b0101

SELECT X'EFFF`

> xEFFF

[zhm]

Also note that I don't think bitstrings are particularly useful to most people, just trying to get some random SQL scripts to parse/deparse ;)

I suppose it's possible that Postgres could introduce new value types, so maybe this all makes sense anyways.

[zhm]

@lfittl also, are there any other scenarios where the caller would care about the type of the Value struct? I had originally put the type info in the _outValue function but I noticed the Value node is used for very trivial things like column names and such. Is there anything other than A_CONST where a bitstring/integer/float would come up?

[lfittl]

@zhm Patch looks good!

From my quick grep-ing through the node definitions, it looks like A_CONST is the only one that really uses different types - all others seem to be strings.

If we wanted to put it into all value nodes, we could consider only outputting the type if its not string (and maybe also if its not null), to avoid bloating the tree. But I'll merge this as-is, we can revisit later if needed.