On Sat, Nov 16, 2019 at 1:25 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> >
>> > updated patch attached
>> >
>>
>> Thanks Pavel for providing updated version.
>> Few comments:
>> I felt the help text seems incomplete:
>> @@ -159,6 +167,7 @@ help(const char *progname)
>> printf(_("\nOptions:\n"));
>> printf(_(" -e, --echo show the commands being
>> sent to the server\n"));
>> printf(_(" -i, --interactive prompt before deleting anything\n"));
>> + printf(_(" -f, --force try to terminate other
>> connection before\n"));
>> printf(_(" -V, --version output version information,
>> then exit\n"));
>> we can change to:
>> printf(_(" -f, --force try to terminate other
>> connection before dropping\n"));
>>
>
> done. maybe alternative can be "first try to terminate other connections". It is shorter. The current text has 78
chars,what should be acceptable
>
>>
>> We can add one test including -e option which validates the command
>> generation including WITH (FORCE):
>> +$node->safe_psql('postgres', 'CREATE DATABASE foobar2');
>> +$node->issues_sql_like(
>> + [ 'dropdb', '--force', 'foobar2' ],
>> + qr/statement: DROP DATABASE foobar2 WITH \(FORCE\);/,
>> + 'SQL DROP DATABASE (FORCE) run');
>> +
>
>
> I don't understand to this point. It is effectively same like existing test
>
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.
>>
>> 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.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
