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 <mich...@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 <br...@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
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>
