On Thu, 6 Oct 2022 at 13:39, Andres Freund wrote:
> I attached a patch to add -Wshadow=compatible-local to our set of warnings.
Since I just committed the patch to fix the final warnings, I think we
should go ahead and commit the patch you wrote to add
-Wshadow=compatible-local to the standard
On Fri, 7 Oct 2022 at 10:57, Tom Lane wrote:
> I poked at this some more by creating a function that intentionally
> does pfree(malloc(N)) for various values of N.
>
> RHEL8, x86_64: the low-order nibble of the header is consistently 0001.
>
> macOS 12.6, arm64: the low-order nibble is
On Fri, 7 Oct 2022 at 09:05, Tom Lane wrote:
>
> Here's a v2 incorporating discussed changes.
>
> In reordering enum MemoryContextMethodID, I arranged to avoid using
> 000 and 111 as valid IDs, since those bit patterns will appear in
> zeroed and wipe_mem'd memory respectively. Those should
On Thu, 6 Oct 2022 at 20:32, Alvaro Herrera wrote:
>
> On 2022-Oct-06, David Rowley wrote:
> > I didn't want to do it that way because all this code is in a while
> > loop and the outer "now" will be reused after it's set by the code
> > above. It's not rea
On Wed, 5 Oct 2022 at 04:55, Tom Lane wrote:
> After studying the existing usages of MemoryContextContains, I think
> there is a better answer, which is to just nuke them.
I was under the impression you wanted to keep that function around in
cassert builds for some of the guc.c changes you were
On Thu, 6 Oct 2022 at 13:39, Andres Freund wrote:
> I attached a patch to add -Wshadow=compatible-local to our set of warnings.
Thanks for writing that and for looking at the patch.
FWIW, I'm +1 for having this part of our default compilation flags. I
don't want to have to revisit this on a
On Thu, 6 Oct 2022 at 11:50, David Rowley wrote:
>
> On Thu, 6 Oct 2022 at 10:40, Andres Freund wrote:
> > Your commit message said the last shadowed variable. But building with
> > -Wshadow=compatible-local triggers a bunch of warnings for me (see trimmed
> > at
&
On Thu, 6 Oct 2022 at 10:40, Andres Freund wrote:
> Your commit message said the last shadowed variable. But building with
> -Wshadow=compatible-local triggers a bunch of warnings for me (see trimmed at
> the end). Looks like it "only" fixed it for src/, without optional
> dependencies like
On Thu, 6 Oct 2022 at 02:34, Alvaro Herrera wrote:
> A simpler idea might be to just remove the inner declaration, and have
> that block set the outer var. There's no damage, since the block is
> going to end and not access the previous value anymore.
>
> diff --git a/src/bin/pgbench/pgbench.c
On Thu, 6 Oct 2022 at 03:19, Tom Lane wrote:
>
> David Rowley writes:
> > I've attached a draft patch for a method I was considering to fix the
> > warnings we're getting from the nested PG_TRY() statement in
> > utility.c.
>
> +1
Pushed.
> > The o
On Wed, 5 Oct 2022 at 21:05, David Rowley wrote:
> 5 warnings remain. 4 of these are for PG_TRY() and co.
I've attached a draft patch for a method I was considering to fix the
warnings we're getting from the nested PG_TRY() statement in
utility.c.
The C preprocessor does not allow n
On Tue, 4 Oct 2022 at 15:30, Justin Pryzby wrote:
> Here, you forgot to change "val < 0".
Thanks. I made another review pass of each change to ensure I didn't
miss any others. There were no other issues, so I pushed the adjusted
patch.
5 warnings remain. 4 of these are for PG_TRY() and co.
On Thu, 29 Sept 2022 at 04:45, Bruce Momjian wrote:
>
> We have discussed the problems caused by the use of pg_stat_reset() and
> pg_stat_reset_shared(), specifically the removal of information needed
> by autovacuum. I don't see these risks documented anywhere. Should we
> do that? Are there
On Tue, 4 Oct 2022 at 13:35, Ranier Vilela wrote:
> Revisiting my work on reducing memory consumption, I found this patch left
> out.
> I'm not sure I can help.
> But basically I was able to write and read the block size, in the chunk.
> Could it be the case of writing and reading the context
On Tue, 30 Aug 2022 at 17:44, Justin Pryzby wrote:
> Would you check if any of these changes are good enough ?
I looked through v5.txt and modified it so that the fix for the shadow
warnings are more aligned to the spreadsheet I created.
I also fixed some additional warnings which leaves just 5
On Thu, 29 Sept 2022 at 18:30, David Rowley wrote:
> Does anyone have any opinions on this?
I by no means think I've nailed the fix in
other_ideas_to_fix_MemoryContextContains.patch, so it would be good to
see if anyone else has any new ideas on how to solve this issue.
Andres did mention to
On Sun, 2 Oct 2022 at 05:34, Arne Roland wrote:
>
> On Tue, Sep 20, 2022 at 4:53 PM David Rowley wrote:
> > Arne sent me an off-list
> > message to say he's planning on working on the patch that uses the
> > existing field instead of the new one he originally a
On Mon, 3 Oct 2022 at 09:59, Tom Lane wrote:
>
> David Rowley writes:
> > For the master version, I think it's safe just to get rid of
> > PlannerInfo.num_groupby_pathkeys now. I only added that so I could
> > strip off the ORDER BY / DISTINCT aggregate PathKeys from t
On Mon, 3 Oct 2022 at 08:10, Tom Lane wrote:
> As attached.
For the master version, I think it's safe just to get rid of
PlannerInfo.num_groupby_pathkeys now. I only added that so I could
strip off the ORDER BY / DISTINCT aggregate PathKeys from the group by
pathkeys before passing to the
On Fri, 30 Sept 2022 at 13:06, Peter Geoghegan wrote:
> I like the idea of just avoiding unparameterized nested loop joins
> altogether when an "equivalent" hash join plan is available because
> it's akin to an execution-time mitigation, despite the fact that it
> happens during planning. While
On Tue, 27 Sept 2022 at 11:28, David Rowley wrote:
> Maybe we could remove the datumCopy() from eval_windowfunction() and
> also document that a window function when returning a non-byval type,
> must allocate the Datum in either ps_ExprContext's
> ecxt_per_tuple_memory or ecxt_per_
We originally did this in 91e9e89dc, but a memory leak was discovered
as I neglected to pfree the datum which is freshly allocated in
tuplesort_getdatum. Because that was discovered late in the PG15
cycle, we opted to just disable the datum sort optimisation for byref
types in 3a5817695.
As was
On Thu, 29 Sept 2022 at 14:32, Peter Geoghegan wrote:
>
> On Wed, Sep 28, 2022 at 6:13 PM David Rowley wrote:
> > Master:
> > latency average = 313.197 ms
> >
> > Patched:
> > latency average = 304.335 ms
> >
> > So not a very impressive speedup th
On Thu, 29 Sept 2022 at 12:07, Peter Geoghegan wrote:
> Also potentially relevant: the 2017 commit fa117ee4 anticipated adding
> a "copy" argument to tuplesort_getdatum() (the same commit added such
> a "copy" argument to tuplesort_gettupleslot()). I see that that still
> hasn't happened to
On Thu, 29 Sept 2022 at 12:30, Michael Paquier wrote:
>
> On Thu, Sep 29, 2022 at 11:58:17AM +1300, David Rowley wrote:
> > I've just pushed the disable byref Datums patch I posted earlier. I
> > only made a small adjustment to make use of the TupleDescAttr() macro.
&g
On Thu, 29 Sept 2022 at 08:57, Tom Lane wrote:
>
> David Rowley writes:
> > I'm wondering if the best way to fix it if doing it that way would be
> > to invent tuplesort_getdatum_nocopy() which would be the same as
> > tuplesort_getdatum() except it wouldn't do the datum
Thanks for investigating this and finding the guilty commit.
On Thu, 29 Sept 2022 at 07:34, Tom Lane wrote:
> After looking at that for a little while, I wonder if we shouldn't
> fix this by restricting the Datum-sort path to be used only with
> pass-by-value data types. That'd require only a
On Tue, 27 Sept 2022 at 06:08, James Coleman wrote:
>
> On Mon, Sep 26, 2022 at 9:59 AM Wolfgang Walther
> wrote:
> > So no need for me to distract this thread from $subject anymore. I think
> > the idea of allowing to create unique constraints on a superset of the
> > columns of an already
On Tue, 20 Sept 2022 at 13:23, Tom Lane wrote:
>
> David Rowley writes:
> > Aside from that, I don't have any ideas on how to get rid of the
> > possible additional datumCopy() from non-Var arguments to these window
> > functions. Should we just suffer it? It'
On Tue, 27 Sept 2022 at 10:45, Tom Lane wrote:
>
> David Rowley writes:
> > On Tue, 13 Sept 2022 at 09:31, Jonathan S. Katz
> > wrote:
> >> Separately, per[1], including dense_rank() in the list of window
> >> functions with optimizations (dense-rank.d
On Tue, 13 Sept 2022 at 09:31, Jonathan S. Katz wrote:
> Separately, per[1], including dense_rank() in the list of window
> functions with optimizations (dense-rank.diff).
This one might have been forgotten... ? I can push it shortly if nobody objects.
David
> [1]
>
On Tue, 2 Aug 2022 at 03:37, Simon Riggs wrote:
> Using the above test case, I'm getting a further 4-7% improvement on
> already committed code with the attached patch, which follows your
> proposal.
>
> The patch passes info via a state object, useful to avoid API churn in
> later patches.
Hi
Thank you for having a look at the patch.
On Tue, 20 Sept 2022 at 18:41, Amit Langote wrote:
> Agreed, though the patch's changes to tests does not seem to have to
> do with join removal? I don't really understand what the test changes
> are all about. I wonder why the patch doesn't instead
On Tue, 20 Sept 2022 at 17:23, Tom Lane wrote:
> "Broken" is a strong claim. There's reason to think it could fail
> in the back branches, but little evidence that it actually has failed
> in the field.
I've posted some code to the security list that shows that I can get
MemoryContextContains()
On Tue, 20 Sept 2022 at 13:23, Tom Lane wrote:
>
> David Rowley writes:
> > Aside from that, I don't have any ideas on how to get rid of the
> > possible additional datumCopy() from non-Var arguments to these window
> > functions. Should we just suffer it? It'
On Tue, 13 Sept 2022 at 20:27, David Rowley wrote:
> I see that one of the drawbacks from not using MemoryContextContains()
> is that window functions such as lead(), lag(), first_value(),
> last_value() and nth_value() may now do the datumCopy() when it's not
> needed. For example, w
On Wed, 13 Jul 2022 at 15:37, David Rowley wrote:
> I'm just in this general area of the code again today and wondered
> about the header comment for the preprocess_groupclause() function.
>
> It says:
>
> * In principle it might be interesting to consider other orderings
On Mon, 19 Sept 2022 at 23:10, Justin Pryzby wrote:
> Find below some others.
Thanks. Pushed.
David
On Mon, 19 Sept 2022 at 15:04, Peter Geoghegan wrote:
> The general structure of the patchset is now a little more worked out.
> Although it's still not close to being commitable, it should give you
> a better idea of the kind of structure that I'm aiming for. I think
> that this should be broken
On Sun, 18 Sept 2022 at 07:59, Peter Geoghegan wrote:
> Attached revision adds a new, third patch. This fixes all the warnings
> from clang-tidy's "readability-named-parameter" check. The extent of
> the code churn seems acceptable to me.
+1 to the idea of aligning the parameter names between
On Sun, 18 Sept 2022 at 07:00, Tom Lane wrote:
>
> Alvaro Herrera writes:
> > After a bit of trawling through the archives, I found it here:
> > https://www.postgresql.org/message-id/20180124162006.pmapfiznhgngwtjf%40alvherre.pgsql
> > I think there was insufficient discussion and you're
On Wed, 3 Aug 2022 at 11:07, Arne Roland wrote:
> Attached a rebased version of the patch.
Firstly, I agree that we should fix the issue of join removals not
working with partitioned tables.
I had a quick look over this and the first thing that I thought was
the same as what Amit mentioned in:
On Wed, 13 Jul 2022 at 05:31, Tom Lane wrote:
> I tried the attached quick-hack patch that just prevents
> remove_useless_groupby_columns from removing anything that
> appears in ORDER BY. That successfully fixes the complained-of
> case, and it doesn't change any existing regression test
On Thu, 15 Sept 2022 at 04:04, Aleksander Alekseev
wrote:
> 1. Forbid setting toast_tuple_target < TOAST_TUPLE_THRESHOLD
> 2. Consider using something like RelationGetToastTupleTarget(rel,
> TOAST_TUPLE_THRESHOLD) in heapam.c:2250, heapam.c:3625 and
> rewriteheap.c:636 and modify the
On Thu, 15 Sept 2022 at 04:08, James Coleman wrote:
> In 29f45e29 we added support for executing NOT IN(values) with a
> hashtable, however this comment still claims that we only do so for
> cases where the ScalarArrayOpExpr's useOr flag is true.
>
> See attached for fix.
Thank you. Pushed.
On Fri, 9 Sept 2022 at 11:33, David Rowley wrote:
> I really think the Assert only form of MemoryContextContains() is the
> best move, and if it's doing Assert only, then we can do the
> loop-over-the-blocks idea as you described and I drafted in [1].
>
> If the need comes up tha
On Tue, 6 Sept 2022 at 13:29, David Rowley wrote:
> I'll hold off a few days before pushing the other patch. Tom stamped
> beta4 earlier, so I'll hold off until after the tag.
I've now pushed this.
David
On Tue, 13 Sept 2022 at 08:56, Jonathan S. Katz wrote:
> What do you think of this (copied from the attached file)
>
> On certain operating systems, PostgreSQL 15 adds support to [prefetch
> pages referenced in
>
On Tue, 13 Sept 2022 at 04:53, Jonathan S. Katz wrote:
> Here is a (penultimate?) draft that includes URLs. Please provide any
> additional feedback no later than 2022-09-14 0:00 AoE. After that, we
> will begin the translation process.
Thanks for drafting these up.
I noticed a couple of
On Fri, 9 Sept 2022 at 10:53, Tom Lane wrote:
> I hate to give up MemoryContextContains altogether. The assertions
> that David nuked in b76fb6c2a had some value I think,
Those can be put back if we decide to keep MemoryContextContains.
Those newly added Asserts just temporarily had to go due
On Thu, 8 Sept 2022 at 09:32, David Rowley wrote:
>
> On Thu, 8 Sept 2022 at 03:08, Tom Lane wrote:
> > Step 4 is annoyingly expensive, but perhaps not too awful given
> > the way we step up alloc block sizes. We should make sure that
> > any context we want to u
On Thu, 8 Sept 2022 at 03:08, Tom Lane wrote:
> 4. For aset.c, I'd be inclined to have it compute the AllocBlock
> address implied by the putative chunk header, and then run through
> the context's alloc blocks and see if any of them match. If we
> do find a match, and the chunk address is
On Thu, 8 Sept 2022 at 01:22, David Rowley wrote:
>
> On Thu, 8 Sept 2022 at 01:05, Julien Rouhaud wrote:
> > FYI lapwing isn't happy with this patch:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2022-09-07%2012%3A40%3A16.
>
> I'll look into it
On Thu, 8 Sept 2022 at 01:05, Julien Rouhaud wrote:
> FYI lapwing isn't happy with this patch:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing=2022-09-07%2012%3A40%3A16.
Thanks. It does seem to be because of the nodeWindowAgg.c usage of
MemoryContextContains.
I'll look into it
On Tue, 6 Sept 2022 at 15:17, David Rowley wrote:
>
> On Tue, 6 Sept 2022 at 14:43, Tom Lane wrote:
> > I think MemoryContextContains' charter is to return
> >
> > GetMemoryChunkContext(pointer) == context
> >
> > *except* that instead of assertin
On Tue, 6 Sept 2022 at 01:41, David Rowley wrote:
>
> On Fri, 2 Sept 2022 at 20:11, David Rowley wrote:
> >
> > On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
> > >
> > > David Rowley writes:
> > > > Maybe we should just consider alwa
On Wed, 7 Sept 2022 at 13:40, Jonathan S. Katz wrote:
> Please provide feedback on the draft no later than Sep 8, 2022 0:00 AoE.
"* Adjust costing to force parallelism with partition aggregates."
If that's about 01474f5698, then it does not need to be mentioned.
All that commit does is update
On Tue, 6 Sept 2022 at 23:25, Ranier Vilela wrote:
> But first, I would like to continue with this correction of using strings.
> In the following cases:
> fprintf -> fputs -> fputc
> printf -> puts -> putchar
>
> There are many occurrences, do you think it would be worth the effort?
I'm pretty
On Thu, 1 Sept 2022 at 20:57, Alvaro Herrera wrote:
> So apparently the way to get a trigger associated with a relation
> (tgconstrrelid) is via CREATE CONSTRAINT TRIGGER, but there doesn't
> appear to be a way to have it associated with a specific *index* on that
> relation (tgconstrindid). So
On Tue, 6 Sept 2022 at 14:43, Tom Lane wrote:
> I think MemoryContextContains' charter is to return
>
> GetMemoryChunkContext(pointer) == context
>
> *except* that instead of asserting what GetMemoryChunkContext asserts,
> it should treat those cases as reasons to return false. So if you
On Tue, 6 Sept 2022 at 14:32, David Rowley wrote:
> I wonder if there are many usages of MemoryContextContains in
> extensions. If there's not, I'd be much happier if we got rid of this
> function and used GetMemoryChunkContext() in nodeAgg.c and
> nodeWindowAgg.c.
I see postgis
On Tue, 6 Sept 2022 at 12:27, Tom Lane wrote:
>
> David Rowley writes:
> > On Tue, 6 Sept 2022 at 11:09, Andres Freund wrote:
> >> I was looking at
> >> MemoryContextContains(). Unless I am missing something, the patch omitted
> >> adjusting that? We'll
On Tue, 6 Sept 2022 at 13:52, Ranier Vilela wrote:
>
> Em seg., 5 de set. de 2022 às 22:29, David Rowley
> escreveu:
>> It feels like it would be good if we had a way to detect a few of
>> these issues much earlier than we are currently. There's been a long
>>
On Tue, 6 Sept 2022 at 06:07, Ranier Vilela wrote:
> I did a search and found a few more places.
> v1 attached.
Thanks. I've done a bit more looking and found a few more places that
we can improve and I've pushed the result.
It feels like it would be good if we had a way to detect a few of
On Tue, 6 Sept 2022 at 11:09, Andres Freund wrote:
> I was looking at
> MemoryContextContains(). Unless I am missing something, the patch omitted
> adjusting that? We'll probably always return false right now.
Oops. Yes. I'll push a fix a bit later.
David
On Fri, 2 Sept 2022 at 20:11, David Rowley wrote:
>
> On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
> >
> > David Rowley writes:
> > > Maybe we should just consider always making room for a sentinel for
> > > chunks that are on dedicated blocks. At most t
On Mon, 5 Sept 2022 at 22:15, David Rowley wrote:
> On Sat, 3 Sept 2022 at 00:37, Ranier Vilela wrote:
> > 6. Avoid overhead when using unnecessary StringInfoData to convert Datum a
> > to Text b.
>
> I've ripped out #4 and #6 for now. I think we should do #6 in master
>
On Sat, 3 Sept 2022 at 00:37, Ranier Vilela wrote:
>> But +1 to fix this and other issues even if they would never crash.
Yeah, I don't think any of this coding would lead to a crash, but it's
pretty weird coding and we should fix it.
> 1. Once that ranges->nranges is invariant, avoid the loop
On Fri, 2 Sept 2022 at 22:06, David Rowley wrote:
> Thanks. I ended up adjusting it to:
>
> "To create a unique or primary key constraint on a partitioned table,"
and pushed.
Thanks for having a look at this Erik.
David
On Fri, 2 Sept 2022 at 22:01, Erik Rijkers wrote:
> Minimal changes:
>
> 'To create a unique or primary key constraints on partitioned table'
>
> should be
>
> 'To create unique or primary key constraints on partitioned tables'
Thanks. I ended up adjusting it to:
"To create a unique or primary
Over on [1], there was a question about why it wasn't possible to
create the following table:
CREATE TABLE foobar(
id BIGINT NOT NULL PRIMARY KEY,
baz VARCHAR NULL DEFAULT NULL
) PARTITION BY HASH(my_func(id));
The above is disallowed by 2 checks in DefineIndex().
1. If the partitioned
On Thu, 1 Sept 2022 at 04:52, Reid Thompson
wrote:
> Add the ability to limit the amount of memory that can be allocated to
> backends.
Are you aware that relcache entries are stored in backend local memory
and that once we've added a relcache entry for a relation that we have
no current code
On Thu, 1 Sept 2022 at 12:46, Tom Lane wrote:
>
> David Rowley writes:
> > Maybe we should just consider always making room for a sentinel for
> > chunks that are on dedicated blocks. At most that's an extra 8 bytes
> > in some allocation that's either over
On Fri, 2 Sept 2022 at 12:55, Ranier Vilela wrote:
> Why not?
> astate_values = accumArrayResult(astate_values,
> PointerGetDatum(a),
> false,
> TEXTOID,
> CurrentMemoryContext);
>
> Is it
On Sat, 27 Aug 2022 at 01:29, Ranier Vilela wrote:
> At function has_matching_range, if variable ranges->nranges == 0,
> we exit quickly with a result equal to false.
>
> This means that nranges can be zero.
> It occurs then that it is possible then to occur an array out of bonds, in
> the
(I think this is a better question for the general mailing list)
On Thu, 1 Sept 2022 at 16:28, Dmitry Markman wrote:
>
> Hi, when I’m trying to access values of my custom enum type I created with
>
> create type colors as enum ('red', 'green', 'brown', 'yellow', 'blue');
>
> I’m getting oid as
On Sat, 20 Aug 2022 at 09:18, Justin Pryzby wrote:
> Is it somwhow possible to call CreateTrigger() to create a FOR EACH ROW
> trigger, with an index, and not internally ?
I've been looking over this and I very much agree that the code looks
very broken. As for whether this is dead code or not,
On Thu, 1 Sept 2022 at 16:06, Tom Lane wrote:
>
> David Rowley writes:
> > Does anyone have any objections to d5ee4db0e in its entirety being
> > backpatched?
>
> It doesn't seem to be fixing any live bug in the back branches,
> but by the same token i
On Tue, 30 Aug 2022 at 13:16, David Rowley wrote:
> I'm also wondering if this should also be backpatched back to v10,
> providing the build farm likes it well enough on master.
Does anyone have any objections to d5ee4db0e in its entirety being backpatched?
David
On Thu, 1 Sept 2022 at 12:23, Tom Lane wrote:
> Is there reason to think we can't validate headers enough to catch
> clobbers?
For non-sentinel chunks, the next byte after the end of the chunk will
be storing the block offset for the following chunk. I think:
if (block !=
On Wed, 31 Aug 2022 at 22:39, David Rowley wrote:
> My current thoughts are that this is a very trivial patch and unless
> there's any objections I plan to push it soon.
Pushed.
David
On Thu, 1 Sept 2022 at 08:53, Tomas Vondra
wrote:
> So the raw size (what we asked for) is ~23.5GB, but in practice we
> allocate ~28.8GB because of the pow-of-2 logic. And by adding the extra
> 1B we end up allocating 31.5GB. That doesn't seem like a huge increase,
> and it's far from the +60%
On Fri, 26 Aug 2022 at 16:48, David Rowley wrote:
> 0003: Changes writetuple to tell it what it should do in regards to
> freeing and adjusting the memory accounting.
>
> Probably 0003 could be done differently. I'm certainly not set on the
> bool args. I understand that I'
On Wed, 31 Aug 2022 at 02:17, Tom Lane wrote:
>
> I wrote:
> > So maybe we should revisit the question. It'd be worth collecting some
> > stats about how much extra space would be needed if we force there
> > to be room for a sentinel.
>
> Actually, after ingesting more caffeine, the problem
On Wed, 31 Aug 2022 at 03:31, David Rowley wrote:
> I've no objections to adding a comment to the enum to
> explain to future devs. My vote would be for that and add the (int)
> cast as proposed by Robert.
Here's a patch which adds a comment to MemoryContextMethodID to Robert's patc
On Wed, 31 Aug 2022 at 01:36, Tom Lane wrote:
>
> David Rowley writes:
> > I think the Assert is useful as if we were ever to add an enum member
> > with the value of 8 and forgot to adjust MEMORY_CONTEXT_METHODID_BITS
> > then bad things would happen insi
On Wed, 31 Aug 2022 at 03:00, Robert Haas wrote:
>
> On Tue, Aug 30, 2022 at 3:14 AM David Rowley wrote:
> > I think the Assert is useful as if we were ever to add an enum member
> > with the value of 8 and forgot to adjust MEMORY_CONTEXT_METHODID_BITS
> > then bad th
On Tue, 30 Aug 2022 at 03:16, Robert Haas wrote:
> ../../../../src/include/utils/memutils_memorychunk.h:186:18: error:
> comparison of constant 7 with expression of type
> 'MemoryContextMethodID' (aka 'enum MemoryContextMethodID') is always
> true
On Tue, 30 Aug 2022 at 15:15, Tom Lane wrote:
> AFAICS that could only happen if "double" has 8-byte alignment
> requirement but int64 does not. I recall some discussion about
> that possibility a month or two back, but I think we concluded
> that we weren't going to support it.
ok
> I guess
On Tue, 30 Aug 2022 at 03:39, Tom Lane wrote:
> I'd suggest reverting df0f4feef as it seems to be
> a red herring.
I think it's useless providing that a 64-bit variable will always be
aligned to 8 bytes on all of our supported 32-bit platforms as,
without the padding, the uint64 hdrmask in
On Tue, 30 Aug 2022 at 13:58, Tomas Vondra
wrote:
>
> On 8/30/22 03:16, David Rowley wrote:
> > Any chance you could run make check-world on your 32-bit Raspberry PI?
> >
>
> Will do, but I think the sentinel fix should be
Thank you. I think Tom is also running make
On Tue, 30 Aug 2022 at 13:55, Tom Lane wrote:
> I wonder if slab ought to artificially bump up such requests when
> MEMORY_CONTEXT_CHECKING is enabled, so there's room for a sentinel.
> I think it's okay for aset.c to not do that, because its power-of-2
> behavior means there usually is room for
On Tue, 30 Aug 2022 at 13:58, Tomas Vondra
wrote:
> armv7l (32-bit rpi4)
>
> +WARNING: chunkSize 216 fullChunkSize 232 header 16
> +WARNING: chunkSize 64 fullChunkSize 80 header 16
>
> aarch64 (64-bit rpi4)
>
> +WARNING: chunkSize 304 fullChunkSize 320 header 16
> +WARNING: chunkSize 80
On Tue, 30 Aug 2022 at 12:45, David Rowley wrote:
> I think the existing sentinel check looks wrong:
>
> if (!sentinel_ok(chunk, slab->chunkSize))
>
> shouldn't that be passing the pointer rather than the chunk?
Here's v2 of the slab-fix patch.
I've included the sentinel check
On Tue, 30 Aug 2022 at 12:22, Tomas Vondra
wrote:
> I also suggested doing a similar check in MemoryChunkGetPointer, so that
> we catch the issue earlier - right after we allocate the chunk. Any
> opinion on that?
I think it's probably a good idea. However, I'm not yet sure if we can
keep it as
On Tue, 30 Aug 2022 at 03:39, Tomas Vondra
wrote:
> The attached patch seems to fix the issue for me - at least it seems
> like that. This probably will need to get backpatched, I guess. Maybe we
> should add an assert to MemoryChunkGetPointer to check alignment?
Hi Tomas,
I just wanted to
On Tue, 30 Aug 2022 at 03:43, Tom Lane wrote:
> I think adding a padding field to SlabBlock would be a less messy
> solution than your patch.
Thank you both of you for looking at this while I was sleeping.
I've read over the emails and glanced at Tomas' patch. I think that
seems good. I think
On Mon, 29 Aug 2022 at 21:37, Amit Kapila wrote:
> 2022-08-29 03:29:56.911 EDT [1056:67] pg_regress/ddl STATEMENT:
> SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
> NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
> TRAP: FailedAssertion("pointer == (void *)
On Mon, 29 Aug 2022 at 21:37, Amit Kapila wrote:
> I am not completely sure if the failure is due to your commit but it
> was showing the line added by this commit. Note that I had also
> committed (commit id: d2169c9985) one patch today but couldn't
> correlate the failure with that so thought
On Mon, 29 Aug 2022 at 10:39, David Rowley wrote:
> One more try to make CFbot happy.
After a bit more revision, mostly updating outdated comments and
naming adjustments, I've pushed this.
Per the benchmark results I showed in [1], due to the performance of
having the AllocSet free l
801 - 900 of 2934 matches
Mail list logo
