Re: Missing column_constraint explanation - Mailing list pgsql-docs

From Stephen Frost
Subject Re: Missing column_constraint explanation
Date
Msg-id 20180114143313.GR2416@tamriel.snowman.net
Whole thread Raw
In response to Re: Missing column_constraint explanation  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Missing column_constraint explanation  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-docs
Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sat, Jan 13, 2018 at 09:06:22PM -0500, Stephen Frost wrote:
> > I'm not really sure that we want to go there for this case though.
> > Perhaps others disagree, but that seems like a lot to avoid this
> > particular duplication, which really isn't all that bad.
> >
> > This patch also seems to have gotten lost in the shuffle of things, but
> > it still applies cleanly and I took another look at it today and it
> > looks good to me, so I'm going to stick it in the CF and mark it as
> > Needs Review for now.  Perhaps someone else can give it another
> > once-over to make sure everything looks good and, if so, mark it as
> > Ready for Committer and then I'll take care of it.
>
> I may be missing something, but no patch is attached to this
> thread. Honestly I think that duplicating this code should be avoided,
> and the patch to produce is not that complicated technically. So are you
> planning to just duplicate hte definitions in CREATE TABLE to ALTER
> TABLE? This is a recipy for forgetting updates in the future on one
> page or the other...

I'm not really sure why the thread keeps getting broken, but the
original was here:


https://www.postgresql.org/message-id/flat/CAB_COdgOoA=G18RhWPoW8zZ+xOxTns7xD7psHA=ct+XccokbUg@mail.gmail.com#CAB_COdgOoA=G18RhWPoW8zZ+xOxTns7xD7psHA=ct+XccokbUg@mail.gmail.com

where the latest patch is from Amit.

I agree that there's some risk that someone changing what
'column_constraint' means will miss changing it everywhere it needs to
be changed in the documentation, but that's hardly new, we have quite a
few places that need to be changed when someone adds such a new feature.

We also do not actually have the exact same definition of it everywhere
either- see CREATE TABLE and CREATE FOREIGN TABLE for example.  In this
case, column_constraint is the same between CREATE TABLE and ALTER
TABLE, and presumably the same is true between CREATE FOREIGN TABLE and
ALTER FOREIGN TABLE, but it's not the same between regular tables and
foreign tables, despite sharing the same name.  As such, someone
extending what column_constraint means would need to at least be
thinking about if their new feature needs to be added into the CREATE
FOREIGN TABLE definition too.  Unless we start marking up the
documentation somehow further to indicate which bits of
column_constraint are accepted for regular tables and which are accepted
for foreign tables, but we may also have cases where one part of
'column_constraint' is actually different between the two for perhaps
entirely appropriate technical reasons.

Further, as mentioned upthread in
https://www.postgresql.org/message-id/20171221031511.GX4628%40tamriel.snowman.net
I'm concerned that this wouldn't be very easy to make work with the way
psql's documentation is generated.  Did you take a look at how that
works with create_help.pl?  I don't see create_help.pl as supporting
'entity'.  We could perhaps fix that, but I'm not really sure it's worth
it and it's certainly quite a bit to add on to this particular patch.

If we used the method that features-supported.sgml uses, where we have
another perl script that's used to build the sgml files up from text
files, then we'd also need to make building psql depend on the docs
having been built, I believe, and I don't think we really want to do
that either.

I wouldn't be against a larger patch which actually refactored all of
these sub-stanzas into independent files (with unique names, unlike how
column_constraint is today) and also taught create_help.pl and the doc
build to be able to work with them, but that's a great deal more work
and nothing in this patch stops that from being done in the future.

In the end, I'm inclinced to simply adopt this change and even consider
back-patching it as a documentation fix.

Thanks!

Stephen

Attachment

pgsql-docs by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Missing column_constraint explanation
Next
From: Michael Paquier
Date:
Subject: Re: Missing column_constraint explanation