On Tue, Aug 19, 2025 at 11:21:06AM +1200, David Rowley wrote:
> Ok. I pushed it like that. Thanks for looking.
Added a -DHASH_STATISTICS to batta, that runs only the master branch.
--
Michael
signature.asc
Description: PGP signature
On Tue, 19 Aug 2025 at 01:34, Tom Lane wrote:
>
> David Rowley writes:
> > I've purposefully left references to HASH_DEBUG in the "Original
> > comments" section near the top of dynahash.c. That comment also
> > references function names that no longer exist.
>
> Hm, there is actually no part of
David Rowley writes:
> I've purposefully left references to HASH_DEBUG in the "Original
> comments" section near the top of dynahash.c. That comment also
> references function names that no longer exist.
Hm, there is actually no part of that comment para that is accurate
anymore, so I cannot imag
On Mon, Aug 18, 2025 at 05:55:33PM +1200, David Rowley wrote:
> I've purposefully left references to HASH_DEBUG in the "Original
> comments" section near the top of dynahash.c. That comment also
> references function names that no longer exist.
Ah, right, the hcreate() and hdestroy() business. LG
On Mon, 18 Aug 2025 at 17:06, Michael Paquier wrote:
>
> On Mon, Aug 18, 2025 at 02:19:06PM +1200, David Rowley wrote:
> > On Mon, 18 Aug 2025 at 13:26, Tom Lane wrote:
> >> Yeah, it's really quite unclear what the existing HASH_DEBUG printout
> >> is good for. At least in our usage, it doesn't
On Mon, Aug 18, 2025 at 02:19:06PM +1200, David Rowley wrote:
> On Mon, 18 Aug 2025 at 13:26, Tom Lane wrote:
>> Yeah, it's really quite unclear what the existing HASH_DEBUG printout
>> is good for. At least in our usage, it doesn't tell you anything
>> you can't discover from static code analysi
On Mon, 18 Aug 2025 at 13:26, Tom Lane wrote:
>
> David Rowley writes:
> > I wondered about that and thought that there might be an above zero
> > chance that someone would want HASH_DEBUG without USE_ASSERT_CHECKING.
> > I don't really know if that person exists. It certainly isn't me.
>
> Yeah,
David Rowley writes:
> On Mon, 18 Aug 2025 at 13:10, Michael Paquier wrote:
>> If we do that, I guess that we could just remove HASH_DEBUG, keeping
>> only HASH_STATISTICS.
> I wondered about that and thought that there might be an above zero
> chance that someone would want HASH_DEBUG without U
On Mon, Aug 18, 2025 at 12:56:02PM +1200, David Rowley wrote:
> One last thing, in order to inform people of breakages sooner than a
> post-commit report from the buildfarm, I wondered is if we should do:
>
> -#ifdef HASH_DEBUG
> +#if defined(HASH_DEBUG) || defined(USE_ASSERT_CHECKING)
>
> The HA
On Mon, 18 Aug 2025 at 13:10, Michael Paquier wrote:
> On Mon, Aug 18, 2025 at 12:56:02PM +1200, David Rowley wrote:
> > -#ifdef HASH_DEBUG
> > +#if defined(HASH_DEBUG) || defined(USE_ASSERT_CHECKING)
> >
> > The HASH_DEBUG does not add any extra fields, so the overhead only
> > amounts to the elo
On Sun, 17 Aug 2025 at 18:54, Michael Paquier wrote:
> -#ifdef HASH_STATISTICS
> -static long hash_accesses,
> - hash_collisions,
> - hash_expansions;
> -#endif
>
> These global counters are as old as d31084e9d111. Removing them
> should not be a proble
On Sun, Aug 17, 2025 at 03:54:16PM +0900, Michael Paquier wrote:
> Side thing.. I'm wondering what prevents us from wiping out entirely
> the use of long in this file. long is 8 bytes everywhere, except on
> WIN32 where it's 4 bytes (as you say), which is a bad practice as we
> have been bitten b
On Sun, Aug 17, 2025 at 12:24:29AM +1200, David Rowley wrote:
> I noticed that the existing code has a set of global variables that
> keeps track of accesses, collisions and expansions for *all* tables in
> the backend. This didn't fit well with the single line elog. I kinda
> though the global inf
On Sat, 16 Aug 2025 at 03:16, Tom Lane wrote:
> * Neither printout identifies which hashtable it's talking about
> in any usable fashion, which is silly when we could print
> hashp->tabname. HASH_DEBUG prints the pointer to the table,
> which is certainly useless unless you've got gdb attached to
David Rowley writes:
> Yeah. Just to aid discussion, let's say the attached patch.
I think if we want to claim that these bits represent actually
usable functionality, we need to do more than s/fprintf/elog/.
Some points:
* Neither printout identifies which hashtable it's talking about
in any us
On Fri, 15 Aug 2025 at 19:09, Michael Paquier wrote:
>
> On Fri, Aug 15, 2025 at 06:37:44PM +1200, David Rowley wrote:
> > Michael, any thoughts on switching from stderr to elog(DEBUG)?
>
> You mean to do that for both flags, right? That would be OK for me.
Yeah. Just to aid discussion, let's sa
On Fri, Aug 15, 2025 at 06:37:44PM +1200, David Rowley wrote:
> I think the votes to keep it should outweigh the votes to get rid of
> it, as someone voting to keep it probably has some idea that it'll be
> useful to them, and keeping it shouldn't really hinder the people who
> don't want it. I thi
On Fri, 15 Aug 2025 at 17:46, Hayato Kuroda (Fujitsu)
wrote:
> Just in case: actually, even if the HASH_DEBUG part is fixed on PG17, it could
> not pass some tests. One example is initdb test [1].
> ISTM, command_like() assumed that there are no outputs in stderr but this
> option
> does. This me
On Fri, 15 Aug 2025 at 17:25, Michael Paquier wrote:
>
> On Fri, Aug 15, 2025 at 04:48:10PM +1200, David Rowley wrote:
> > Yes, I'm about to push the HASH_STATISTICS one and backpatch to v17.
>
> Thanks.
Done
> > I think we should fix and backpatch the HASH_DEBUG one. We can have a
> > separate
Dear David,
> FWIW, I've no personal need to keep HASH_DEBUG, but if someone does,
> then let's keep it. Maybe we can make it use elog(DEBUG) rather
> than fprintf and at least build with it in some BF member so we notice
> sooner if someone breaks it again. (I've not checked if there's a good
> r
On Fri, Aug 15, 2025 at 04:48:10PM +1200, David Rowley wrote:
> Yes, I'm about to push the HASH_STATISTICS one and backpatch to v17.
Thanks.
> I think we should fix and backpatch the HASH_DEBUG one. We can have a
> separate debate on removing it in master only. It's hard to imagine
> anyone objec
On Fri, 15 Aug 2025 at 15:42, Michael Paquier wrote:
> It looks like I'm responsible for breaking one of them, at least,
> sorry for that. As far as I can see, the fixes are not complicated,
> so I'd rather fix and backpatch things rather than removing both as
> both combined can be quite powerfu
"Hayato Kuroda (Fujitsu)" writes:
>> There might be a case for removing HASH_STATISTICS too, but
>> it seems weaker. I do recall having made use of that code
>> once years ago ...
> I have never used both options, but one point is that hash_stats() has been
> exported and extensions have called
On Fri, Aug 15, 2025 at 03:22:18AM +, Hayato Kuroda (Fujitsu) wrote:
>> Based on this, I think we should remove the HASH_DEBUG support.
>> It's been broken for six of the last nine years and only one
>> person ever noticed. Moreover, if you were trying to find a
>> problem in dynahash, you'd p
Dear Tom,
Thanks for the analysis.
> Based on this, I think we should remove the HASH_DEBUG support.
> It's been broken for six of the last nine years and only one
> person ever noticed. Moreover, if you were trying to find a
> problem in dynahash, you'd probably want different debug logging
> t
On Fri, 15 Aug 2025 at 02:04, Tom Lane wrote:
> As I just responded to Hayato-san, I think there's a good case
> for removing the HASH_DEBUG code rather than fixing it (again).
No objections here. It feels like it must not get used very often if
it's taken almost 5 years for someone to notice the
David Rowley writes:
> I'll address these and backpatch once the freeze is lifted from the
> backbranches. That should be quite soon.
The freeze was over when we tagged the releases, a day and a half
ago now.
As I just responded to Hayato-san, I think there's a good case
for removing the HASH_DE
"Hayato Kuroda (Fujitsu)" writes:
> I found that postgres could not be built if a complier option HASH_STATISTICS
> is
> set [1]. Also, I found HASH_DEBUG option caused warnings [2]. Usage of the are
> mentioned at the code comments in dynahash.c.
> I'm not sure whether we would keep supporting
On Thu, 14 Aug 2025 at 23:48, Hayato Kuroda (Fujitsu)
wrote:
> I found that postgres could not be built if a complier option HASH_STATISTICS
> is
> set [1]. Also, I found HASH_DEBUG option caused warnings [2]. Usage of the are
> mentioned at the code comments in dynahash.c.
> [1]:
> ```
> dynaha
29 matches
Mail list logo
