Обсуждение: Typo in the Section "3.6. Inheritance"
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
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 <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
Вложения
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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
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
Вложения
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);
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.
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
Вложения
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
Вложения
On Fri, Aug 21, 2020 at 4:36 PM Bruce Momjian <bruce@momjian.us> 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.
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 <bruce@momjian.us> 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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee