Re: Reducing output size of nodeToString

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Michel Pelletier <pelletier(dot)michel(at)gmail(dot)com>
Subject: Re: Reducing output size of nodeToString
Date: 2024-03-20 11:49:52
Message-ID: 01e17c3a-b37b-4a08-96f6-0b85be6ffadd@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 19.03.24 17:13, Peter Eisentraut wrote:
> On 11.03.24 21:52, Matthias van de Meent wrote:
>>> * v7-0003-gen_node_support.pl-Mark-location-fields-as-type-.patch
>>>
>>> This looks sensible, but maybe making Location a global type is a bit
>>> much?  Maybe something more specific like ParseLocation, or ParseLoc, to
>>> keep it under 12 characters.
>> I've gone with ParseLoc in the attached v8 patchset.
>
> I have committed this one.

Next, I was looking at
v8-0003-pg_node_tree-Don-t-store-query-text-locations-in-.patch. After
applying that, I was looking how many uses of nodeToString() (with
locations) were left. I think your patch forgot to convert a number of
them, and there also might have been a few new ones that came in with
other recent patches. Might be hard to make sure all new developments
do this right. Plus, there are various mentions in the documentation
that should be updated. After considering all that, there weren't
really many callers of nodeToString() left. It's really only debugging
support in postgres.c and print.c, and a few places were it doesn't
matter, like the few places where it initializes "cooked expressions",
which were in turn already stripped of location fields at some earlier time.

So anyway, my idea was that we should turn this around and make
nodeToString() always drop location information, and instead add
nodeToStringWithLocations() for the few debugging uses. And this would
also be nice because then it matches exactly with the existing
stringToNodeWithLocations().

Attached patch shows this.

Attachment Content-Type Size
0001-Do-not-output-actual-value-of-location-fields-in-nod.patch text/plain 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2024-03-20 11:52:47 Re: DOCS: add helpful partitioning links
Previous Message jian he 2024-03-20 11:46:00 Re: remaining sql/json patches