po 18. 11. 2019 v 4:43 odesÃlatel vignesh C <vignes...@gmail.com> napsal:
> On Sat, Nov 16, 2019 at 1:25 PM Pavel Stehule <pavel.steh...@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.
>
still I don't understand. The created query is tested already by current
test.
Do you want to test just -e option? Then it should be done as separate
issue. Do this now is little bit messy.
> >>
> >> 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
> Regards,
> Vignesh
> EnterpriseDB: http://www.enterprisedb.com
>
