Re: Row pattern recognition

From: Henson Choi <assam258(at)gmail(dot)com>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>, jian(dot)universality(at)gmail(dot)com
Cc: zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Row pattern recognition
Date: 2026-09-12 02:49:48
Message-ID: CAAAe_zCUtfzBH5c1kFTKo5pSxXjQ0wjcuw5sXRGyoNAZ1+Tkwg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Tatsuo, Jian,

> Since v51 "pg_nodiscard" has been introduced in the RPR patches for
> some static functions. Are these "pg_nodiscard" really necessary?

They are not, and some history explains how they got there.

The seven marked functions are the pattern optimization passes in
the RPR planner file. Jian proposed changing them to modify the
list through the argument -- a node pointer, or List ** -- instead
of returning it. I preferred keeping the return, including in the
passes that do modify the list in place. His argument was that a
caller which drops the return value silently loses what the pass
did; mine was that these are static functions in a single file,
that every call site assigns the result back, and that the list
API itself is built the same way -- lappend(), list_delete_*() and
list_truncate() all modify and return, and all of them carry
pg_nodiscard.

pg_nodiscard was what I did with his concern in place of his
proposal: the return stays, and dropping it is no longer silent.
It was not an agreement -- Jian never signed off on the marker; it
went into the series because that is how I answered an offlist
proposal, and with several defects being worked through at once it
never got further than that. That is how a marker with no
precedent on a static function anywhere in the tree ended up in
the series. You are right that there is none -- I checked again,
and the existing uses are all extern.

Which of the two the series should use is yours to decide, and I
will follow it either way. If the return stays, I am happy to
drop the markers: every call site assigns today, and matching the
tree's convention is worth more than the warning.

Jian, is that a fair account of where each of us stood? Now that
we are no longer pressed by defect fixes, I think we can afford to
spend more time discussing things like this.

And one last thing. Look at the shape the patches proposed so far
have finally taken, and if anything you raised offlist is still
something you want reflected, let us work through it among the
three of us. For my part, where Tatsuo has reached a conclusion
after weighing it, I will go along with it as long as it does not
introduce a defect.

Best regards,
Henson

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kirk Roybal 2026-09-12 02:50:46 Re: [PATCH] Add PROMPT_COMMAND and dynamic prompt support to psql
Previous Message Xuneng Zhou 2026-09-12 02:48:06 Re: Reject WAIT FOR earlier in transaction-snapshot mode