On Sun, Feb 4, 2018 at 9:42 AM, Andres Freund wrote:
> Wheee! Congrats Peter, Rushash, and everyone else involved!
Thanks!
--
Peter Geoghegan
On Sun, Feb 4, 2018 at 10:11 AM, Tom Lane wrote:
> I'll be happier about it when the valgrind buildfarm animals are
> happy.
I don't know if you noticed, but I did post a patch for that on Friday.
--
Peter Geoghegan
he suppression is actually slightly better scoped than
that, since, for example, that won't just affect writes of
uninitialized bytes from the buffer. But I'll do it that way.
--
Peter Geoghegan
test the code. I had to push to get
us to give external sorts test coverage at one point about 18 months
ago, because of concerns about the overhead/duration of external
sorts. Not that I feel strongly about this myself.
--
Peter Geoghegan
On Wed, Feb 21, 2018 at 10:55 AM, Peter Geoghegan wrote:
> Sure, but it looks like it has the exact same underlying cause to the
> LogicalTapeFreeze() issue. It shouldn't be very hard to write an
> equivalent patch for LogicalTapeRewindForRead() -- I pointed out that
> this c
On Thu, Feb 22, 2018 at 6:32 AM, Robert Haas wrote:
>> Attached patch does this. I cannot recreate this issue locally, but
>> this should still fix it on skink.
>
> Committed.
Looks like it worked.
--
Peter Geoghegan
ing
in the first place? We don't bother to check that when we already know
the page is rightmost within _bt_moveright() or _bt_findinsertloc().
I also noticed that favorable/favourable is misspelled "favourble".
Also, "workout" is a noun.
This patch was committed in haste, and it shows.
--
Peter Geoghegan
, since page
deletion cannot delete the right most page among an internal page's
children, but actually omitting the P_IGNORE() doesn't seem like an
improvement. That's probably worth a comment, though.
--
Peter Geoghegan
On Wed, Mar 28, 2018 at 11:56 AM, Peter Geoghegan wrote:
>> Previously, we agreed that P_IGNORE() is required. So I assume no issues
>> there. The other tests seem required too for us to conclusively decide to
>> use the cached block.
>
> Actually, you're right tha
uld need
> to insert into parent.
But, to repeat myself, it actually can do that. Try removing the space
check in the main _bt_doinsert() test for yourself -- the regression
tests continue to pass. I would like there to be an assertion failure
instead.
> I don't see any need to change any of these things. This is a tuning
> patch and none of the above affects correctness of what has been
> committed.
I think that there has been a misunderstanding. I'm only asking for a
new assertion when I talk about the stack. That's all.
--
Peter Geoghegan
seems like a good idea.
--
Peter Geoghegan
ght of that, but didn't.
> But thank you for the leeway.
FWIW, I think that a few hours should be standard. After that, it
should be possible for allowances to be made based on extenuating
circumstances. (I don't think that it's necessary to formalize that,
though.)
--
Peter Geoghegan
't want to do the whole thing in unsigned, because this ties back
fairly directly to an int8 argument from the test_bloomfilter() SQL
function interface.
I proposed the attached, which makes the buffer one byte larger, per
your suggestion.
Thanks
--
Peter Geoghegan
From 4464e134eae415c47c4373
't ideal, and
particularly hinders automated testing.
--
Peter Geoghegan
d so even a RecentGlobalXmin style interlock isn't required.
We cannot fail to detect that our hint was invalidated, because there
can only be one such page in the B-Tree at any time. It's possible
that the page will be deleted and recycled without a backend's cached
page also being detected as invalidated, but only when we happen to
recycle a page that once again becomes the rightmost leaf page.
Once those changes are made, this should be fine to commit.
--
Peter Geoghegan
it.
I agree that this was handled in a way that was just unsatisfactory.
It was also unnecessary, since we still have a few days left until
feature freeze.
--
Peter Geoghegan
ch contains some redundant variabled. See warnings
> produced
> by gcc-7.
I also see an assertion failure within _bt_getrootheight():
TRAP: FailedAssertion("!(metad->btm_version == 3)", File:
"/home/pg/postgresql/root/build/../source/src/backend/access/nbtree/nbtpage.c",
Line: 619)
--
Peter Geoghegan
--- what's the test case?
This was discovered while testing/reviewing the latest version of the
INCLUDE covering indexes patch. It now seems to be unrelated.
Sorry for the noise.
--
Peter Geoghegan
re-existing indexes.
>
> In short, this sounds like a place that did not get the memo about
> how to cope with un-upgraded indexes.
Sounds plausible.
--
Peter Geoghegan
on itself? Looks like I'm too tired to think sensibly.
I know the feeling.
I think you can take that wording almost verbatim. Obviously it should
refer to the optimization by name, and blend into the surrounding text
in the README. I suggest putting a small section before "On-the-Fly
Deletion Of Index Tuples", but after the main discussion of deletion +
recycling. It's essentially an exception to the general rule, so that
placement makes sense to me.
Thanks
--
Peter Geoghegan
rm-culicidae/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/data
-type f ! -perm 640
+ wc -l
+ [ 1778 -ne 0 ]
+ echo files in PGDATA with permission != 640
files in PGDATA with permission != 640
+ exit 1
+ rm -rf /tmp/pg_upgrade_check-h6epmk
make: *** [Makefile:40: check] Error 1
"""
--
Peter Geoghegan
On Thu, Apr 5, 2018 at 10:16 AM, Peter Geoghegan wrote:
> I think you can take that wording almost verbatim. Obviously it should
> refer to the optimization by name, and blend into the surrounding text
> in the README. I suggest putting a small section before "On-the-Fly
>
l, cachedBlock);
I suggest putting this in nbtree.h instead. You can put it just after
BTREE_NONLEAF_FILLFACTOR, with a comment, in a separate block/section.
Other than that, looks good to me.
Thanks
--
Peter Geoghegan
On Tue, Apr 10, 2018 at 12:30 PM, Peter Geoghegan wrote:
>> Apart from that, other changes requested are included in the patch. This
>> also takes care of Heikki's observation regarding UNLOGGED tables on the
>> other thread.
>
> Cool.
BTW, Heikki's commit
mization".
The comments still say "Check if the page...no split is in progress".
Despite the fact that that's just an assertion now.
--
Peter Geoghegan
On Tue, Apr 10, 2018 at 3:37 PM, Andrew Dunstan
wrote:
>> The comments still say "Check if the page...no split is in progress".
>> Despite the fact that that's just an assertion now.
>>
>
> Fixed.
Thanks.
--
Peter Geoghegan
Valgrind to make sure that a patch + tests don't have
a problem like this before pushing. That's not perfect, of course, but
it's an easy way to save yourself some trouble.
--
Peter Geoghegan
8, to be consistent. (It should not be double because
we always avoid using anything other types with explicit typedef'd
widths in WAL records.)
--
Peter Geoghegan
Fix typo in JIT README.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/3747d478-41f9-439f-8074-ac81a5c76...@yesql.se
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/f6b95ff434bff28c0d9b390d5a0ff316847c4fb7
Modified Files
--
src/backend/jit/RE
On Sat, Jun 9, 2018 at 10:55 AM, Andres Freund wrote:
> Congrats!
Thanks Andres!
--
Peter Geoghegan
Remove INCLUDE attributes section from docs.
Discussing covering indexes in a chapter that is mostly about the
behavior of B-Tree operator classes is unnecessary. The CREATE INDEX
documentation's handling of covering indexes seems sufficient.
Discussion:
https://postgr.es/m/CAH2-WzmpU=l_6vjhhoa
Remove obsolete comment block in nbtsort.c.
Building a new nbtree index through incremental insertions would always
be slower than our actual approach of sorting using tuplesort,
assembling leaf pages from tuplesort output, and writing and WAL-logging
whole pages. Remove a comment block from the
Correct a comment on logtape.c's leader tape.
randomAccess parallel tuplesorts are disallowed because the leader would
try to write to its own leader tape, not because the leader would try to
write to a worker tape directly.
Cleanup from commit 9da0cc35284.
Branch
--
master
Details
---
t minute change. The points are organized in a way that makes a run
down quick and easy, even when committing a trivial patch.
--
Peter Geoghegan
editor having a "gitcommit" syntax.
I find it useful to have a routine checklist for things like
committing, because it frees up a little space in my head for other
things. I do the same thing when preparing for a trip.
--
Peter Geoghegan
Correct obsolete unique index insertion comment.
Commit bc292937ae6 failed to update a comment about unique index
checking. _bt_insertonpg() is no longer responsible for finding an
insertion location while preventing conflicting insertions.
Branch
--
master
Details
---
https://git.postg
On Mon, Jul 9, 2018 at 11:24 AM, Alvaro Herrera
wrote:
> Maybe we should add an XML comment
>
> at the end of the table :-)
+1. I made the same mistake at one point.
--
Peter Geoghegan
ee we won't all want the exact same checklist. Still, it wouldn't hurt
> to have a wiki page of checklist entry ideas from which folks cherry-pick the
> entries they like.
You've convinced me that we should definitely have such a list. I've
put it on my TODO list.
--
Peter Geoghegan
On Wed, Jul 11, 2018 at 4:35 PM, Peter Geoghegan wrote:
> You've convinced me that we should definitely have such a list. I've
> put it on my TODO list.
I started this Wiki page:
https://wiki.postgresql.org/wiki/Committing_checklist
I've tried to avoid being too prescripti
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
ommit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers(). (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)
Author: Peter Geoghegan
Reported-By: Luca Fe
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
Doc: Correct description of amcheck example query.
The amcheck documentation incorrectly claimed that its example query
verifies every catalog index in the database. In fact, the query only
verifies the 10 largest indexes (as determined by pg_class.relpages).
Adjust the description accordingly.
per.c
backend state as part of standard parallel initialization (Cf. commit
29d58fd3). On v11, simply disallow builds against mapped catalog
relations by deeming them parallel unsafe.
Author: Peter Geoghegan
Reported-By: "death lock"
Reviewed-By: Tom Lane, Amit Kapila
Bug: #15309
Dis
per.c
backend state as part of standard parallel initialization (Cf. commit
29d58fd3). On v11, simply disallow builds against mapped catalog
relations by deeming them parallel unsafe.
Author: Peter Geoghegan
Reported-By: "death lock"
Reviewed-By: Tom Lane, Amit Kapila
Bug: #15309
Dis
Adjust trace_sort log messages.
The project message style guide dictates: "When citing the name of an
object, state what kind of object it is". The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type. Bring th
Adjust trace_sort log messages.
The project message style guide dictates: "When citing the name of an
object, state what kind of object it is". The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type. Bring th
Have BufFileSize() ereport() on FileSize() failure.
Move the responsibility for checking for and reporting a failure from
the only current BufFileSize() caller, logtape.c, to BufFileSize()
itself. Code within buffile.c is generally responsible for interfacing
with fd.c to report irrecoverable fai
Have BufFileSize() ereport() on FileSize() failure.
Move the responsibility for checking for and reporting a failure from
the only current BufFileSize() caller, logtape.c, to BufFileSize()
itself. Code within buffile.c is generally responsible for interfacing
with fd.c to report irrecoverable fai
Correct obsolete nbtree recovery comments.
Commit 40dae7ec537, which made the handling of interrupted nbtree page
splits more robust, removed an nbtree-specific end-of-recovery cleanup
step. This meant that it was no longer possible to complete an
interrupted page split during recovery. However,
Correct obsolete nbtree recovery comments.
Commit 40dae7ec537, which made the handling of interrupted nbtree page
splits more robust, removed an nbtree-specific end-of-recovery cleanup
step. This meant that it was no longer possible to complete an
interrupted page split during recovery. However,
Remove obsolete nbtree duplicate entries comment.
Remove a comment from the Berkeley days claiming that nbtree must
disambiguate duplicate keys within _bt_moveright(). There is no special
care taken around duplicates within _bt_moveright(), at least since
commit 9e85183bfc3 removed inscrutable _b
Avoid amcheck inline compression false positives.
The previous tacit assumption that index_form_tuple() hides differences
in the TOAST state of its input datums was wrong. Normalize input
varlena datums by decompressing compressed values, and forming a new
index tuple for fingerprinting using unc
Avoid amcheck inline compression false positives.
The previous tacit assumption that index_form_tuple() hides differences
in the TOAST state of its input datums was wrong. Normalize input
varlena datums by decompressing compressed values, and forming a new
index tuple for fingerprinting using unc
diff command, so that we don't see the full absolute
> path in the diff header, which makes the diff unnecessarily verbose
> and harder to read.
This broke some of my tooling for quickly reconciling expected and
actual test outputs from my text editor.
I don't think that this was a great idea.
--
Peter Geoghegan
Correct obsolete nbtree page deletion comment.
Commit efada2b8e92, which made the nbtree page deletion algorithm more
robust, removed _bt_getstackbuf() calls from _bt_pagedel(). It failed
to update a comment that referenced the earlier approach. Update the
comment to explain that the _bt_getstac
Remove unneeded argument from _bt_getstackbuf().
_bt_getstackbuf() is called at exactly two points following commit
efada2b8e92 (one call site is concerned with page splits, while the
other is concerned with page deletion). The parent buffer returned by
_bt_getstackbuf() is write-locked in both c
Correct obsolete nbtree page split WAL comment.
Commit 2c03216d831, which revamped the WAL record format, failed to
update a comment referencing the old API. Update the comment.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/72c7c4e38610297b200721a7d5201f79e7ceef7
Note case where nbtree VACUUM finishes splits.
The nbtree README claims that VACUUM can never finish interrupted page
splits by design. That isn't entirely accurate, though. Note an
exception to the general rule.
Discussion:
https://postgr.es/m/CAH2-Wz=_xvv8byzk_lvy4ci76ogshcqzokf7we8yg9wao7j.
Correct obsolete nbtree page split comment.
Commit 40dae7ec537, which made the nbtree page split algorithm more
robust, made _bt_insert_parent() only unlock the right child of the
parent page before inserting a new downlink into the parent. Update a
comment from the Berkeley days claiming that bo
Tweak nbtsearch.c function prototype order.
nbtsearch.c's static function prototypes were slightly out of order.
Make the order consistent with static function definition order.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/1009920aaa39e19ecb36409447ece2f8102f4225
ID tiebreaker column must be used in comparisons for the
restored binary search (unlike the original _bt_check_unique() binary
search, where scankey's heap TID column must be omitted).
Author: Peter Geoghegan, Heikki Linnakangas
Reviewed-By: Heikki Linnakangas, Andrey Lepikhov
Discussion:
https:
d by an amount equal to
the space required to store an extra MAXALIGN()'d TID in a new high key
during leaf page splits. The user-facing definition of the "1/3 of a
page" restriction is already imprecise, and so does not need to be
revised. However, there should be a compatibility note
E
indexes. Grouping relatively similar tuples together is beneficial in
and of itself, since it reduces the number of leaf pages that must be
accessed by subsequent index scans.
Author: Peter Geoghegan
Reviewed-By: Heikki Linnakangas
Discussion:
https://postgr.es/m/cah2-wzmmolnqoj9mad78iqhfwlj
nough to give wrong
answers to some queries, and yet not corrupt enough to allow the problem
to be detected without verifying agreement between the leaf page and the
root page, skipping at least one internal page level. The existing
bt_index_parent_check() checks never cross more than a single level.
On Wed, Mar 20, 2019 at 10:05 AM Peter Geoghegan wrote:
> Make heap TID a tiebreaker nbtree index column.
I see that this has caused SELinux test failures on rhinoceros (the
ddl test fails). It looks like the output order is affected by the
implementation details of nbtree, likely for s
it would be fine to rely on the new output ordering, even
though it theoretically isn't any more stable.
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 11:09 AM Andres Freund wrote:
> FWIW, I just fix up the tests using the regression output from
> rhinoceros when that happens. Sometimes takes more than a single round,
> but it builds frequently enough...
I'll give that a go, provided Tom is okay with
On Wed, Mar 20, 2019 at 11:30 AM Peter Geoghegan wrote:
> Your work on test stability probably eliminated 98% of the problems.
> It's still early, but the buildfarm is mostly fine.
batfish just had a similar failure, this time in foreign_data -- two
lines of "DETAIL" output
Suppress DETAIL output from a foreign_data test.
Unstable sort order related to changes to nbtree from commit dd299df8
can cause two lines of DETAIL output to be in opposite-of-expected
order. Suppress the output using the same VERBOSITY hack that is used
elsewhere in the foreign_data tests.
Not
witem' was declared here
> nbtxlog.c:271: warning: 'newitemsz' may be used uninitialized in this function
> nbtxlog.c:271: note: 'newitemsz' was declared here
Sure. Will do that one next.
--
Peter Geoghegan
y adjusting the output, rather than waiting for your
patch. Whether or not the verbosity hack can be ripped out can be
considered later, in a separate pass.
--
Peter Geoghegan
Fix spurious compiler warning in nbtxlog.c.
Cleanup from commit dd299df8.
Per complaint from Tom Lane.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/3d0dcc5c7fb9cfc349d1b2d476a1c0c5d64522bd
Modified Files
--
src/backend/access/nbtree/nbtxlog.c | 8 ++
tree code used to provide mostly-reverse-insertion-order
> scan order.
That's good. I'm trying to fix it by hand right now, in the way that
Andres suggested. It is both tedious and error-prone.
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 3:08 PM Tom Lane wrote:
> And done. Should be possible to revert 7d3bf73ac, if you wish.
Will do.
Thanks!
--
Peter Geoghegan
On Wed, Mar 20, 2019 at 3:11 PM Peter Geoghegan wrote:
> On Wed, Mar 20, 2019 at 3:08 PM Tom Lane wrote:
> > And done. Should be possible to revert 7d3bf73ac, if you wish.
>
> Will do.
Actually, I'm not sure why it would be fine to revert 7d3bf73ac now.
Might the problem a
Revert "Suppress DETAIL output from a foreign_data test."
This should be superseded by commit 8aa9dd74.
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/fff518d051285bc47e2694a349d410e01972730b
Modified Files
--
src/test/regress/expected/foreign_data.out
w,
> not only that one. They should not be necessary any more, and
> they might be hiding things we need to know about, now or in
> the future. But I haven't got round to it.
Seems like a useful goal.
--
Peter Geoghegan
ge to DROP OWNED BY. Something seemed amiss to me
for these reasons.
--
Peter Geoghegan
the server
> log could be pretty unfriendly in some contexts, too.
All of these options seem acceptable. However, the problem is unlikely
to get any worse, so going to the trouble of option 1 or 1a might not
be the best use of time.
--
Peter Geoghegan
On Fri, Mar 22, 2019 at 10:38 AM Tom Lane wrote:
> So that means that the de-revert is probably the best option for
> now. Will you do the honors?
I'll take care of that shortly.
Thanks
--
Peter Geoghegan
Go back to suppressing foreign_data DETAIL test output.
This is almost a straight revert of commit fff518d, which itself was a
revert of 7d3bf73ac.
It turns out that commit 8aa9dd74, which sorted dependent objects before
deletion in DROP OWNED BY, was not sufficient to make all remaining
unstable
only used for backwards scans. Backward scans continue to
opportunistically check the final non-pivot tuple, which is actually the
first non-pivot tuple on the page (not the last).
Note that even pg_upgrade'd v3 indexes make use of this optimization.
Author: Peter Geoghegan, Heikki Linnakan
Suppress DETAIL output from an event_trigger test.
Suppress 3 lines of unstable DETAIL output from a DROP ROLE statement in
event_trigger.sql. This is further cleanup for commit dd299df8.
Note that the event_trigger test instability issue is very similar to
the recently suppressed foreign_data t
ch was what almost always happened
before dd299df81. I think that pruning could affect the results, for
example. Previously, it was mostly a matter of index insertion order,
which looked like it made heap TIDs among duplicates be in descending
order, though that certainly didn't happen reliably.
--
Peter Geoghegan
On Sun, Mar 24, 2019 at 9:47 AM Peter Geoghegan wrote:
> On Sun, Mar 24, 2019 at 8:32 AM Tom Lane wrote:
> > So here's another failure of the same ilk:
> >
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=urocryon&dt=2019-03-24%2006%3A12%3A35
>
>
ood bit more memory than an ObjectAddress
> array in any case, so if we're not going to sweat about OOM for the
> message then I'm not sure we need to be paranoid about the sort memory.
I agree that it isn't worth worrying about an OOM for the sort here.
--
Peter Geoghegan
x this evening if, for whatever reason, it seems
necessary.
Thanks
--
Peter Geoghegan
Remove dead code from nbtsplitloc.c.
It doesn't make sense to consider the possibility that there will only
be one candidate split point when choosing among split points to find
the split with the lowest penalty. This is a vestige of an earlier
version of the patch that became commit fab25024.
I
ward.
> If no objections, I'll push shortly.
Sounds good.
--
Peter Geoghegan
that even pg_upgrade'd v3 indexes make use of this optimization.
Author: Peter Geoghegan
Reviewed-By: Heikki Linnakangas
Discussion:
https://postgr.es/m/cah2-wzkpkezjrxvr_p7vsy1b-s85e3ghytbzqzr0bkj5lrw...@mail.gmail.com
Branch
--
master
Details
---
https://git.postgresql.or
reason why you didn't remove the heap_hot_search() wrapper
function entirely?
--
Peter Geoghegan
at there are any external users, though.
--
Peter Geoghegan
the failure on it.
>
> pushed a fix
Buildfarm member snapper is still unhappy about this:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snapper&dt=2019-03-26%2013%3A01%3A31
--
Peter Geoghegan
1 - 100 of 819 matches
Mail list logo
