Re: [HACKERS] Add regression tests for DISCARD
On Sat, Jul 6, 2013 at 11:49 PM, Robins Tharakan thara...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for DISCARD
On Sun, Jul 7, 2013 at 12:49 AM, Robins Tharakan thara...@gmail.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. Ok. Some basic checks on this patch: - Removed unnecessary extra-lines: Yes - Cleanly applies to Git-Head: Yes - Documentation Updated: N/A - Tests Updated: Yes - All tests pass: Yes. - Does it Work: Yes - Do we want it?: Yes - Is this a new feature: No - Does it support pg_dump: No - Does it follow coding guidelines: Yes - Any visible issues: No - Any corner cases missed out: No - Performance tests required: No - Any compiler warnings: No - Are comments sufficient: Yes - Others: N/A Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] Add regression tests for DISCARD
On Sat, Jul 6, 2013 at 8:49 PM, Robins Tharakan thara...@gmail.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. Why does serial_schedule even exist? Couldn't we just run the parallel schedule serially, like what happens when MAX_CONNECTIONS=1? Cheers, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for DISCARD
On 7 July 2013 14:08, Jeff Janes jeff.ja...@gmail.com wrote: Why does serial_schedule even exist? Couldn't we just run the parallel schedule serially, like what happens when MAX_CONNECTIONS=1? Well, I am sure it works that way, without errors. The 'why' still eludes me though, just that its required for this submission. -- Robins Tharakan
Re: [HACKERS] Add regression tests for DISCARD
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. -- Robins Tharakan On 2 July 2013 09:32, Fabrízio de Royes Mello fabriziome...@gmail.comwrote: On Mon, Jul 1, 2013 at 5:59 PM, Robins Tharakan thara...@gmail.comwrote: Thanks Marko for pointing out about guc.sql. Please find attached a patch to move DISCARD related tests from guc.sql to discard.sql. It adds an extra test for a DISCARD PLANS line, although I amn't sure on how to validate that its working. Personally, I wouldn't call this a great patch, since most of the tests were already running, although in a generic script. The separation of DISCARD related tests to another file is arguably good for the long-term though. Robins, You must add this new test case called discard to src/test/regress/parallel_schedule and src/test/regress/serial_schedule, because if we do make check the new discard test case is not executed. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello regress_discard_v5.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for DISCARD
On Mon, Jul 1, 2013 at 5:59 PM, Robins Tharakan thara...@gmail.com wrote: Thanks Marko for pointing out about guc.sql. Please find attached a patch to move DISCARD related tests from guc.sql to discard.sql. It adds an extra test for a DISCARD PLANS line, although I amn't sure on how to validate that its working. Personally, I wouldn't call this a great patch, since most of the tests were already running, although in a generic script. The separation of DISCARD related tests to another file is arguably good for the long-term though. Robins, You must add this new test case called discard to src/test/regress/parallel_schedule and src/test/regress/serial_schedule, because if we do make check the new discard test case is not executed. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Blog sobre TI: http://fabriziomello.blogspot.com Perfil Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] Add regression tests for DISCARD
On 17 June 2013 18:14, Marko Kreen mark...@gmail.com wrote: Perhaps existing tests in guc.sql should be merged into it? Thanks Marko for pointing out about guc.sql. Please find attached a patch to move DISCARD related tests from guc.sql to discard.sql. It adds an extra test for a DISCARD PLANS line, although I amn't sure on how to validate that its working. Personally, I wouldn't call this a great patch, since most of the tests were already running, although in a generic script. The separation of DISCARD related tests to another file is arguably good for the long-term though. -- Robins Tharakan regress_discard_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for DISCARD
On Mon, May 13, 2013 at 2:58 AM, Robins Tharakan thara...@gmail.com wrote: Please find attached a patch that adds basic regression tests for DISCARD command. Any and all feedback is obviously welcome. Perhaps existing tests in guc.sql should be merged into it? -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
