On 18.01.23 08:04, Michael Paquier wrote:
On Tue, Jan 17, 2023 at 04:52:28PM +0900, Michael Paquier wrote:
On Tue, Jan 17, 2023 at 08:43:44AM +0100, Peter Eisentraut wrote:
Ok, I understand now, and I agree with this approach over the opposite. I
was confused because the snippet you showed above used "jumble_ignore", but
your patch is correct as it uses "jumble_location".
Okay. I'll refresh the patch set so as we have only "jumble_ignore",
then, like v1, with preparatory patches for what you mentioned and
anything that comes into mind.
This is done as of the patch series v3 attached:
- 0001 reformats all the comments of the nodes.
- 0002 moves the current files for query jumble as of queryjumble.c ->
queryjumblefuncs.c and utils/queryjumble.h -> nodes/queryjumble.h.
- 0003 is the core feature, where I have done a second pass over the
nodes to make sure that things map with HEAD, incorporating the extra
docs coming from v2, adding a bit more.
This patch structure looks good.
That said, the term "jumble" is really weird, because in the sense that we
are using it here it means, approximately, "to mix together", "to unify".
So what we are doing with the Const nodes is really to *not* jumble the
location, but for all other node types we are jumbling the location. At
least that is my understanding.
I am quite familiar with this term, FWIW. That's what we've inherited
from the days where this has been introduced in pg_stat_statements.
I have renamed the node attributes to query_jumble_ignore and
no_query_jumble at the end, after considering Peter's point that only
"jumble" could be fuzzy here. The file names are changed in
consequence.
I see that in the 0003 patch, most location fields now have an explicit
markup with query_jumble_ignore. I thought we had previously resolved
to consider location fields to be automatically ignored unless
explicitly included (like for the Const node). This appears to invert
that? Am I missing something?
