Re: Typo in the Section "3.6. Inheritance" - Mailing list pgsql-docs

On Wed, Aug  5, 2020 at 11:29:31PM -0700, David G. Johnston wrote:
> On Wed, Aug 5, 2020 at 10:41 PM Michael Paquier <michael@paquier.xyz> wrote:
> 
>     On Wed, Aug 05, 2020 at 05:36:56PM -0400, Bruce Momjian wrote:
>     > On Wed, Aug  5, 2020 at 05:12:43PM -0400, Bruce Momjian wrote:
>     > > !     type for variable length character strings.  The
>     > > !     <classname>capitals</classname> table has
>     > > !     an extra column, <structfield>state</structfield>, which shows
>     their states.  In
>     >                                                                         
>         ------
>     >
>     > Uh, I thought I was fixing this by changing "state" to "states", but I
>     > now think "state" was correct, right?
> 
> 
> The main wording problem with the direct phrasing shown here is the inability
> to clarify that while there are multiple capitals and multiple states each
> capital exists in a single, mutually exclusive, state.  Frankly it doesn't seem
> wrong, or at least any worse than the general content on the page, and probably
> should just be left alone until someone writes a better tutorial.
>  
> Tangentially, I personally think "additional" is a better word choice than
> "extra"; not enough to patch by itself but to consider if patching anyway.
> 
> 
>     What if you used an entirely different wording for the second part of
>     the sentence?  Say, "which shows the state of each capital".
> 
> 
> The <classname>capitals</classname> table has an additional column,
> <structfield>state</structfield>, which holds a state abbreviation.
> 
> As an aside, that field should be constrained NOT NULL and UNIQUE.  I have no
> qualms using those features in a chapter named "Advanced Features".  The cities
> table also doesn't have a PK (name, though in practice that is a poor choice),
> which it should, and the capitals table should have a unique index created over
> its inherited name field.  The note at the bottom says as much but showing the
> additional code in an example seems worthwhile.
> 
> Another option is to define capitals as:
> 
> CREATE TABLE capitals (
>   capital_dedication date NOT NULL
> ) INHERITS (cities);
> 
> "The capitals table has an additional column, capital_dedication, that holds
> the date on which the city became a capital."
> 
> Removing "char" from the tutorial is a nice side-effect that we probably want
> to do even if we keep "state".

I think CHAR(2) is fine because it is always 2 characters.

> The fact that this limited scope cities/capitals model is best defined using a
> single table with an "is_captial boolean not null" field sours me entirely as
> to the model choice for this page.

Understood.

How is the attached patch, based on your suggestions?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee