Re: dropdb --force
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Dilip Kumar <dilipbalaut@gmail.com>, Ryan Lambert <ryan@rustprooflabs.com>, Thomas Munro <thomas.munro@gmail.com>, Anthony Nowocien <anowocien@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Filip Rembiałkowski <filip.rembialkowski@gmail.com>
Date: 2019-11-18T05:24:17Z
Lists: pgsql-hackers
Attachments
- dropdb-f-20191118.amit.patch (application/octet-stream) patch
On Mon, Nov 18, 2019 at 10:33 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> po 18. 11. 2019 v 4:43 odesílatel vignesh C <vignesh21@gmail.com> napsal:
>>
>>
>> When we don't specify -e option, the query used to drop db will not be
>> printed like below:
>> ./dropdb testdb1
>> When we specify -e option, the query used to drop db will be printed like below:
>> ./dropdb -e testdb2
>> SELECT pg_catalog.set_config('search_path', '', false);
>> DROP DATABASE testdb2;
>> If we specify -e option, the query that is being used to drop db will
>> be printed. In the existing test I could not see the inclusion of -e
>> option. I was thinking to add a test including -e that way the query
>> that includes force option gets validated.
>
>
> still I don't understand. The created query is tested already by current test.
>
> Do you want to test just -e option?
>
Yeah, it seems Vignesh wants to do that. It will help in verifying
that the command generated by code is correct. However, I think there
is no pressing need to have an additional test for this.
> Then it should be done as separate issue.
>
Yeah, I agree. I think this can be done as a separate test patch to
improve coverage if someone wants.
>>
>> >>
>> >> Also should we include one test where one session is connected to db
>> >> and another session tries dropping with -f option?
>> >
>> >
>> > I afraid so test API doesn't allow asynchronous operations. Do you have any idea, how to it?
>> >
>>
>> I had seen that isolation test(src/test/isolation) has a framework to
>> support this. You can have a look to see if it can be handled using
>> that.
>
>
> I'll look there
>
If we want to have a test for this, then you might want to look at
test src/test/recovery/t/013_crash_restart. In that test, we keep a
connection open and then validate whether it is terminated. Having
said that, I think it might be better to add this as a separate test
patch apart from main patch because it is a bit of a timing-dependent
test and might fail on some slow machines. We can always revert this
if it turns out to be an unstable test.
I have slightly modified the main patch and attached is the result.
Basically, I don't see any need to repeat what is mentioned in the
Drop Database page. Let me know what you guys think?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Commits
-
Add tests for '-f' option in dropdb utility.
- 8a7e9e9dad56 13.0 landed
-
Move pump_until to TestLib.pm.
- 290acac92b1d 13.0 landed
-
Add the support for '-f' option in dropdb utility.
- 80e05a088e4e 13.0 landed
-
Introduce the 'force' option for the Drop Database command.
- 1379fd537f9f 13.0 landed
-
Improve CREATE/DROP/RENAME DATABASE so that when failing because the source
- 4abd7b49f1e9 8.4.0 cited