rename es_epq_active to es_epqstate
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t50920psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 27, 2026 at 10:27 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t50920_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t50920_1 && git checkout t50920_1Patchset v1 (message #1) is on t50920_1
Hi hackers,
While reviewing the ExecSeqScan optimizations patch[1]/messages/by-id/CA+HiwqF+oydVreYN3Xp7U6x_LKi9ZL+No2X6WUv8X_kN+yHSLA@mail.gmail.com, I found that
es_epq_active might not be well named, my intuition told me that this
is a boolean field because of the "active" suffix.
es_epq_active was introduced in 27cc7cd, in the original discussion[2]/messages/by-id/20190828030201.v5u76ty47mtw2efp@alap3.anarazel.de,
Tom and Andres discussed the field name "es_active_epq" a little bit,
let me quote some:
------- quoted content begin
Also I dislike the field name "es_active_epq", as what that suggests
to me is exactly backwards from the way you apparently are using it.
Maybe "es_parent_epq" instead? The comment for it is pretty opaque
as well, not to mention that it misspells EState.
I think what I was trying to signal was that EPQ is currently active
from the POV of executor nodes. Ought to have been es_epq_active, for
that, I guess. For me "if (estate->es_epq_active)" explains the meaning
of the check more than "if (estate->es_parent_epq)".
I went with es_epq_active, as I suggested in my earlier email, which
still seems accurate to me.
I really want to move consideration of es_ep_active to ExecInitNode()
time, rather than execution time. If we add an execScan helper, we can
have it set the corresponding executor node's ExecProcNode to
a) a function that performs qual checks and projection
b) a function that performs projection
c) the fetch method from the scan node
d) basically the current ExecScan, when es_epq_active
-------- quoted content end
ISTM Andres tend to use *es_epq_active* in a boolean way,
like `if (es_epq_active) then`, but in the code base, all its usages
follow pattern `if (es_epq_active == NULL) then`, so I propose to
change es_epq_active to es_epqstate.
[1]: /messages/by-id/CA+HiwqF+oydVreYN3Xp7U6x_LKi9ZL+No2X6WUv8X_kN+yHSLA@mail.gmail.com
[2]: /messages/by-id/20190828030201.v5u76ty47mtw2efp@alap3.anarazel.de
--
Regards
Junwang Zhao
Junwang Zhao <zhjwpku@gmail.com> writes:
ISTM Andres tend to use *es_epq_active* in a boolean way,
like `if (es_epq_active) then`, but in the code base, all its usages
follow pattern `if (es_epq_active == NULL) then`, so I propose to
change es_epq_active to es_epqstate.
While I didn't especially love "es_epq_active" at the time,
I don't see that "es_epqstate" is much of an improvement:
it's an extremely generic name that conveys little information.
And renaming it now, years later, seems to add little except
back-patching hazards. So I'd vote for leaving it alone.
regards, tom lane
On Sat, Jan 18, 2025 at 9:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Junwang Zhao <zhjwpku@gmail.com> writes:
ISTM Andres tend to use *es_epq_active* in a boolean way,
like `if (es_epq_active) then`, but in the code base, all its usages
follow pattern `if (es_epq_active == NULL) then`, so I propose to
change es_epq_active to es_epqstate.While I didn't especially love "es_epq_active" at the time,
I don't see that "es_epqstate" is much of an improvement:
it's an extremely generic name that conveys little information.
And renaming it now, years later, seems to add little except
back-patching hazards. So I'd vote for leaving it alone.regards, tom lane
Ok, let's keep it as is, thanks for the explanation.
--
Regards
Junwang Zhao