Re: Add regression tests for DISCARD

On Sat, Jul 6, 2013 at 11:49 PM, Robins Tharakan <tharakan(at)gmail(dot)com> wrote:
> Thanks Fabrizio.
>
> Although parallel_schedule was a miss for this specific patch, however, I
> guess I seem to have missed out serial_schedule completely (in all patches)
> and then thanks for pointing this out. Subsequently Robert too noticed the
> miss at the serial_schedule end.
>
> Please find attached a patch, updated towards serial_schedule /
> parallel_schedule as well as the role name change as per Robert's feedback
> on CREATE OPERATOR thread.

Aside from some reorganization, this patch just checks four new things:

- That DISCARD TEMPORARY works like DISCARD TEMP.
- That DISCARD PLANS does not throw an error.
- That DISCARD ALL fails from within a transaction.
- That DISCARD invalid_option fails.

The last of these fails with a parse error and therefore, per
discussion on the other thread, is not wanted. I'd be more inclined
to include the remaining three tests in the existing test file rather
than reorganize things into a new file. Reorganizing code makes
back-patching harder, and we do back-patch changes that update the
regression tests, and I don't think three new tests are are enough
justification to add a whole new file.

Possibly the test that DISCARD ALL fails from within a transaction
ought to be part of a more general category of tests for
PreventTransactionChain(). I notice that we currently have NO
regression tests that trip that function; we could consider testing
some of the others as well. But it's a bit tricky because the
CREATE/DROP DATABASE/TABLESPACE commands manipulate global objects -
which is a bit hairy - and COMMIT/ROLLBACK PREPARED require special
test setup. However, I think we could test CREATE INDEX CONCURRENTLY,
DROP INDEX CONCURRENTLY and CLUSTER in addition to DISCARD ALL. Or at
least some of those.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company