On Tue, Mar 2, 2021 at 1:24 PM Robert Haas wrote:
> Other than that 0001 looks to me to be in pretty good shape now.
Incidentally, we might want to move this to a new thread with a better
subject line, since the current subject line really doesn't describe
the uncommitted portion of the work.
On Tue, Mar 2, 2021 at 12:10 PM Mark Dilger
wrote:
> On further reflection, I decided to implement these changes and not worry
> about the behavioral change.
Thanks.
> I skipped this part. The initcmd argument is only handed to
> ParallelSlotsGetIdle(). Doing as you suggest would not really
> On Mar 1, 2021, at 1:14 PM, Robert Haas wrote:
>
> On Wed, Feb 24, 2021 at 1:55 PM Mark Dilger
> wrote:
>> [ new patches ]
>
> Regarding 0001:
>
> There seem to be whitespace-only changes to the comment for select_loop().
>
> I wonder if the
On Wed, Feb 24, 2021 at 1:55 PM Mark Dilger
wrote:
> [ new patches ]
Regarding 0001:
There seem to be whitespace-only changes to the comment for select_loop().
I wonder if the ParallelSlotsSetupOneDB()/ParallelSlotsSetupMinimal()
changes could be simpler. First idea: Suppose you had
On Tue, Feb 23, 2021 at 12:38 PM Mark Dilger
wrote:
> This is changed in v40 as you propose to exit on FATAL and PANIC level errors
> and on error to send a query. On lesser errors (which includes all
> corruption reports about btrees and some heap corruption related errors), the
> slot's
> On Feb 17, 2021, at 12:56 PM, Robert Haas wrote:
>
> On Wed, Feb 17, 2021 at 1:46 PM Mark Dilger
> wrote:
>> It will reconnect and retry a command one time on error. That should cover
>> the case that the connection to the database was merely lost. If the second
>> attempt also fails,
On Wed, Feb 17, 2021 at 1:46 PM Mark Dilger
wrote:
> Reworking the code took a while. Version 39 patches attached.
Regarding the documentation, I think the Usage section at the top is
far too extensive and duplicates the option description section to far
too great an extent. You have 21 usage
On Wed, Feb 17, 2021 at 1:46 PM Mark Dilger
wrote:
> It will reconnect and retry a command one time on error. That should cover
> the case that the connection to the database was merely lost. If the second
> attempt also fails, no further retry of the same command is attempted, though
>
On Thu, Feb 4, 2021 at 11:10 AM Mark Dilger
wrote:
> Numbered 0001 in this next patch set.
Hi,
I committed 0001 as you had it and 0002 with some more cleanups. Things I did:
- Adjusted some comments.
- Changed processQueryResult so that it didn't do foo(bar) with foo
being a pointer. Generally
On Thu, Feb 4, 2021 at 11:10 AM Mark Dilger
wrote:
> I also made changes to clean up 0003 (formerly numbered 0004)
"deduplice" is a typo.
I'm not sure that I agree with check_each_database()'s commentary
about why it doesn't make sense to optimize the resolve-the-databases
step. Like, suppose I
On Tue, Feb 2, 2021 at 6:10 PM Mark Dilger wrote:
> 0001 -- no changes
Committed.
> 0002 -- fixing omissions in @pgfeutilsfiles in file
> src/tools/msvc/Mkvcbuild.pm
Here are a few minor cosmetic issues with this patch:
- connect_utils.c lacks a file header comment.
- Some or perhaps all of
> On Jan 28, 2021, at 9:41 AM, Mark Dilger wrote:
>
>
>
>> On Jan 28, 2021, at 9:13 AM, Robert Haas wrote:
>>
>> I like 0007 quite a bit and am inclined to commit it soon, as it
>> doesn't depend on the earlier patches. But:
>>
>> - I think the residual comment in processSQLNamePattern
> On Jan 28, 2021, at 9:49 AM, Robert Haas wrote:
>
> On Thu, Jan 28, 2021 at 12:40 PM Mark Dilger
> wrote:
>>> On Jan 28, 2021, at 9:13 AM, Robert Haas wrote:
>>> If I run pg_amcheck --all -j4 do I get a serialization boundary across
>>> databases? Like, I have to completely finish db1
On Thu, Jan 28, 2021 at 12:40 PM Mark Dilger
wrote:
> > On Jan 28, 2021, at 9:13 AM, Robert Haas wrote:
> > If I run pg_amcheck --all -j4 do I get a serialization boundary across
> > databases? Like, I have to completely finish db1 before I can go onto
> > db2, even though maybe only one worker
> On Jan 28, 2021, at 9:13 AM, Robert Haas wrote:
>
> I like 0007 quite a bit and am inclined to commit it soon, as it
> doesn't depend on the earlier patches. But:
>
> - I think the residual comment in processSQLNamePattern beginning with
> "Note:" could use some wordsmithing to account for
> On Jan 28, 2021, at 9:13 AM, Robert Haas wrote:
>
> If I run pg_amcheck --all -j4 do I get a serialization boundary across
> databases? Like, I have to completely finish db1 before I can go onto
> db2, even though maybe only one worker is still busy with it?
Yes, you do. That's patterned
I like 0007 quite a bit and am inclined to commit it soon, as it
doesn't depend on the earlier patches. But:
- I think the residual comment in processSQLNamePattern beginning with
"Note:" could use some wordsmithing to account for the new structure
of things -- maybe just "this pass" -> "this
On Mon, Jan 11, 2021 at 1:16 PM Mark Dilger
wrote:
> Added in v32, along with adding pg_amcheck to @contrib_uselibpq,
> @contrib_uselibpgport, and @contrib_uselibpgcommon
exit_utils.c fails to achieve the goal of making this code independent
of pg_dump, because of:
#ifdef WIN32
if
On Fri, Jan 8, 2021 at 6:33 AM Mark Dilger wrote:
> The attached patches, v31, are mostly the same, but with "getopt_long.h"
> included from pg_amcheck.c per Thomas's review, and a .gitignore file added
> in contrib/pg_amcheck/
I couple more little things from Windows CI:
> On Nov 19, 2020, at 11:47 AM, Peter Geoghegan wrote:
>
> On Thu, Nov 19, 2020 at 9:06 AM Robert Haas wrote:
>> I'm also not sure if these descriptions are clear enough, but it may
>> also be hard to do a good job in a brief space. Still, comparing this
>> to the documentation of
On Tue, Oct 27, 2020 at 5:12 AM Mark Dilger
wrote:
> The v20 patches 0002, 0003, and 0005 still apply cleanly, but 0004 required a
> rebase. (0001 was already committed last week.)
>
> Here is a rebased set of 4 patches, numbered 0002..0005 to be consistent with
> the previous naming. There
On Thu, Nov 19, 2020 at 1:50 PM Mark Dilger
wrote:
> It makes sense to me to have a "don't run through minefields" option, and a
> "go ahead, run through minefields" option for pg_amcheck, given that users in
> differing situations will have differing business consequences to bringing
> down
> On Nov 19, 2020, at 11:47 AM, Peter Geoghegan wrote:
>
>> I think in general you're worrying too much about the possibility of
>> this tool causing backend crashes. I think it's good that you wrote
>> the heapcheck code in a way that's hardened against that, and I think
>> we should try to
On Thu, Nov 19, 2020 at 12:06 PM Robert Haas wrote:
> I originally intended to review the docs and regression tests in the
> same email as the patch itself, but this email has gotten rather long
> and taken rather longer to get together than I had hoped, so I'm going
> to stop here for now and
On Thu, Nov 19, 2020 at 2:48 PM Peter Geoghegan wrote:
> Ideally heapallindex verification would verify 1:1 correspondence. It
> doesn't do that right now, but it could.
Well, that might be a cool new mode, but it doesn't necessarily have
to supplant the thing we have now. The problem
On Thu, Nov 19, 2020 at 9:06 AM Robert Haas wrote:
> I'm also not sure if these descriptions are clear enough, but it may
> also be hard to do a good job in a brief space. Still, comparing this
> to the documentation of heapallindexed makes me rather nervous. This
> is only trying to verify that
On Mon, Oct 26, 2020 at 12:12 PM Mark Dilger
wrote:
> The v20 patches 0002, 0003, and 0005 still apply cleanly, but 0004 required a
> rebase. (0001 was already committed last week.)
>
> Here is a rebased set of 4 patches, numbered 0002..0005 to be consistent with
> the previous naming. There
> On Oct 26, 2020, at 9:12 AM, Tom Lane wrote:
>
> Robert Haas writes:
>> On Wed, Oct 21, 2020 at 11:45 PM Mark Dilger
>> wrote:
>>> Done that way in the attached, which also include Robert's changes from v19
>>> he posted earlier today.
>
>> Committed. Let's see what the buildfarm
Robert Haas writes:
> On Wed, Oct 21, 2020 at 11:45 PM Mark Dilger
> wrote:
>> Done that way in the attached, which also include Robert's changes from v19
>> he posted earlier today.
> Committed. Let's see what the buildfarm thinks.
Another thing that the buildfarm is pointing out is
[WARN]
Hi,
On October 26, 2020 7:13:15 AM PDT, Tom Lane wrote:
>Robert Haas writes:
>> On Mon, Oct 26, 2020 at 9:56 AM Mark Dilger
>> wrote:
hornet is still unhappy even after
321633e17b07968e68ca5341429e2c8bbf15c331?
>
>>> That appears to be a failed test for pg_surgery rather than for
Robert Haas writes:
> On Mon, Oct 26, 2020 at 9:56 AM Mark Dilger
> wrote:
>>> hornet is still unhappy even after
>>> 321633e17b07968e68ca5341429e2c8bbf15c331?
>> That appears to be a failed test for pg_surgery rather than for amcheck. Or
>> am I reading the log wrong?
> Oh, yeah, you're
> On Oct 26, 2020, at 7:00 AM, Robert Haas wrote:
>
> On Mon, Oct 26, 2020 at 9:56 AM Mark Dilger
> wrote:
>> Much of the test in 0002 could be ported to work without committing the rest
>> of 0002, if the pg_amcheck command line utiilty is not wanted.
>
> How much consensus do we think we
On Mon, Oct 26, 2020 at 9:56 AM Mark Dilger
wrote:
> Much of the test in 0002 could be ported to work without committing the rest
> of 0002, if the pg_amcheck command line utiilty is not wanted.
How much consensus do we think we have around 0002 at this point? I
think I remember a vote in favor
> On Oct 26, 2020, at 6:37 AM, Robert Haas wrote:
>
> On Fri, Oct 23, 2020 at 2:04 PM Tom Lane wrote:
>> Seems to work, so I pushed it (after some compulsive fooling
>> about with whitespace and perltidy-ing). It appears to me that
>> the code coverage for verify_heapam.c is not very good
On Fri, Oct 23, 2020 at 2:04 PM Tom Lane wrote:
> Seems to work, so I pushed it (after some compulsive fooling
> about with whitespace and perltidy-ing). It appears to me that
> the code coverage for verify_heapam.c is not very good though,
> only circa 50%. Do we care to expend more effort on
> On Oct 23, 2020, at 4:12 PM, Tom Lane wrote:
>
> Mark Dilger writes:
>> You certainly appear to be right about that. I've added the extra checks,
>> and extended the regression test to include them. Patch attached.
>
> Pushed with some more fooling about. The "bit reversal" idea is
Mark Dilger writes:
> You certainly appear to be right about that. I've added the extra checks,
> and extended the regression test to include them. Patch attached.
Pushed with some more fooling about. The "bit reversal" idea is not
a sufficient guide to picking values that will hit all the
Mark Dilger writes:
>> On Oct 23, 2020, at 11:51 AM, Tom Lane wrote:
>> I do not have 64-bit big-endian hardware to play with unfortunately.
>> But what I suspect is happening here is less about endianness and
>> more about alignment pickiness; or maybe we were unlucky enough to
>> index off the
> On Oct 23, 2020, at 11:51 AM, Tom Lane wrote:
>
> Hmm, we're not out of the woods yet: thorntail is even less happy
> than before.
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thorntail=2020-10-23%2018%3A08%3A11
>
> I do not have 64-bit big-endian hardware to play with
On Fri, Oct 23, 2020 at 11:51 AM Tom Lane wrote:
> /* Set up context information about this next tuple */
> ctx.lp_len = ItemIdGetLength(ctx.itemid);
> ctx.tuphdr = (HeapTupleHeader) PageGetItem(ctx.page, ctx.itemid);
> ctx.natts =
Hmm, we're not out of the woods yet: thorntail is even less happy
than before.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thorntail=2020-10-23%2018%3A08%3A11
I do not have 64-bit big-endian hardware to play with unfortunately.
But what I suspect is happening here is less about
> On Oct 23, 2020, at 11:04 AM, Tom Lane wrote:
>
> I wrote:
>> Mark Dilger writes:
>>> The patch I *should* have attached last night this time:
>
>> Thanks, I'll do some big-endian testing with this.
>
> Seems to work, so I pushed it (after some compulsive fooling
> about with whitespace
I wrote:
> Mark Dilger writes:
>> The patch I *should* have attached last night this time:
> Thanks, I'll do some big-endian testing with this.
Seems to work, so I pushed it (after some compulsive fooling
about with whitespace and perltidy-ing). It appears to me that
the code coverage for
Mark Dilger writes:
> The patch I *should* have attached last night this time:
Thanks, I'll do some big-endian testing with this.
regards, tom lane
> On Oct 22, 2020, at 9:21 PM, Mark Dilger wrote:
>
>
>
>> On Oct 22, 2020, at 7:01 PM, Tom Lane wrote:
>>
>> Mark Dilger writes:
>>> Ahh, crud. It's because
>>> syswrite($fh, '\x77\x77\x77\x77', 500)
>>> is wrong twice. The 500 was wrong, but the string there isn't the bit
>>>
> On Oct 22, 2020, at 7:01 PM, Tom Lane wrote:
>
> Mark Dilger writes:
>> Ahh, crud. It's because
>> syswrite($fh, '\x77\x77\x77\x77', 500)
>> is wrong twice. The 500 was wrong, but the string there isn't the bit
>> pattern we want -- it's just a string literal with backslashes and
Mark Dilger writes:
> Ahh, crud. It's because
> syswrite($fh, '\x77\x77\x77\x77', 500)
> is wrong twice. The 500 was wrong, but the string there isn't the bit
> pattern we want -- it's just a string literal with backslashes and such. It
> should have been double-quoted.
Argh. So we
> On Oct 22, 2020, at 6:50 PM, Mark Dilger wrote:
>
>
>
>> On Oct 22, 2020, at 6:46 PM, Tom Lane wrote:
>>
>> I wrote:
>>> I get
>>> off = , flags = 2, len = 3bbb
>>> on a little-endian machine, and
>>> off = 3bbb, flags = 2, len =
>>> on big-endian. It'd be less symmetric if
> On Oct 22, 2020, at 6:46 PM, Tom Lane wrote:
>
> I wrote:
>> I get
>> off = , flags = 2, len = 3bbb
>> on a little-endian machine, and
>> off = 3bbb, flags = 2, len =
>> on big-endian. It'd be less symmetric if the bytes weren't
>> all the same ...
>
> ... but given that this is
> On Oct 22, 2020, at 6:41 PM, Tom Lane wrote:
>
> I wrote:
>> So now I think this is a REDIRECT on either architecture, but the
>> offset and length fields have different values, causing the redirect
>> pointer to point to different places. Maybe it happens to point
>> at a DEAD tuple in
I wrote:
> I get
> off = , flags = 2, len = 3bbb
> on a little-endian machine, and
> off = 3bbb, flags = 2, len =
> on big-endian. It'd be less symmetric if the bytes weren't
> all the same ...
... but given that this is the test value we are using, why
don't both endiannesses whine
I wrote:
> So now I think this is a REDIRECT on either architecture, but the
> offset and length fields have different values, causing the redirect
> pointer to point to different places. Maybe it happens to point
> at a DEAD tuple in the big-endian case.
Just to make sure, I tried this test
Mark Dilger writes:
>> On Oct 22, 2020, at 2:06 PM, Tom Lane wrote:
>> Oh, wait a second. ItemIdData has the flag bits in the middle:
>> meaning that for that particular bit pattern, one endianness
>> is going to see the flags as 01 (LP_NORMAL) and the other as 10
>> (LP_REDIRECT).
> Well, the
On Thu, Oct 22, 2020 at 4:04 PM Mark Dilger
wrote:
> I think the compiler warning was about fxid not being set. The callers pass
> NULL for status if they don't want status checked, so writing *status
> unconditionally would be an error. Also, if the xid being checked is out of
> bounds, we
On Thu, Oct 22, 2020 at 2:39 PM Mark Dilger
wrote:
> > This is great work. Thanks Mark and Robert.
>
> That's the first time I've laughed today. Having turned the build-farm red,
> this is quite ironic feedback! Thanks all the same for the sentiment.
Breaking the buildfarm is not a capital
> On Oct 22, 2020, at 2:26 PM, Peter Geoghegan wrote:
>
> On Thu, Oct 22, 2020 at 5:51 AM Robert Haas wrote:
>> Committed. Let's see what the buildfarm thinks.
>
> This is great work. Thanks Mark and Robert.
That's the first time I've laughed today. Having turned the build-farm red,
this
On Thu, Oct 22, 2020 at 5:51 AM Robert Haas wrote:
> Committed. Let's see what the buildfarm thinks.
This is great work. Thanks Mark and Robert.
--
Peter Geoghegan
Mark Dilger writes:
> Yeah, I'm already looking at that. The logic in verify_heapam skips over
> line pointers that are unused or dead, and the test is reporting zero
> corruption (and complaining about that), so it's probably not going to help
> to overwrite all the line pointers with this
> On Oct 22, 2020, at 2:06 PM, Tom Lane wrote:
>
> I wrote:
>> Mark Dilger writes:
>>> It is seeking to position 32 and writing '\x77\x77\x77\x77'. x86_64 is
>>> little-endian, and ppc32 and sparc64 are both big-endian, right?
>
>> They are, but that should not meaningfully affect the
> On Oct 22, 2020, at 2:06 PM, Tom Lane wrote:
>
> I wrote:
>> Mark Dilger writes:
>>> It is seeking to position 32 and writing '\x77\x77\x77\x77'. x86_64 is
>>> little-endian, and ppc32 and sparc64 are both big-endian, right?
>
>> They are, but that should not meaningfully affect the
I wrote:
> Mark Dilger writes:
>> It is seeking to position 32 and writing '\x77\x77\x77\x77'. x86_64 is
>> little-endian, and ppc32 and sparc64 are both big-endian, right?
> They are, but that should not meaningfully affect the results of
> that corruption step. You zapped only one line
Mark Dilger writes:
>> On Oct 22, 2020, at 1:31 PM, Tom Lane wrote:
>> Hm, but why are we seeing the failure only on specific machine
>> architectures? sparc64 and ppc32 is a weird pairing, too.
> It is seeking to position 32 and writing '\x77\x77\x77\x77'. x86_64 is
> little-endian, and
> On Oct 22, 2020, at 1:31 PM, Tom Lane wrote:
>
> Mark Dilger writes:
>>> On Oct 22, 2020, at 1:09 PM, Tom Lane wrote:
>>> ooh, looks like prairiedog sees the problem too. That means I should be
>>> able to reproduce it under a debugger, if you're not certain yet where
>>> the problem
Mark Dilger writes:
>> On Oct 22, 2020, at 1:09 PM, Tom Lane wrote:
>> ooh, looks like prairiedog sees the problem too. That means I should be
>> able to reproduce it under a debugger, if you're not certain yet where
>> the problem lies.
> Thanks, Tom, but I question whether the regression
> On Oct 22, 2020, at 1:23 PM, Tom Lane wrote:
>
> ... btw, having now looked more closely at get_xid_status(), I wonder
> how come there aren't more compilers bitching about it, because it
> is very very obviously broken. In particular, the case of
> requesting status for an xid that is
> On Oct 22, 2020, at 1:09 PM, Tom Lane wrote:
>
> ooh, looks like prairiedog sees the problem too. That means I should be
> able to reproduce it under a debugger, if you're not certain yet where
> the problem lies.
Thanks, Tom, but I question whether the regression test failures are from a
... btw, having now looked more closely at get_xid_status(), I wonder
how come there aren't more compilers bitching about it, because it
is very very obviously broken. In particular, the case of
requesting status for an xid that is BootstrapTransactionId or
FrozenTransactionId *will* fall through
ooh, looks like prairiedog sees the problem too. That means I should be
able to reproduce it under a debugger, if you're not certain yet where
the problem lies.
regards, tom lane
> On Oct 22, 2020, at 1:00 PM, Robert Haas wrote:
>
> On Thu, Oct 22, 2020 at 3:15 PM Mark Dilger
> wrote:
>> The 0001 attached patch addresses the -Werror=maybe-uninitialized problem.
>
> I am skeptical. Why so much code churn to fix a compiler warning? And
> even in the revised code,
On Thu, Oct 22, 2020 at 3:15 PM Mark Dilger
wrote:
> The 0001 attached patch addresses the -Werror=maybe-uninitialized problem.
I am skeptical. Why so much code churn to fix a compiler warning? And
even in the revised code, *status isn't set in all cases, so I don't
see why this would satisfy
> On Oct 22, 2020, at 9:01 AM, Mark Dilger wrote:
>
>
>
>> On Oct 22, 2020, at 7:06 AM, Robert Haas wrote:
>>
>> On Thu, Oct 22, 2020 at 8:51 AM Robert Haas wrote:
>>> Committed. Let's see what the buildfarm thinks.
>>
>> It is mostly happy, but thorntail is not:
>>
>>
> On Oct 22, 2020, at 7:06 AM, Robert Haas wrote:
>
> On Thu, Oct 22, 2020 at 8:51 AM Robert Haas wrote:
>> Committed. Let's see what the buildfarm thinks.
>
> It is mostly happy, but thorntail is not:
>
>
lapwing just spit up a possibly relevant issue:
ccache gcc -std=gnu99 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -g -O2 -Werror -fPIC
On Thu, Oct 22, 2020 at 10:28 AM Tom Lane wrote:
> Considering this is a TAP test, why in the world is it designed to hide
> all details of any unexpected amcheck messages? Surely being able to
> see what amcheck is saying would be helpful here.
>
> IOW, don't have the tests abbreviate the
Robert Haas writes:
> The messages in the log aren't very
> illuminating, unfortunately. :-(
Considering this is a TAP test, why in the world is it designed to hide
all details of any unexpected amcheck messages? Surely being able to
see what amcheck is saying would be helpful here.
IOW, don't
On Thu, Oct 22, 2020 at 8:51 AM Robert Haas wrote:
> Committed. Let's see what the buildfarm thinks.
It is mostly happy, but thorntail is not:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=thorntail=2020-10-22%2012%3A58%3A11
I thought that the problem might be related to the fact
On Wed, Oct 21, 2020 at 11:45 PM Mark Dilger
wrote:
> Done that way in the attached, which also include Robert's changes from v19
> he posted earlier today.
Committed. Let's see what the buildfarm thinks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL
> On Oct 21, 2020, at 1:51 PM, Tom Lane wrote:
>
> Mark Dilger writes:
>> There is still something screwy here, though, as this compiles, links and
>> runs fine for me on mac and linux, but not for Robert.
>
> Are you using --enable-nls at all on your Mac build? Because for sure it
>
Mark Dilger writes:
> There is still something screwy here, though, as this compiles, links and
> runs fine for me on mac and linux, but not for Robert.
Are you using --enable-nls at all on your Mac build? Because for sure it
should not work there, given the failure to include -lintl in
Robert Haas writes:
> I was about to commit 0001, after making some cosmetic changes, when I
> discovered that it won't link for me. I think there must be something
> wrong with the NLS stuff. My version of 0001 is attached. The error I
> got is:
Well, the short answer would be "you need to add
> On Oct 21, 2020, at 1:13 PM, Alvaro Herrera wrote:
>
> On 2020-Oct-21, Robert Haas wrote:
>
>> On Wed, Oct 7, 2020 at 9:01 PM Mark Dilger
>> wrote:
>>> This next version, attached, has the acl checking and associated
>>> documentation changes split out into patch 0005, making it easier
On 2020-Oct-21, Robert Haas wrote:
> On Wed, Oct 7, 2020 at 9:01 PM Mark Dilger
> wrote:
> > This next version, attached, has the acl checking and associated
> > documentation changes split out into patch 0005, making it easier to review
> > in isolation from the rest of the patch series.
> >
On Wed, Oct 7, 2020 at 9:01 PM Mark Dilger wrote:
> This next version, attached, has the acl checking and associated
> documentation changes split out into patch 0005, making it easier to review
> in isolation from the rest of the patch series.
>
> Independently of acl considerations, this
On Mon, Oct 5, 2020 at 5:24 PM Mark Dilger wrote:
> > I don't see how verify_heapam will avoid raising an error during basic
> > validation from PageIsVerified(), which will violate the guarantee
> > about not throwing errors. I don't see that as a problem myself, but
> > presumably you will.
>
>
> On Oct 6, 2020, at 11:27 PM, Andrey Borodin wrote:
>
>
>
>> 7 окт. 2020 г., в 04:20, Mark Dilger
>> написал(а):
>>
>>
>>
>>> On Oct 5, 2020, at 5:24 PM, Mark Dilger
>>> wrote:
>>>
>>> - This version does not change clog handling, which leaves Andrey's concern
>>> unaddressed.
> 7 окт. 2020 г., в 04:20, Mark Dilger
> написал(а):
>
>
>
>> On Oct 5, 2020, at 5:24 PM, Mark Dilger wrote:
>>
>> - This version does not change clog handling, which leaves Andrey's concern
>> unaddressed. Peter also showed some support for (or perhaps just a lack of
>> opposition
On Tue, Aug 25, 2020 at 07:36:53AM -0700, Mark Dilger wrote:
> Removed.
This patch is failing to compile on Windows:
C:\projects\postgresql\src\include\fe_utils/print.h(18): fatal error
C1083: Cannot open include file: 'libpq-fe.h': No such file or
directory
Greetings,
* Peter Geoghegan (p...@bowt.ie) wrote:
> On Tue, Sep 22, 2020 at 12:41 PM Robert Haas wrote:
> > But now I see that there's no secondary permission check in the
> > verify_nbtree.c code. Is that intentional? Peter, what's the
> > justification for that?
>
> As noted by comments in
Peter Geoghegan writes:
> On Mon, Sep 21, 2020 at 2:09 PM Robert Haas wrote:
>> +REVOKE ALL ON FUNCTION
>> +verify_heapam(regclass, boolean, boolean, cstring, bigint, bigint)
>> +FROM PUBLIC;
>>
>> This too.
> Do we really want to use a cstring as an enum-like argument?
Ugh. We should not be
On Sat, Aug 29, 2020 at 10:48 AM Mark Dilger
wrote:
> I had an earlier version of the verify_heapam patch that included a
> non-throwing interface to clog. Ultimately, I ripped that out. My reasoning
> was that a simpler patch submission was more likely to be acceptable to the
> community.
On Mon, Sep 21, 2020 at 2:09 PM Robert Haas wrote:
> +REVOKE ALL ON FUNCTION
> +verify_heapam(regclass, boolean, boolean, cstring, bigint, bigint)
> +FROM PUBLIC;
>
> This too.
Do we really want to use a cstring as an enum-like argument?
I think that I see a bug at this point in check_tuple()
On Tue, Sep 22, 2020 at 12:41 PM Robert Haas wrote:
> But now I see that there's no secondary permission check in the
> verify_nbtree.c code. Is that intentional? Peter, what's the
> justification for that?
As noted by comments in contrib/amcheck/sql/check_btree.sql (the
verify_nbtree.c tests),
On Tue, Sep 22, 2020 at 1:55 PM Mark Dilger
wrote:
> I am inclined to just restrict verify_heapam() to superusers and be done.
> What do you think?
I think that's an old and largely failed approach. If you want to use
pg_class_ownercheck here rather than pg_class_aclcheck or something
like
On Tue, Sep 22, 2020 at 10:55 AM Mark Dilger
wrote:
> I am inclined to just restrict verify_heapam() to superusers and be done.
> What do you think?
The existing amcheck functions were designed to have execute privilege
granted to non-superusers, though we never actually advertised that
fact.
> On Sep 21, 2020, at 2:09 PM, Robert Haas wrote:
>
> I think there should be a call to pg_class_aclcheck() here, just like
> the one in pg_prewarm, so that if the superuser does choose to grant
> access, users given access can check tables they anyway have
> permission to access, but not
On Tue, Aug 25, 2020 at 10:36 AM Mark Dilger
wrote:
> Thanks for the review!
+ msg OUT text
+ )
Looks like atypical formatting.
+REVOKE ALL ON FUNCTION
+verify_heapam(regclass,
> On Aug 29, 2020, at 3:27 AM, Andrey M. Borodin wrote:
>
>
>
>> 29 авг. 2020 г., в 00:56, Robert Haas написал(а):
>>
>> On Fri, Aug 28, 2020 at 2:10 PM Andrey M. Borodin
>> wrote:
>>> I don't think so. ISTM It's the same problem of xmax>> just hidden behind detoasing.
>>> Our regular
> 29 авг. 2020 г., в 00:56, Robert Haas написал(а):
>
> On Fri, Aug 28, 2020 at 2:10 PM Andrey M. Borodin
> wrote:
>> I don't think so. ISTM It's the same problem of xmax> just hidden behind detoasing.
>> Our regular heap_check was checking xmin\xmax invariants for tables, but
>> failed to
On Fri, Aug 28, 2020 at 2:10 PM Andrey M. Borodin wrote:
> I don't think so. ISTM It's the same problem of xmax just hidden behind detoasing.
> Our regular heap_check was checking xmin\xmax invariants for tables, but
> failed to recognise the problem in toast (while toast was accessible until
>
> On Aug 28, 2020, at 11:10 AM, Andrey M. Borodin wrote:
>
>
>
>> 28 авг. 2020 г., в 18:58, Robert Haas написал(а):
>> In the case
>> you mention, I think we should view that as a problem with clog rather
>> than a problem with the table, and thus out of scope.
>
> I don't think so. ISTM
1 - 100 of 194 matches
Mail list logo
