pg.ddx.io pgsql-docs@postgresql.org mailing list archive
help / color / mirror / Atom feedRe: Typo in the Section "3.6. Inheritance"
8+ messages / 3 participants
[nested] [flat]
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-05 21:12 Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 8+ messages in thread
From: Bruce Momjian @ 2020-08-05 21:12 UTC (permalink / raw)
To: 2552891@gmail.com; pgsql-docs@lists.postgresql.org
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
Attachments:
diff (text/plain, 1.2K) download
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
new file mode 100644
index f6c4627..27b6d47
*** a/doc/src/sgml/advanced.sgml
--- b/doc/src/sgml/advanced.sgml
*************** CREATE TABLE capitals (
*** 628,635 ****
<firstterm>parent</firstterm>, <classname>cities</classname>. The
type of the column <structfield>name</structfield> is
<type>text</type>, a native <productname>PostgreSQL</productname>
! type for variable length character strings. State capitals have
! an extra column, <structfield>state</structfield>, that shows their state. In
<productname>PostgreSQL</productname>, a table can inherit from
zero or more other tables.
</para>
--- 628,636 ----
<firstterm>parent</firstterm>, <classname>cities</classname>. The
type of the column <structfield>name</structfield> is
<type>text</type>, a native <productname>PostgreSQL</productname>
! type for variable length character strings. The
! <classname>capitals</classname> table has
! an extra column, <structfield>state</structfield>, which shows their states. In
<productname>PostgreSQL</productname>, a table can inherit from
zero or more other tables.
</para>
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-05 21:36 Bruce Momjian <bruce@momjian.us>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 8+ messages in thread
From: Bruce Momjian @ 2020-08-05 21:36 UTC (permalink / raw)
To: 2552891@gmail.com; pgsql-docs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-06 05:41 Michael Paquier <michael@paquier.xyz>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2020-08-06 05:41 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: 2552891@gmail.com; pgsql-docs@lists.postgresql.org
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-06 06:29 David G. Johnston <david.g.johnston@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 8+ messages in thread
From: David G. Johnston @ 2020-08-06 06:29 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; 2552891@gmail.com, Pg Docs <pgsql-docs@lists.postgresql.org>
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".
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.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-21 23:36 Bruce Momjian <bruce@momjian.us>
parent: David G. Johnston <david.g.johnston@gmail.com>
0 siblings, 2 replies; 8+ messages in thread
From: Bruce Momjian @ 2020-08-21 23:36 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; 2552891@gmail.com, Pg Docs <pgsql-docs@lists.postgresql.org>
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
Attachments:
cities.diff (text/x-diff, 877B) download
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
index d77312600f..2d4ab85d45 100644
--- a/doc/src/sgml/advanced.sgml
+++ b/doc/src/sgml/advanced.sgml
@@ -616,7 +616,7 @@ CREATE TABLE cities (
);
CREATE TABLE capitals (
- state char(2)
+ state char(2) UNIQUE NOT NULL
) INHERITS (cities);
</programlisting>
</para>
@@ -630,7 +630,8 @@ CREATE TABLE capitals (
<type>text</type>, a native <productname>PostgreSQL</productname>
type for variable length character strings. The
<classname>capitals</classname> table has
- an extra column, <structfield>state</structfield>, which shows their states. In
+ an additional column, <structfield>state</structfield>, which shows its
+ state abbreviation. In
<productname>PostgreSQL</productname>, a table can inherit from
zero or more other tables.
</para>
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-22 01:36 Michael Paquier <michael@paquier.xyz>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 0 replies; 8+ messages in thread
From: Michael Paquier @ 2020-08-22 01:36 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; 2552891@gmail.com, Pg Docs <pgsql-docs@lists.postgresql.org>
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-22 02:09 David G. Johnston <david.g.johnston@gmail.com>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 1 reply; 8+ messages in thread
From: David G. Johnston @ 2020-08-22 02:09 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Michael Paquier <michael@paquier.xyz>; 2552891@gmail.com, Pg Docs <pgsql-docs@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Typo in the Section "3.6. Inheritance"
@ 2020-08-31 17:43 Bruce Momjian <bruce@momjian.us>
parent: David G. Johnston <david.g.johnston@gmail.com>
0 siblings, 0 replies; 8+ messages in thread
From: Bruce Momjian @ 2020-08-31 17:43 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; 2552891@gmail.com, Pg Docs <pgsql-docs@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2020-08-31 17:43 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05 21:12 Re: Typo in the Section "3.6. Inheritance" Bruce Momjian <bruce@momjian.us>
2020-08-05 21:36 ` Bruce Momjian <bruce@momjian.us>
2020-08-06 05:41 ` Michael Paquier <michael@paquier.xyz>
2020-08-06 06:29 ` David G. Johnston <david.g.johnston@gmail.com>
2020-08-21 23:36 ` Bruce Momjian <bruce@momjian.us>
2020-08-22 01:36 ` Michael Paquier <michael@paquier.xyz>
2020-08-22 02:09 ` David G. Johnston <david.g.johnston@gmail.com>
2020-08-31 17:43 ` Bruce Momjian <bruce@momjian.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox