Обсуждение: Minor improvements in alter_table.sgml
Attached is a patch to improve the manual page for the ALTER TABLE command. Thanks, Best regards, Etsuro Fujita
Вложения
On Tue, Apr 8, 2014 at 5:05 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote: > Attached is a patch to improve the manual page for the ALTER TABLE command. Do we really need to add a section for "type_name" when we already have a section for "OF type_name"? constraint_name is also used for adding a constraint using an index. So it could not only be a constraint to alter, validate, or drop, but also a new constraint name to be added. Honestly, how much value is there in even having a section for this? Do we really want to document constraint_name as "name of an existing constraint, or the name of a new constraint to be added"? It would be accurate, then, but it also doesn't really tell you anything you didn't know already. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
(2014/04/09 1:23), Robert Haas wrote: > On Tue, Apr 8, 2014 at 5:05 AM, Etsuro Fujita > <fujita.etsuro@lab.ntt.co.jp> wrote: >> Attached is a patch to improve the manual page for the ALTER TABLE command. > > Do we really need to add a section for "type_name" when we already > have a section for "OF type_name"? I think that the section for "type_name" would be necessary as that in chapter "Parameters", not in chapter "Description", which includes the section for "OF type_name". > constraint_name is also used for adding a constraint using an index. > So it could not only be a constraint to alter, validate, or drop, but > also a new constraint name to be added. I overlooked that. > Honestly, how much value is > there in even having a section for this? Do we really want to > document constraint_name as "name of an existing constraint, or the > name of a new constraint to be added"? It would be accurate, then, > but it also doesn't really tell you anything you didn't know already. You have a point there, but I feel odd about the documentation as is, because some are well written (eg, column_name) and some are not (eg, constraint_name). So, if there are no objections, I'd like to update the patch. Thanks, Best regards, Etsuro Fujita
(2014/04/09 12:03), Etsuro Fujita wrote: > (2014/04/09 1:23), Robert Haas wrote: >> On Tue, Apr 8, 2014 at 5:05 AM, Etsuro Fujita >> <fujita.etsuro@lab.ntt.co.jp> wrote: >>> Attached is a patch to improve the manual page for the ALTER TABLE >>> command. >> >> Do we really need to add a section for "type_name" when we already >> have a section for "OF type_name"? > > I think that the section for "type_name" would be necessary as that in > chapter "Parameters", not in chapter "Description", which includes the > section for "OF type_name". > >> constraint_name is also used for adding a constraint using an index. >> So it could not only be a constraint to alter, validate, or drop, but >> also a new constraint name to be added. > > I overlooked that. > > > Honestly, how much value is >> there in even having a section for this? Do we really want to >> document constraint_name as "name of an existing constraint, or the >> name of a new constraint to be added"? It would be accurate, then, >> but it also doesn't really tell you anything you didn't know already. > > You have a point there, but I feel odd about the documentation as is, > because some are well written (eg, column_name) and some are not (eg, > constraint_name). So, if there are no objections, I'd like to update > the patch. Attached is an updated version of the patch. Thanks, Best regards, Etsuro Fujita
Вложения
On Fri, Apr 11, 2014 at 5:00 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote: > Attached is an updated version of the patch. I applied the first two hunks of this, which seem like clear oversights; and also the bit fixing the constraint_name language. I think the other changes deserve to be considered separately, and in particular I'm still not sure it's a good idea to document both OF type_name and type_name. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
(2014/04/14 23:53), Robert Haas wrote: > On Fri, Apr 11, 2014 at 5:00 AM, Etsuro Fujita > <fujita.etsuro@lab.ntt.co.jp> wrote: >> Attached is an updated version of the patch. > > I applied the first two hunks of this, which seem like clear > oversights; and also the bit fixing the constraint_name language. > > I think the other changes deserve to be considered separately, and in > particular I'm still not sure it's a good idea to document both OF > type_name and type_name. OK, Thanks! Best regards, Etsuro Fujita
(2014/04/15 15:27), Etsuro Fujita wrote: > (2014/04/14 23:53), Robert Haas wrote: >> On Fri, Apr 11, 2014 at 5:00 AM, Etsuro Fujita >> <fujita.etsuro@lab.ntt.co.jp> wrote: >>> Attached is an updated version of the patch. >> I think the other changes deserve to be considered separately, and in >> particular I'm still not sure it's a good idea to document both OF >> type_name and type_name. I've agreed on that point, but I think apart from the others, the trivial typo should be corrected. Patch attached (doc-altertable-typo.patch). I noticed the description of index_name should also be corrected, because it is currently used not only in CLUSTER ON, but in ADD table_constraint_using_index. (It will also be used in REPLICA IDENTITY in 9.4.) Patch attached (doc-altertable-indexname.patch). Thanks, Best regards, Etsuro Fujita
Вложения
On Wed, Apr 23, 2014 at 3:46 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote: > (2014/04/15 15:27), Etsuro Fujita wrote: >> (2014/04/14 23:53), Robert Haas wrote: >>> >>> On Fri, Apr 11, 2014 at 5:00 AM, Etsuro Fujita >>> <fujita.etsuro@lab.ntt.co.jp> wrote: >>>> >>>> Attached is an updated version of the patch. > > >>> I think the other changes deserve to be considered separately, and in >>> particular I'm still not sure it's a good idea to document both OF >>> type_name and type_name. > > > I've agreed on that point, but I think apart from the others, the trivial > typo should be corrected. Patch attached (doc-altertable-typo.patch). > > I noticed the description of index_name should also be corrected, because it > is currently used not only in CLUSTER ON, but in ADD > table_constraint_using_index. (It will also be used in REPLICA IDENTITY in > 9.4.) Patch attached (doc-altertable-indexname.patch). Agreed. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company