Re: Extending amcheck to check toast size and compression

2021-11-06 Thread Mark Dilger
> On Nov 5, 2021, at 8:56 PM, Tom Lane wrote: > > Some of the buildfarm is unimpressed with this --- looks like the test > output is less stable than you thought. Yes, it does. I had to play with it a bit to be sure the test itself is faulty, and I believe that it is. — Mark Dilger Enterpr

Re: Extending amcheck to check toast size and compression

2021-11-05 Thread Tom Lane
Robert Haas writes: > OK, I've committed this version. Some of the buildfarm is unimpressed with this --- looks like the test output is less stable than you thought. regards, tom lane

Re: Extending amcheck to check toast size and compression

2021-11-05 Thread Robert Haas
On Thu, Nov 4, 2021 at 6:58 PM Mark Dilger wrote: > It only takes about 20 additional lines in the regression test to check the > code paths for raw sizes which are too large and too small, so I've done that > in this next version. Testing corrupt compressed data in a deterministic, > cross pl

Re: Extending amcheck to check toast size and compression

2021-11-04 Thread Mark Dilger
> On Nov 4, 2021, at 7:53 AM, Robert Haas wrote: > > But, is it plausible to add test coverage for the new checks, or is > that going to be too much of a pain? It only takes about 20 additional lines in the regression test to check the code paths for raw sizes which are too large and too smal

Re: Extending amcheck to check toast size and compression

2021-11-04 Thread Robert Haas
On Wed, Nov 3, 2021 at 6:56 PM Mark Dilger wrote: > Done that way. I agree with what others have said: this looks fine. But, is it plausible to add test coverage for the new checks, or is that going to be too much of a pain? -- Robert Haas EDB: http://www.enterprisedb.com

Re: Extending amcheck to check toast size and compression

2021-11-03 Thread Mark Dilger
> On Oct 20, 2021, at 12:06 PM, Mark Dilger > wrote: > > Ok. How about: Done that way. v3-0001-Adding-more-toast-pointer-checks-to-amcheck.patch Description: Binary data — Mark Dilger EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company

Re: Extending amcheck to check toast size and compression

2021-10-20 Thread Mark Dilger
> On Oct 20, 2021, at 11:42 AM, Greg Stark wrote: > > > > On Wed., Oct. 20, 2021, 12:41 Mark Dilger, > wrote: > > I used a switch statement to trigger a compiler warning in such an event. > > Catching better compiler diagnostics is an excellent reason to choose this > structure. I guess

Re: Extending amcheck to check toast size and compression

2021-10-20 Thread Greg Stark
On Wed., Oct. 20, 2021, 12:41 Mark Dilger, wrote: > > I used a switch statement to trigger a compiler warning in such an event. > Catching better compiler diagnostics is an excellent reason to choose this structure. I guess all I could ask is that the comment saying no default branch say this is

Re: Extending amcheck to check toast size and compression

2021-10-20 Thread Mark Dilger
> On Oct 19, 2021, at 1:58 PM, Greg Stark wrote: > > Right so here's a review. > > I think the patch is committable as is. It's an improvement and it > does the job as promised. I do have some comments but I don't think > they're serious issues and would actually be pretty happy committing >

Re: Extending amcheck to check toast size and compression

2021-10-19 Thread Greg Stark
Right so here's a review. I think the patch is committable as is. It's an improvement and it does the job as promised. I do have some comments but I don't think they're serious issues and would actually be pretty happy committing it as is. Fwiw I didn't realize how short the patch was at first and

Re: Extending amcheck to check toast size and compression

2021-07-14 Thread Mark Dilger
> On Jul 14, 2021, at 7:57 AM, Mark Dilger wrote: > > so no valid toast pointer should contain a va_rawsize field greater than > MaxAllocSize ... nor should any valid toast pointer contain a va_extinfo field encoding a va_extsize greater than va_rawsize - VARHDRSZ. Violations of either of

Re: Extending amcheck to check toast size and compression

2021-07-14 Thread Mark Dilger
> On Jul 14, 2021, at 3:33 AM, Heikki Linnakangas wrote: > >> +/* The largest valid toast va_rawsize */ >> +#define VARLENA_SIZE_LIMIT 0x3FFF >> + > > Hmm, a toasted datum cannot be larger than MaxAllocSize, because it's > reconstituted in a palloc'd datum, right? No datum size exceeds

Re: Extending amcheck to check toast size and compression

2021-07-14 Thread Heikki Linnakangas
@@ -30,6 +30,9 @@ PG_FUNCTION_INFO_V1(verify_heapam); /* The number of columns in tuples returned by verify_heapam */ #define HEAPCHECK_RELATION_COLS 4 +/* The largest valid toast va_rawsize */ +#define VARLENA_SIZE_LIMIT 0x3FFF + Hmm, a toasted datum cannot be larger than MaxAllocSize,

Re: Extending amcheck to check toast size and compression

2021-05-12 Thread Aleksander Alekseev
Hi hackers, > make installcheck-world: tested, failed > Implements feature: tested, failed > Spec compliant: tested, failed > Documentation:tested, failed Very sorry about these "failed" checkboxes. Didn't use the commitfest webapp for a while. The patch is fine. > T

Re: Extending amcheck to check toast size and compression

2021-05-12 Thread Aleksander Alekseev
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, failed Spec compliant: tested, failed Documentation:tested, failed This patch looks good to me. Considering a positive response

Re: Extending amcheck to check toast size and compression

2021-05-04 Thread Mark Dilger
> On May 4, 2021, at 9:43 AM, Justin Pryzby wrote: > > + /* Oversized toasted attributes should never be stored */ > + if (toast_pointer.va_rawsize > VARLENA_SIZE_LIMIT) > + report_corruption(ctx, > + psprintf("toast valu

Re: Extending amcheck to check toast size and compression

2021-05-04 Thread Justin Pryzby
+ /* Oversized toasted attributes should never be stored */ + if (toast_pointer.va_rawsize > VARLENA_SIZE_LIMIT) + report_corruption(ctx, + psprintf("toast value %u rawsize %u exceeds limit %u", +

Extending amcheck to check toast size and compression

2021-05-04 Thread Mark Dilger
Hackers, During the version 14 development period, a few checks of toasted attributes were written but never committed. For the version 15 development cycle, I'd like to consider extending the checks of toasted attributes. First, no toasted attribute should ever have a rawsize larger than the