On Thu, Dec 25, 2025 at 04:06:52PM +0000, Bernice Southey wrote:
> Robert Treat <[email protected]> wrote:
> > I think there were multiple goals at play, but IMHO they resulted in
> > an example that was too clever by half. While I have used multiple
> > versions of the technique they were trying to highlight myself, I
> > think it is out of place to add such complex examples in the
> > documentation where we are relying on the behavioral side-effects
> > (locking and ordering) of what is essentially an implementation detail
> > (ctid) and a detail which we really do not recommend users interact
> > with in any general way.
> 
> Thanks for this. Now I see why trying to fix these examples is so
> hard. They're obviously in the wrong place. Like you, I use this
> technique extensively, with and without ctid, and so I fully agree
> with the patch writer's aim. As I understand, it's to encourage
> batching for the performance benefits, how to resolve the inevitable
> deadlocks, and the added boost of ctid.

Yes, the issue is that a lot of discussion went into the existing docs,
so even though they are not ideal, we don't want to over-react and
change them more than reasonable, because then you need another set of
changes to adjust them to reasonable.

Also, original reporters tend to think the problem is worse than actual
because they had the problem.  This example has been published since PG
17 and this is the first reported complaint, and frankly the complaint
is that inaccurate assumptions were made from the example, and not
warned about.

Original reporters often want to add a lot of text to avoid others
having similar problem, even when clearly very few people have had the
problem.

> What if we remove the examples from update and delete completely?
> Instead we create a new subsection in the Performance Tips chapter
> called Batching. This keeps all this good advice together, in a place
> people like me, who wanted this guidance, will go looking. This is
> preferable to splitting it up into unread unrelated corners of the
> docs. The Batching doc could be the current UPDATE doc text expanded.
> It can properly explain the locking options, it can briefly explain
> what ctid is, why it's fast, and how to use it safely with locks.
> 
> I'd also like to propose including another batching trick in this new
> section: using copy to populate reusable session temp tables for batch
> processing. I expect there are other useful batching patterns
> community members can contribute in future.
> 
> I also considered a new "Updates" section in Performance Tips, a bit
> like the populating a database section. But this would need lots of
> other additions (like minimizing updates, checking if a record is
> actually changed, HOT updates, truncating partitions....), and it
> could become incohesive. I'm sure there's other potential places I'm
> unaware of.

Yep, "incohesive" is the risk.  Right now the UPDATE and DELETE examples
are different enough that explaining them in a separate section could
be confusing.

> Unfortunately I was wrong about the examples in UPDATE and DELETE
> being a safe use of ctid because they're called repeatedly - the final
> update/delete calls aren't safe. The examples as written have the same
> problem they describe for skip locked, i.e. a final execution is
> needed for any missed rows.  Using a select for update wait lock, with
> a ctid self-join, is the equivalent of "wait skip".

Yes, these are hard to get right.  We already have users running the
query repeatedly, so adding SKIP LOCKED to all the queries but the last
one is certainly possible and explainable.

At this point I have applied the attached patch back to PG 17 to
highlight that "ctid" is used in the update/delete queries only.  If we
want a new section or to move things around, that will only be done in
master, so it makes sense to just fix what we have now.

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

  Do not let urgent matters crowd out time for investment in the future.
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index cea28c00f8a..9070aaa5a7c 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -1558,7 +1558,7 @@ CREATE TABLE circles (
       locate the row version very quickly, a row's
       <structfield>ctid</structfield> will change if it is
       updated or moved by <command>VACUUM FULL</command>.  Therefore
-      <structfield>ctid</structfield> is useless as a long-term row
+      <structfield>ctid</structfield> should not be used as a row
       identifier.  A primary key should be used to identify logical rows.
      </para>
     </listitem>
diff --git a/doc/src/sgml/ref/delete.sgml b/doc/src/sgml/ref/delete.sgml
index 5b52f77e28f..b9367f2b23c 100644
--- a/doc/src/sgml/ref/delete.sgml
+++ b/doc/src/sgml/ref/delete.sgml
@@ -323,6 +323,9 @@ DELETE FROM user_logs AS dl
   USING delete_batch AS del
   WHERE dl.ctid = del.ctid;
 </programlisting>
+   This use of <structfield>ctid</structfield> is only safe because
+   the query is repeatedly run, avoiding the problem of changed
+   <structfield>ctid</structfield>s.
   </para>
  </refsect1>
 
diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml
index 40cca063946..b523766abe3 100644
--- a/doc/src/sgml/ref/update.sgml
+++ b/doc/src/sgml/ref/update.sgml
@@ -503,6 +503,9 @@ UPDATE work_item SET status = 'failed'
   WHERE work_item.ctid = emr.ctid;
 </programlisting>
    This command will need to be repeated until no rows remain to be updated.
+   (This use of <structfield>ctid</structfield> is only safe because
+   the query is repeatedly run, avoiding the problem of changed
+   <structfield>ctid</structfield>s.)
    Use of an <literal>ORDER BY</literal> clause allows the command to
    prioritize which rows will be updated; it can also prevent deadlock
    with other update operations if they use the same ordering.

Reply via email to