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
Dear hackers,
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 them because no one may not hav
30 matches
Mail list logo
