Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps
 (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1kWJCS-0007iv-G9
	for pgsql-hackers@arkaria.postgresql.org; Sat, 24 Oct 2020 13:03:12 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1kWJCO-0006dX-MI
	for pgsql-hackers@arkaria.postgresql.org; Sat, 24 Oct 2020 13:03:08 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1kWJCN-0006ct-U5
	for pgsql-hackers@lists.postgresql.org; Sat, 24 Oct 2020 13:03:08 +0000
Received: from tamriel.snowman.net ([70.109.60.50])
	by makus.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1kWJCH-0008Qn-U5
	for pgsql-hackers@postgresql.org; Sat, 24 Oct 2020 13:03:04 +0000
Received: by tamriel.snowman.net (Postfix, from userid 1000)
	id DC1DF5F799; Sat, 24 Oct 2020 09:02:59 -0400 (EDT)
Date: Sat, 24 Oct 2020 09:02:59 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Ian Lawrence Barwick <barwick@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [doc] remove reference to pg_dump pre-8.1 switch behaviour
Message-ID: <20201024130259.GU16415@tamriel.snowman.net>
References: 
 <CAB8KJ=jYHgnxLLZSNJz7gBTck4TxomngCmGkw3nEMSNF0yL6wA@mail.gmail.com>
 <CAB8KJ=h-XALik4M7gv-pX48=+SPWexfaYwa+yTnPwD3DxceXrg@mail.gmail.com>
 <e4934710-73cb-6eef-fd35-6fcfd05f381b@iki.fi>
 <20201023134611.GG16415@tamriel.snowman.net>
 <145522.1603461363@sss.pgh.pa.us>
 <20201023141211.GH16415@tamriel.snowman.net>
 <147812.1603464663@sss.pgh.pa.us>
 <fd93f1c5-7818-a02c-01e5-1075ac0d4def@iki.fi>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Gle0L8H5l8ToQqD4"
Content-Disposition: inline
In-Reply-To: <fd93f1c5-7818-a02c-01e5-1075ac0d4def@iki.fi>
User-Agent: Mutt/1.5.24 (2015-08-30)
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk


--Gle0L8H5l8ToQqD4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
> On 23/10/2020 17:51, Tom Lane wrote:
> >But anyway, this was about documentation not code.  What I'm wondering
> >about is when to drop things like, say, this bit in the regex docs:
> >
> >     Two significant incompatibilities exist between AREs and the ERE sy=
ntax
> >     recognized by pre-7.4 releases of <productname>PostgreSQL</productn=
ame>:
> >     (etc etc)
> >
> >Seems like we could have gotten rid of that by now, but when exactly
> >does it become fair game?  And can we have a non-ad-hoc process for
> >getting rid of such cruft?
>=20
> Let's try to zoom in on a rule:
>=20
> Anything that talks about 9.4 or above (min supported version - 1) should
> definitely be left in place.

Sure.

> Something around 9.0 is possibly still useful to someone upgrading or
> updating an application. Or someone might still bump into old blog posts
> from that era.

Right- going back ~10 years.  (I do think it'd be good to have an actual
policy rather than just "well, right now, it seems like 9.0 is about
right").

> Before that, I don't see much value. Although you could argue that I jump=
ed
> the gun on the notice about pre-8.2 pg_dump -t behavior. pg_dump still
> supports servers down to 8.0, so someone might also have an 8.0 pg_dump
> binary lying around, and might be confused that -t behaves differently. On
> the whole though, I think removing it was fair game.

I don't really have an issue with it, to be clear.  I had been hoping we
might be able to come up with a general rule to apply across both
documentation and code (in particular, pg_dump), but that doesn't seem
to be the case.  That does mean that *some* documentation might end up
needing to keep notes from before 9.0, where that documentation is about
pg_dump and older versions.

> I did some grepping for strings like "version 7", "pre-8" and so on. I
> couldn't come up with a clear rule on what could be removed. Context
> matters. In text that talks about protocol versions or libpq functions li=
ke
> PQlibVersion() it seems sensible to go back as far as possible, for the
> completeness. And subtle user-visible differences in behavior are more
> important to document than changes in internal C APIs that cause a compil=
er
> failure, for example.

I agree that context matters.

> Other notices are about old syntax that's kept for backwards compatibilit=
y,
> but still works. It makes sense to mention the old version in those cases,
> even if it's very old, because the alternative would be to just say
> something like "very old version", which is not any shorter, just less
> precise.

I would argue that we shouldn't be keeping things around for backwards
compatibility, in general.  If we feel that it's a good feature to keep
then let's keep it and just document it as an alternative syntax.

> Findings in detail follow. And attached is a patch about the stuff that I
> think can be removed pretty straightforwardly.

Thanks a lot for spending the time going through all of this!

> array.sgml:
>   <para>
>    If the value written for an element is <literal>NULL</literal> (in any
> case
>    variant), the element is taken to be NULL.  The presence of any quotes
>    or backslashes disables this and allows the literal string value
>    <quote>NULL</quote> to be entered.  Also, for backward compatibility w=
ith
>    pre-8.2 versions of <productname>PostgreSQL</productname>, the <xref
>    linkend=3D"guc-array-nulls"/> configuration parameter can be turned
>    <literal>off</literal> to suppress recognition of <literal>NULL</liter=
al>
> as a NULL.
>   </para>
>=20
> The GUC still exists, so we should keep this.

I agree we should keep the documentation as long as the GUC exists- but
we should be considering getting rid fo the GUC.

> catalogs.sgml:
>   <para>
>    The view <structname>pg_group</structname> exists for backwards
>    compatibility: it emulates a catalog that existed in
>    <productname>PostgreSQL</productname> before version 8.1.
>    It shows the names and members of all roles that are marked as not
>    <structfield>rolcanlogin</structfield>, which is an approximation to t=
he
> set
>    of roles that are being used as groups.
>   </para>
>=20
> pg_group still exists, and that paragraph explains why. We should keep it.
> (There's a similar paragraph for pg_shadow)

When I wrote that, many many years ago (apparently about 15, looking
back at the commit responsible), I certainly didn't expect we'd still
have them today.  These views, which have been only haphazardly
maintained and which don't really represent the current system terribly
well, need to go.  In hindsight, introducing them was a mistake in the
first place.  We support 5 major versions for a reason and people should
be updating their code as we make changes- as they have to do in lots of
other parts of the system and for other catalogs (consider the v10
changes of XLOG -> WAL).

If we keep the views we should keep the documentation, of course, but
it's long, long past time to rip out pg_user, pg_group, and pg_shadow.
Now that we have column-level privileges, I'd think we could probably
get rid of pg_roles too as not really providing much value.

> config.sgml (on synchronized_scans):
>=20
>        <para>
>         This allows sequential scans of large tables to synchronize with
> each
>         other, so that concurrent scans read the same block at about the
>         same time and hence share the I/O workload.  When this is enabled,
>         a scan might start in the middle of the table and then <quote>wrap
>         around</quote> the end to cover all rows, so as to synchronize wi=
th
> the
>         activity of scans already in progress.  This can result in
>         unpredictable changes in the row ordering returned by queries that
>         have no <literal>ORDER BY</literal> clause.  Setting this paramet=
er
> to
>         <literal>off</literal> ensures the pre-8.3 behavior in which a
> sequential
>         scan always starts from the beginning of the table.  The default
>         is <literal>on</literal>.
>        </para>
>=20
> We could remove the reference to 8.3 version. I'm inclined to keep it
> though.

I'm fine keeping the reference, if we keep the GUC.  I'm not really
inclined to keep this GUC though, at least not for the purpose of having
it to match pre-8.3 behavior explicitly.  If there's some usefulness to
this GUC then we should document what that is.  If there isn't, then
let's remove it.

> func.sgml (String Functions and Operators):
>     <note>
>     <para>
>      Before <productname>PostgreSQL</productname> 8.3, these functions wo=
uld
>      silently accept values of several non-string data types as well, due=
 to
>      the presence of implicit coercions from those data types to
>      <type>text</type>.  Those coercions have been removed because they
> frequently
>      caused surprising behaviors.  However, the string concatenation
> operator
>      (<literal>||</literal>) still accepts non-string input, so long as at
> least one
>      input is of a string type, as shown in <xref
>      linkend=3D"functions-string-sql"/>.  For other cases, insert an expl=
icit
>      coercion to <type>text</type> if you need to duplicate the previous
> behavior.
>     </para>
>    </note>
>=20
> Could remove the reference to 8.3, but the information about || still mak=
es
> sense. I'm inclined to just keep it.

I'd rather we rip out what the pre-8.3 behavior was as no longer
relevant and shorten this up quite a bit:

The string concatenation operator (<literal>||</literal>) will accept
non-string input, so long as at least one input is of string type, as
shown in <xref linkend=3D"functions-string-sql"/>.  For other cases,
inserting an explicit coercion to <type>text</type> can be used to have
non-string input accepted.

We're talking about v14 for this, after all.

> func.sgml:
>    <note>
>      <para>
>      Before <productname>PostgreSQL</productname> 8.2, the containment
>      operators <literal>@&gt;</literal> and <literal>&lt;@</literal> were
> respectively
>      called <literal>~</literal> and <literal>@</literal>.  These names a=
re
> still
>      available, but are deprecated and will eventually be removed.
>     </para>
>    </note>
>=20
> The old names are still available, so should keep this.

We should either remove them, or document them directly as alternative
spellings and admit to ourselves that they aren't deprecated and aren't
going to be removed.

> func.sgml:
>    <para>
>     Before <productname>PostgreSQL</productname> 8.1, the arguments of the
>     sequence functions were of type <type>text</type>, not
> <type>regclass</type>, and
>     the above-described conversion from a text string to an OID value wou=
ld
>     happen at run time during each call.  For backward compatibility, this
>     facility still exists, but internally it is now handled as an implicit
>     coercion from <type>text</type> to <type>regclass</type> before the
> function is
>     invoked.
>    </para>
>=20
> Let's remove this.

+1

> func.sqml:
>   <para>
>    <xref linkend=3D"array-operators-table"/> shows the specialized operat=
ors
>    available for array types.
>    In addition to those, the usual comparison operators shown in <xref
>    linkend=3D"functions-comparison-op-table"/> are available for
>    arrays.  The comparison operators compare the array contents
>    element-by-element, using the default B-tree comparison function for
>    the element data type, and sort based on the first difference.
>    In multidimensional arrays the elements are visited in row-major order
>    (last subscript varies most rapidly).
>    If the contents of two arrays are equal but the dimensionality is
>    different, the first difference in the dimensionality information
>    determines the sort order.  (This is a change from versions of
>    <productname>PostgreSQL</productname> prior to 8.2: older versions wou=
ld
> claim
>    that two arrays with the same contents were equal, even if the
>    number of dimensions or subscript ranges were different.)
>   </para>
>=20
> Could remove it.

+1 to remove.

>    <note>
>      <para>
>      There are two differences in the behavior of
> <function>string_to_array</function>
>      from pre-9.1 versions of <productname>PostgreSQL</productname>.
>      First, it will return an empty (zero-element) array rather
>      than <literal>NULL</literal> when the input string is of zero length.
>      Second, if the delimiter string is <literal>NULL</literal>, the
> function
>      splits the input into individual characters, rather than
>      returning <literal>NULL</literal> as before.
>     </para>
>    </note>
>=20
> Feels too early to remove.

+0 to remove, for my part.

>   <note>
>    <para>
>     Prior to <productname>PostgreSQL</productname> 8.2, the
>     <literal>&lt;</literal>, <literal>&lt;=3D</literal>,
> <literal>&gt;</literal> and <literal>&gt;=3D</literal>
>     cases were not handled per SQL specification.  A comparison like
>     <literal>ROW(a,b) &lt; ROW(c,d)</literal>
>     was implemented as
>     <literal>a &lt; c AND b &lt; d</literal>
>     whereas the correct behavior is equivalent to
>     <literal>a &lt; c OR (a =3D c AND b &lt; d)</literal>.
>    </para>
>   </note>
>=20
> Important incompatibility. Although very old. I'm inclined to keep it. If=
 we
> remove it, it'd still be useful to explain the new behavior.

+1 to remove and replace with an explanation of the new behavior.

> gin.sqml:
> <title>GIN Tips and Tricks</title>
>=20
>  <variablelist>
>   <varlistentry>
>    <term>Create vs. insert</term>
>    <listitem>
>     <para>
>      Insertion into a <acronym>GIN</acronym> index can be slow
>      due to the likelihood of many keys being inserted for each item.
>      So, for bulk insertions into a table it is advisable to drop the GIN
>      index and recreate it after finishing bulk insertion.
>     </para>
>=20
>     <para>
>      As of <productname>PostgreSQL</productname> 8.4, this advice is less
>      necessary since delayed indexing is used (see <xref
>      linkend=3D"gin-fast-update"/> for details).  But for very large upda=
tes
>      it may still be best to drop and recreate the index.
>     </para>
>    </listitem>
>   </varlistentry>
>=20
> I think that's old enough, but the paragraph would need some copy-editing,
> not just removal.

How about:

Building a <acronym>GIN</acronym> index after all of the data has been
loaded will typically be faster than creating the index and then filling
it.  There may also be cases where, for a sufficiently large update,
dropping the <acronym>GIN</acronym> index, then performing the update,
and then recreating the index will be faster than a routine update,
however, one should review the delayed indexing technique used for
<acronym>GIN</acronym> (see <xref linkend=3D"gin-fast-update"/> for
details) and the options it provides.

> high-availability.sgml (Record-based log shipping)
>   <sect2 id=3D"warm-standby-record">
>    <title>Record-Based Log Shipping</title>
>=20
>    <para>
>     It is also possible to implement record-based log shipping using this
>     alternative method, though this requires custom development, and chan=
ges
>     will still only become visible to hot standby queries after a full WAL
>     file has been shipped.
>    </para>
>=20
>    <para>
>     An external program can call the
> <function>pg_walfile_name_offset()</function>
>     function (see <xref linkend=3D"functions-admin"/>)
>     to find out the file name and the exact byte offset within it of
>     the current end of WAL.  It can then access the WAL file directly
>     and copy the data from the last known end of WAL through the current =
end
>     over to the standby servers.  With this approach, the window for data
>     loss is the polling cycle time of the copying program, which can be v=
ery
>     small, and there is no wasted bandwidth from forcing partially-used
>     segment files to be archived.  Note that the standby servers'
>     <varname>restore_command</varname> scripts can only deal with whole W=
AL
> files,
>     so the incrementally copied data is not ordinarily made available to
>     the standby servers.  It is of use only when the primary dies &mdash;
>     then the last partial WAL file is fed to the standby before allowing
>     it to come up.  The correct implementation of this process requires
>     cooperation of the <varname>restore_command</varname> script with the
> data
>     copying program.
>    </para>
>=20
>    <para>
>     Starting with <productname>PostgreSQL</productname> version 9.0, you =
can
> use
>     streaming replication (see <xref linkend=3D"streaming-replication"/>)=
 to
>     achieve the same benefits with less effort.
>    </para>
>   </sect2>
>=20
> I think we should remove this whole section. Writing your own record-level
> log shipping by polling pg_walfile_name_offset() is malpractice on modern
> versions, when you could use streaming replication instead. The whole
> "Alternative Method for Log Shipping" section is pretty outdated.

+1

> indexam.sgml:
>   <para>
>    As of <productname>PostgreSQL</productname> 8.4,
>    <function>amvacuumcleanup</function> will also be called at completion=
 of
> an
>    <command>ANALYZE</command> operation.  In this case
> <literal>stats</literal> is always
>    NULL and any return value will be ignored.  This case can be
> distinguished
>    by checking <literal>info-&gt;analyze_only</literal>.  It is recommend=
ed
>    that the access method do nothing except post-insert cleanup in such a
>    call, and that only in an autovacuum worker process.
>   </para>
>=20
> Let's remove the "As of PostgreSQL 8.4".

+1

>    <para>
>     The standard installation provides all the header files needed for
> client
>     application development as well as for server-side program
>     development, such as custom functions or data types written in C.
>     (Prior to <productname>PostgreSQL</productname> 8.0, a separate
> <literal>make
>     install-all-headers</literal> command was needed for the latter, but
> this
>     step has been folded into the standard install.)
>    </para>
>=20
> Remove.

+1

>      <listitem>
>       <para>
>        Interrogates the frontend/backend protocol being used.
> <synopsis>
> int PQprotocolVersion(const PGconn *conn);
> </synopsis>
>        Applications might wish to use this function to determine whether
> certain
>        features are supported.  Currently, the possible values are 2 (2.0
>        protocol), 3 (3.0 protocol), or zero (connection bad).  The
>        protocol version will
>        not change after connection startup is complete, but it could
>        theoretically change during a connection reset.  The 3.0 protocol
>        will normally be used when communicating with
>        <productname>PostgreSQL</productname> 7.4 or later servers; pre-7.4
> servers
>        support only protocol 2.0.  (Protocol 1.0 is obsolete and not
>        supported by <application>libpq</application>.)
>       </para>
>      </listitem>
>=20
> Talking about old versions, even very old ones, seems appropriate for a
> function like PQprotocolVersion().

Agreed, at least until we rip out the 2.0 protocol..

> libpq.sgml, on PQlibVersion():
>      <note>
>       <para>
>        This function appeared in <productname>PostgreSQL</productname>
> version 9.1, so
>        it cannot be used to detect required functionality in earlier
>        versions, since calling it will create a link dependency
>        on version 9.1 or later.
>       </para>
>      </note>
>=20
> Seems appropriate to keep.

+0 to remove, for my part.  People building against libraries should
realize that they're creating a link dependency on things they're
calling..

> libpq.sgml:
>       <para>
>        <xref linkend=3D"libpq-PQinitSSL"/> has been present since
>        <productname>PostgreSQL</productname> 8.0, while <xref
> linkend=3D"libpq-PQinitOpenSSL"/>
>        was added in <productname>PostgreSQL</productname> 8.4, so <xref
> linkend=3D"libpq-PQinitSSL"/>
>        might be preferable for applications that need to work with older
>        versions of <application>libpq</application>.
>       </para>
>=20
> Keep.

+1 to just remove PQinitSSL.

> lobj.sgml:
>     <para>
>      <indexterm><primary>lo_creat</primary></indexterm>
>      The function
> <synopsis>
> Oid lo_creat(PGconn *conn, int mode);
> </synopsis>
>      creates a new large object.
>      The return value is the OID that was assigned to the new large objec=
t,
>      or <symbol>InvalidOid</symbol> (zero) on failure.
>=20
>      <replaceable class=3D"parameter">mode</replaceable> is unused and
>      ignored as of <productname>PostgreSQL</productname> 8.1; however, for
>      backward compatibility with earlier releases it is best to
>      set it to <symbol>INV_READ</symbol>, <symbol>INV_WRITE</symbol>,
>      or <symbol>INV_READ</symbol> <literal>|</literal>
> <symbol>INV_WRITE</symbol>.
>      (These symbolic constants are defined
>      in the header file <filename>libpq/libpq-fs.h</filename>.)
>     </para>
>=20
> We need to say something about 'mode'. Keep.

We should drop that parameter and get rid of this.

> pgfreespacemap.sgml:
>   <note>
>    <para>
>     The interface was changed in version 8.4, to reflect the new FSM
>     implementation introduced in the same version.
>    </para>
>   </note>
>=20
> Remove.

+1

> pgstandby.sgml:
>   <para>
>    <application>pg_standby</application> is designed to work with
>    <productname>PostgreSQL</productname> 8.2 and later.
>   </para>
>=20
> IMHO we should remove pg_standby altogether. Until we get around to that,=
 I
> think we should keep that note because it gives you a hint that it's old
> :-).

+1 to remove pg_standby

> pgarchivecleanup.sgml:
>   <para>
>    <application>pg_archivecleanup</application> is designed to work with
>    <productname>PostgreSQL</productname> 8.0 and later when used as a
> standalone utility,
>    or with <productname>PostgreSQL</productname> 9.0 and later when used =
as
> an
>    archive cleanup command.
>   </para>
>=20
> Ditto.

+1 to remove pg_archivecleanup

> planstats.sgml:
>   <para>
>    The examples shown below use tables in the
> <productname>PostgreSQL</productname>
>    regression test database.
>    The outputs shown are taken from version 8.3.
>    The behavior of earlier (or later) versions might vary.
>=20
> Should refresh the outputs..

+1

> plpgsql.sgml:
>        <para>
>         When used with a
>         <literal>BEGIN</literal> block, <literal>EXIT</literal> passes
>         control to the next statement after the end of the block.
>         Note that a label must be used for this purpose; an unlabeled
>         <literal>EXIT</literal> is never considered to match a
>         <literal>BEGIN</literal> block.  (This is a change from
>         pre-8.4 releases of <productname>PostgreSQL</productname>, which
>         would allow an unlabeled <literal>EXIT</literal> to match
>         a <literal>BEGIN</literal> block.)
>        </para>
>=20
> Maybe keep for a couple more years.

+1 to remove

> protocol.sgml:
>  <para>
>   This document describes version 3.0 of the protocol, implemented in
>   <productname>PostgreSQL</productname> 7.4 and later.  For descriptions
>   of the earlier protocol versions, see previous releases of the
>   <productname>PostgreSQL</productname> documentation.  A single server
>   can support multiple protocol versions.  The initial startup-request
>   message tells the server which protocol version the client is attempting
> to
>   use.  If the major version requested by the client is not supported by
>   the server, the connection will be rejected (for example, this would oc=
cur
>   if the client requested protocol version 4.0, which does not exist as of
>   this writing).  If the minor version requested by the client is not
>   supported by the server (e.g., the client requests version 3.1, but the
>   server supports only 3.0), the server may either reject the connection =
or
>   may respond with a NegotiateProtocolVersion message containing the high=
est
>   minor protocol version which it supports.  The client may then choose
> either
>   to continue with the connection using the specified protocol version or
>   to abort the connection.
>  </para>
>=20
> Keep.

+1 to keep.

>      <varlistentry>
>       <term>AuthenticationSCMCredential</term>
>       <listitem>
>        <para>
>         This response is only possible for local Unix-domain connections
>         on platforms that support SCM credential messages.  The frontend
>         must issue an SCM credential message and then send a single data
>         byte.  (The contents of the data byte are uninteresting; it's
>         only used to ensure that the server waits long enough to receive
>         the credential message.)  If the credential is acceptable,
>         the server responds with an
>         AuthenticationOk, otherwise it responds with an ErrorResponse.
>         (This message type is only issued by pre-9.1 servers.  It may
>         eventually be removed from the protocol specification.)
>        </para>
>       </listitem>
>      </varlistentry>
>=20
> Keep. It's surely still referred to in client libraries.

I'd rather have something like:

AuthenticationSCMCredential

Only issued by pre-9.1 servers, no longer used.  See older documentation
for details.

Or something along those lines.

>    <para>
>     Data of a particular data type might be transmitted in any of several
>     different <firstterm>formats</firstterm>.  As of
> <productname>PostgreSQL</productname> 7.4
>     the only supported formats are <quote>text</quote> and
> <quote>binary</quote>,
>     but the protocol makes provision for future extensions.  The desired
>     format for any value is specified by a <firstterm>format
> code</firstterm>.
>     Clients can specify a format code for each transmitted parameter value
>     and for each column of a query result.  Text has format code zero,
>     binary has format code one, and all other format codes are reserved
>     for future definition.
>    </para>
>=20
> Could replace the "as of PostgreSQL 7.4" with "Currently", but it's not m=
uch
> shorter.

Seems a bit confusing to say "several different formats" and then say
"well, really only text and binary".  How about:

Data of a particular data type may be transmitted in either
<quote>text</quote> or <quote>binary</quote> format.  The desired format
for any value is specified by a <firstterm>format code</firstterm>.
Clients can specify a format code for each transmitted parameter
value and for each column of a query result.  Text has format code zero,
binary has format code one, and all other format codes are reserved for
future definition.

(Do we also point out that not everything supports binary..?  If not,
seems like we should, but maybe that's covered)

>        <para>
>         For a <command>COPY</command> command, the tag is
>         <literal>COPY <replaceable>rows</replaceable></literal> where
>         <replaceable>rows</replaceable> is the number of rows copied.
>         (Note: the row count appears only in
>         <productname>PostgreSQL</productname> 8.2 and later.)
>        </para>
>=20
> I think we should keep, since we mentioned earlier that the protocol
> documentation is for 7.4 and later.

+0 to remove.  Someone working with a 8.1 or older server could look at
those docs.

> alter_opfamily.sgml and create_opclass.sgml:
>   <para>
>    Before <productname>PostgreSQL</productname> 8.4, the
> <literal>OPERATOR</literal>
>    clause could include a <literal>RECHECK</literal> option.  This is no
> longer
>    supported because whether an index operator is <quote>lossy</quote> is
> now
>    determined on-the-fly at run time.  This allows efficient handling of
>    cases where an operator might or might not be lossy.
>   </para>
>=20
> Keep, since the syntax is still supported (but ignored).

We should remove the syntax.  Having things that are accepted but
ignored isn't good, imv.

> cluster.sgml:
>   <para>
>    The syntax
> <synopsis>
> CLUSTER <replaceable class=3D"parameter">index_name</replaceable> ON
> <replaceable class=3D"parameter">table_name</replaceable>
> </synopsis>
>   is also supported for compatibility with pre-8.3
> <productname>PostgreSQL</productname>
>   versions.
>   </para>
>=20
> Keep, since the syntax is still supported.

We should remove the syntax, or just document it as alternative syntax.

> copy.sgml:
>   <para>
>    The following syntax was used before
> <productname>PostgreSQL</productname>
>    version 9.0 and is still supported:
> ...
>   <para>
>    The following syntax was used before
> <productname>PostgreSQL</productname>
>    version 7.3 and is still supported:
>=20
> Keep, since the syntax is still supported.

+0 to keep these references to when they were introduced.

> create_function.sgml:
>    <para>
>     Before <productname>PostgreSQL</productname> version 8.3, the
>     <literal>SET</literal> clause was not available, and so older functio=
ns
> may
>     contain rather complicated logic to save, set, and restore
>     <varname>search_path</varname>.  The <literal>SET</literal> clause is
> far easier
>     to use for this purpose.
>    </para>
>=20
> Keep, those old functions with complicated might still exist in the wild.

+1

> create_type.sgml:
>   <para>
>    Before <productname>PostgreSQL</productname> version 8.3, the name of
>    a generated array type was always exactly the element type's name with
> one
>    underscore character (<literal>_</literal>) prepended.  (Type names we=
re
>    therefore restricted in length to one less character than other names.)
>    While this is still usually the case, the array type name may vary from
>    this in case of maximum-length names or collisions with user type names
>    that begin with underscore.  Writing code that depends on this convent=
ion
>    is therefore deprecated.  Instead, use
>    <structname>pg_type</structname>.<structfield>typarray</structfield> to
> locate the array type
>    associated with a given type.
>   </para>
>=20
> Let's keep it. We could remove the reference to 8.3, but would still need=
 to
> explain the behaviour, and I think it's easiest to explain through its
> history.

+1

> create_type.sgml:
>   <para>
>    Before <productname>PostgreSQL</productname> version 8.2, the shell-ty=
pe
>    creation syntax
>    <literal>CREATE TYPE <replaceable>name</replaceable></literal> did not
> exist.
>    The way to create a new base type was to create its input function fir=
st.
>    In this approach, <productname>PostgreSQL</productname> will first see
>    the name of the new data type as the return type of the input function.
>    The shell type is implicitly created in this situation, and then it
>    can be referenced in the definitions of the remaining I/O functions.
>    This approach still works, but is deprecated and might be disallowed in
>    some future release.  Also, to avoid accidentally cluttering
>    the catalogs with shell types as a result of simple typos in function
>    definitions, a shell type will only be made this way when the input
>    function is written in C.
>   </para>
>=20
> The deprecated way still works, so keep.

Bleh, I disagree, +1 to remove.  We shouldn't be documenting how not to
do things in modern versions, just because that's how some old version
required it to be done.

> grant.sgml:
>    <para>
>     Since <productname>PostgreSQL</productname> 8.1, the concepts of users
> and
>     groups have been unified into a single kind of entity called a role.
>     It is therefore no longer necessary to use the keyword
> <literal>GROUP</literal>
>     to identify whether a grantee is a user or a group.
> <literal>GROUP</literal>
>     is still allowed in the command, but it is a noise word.
>    </para>
>=20
> The GROUP keyword is still accepted, so let's keep it.

=3D1 to remove the GROUP keyword as being accepted here.

> pg_config-ref.sgml:
>   <para>
>    The options <option>--docdir</option>, <option>--pkgincludedir</option=
>,
>    <option>--localedir</option>, <option>--mandir</option>,
>    <option>--sharedir</option>, <option>--sysconfdir</option>,
>    <option>--cc</option>, <option>--cppflags</option>,
>    <option>--cflags</option>, <option>--cflags_sl</option>,
>    <option>--ldflags</option>, <option>--ldflags_sl</option>,
>    and <option>--libs</option> were added in
> <productname>PostgreSQL</productname> 8.1.
>    The option <option>--htmldir</option> was added in
> <productname>PostgreSQL</productname> 8.4.
>    The option <option>--ldflags_ex</option> was added in
> <productname>PostgreSQL</productname> 9.0.
>   </para>
>=20
> Let's keep these. This could still be relevant if someone is maintaining =
an
> extension that's backwards compatible to old versions.

+0 to keep.

> pg_dumpall.sgml:
>      <varlistentry>
>       <term><option>--lock-wait-timeout=3D<replaceable
> class=3D"parameter">timeout</replaceable></option></term>
>       <listitem>
>        <para>
>         Do not wait forever to acquire shared table locks at the beginning
> of
>         the dump. Instead, fail if unable to lock a table within the
> specified
>         <replaceable class=3D"parameter">timeout</replaceable>. The timeo=
ut
> may be
>         specified in any of the formats accepted by <command>SET
>         statement_timeout</command>.  Allowed values vary depending on the
> server
>         version you are dumping from, but an integer number of millisecon=
ds
>         is accepted by all versions since 7.3.  This option is ignored wh=
en
>         dumping from a pre-7.3 server.
>        </para>
>       </listitem>
>      </varlistentry>
>=20
> pg_dump no longer supports pre-8.0 versions, so this is definitely obsole=
te.
> Remove.

+1

> psql-ref.sgml:
>       <listitem>
>       <para>
>        Before <productname>PostgreSQL</productname> 8.4,
>        <application>psql</application> allowed the
>        first argument of a single-letter backslash command to start
>        directly after the command, without intervening whitespace.
>        Now, some whitespace is required.
>       </para>
>       </listitem>
>=20
> Keep for a few more years.

+0 to remove.

> psql-ref.sgml:
>           <para><literal>old-ascii</literal> style uses plain
> <acronym>ASCII</acronym>
>           characters, using the formatting style used
>           in <productname>PostgreSQL</productname> 8.4 and earlier.
>           Newlines in data are shown using a <literal>:</literal>
>           symbol in place of the left-hand column separator.
>           When the data is wrapped from one line
>           to the next without a newline character, a <literal>;</literal>
>           symbol is used in place of the left-hand column separator.
>           </para>
>=20
> Keep, as long as we keep the format.

+1 to drop the format

>    <note>
>      <para>
>      Before <productname>PostgreSQL</productname> 8.2, the
>      <literal>.*</literal> syntax was not expanded in row constructors, so
>      that writing <literal>ROW(t.*, 42)</literal> created a two-field row
> whose first
>      field was another row value.  The new behavior is usually more usefu=
l.
>      If you need the old behavior of nested row values, write the inner
>      row value without <literal>.*</literal>, for instance
>      <literal>ROW(t, 42)</literal>.
>     </para>
>    </note>
>=20
> I'm inclined to keep this, someone might still need that behaviour, not
> necessary for backwards-compatibility but because you might want to do th=
at
> in an application. Or rewrite without the reference to 8.2.

I'd suggest rewriting to discuss the options and move away from it being
a history lesson.

>   <para>
>    For comparison, the <productname>PostgreSQL</productname> 8.1
> documentation
>    contained 10,441 unique words, a total of 335,420 words, and the most
>    frequent word <quote>postgresql</quote> was mentioned 6,127 times in 6=
55
>    documents.
>   </para>
>=20
>    <!-- TODO we need to put a date on these numbers? -->
>   <para>
>    Another example &mdash; the <productname>PostgreSQL</productname> mail=
ing
>    list archives contained 910,989 unique words with 57,491,343 lexemes in
>    461,020 messages.
>   </para>
>=20
> Refresh the numbers.

+1

>   <note>
>    <para>
>     In the SQL standard, there is a clear distinction between users and
> roles,
>     and users do not automatically inherit privileges while roles do.  Th=
is
>     behavior can be obtained in <productname>PostgreSQL</productname> by
> giving
>     roles being used as SQL roles the <literal>INHERIT</literal> attribut=
e,
> while
>     giving roles being used as SQL users the <literal>NOINHERIT</literal>
> attribute.
>     However, <productname>PostgreSQL</productname> defaults to giving all
> roles
>     the <literal>INHERIT</literal> attribute, for backward compatibility
> with pre-8.1
>     releases in which users always had use of permissions granted to grou=
ps
>     they were members of.
>    </para>
>   </note>
>=20
> Keep, since that's still how it behaves.

I'd just say that then:

<productname>PostgreSQL</productname> defaults to giving all roles the
<literal>INHERIT</literal> attribute, as this is generally seen as more
useful.

> xindex.sgml:
>   <note>
>     <para>
>     Prior to <productname>PostgreSQL</productname> 8.3, there was no conc=
ept
>     of operator families, and so any cross-data-type operators intended to
> be
>     used with an index had to be bound directly into the index's operator
>     class.  While this approach still works, it is deprecated because it
>     makes an index's dependencies too broad, and because the planner can
>     handle cross-data-type comparisons more effectively when both data ty=
pes
>     have operators in the same operator family.
>    </para>
>   </note>
>=20
> Keep, because the old method still works.

+1 to remove.

(Didn't look at the actual patch, I'm sure it does what you had said
above)

Thanks,

Stephen

--Gle0L8H5l8ToQqD4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJflCYDAAoJEO1sijiDR2RVGDAP/2bdg6mUylGH6QZnZaWh3pu4
l4/9lSJCFENGkCnnMRrm/SuxUUdg+WZSlaKoE/NMXg7uBq8Z3bmQDrcFzB6BiMVm
oRH3N9Wwd4hTL2tt39WpPuhyp7IYAI4rXYzwhwjVvqKyqBWg7KrMmn7pey5sAATT
84HypvmyW7B0sPTyYqKdaxHA/pxM7llrqGhTZmRYvTfc98uzoa+srYOVCt1iU+b9
osohmZGuUih2HM1HPDHTvLVpYlFQlhOTTdzwCqt9yFKN9Vp8/7G28qvCBdeMxxm5
9MdZSBJCXGmIjWfYmKUtvOdzR/wJbKk2Xsi71dbqJv1Kbh08ZkB8Ltu//ymcphRf
qVjWuN06lZ8aCqQ4rRrNL8C90JZfcXUWiL2JT5EL2oS4m2nrPOL7DAJVOU+Uar14
+kIk8T2peC0UoEzqghIi9LaZxc5NXXyzLQZmAvMwcUi55wZtvZ0GgiXLJA8Qbqri
GrWkuaiUL7QO/ihVq2oY6JMOhNYbTurH4lhMV2YtaKVFK4HA7bO9QVk833cu3Kz3
Sc0CIasvjm4Qi2CHrqEK6R4op+m2Om1OmyH3tpbZzKUm0pl0g5tF7gAj3KdDN7k6
IeWtcF9OtCage6K6guY7bofMhIUg7hUXJcZ0cxcoQYN/6CsdjXbnsc7tLKXxhWDM
mIP44sgxJ7ql2k7JWGg9
=mwM1
-----END PGP SIGNATURE-----

--Gle0L8H5l8ToQqD4--