Horton

Thread

Commits

  1. Tab completion: don't offer valid constraints in VALIDATE CONSTRAINT.

  1. Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    David Fetter <david@fetter.org> — 2021-04-27T00:24:34Z

    Folks,

I noticed that $subject completes with already valid constraints,
please find attached a patch that fixes it. I noticed that there are
other places constraints can be validated, but didn't check whether
similar bugs exist there yet.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  2. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Aleksander Alekseev <aleksander@timescale.com> — 2021-04-27T09:33:25Z

    Hi David,

> I noticed that $subject completes with already valid constraints,
> please find attached a patch that fixes it. I noticed that there are
> other places constraints can be validated, but didn't check whether
> similar bugs exist there yet.

There was a typo in the patch ("... and and not convalidated"). I've fixed
it. Otherwise the patch passes all the tests and works as expected:

eax=# create table foo (x int);
CREATE TABLE
eax=# alter table foo add constraint bar check (x < 3) not valid;
ALTER TABLE
eax=# alter table foo add constraint baz check (x <> 5) not valid;
ALTER TABLE
eax=# alter table foo validate constraint ba
bar baz
eax=# alter table foo validate constraint bar;
ALTER TABLE

-- 
Best regards,
Aleksander Alekseev

  3. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Aleksander Alekseev <aleksander@timescale.com> — 2021-04-27T09:58:52Z

    Hi hackers,

> Otherwise the patch passes all the tests and works as expected

I've noticed there is no tab completion for ALTER TABLE xxx ADD. Here
is an alternative version of the patch that fixes this as well. Not
sure if this should be in the same commit though.

-- 
Best regards,
Aleksander Alekseev

  4. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    David Fetter <david@fetter.org> — 2021-04-28T02:53:08Z

    On Tue, Apr 27, 2021 at 12:33:25PM +0300, Aleksander Alekseev wrote:
> Hi David,
> 
> > I noticed that $subject completes with already valid constraints,
> > please find attached a patch that fixes it. I noticed that there are
> > other places constraints can be validated, but didn't check whether
> > similar bugs exist there yet.
> 
> There was a typo in the patch ("... and and not convalidated"). I've fixed
> it. Otherwise the patch passes all the tests and works as expected:
> 
> eax=# create table foo (x int);
> CREATE TABLE
> eax=# alter table foo add constraint bar check (x < 3) not valid;
> ALTER TABLE
> eax=# alter table foo add constraint baz check (x <> 5) not valid;
> ALTER TABLE
> eax=# alter table foo validate constraint ba
> bar baz
> eax=# alter table foo validate constraint bar;
> ALTER TABLE

Sorry about that typo, and thanks for poking at this!

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  5. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Michael Paquier <michael@paquier.xyz> — 2021-05-19T07:53:18Z

    On Tue, Apr 27, 2021 at 12:58:52PM +0300, Aleksander Alekseev wrote:
> I've noticed there is no tab completion for ALTER TABLE xxx ADD. Here
> is an alternative version of the patch that fixes this as well. Not
> sure if this should be in the same commit though.

-   /* If we have ALTER TABLE <sth> DROP, provide COLUMN or CONSTRAINT */
-   else if (Matches("ALTER", "TABLE", MatchAny, "DROP"))
+   /* If we have ALTER TABLE <sth> ADD|DROP, provide COLUMN or CONSTRAINT */
+   else if (Matches("ALTER", "TABLE", MatchAny, "ADD|DROP"))
Seems to me that the behavior to not complete with COLUMN or
CONSTRAINT for ADD is intentional, as it is possible to specify a
constraint or column name without the object type first.  This
introduces a inconsistent behavior with what we do for columns with
ADD, for one.  So a more consistent approach would be to list columns,
constraints, COLUMN and CONSTRAINT in the list of options available
after ADD.

+   else if (Matches("ALTER", "TABLE", MatchAny, "VALIDATE", "CONSTRAINT"))
+   {
+       completion_info_charp = prev3_wd;
+       COMPLETE_WITH_QUERY(Query_for_nonvalid_constraint_of_table);
+   }
Specifying valid constraints is an authorized grammar, so it does not
seem that bad to keep things as they are, either.  I would leave that
alone.
--
Michael

  6. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Daniel Gustafsson <daniel@yesql.se> — 2021-09-03T18:27:55Z

    > On 19 May 2021, at 09:53, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Tue, Apr 27, 2021 at 12:58:52PM +0300, Aleksander Alekseev wrote:
>> I've noticed there is no tab completion for ALTER TABLE xxx ADD. Here
>> is an alternative version of the patch that fixes this as well. Not
>> sure if this should be in the same commit though.
> 
> -   /* If we have ALTER TABLE <sth> DROP, provide COLUMN or CONSTRAINT */
> -   else if (Matches("ALTER", "TABLE", MatchAny, "DROP"))
> +   /* If we have ALTER TABLE <sth> ADD|DROP, provide COLUMN or CONSTRAINT */
> +   else if (Matches("ALTER", "TABLE", MatchAny, "ADD|DROP"))
> Seems to me that the behavior to not complete with COLUMN or
> CONSTRAINT for ADD is intentional, as it is possible to specify a
> constraint or column name without the object type first.  This
> introduces a inconsistent behavior with what we do for columns with
> ADD, for one.  So a more consistent approach would be to list columns,
> constraints, COLUMN and CONSTRAINT in the list of options available
> after ADD.
> 
> +   else if (Matches("ALTER", "TABLE", MatchAny, "VALIDATE", "CONSTRAINT"))
> +   {
> +       completion_info_charp = prev3_wd;
> +       COMPLETE_WITH_QUERY(Query_for_nonvalid_constraint_of_table);
> +   }
> Specifying valid constraints is an authorized grammar, so it does not
> seem that bad to keep things as they are, either.  I would leave that
> alone.

This has stalled being marked Waiting on Author since May, and reading the
above it sounds like marking it Returned with Feedback is the logical next step
(patch also no longer applies).

--
Daniel Gustafsson		https://vmware.com/

  7. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    David Fetter <david@fetter.org> — 2021-09-15T06:06:04Z

    On Fri, Sep 03, 2021 at 08:27:55PM +0200, Daniel Gustafsson wrote:
> > On 19 May 2021, at 09:53, Michael Paquier <michael@paquier.xyz> wrote:
> > 
> > On Tue, Apr 27, 2021 at 12:58:52PM +0300, Aleksander Alekseev wrote:
> >> I've noticed there is no tab completion for ALTER TABLE xxx ADD. Here
> >> is an alternative version of the patch that fixes this as well. Not
> >> sure if this should be in the same commit though.
> > 
> > -   /* If we have ALTER TABLE <sth> DROP, provide COLUMN or CONSTRAINT */
> > -   else if (Matches("ALTER", "TABLE", MatchAny, "DROP"))
> > +   /* If we have ALTER TABLE <sth> ADD|DROP, provide COLUMN or CONSTRAINT */
> > +   else if (Matches("ALTER", "TABLE", MatchAny, "ADD|DROP"))
> > Seems to me that the behavior to not complete with COLUMN or
> > CONSTRAINT for ADD is intentional, as it is possible to specify a
> > constraint or column name without the object type first.  This
> > introduces a inconsistent behavior with what we do for columns with
> > ADD, for one.  So a more consistent approach would be to list columns,
> > constraints, COLUMN and CONSTRAINT in the list of options available
> > after ADD.
> > 
> > +   else if (Matches("ALTER", "TABLE", MatchAny, "VALIDATE", "CONSTRAINT"))
> > +   {
> > +       completion_info_charp = prev3_wd;
> > +       COMPLETE_WITH_QUERY(Query_for_nonvalid_constraint_of_table);
> > +   }
> > Specifying valid constraints is an authorized grammar, so it does not
> > seem that bad to keep things as they are, either.  I would leave that
> > alone.
> 
> This has stalled being marked Waiting on Author since May, and reading the
> above it sounds like marking it Returned with Feedback is the logical next step
> (patch also no longer applies).

Please find attached the next revision :)

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  8. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Aleksander Alekseev <aleksander@timescale.com> — 2021-09-24T12:35:43Z

    Hi David,

> Please find attached the next revision :)

The patch didn't apply and couldn't pass cfbot [1]. The (hopefully)
corrected patch is attached. Other than that it looks OK to me but let's
see what cfbot will tell.

[1]: http://cfbot.cputube.org/patch_34_3113.log

-- 
Best regards,
Aleksander Alekseev

  9. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Aleksander Alekseev <afiskon@gmail.com> — 2021-09-24T13:51:25Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

The cfbot seems to be happy with the updated patch.

The new status of this patch is: Ready for Committer

  10. Re: Bug fix for tab completion of ALTER TABLE ... VALIDATE CONSTRAINT ...

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-01-03T23:15:10Z

    Aleksander Alekseev <afiskon@gmail.com> writes:
> The cfbot seems to be happy with the updated patch.
> The new status of this patch is: Ready for Committer

Pushed.

			regards, tom lane