Typo in the Section "3.6. Inheritance"

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Typo in the Section "3.6. Inheritance"

PG Doc comments form
The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/12/tutorial-inheritance.html
Description:

Hello Team,

https://www.postgresql.org/docs/current/tutorial-inheritance.html

There is the sentence in section 3.6: "State capitals have an extra column,
state, that shows their state."
I think it would be more correct to rewrite it as "Table capitals has
.....", since "capitals" is the table name in this context, but not the
state.

Best regards,
Vyacheslav Shablistyy
Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Bruce Momjian
On Mon, Jul 27, 2020 at 02:47:07PM +0000, PG Doc comments form wrote:

> The following documentation comment has been logged on the website:
>
> Page: https://www.postgresql.org/docs/12/tutorial-inheritance.html
> Description:
>
> Hello Team,
>
> https://www.postgresql.org/docs/current/tutorial-inheritance.html
>
> There is the sentence in section 3.6: "State capitals have an extra column,
> state, that shows their state."
> I think it would be more correct to rewrite it as "Table capitals has
> .....", since "capitals" is the table name in this context, but not the
> state.
Agreed.  Patch attached and applied through 9.5.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

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

Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Bruce Momjian
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?

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

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



Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Michael Paquier-2
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?

What if you used an entirely different wording for the second part of
the sentence?  Say, "which shows the state of each capital".
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

David G Johnston
On Wed, Aug 5, 2020 at 10:41 PM Michael Paquier <[hidden email]> 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".

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.

David J.

Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Bruce Momjian
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 <[hidden email]> 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  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

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


cities.diff (901 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Michael Paquier-2
On Fri, Aug 21, 2020 at 07:36:41PM -0400, Bruce Momjian wrote:
> How is the attached patch, based on your suggestions?

Looks good to me.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

David G Johnston
In reply to this post by Bruce Momjian
On Fri, Aug 21, 2020 at 4:36 PM Bruce Momjian <[hidden email]> wrote:
On Wed, Aug  5, 2020 at 11:29:31PM -0700, David G. Johnston wrote:

> 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.

You imply "it is always two non-blank characters" though that isn't what CHAR(2) means.  Adding CHECK (state ~ '^[A-Z]{2}$') and leaving the type as text would be best from a pure model perspective - but this isn't the place to teach that and for the same reason char(2) isn't terrible.

How is the attached patch, based on your suggestions?

Works for me.

David J.
Reply | Threaded
Open this post in threaded view
|

Re: Typo in the Section "3.6. Inheritance"

Bruce Momjian
On Fri, Aug 21, 2020 at 07:09:31PM -0700, David G. Johnston wrote:

> On Fri, Aug 21, 2020 at 4:36 PM Bruce Momjian <[hidden email]> wrote:
>
>     On Wed, Aug  5, 2020 at 11:29:31PM -0700, David G. Johnston wrote:
>
>     > 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.
>
>
> You imply "it is always two non-blank characters" though that isn't what CHAR
> (2) means.  Adding CHECK (state ~ '^[A-Z]{2}$') and leaving the type as text
> would be best from a pure model perspective - but this isn't the place to teach
> that and for the same reason char(2) isn't terrible.
>
>
>     How is the attached patch, based on your suggestions?
>
>
> Works for me.

Patch applied back through 9.5.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

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