Hi hackers, While reviewing the ExecSeqScan optimizations patch[1], 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], 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] https://www.postgresql.org/message-id/CA%2BHiwqF%2BoydVreYN3Xp7U6x_LKi9ZL%2BNo2X6WUv8X_kN%2ByHSLA%40mail.gmail.com [2] https://www.postgresql.org/message-id/20190828030201.v5u76ty47mtw2efp%40alap3.anarazel.de -- Regards Junwang Zhao
v1-0001-rename-es_epq_active-to-es_epqstate.patch
Description: Binary data
