Handle deparse of INTERVAL correctly when used in SET statements

[coderdan]

I recently found an issue when parsing and then deparsing statements in pg_query which turned out to be a bug (or odd edgecase) in libpg_query.

Given the following statement:

SET TIME ZONE INTERVAL '+00:00' HOUR TO MINUTE;

The parsing and then deparsing results in:

SET timezone TO '+00:00'::interval hour to minute

But this is not valid in PostgreSQL 15.1:

pgtest=# SET timezone TO '+00:00'::interval hour to minute;
ERROR:  syntax error at or near "::"
LINE 1: SET timezone TO '+00:00'::interval hour to minute;

Docs are a little thin but it appears that PG has special case interval handling when used with SET TIME ZONE.
See https://www.postgresql.org/docs/15/sql-set.html

Approach in this PR

I added a variant to the DeparseContext enum so that I can handle deparsing of TypeCast nodes for interval when used in a SET statement. Unclear if this is the best approach or not and I welcome feedback/guidance.

The tests still don't pass due to a remaining (odd) problem.

The test case I added to test/deparse_tests.c looks as follows:

SET time zone interval '+00:00' hour TO minute

However, the parse/deparse round-trip converts it back to:

SET timezone interval '+00:00' hour TO minute

Note the missing space between time and zone. While this is valid SQL the strings no longer match.
Oddly, changing the test to have no space results in a SEGFAULT! Perhaps I'm doing something wrong or I've uncovered another bug. Seeking input!

Many thanks.

[coderdan]

Just giving this one a bump. Maybe @lfittl you could take a look if you're around?

[freshtonic]

Giving this another bump. I can also chip in and help once @lfittl has provided some guidance.

[lfittl]

@coderdan Thanks for the contribution!

I've just pushed up a review commit (thanks for allowing maintainer edits), that should fix the "SET TIME ZONE" vs "SET timezone" output problem.

It seems the Postgres grammar is actually special casing "SET TIME ZONE" (see gram.y), so we need to make sure to generate that on output when an interval is involved (even though the VariableSetStmt internally calls it "timezone").

This should make things work, but I'm still surprised why there is a hard crash when using "SET TIMEZONE" with an interval (it should just error out). Still trying to see if I can find the root cause of that.

[lfittl]

A-ha! just found the problem with the crash - the deparse tests were not handling errors correctly, and if we use "SET timezone" with an interval it will error out like this:

ERROR for "SET TIMEZONE interval '+00:00' hour to minute"
  syntax error at or near "interval"

So this should now work as expected.

@coderdan @freshtonic Could you look over the changes I made and see if that looks good to you as well?

[coderdan]

Thanks @lfittl! Appreciate the review and tweaks. Your additions look very reasonable to me. Glad I was on the right track :)

[freshtonic]

Awesome, thank you @lfittl! LGTM.

[coderdan]

@lfittl is this OK to merge, no?

[lfittl]

@coderdan @freshtonic Yep! Merged now - thanks for your patience.