pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
amcheck is using a wrong macro to check compressed-ness
7+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2022-06-22 e243de03fb  amcheck: Fix incorrect use of VARATT_IS_COMPRESSED.
* amcheck is using a wrong macro to check compressed-ness
@ 2022-05-17 07:27 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 7+ messages in thread

From: Kyotaro Horiguchi @ 2022-05-17 07:27 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

Hello.

While I looked into a patch, I noticed that check_tuple_attribute does
not run the check for compessed data even if a compressed data is
given.

check_tuple_attribute()
..
	struct varatt_external toast_pointer;
..
	VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
..
	if (VARATT_IS_COMPRESSED(&toast_pointer))
    {

Since toast_pointer is a varatt_exteral it should be
VARATT_EXTERNAL_IS_COMPRESSED instead.  Since the just following
corss-check is just the reverse of what the macro does, it is useless.

What do you think about the attached?  The problem code is new in
PG15.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Use-correct-macro-to-check-compressed-ness-in-amchec.patch (text/x-patch, 1.5K) download From 00ae35ca30a6d3168dc4905ab5ba9db1339fe377 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Tue, 17 May 2022 16:19:27 +0900 Subject: [PATCH] Use correct macro to check compressed-ness in amcheck check_tuple_attribute uses VARATT_IS_COMPRESSED() for varatt_external. This should be VARATT_EXTERNAL_IS_COMPRESSED() instead. Remove the following cross check for data size since the ycondition cannot be true. --- contrib/amcheck/verify_heapam.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index c875f3e5a2..e488f5e234 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1385,19 +1385,11 @@ check_tuple_attribute(HeapCheckContext *ctx) toast_pointer.va_rawsize, VARLENA_SIZE_LIMIT)); - if (VARATT_IS_COMPRESSED(&toast_pointer)) + if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) { ToastCompressionId cmid; bool valid = false; - /* Compression should never expand the attribute */ - if (VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer) > toast_pointer.va_rawsize - VARHDRSZ) - report_corruption(ctx, - psprintf("toast value %u external size %u exceeds maximum expected for rawsize %d", - toast_pointer.va_valueid, - VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer), - toast_pointer.va_rawsize)); - /* Compressed attributes should have a valid compression method */ cmid = TOAST_COMPRESS_METHOD(&toast_pointer); switch (cmid) -- 2.27.0
^ permalink raw reply [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2022-05-17 07:58 ` Michael Paquier <michael@paquier.xyz>
  2022-05-18 00:55   ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-05-17 07:58 UTC (permalink / raw)
  To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org

On Tue, May 17, 2022 at 04:27:19PM +0900, Kyotaro Horiguchi wrote:
> What do you think about the attached?  The problem code is new in
> PG15.

Adding Robert in CC, as this has been added with bd807be.  I have
added an open item for now.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
@ 2022-05-18 00:55   ` Michael Paquier <michael@paquier.xyz>
  2022-06-09 01:48     ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-05-18 00:55 UTC (permalink / raw)
  To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>

On Tue, May 17, 2022 at 04:58:11PM +0900, Michael Paquier wrote:
> Adding Robert in CC, as this has been added with bd807be.  I have
> added an open item for now.

With the individual in CC, that's even better.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-05-18 00:55   ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
@ 2022-06-09 01:48     ` Michael Paquier <michael@paquier.xyz>
  2022-06-22 01:56       ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-06-09 01:48 UTC (permalink / raw)
  To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>

On Wed, May 18, 2022 at 09:55:07AM +0900, Michael Paquier wrote:
> On Tue, May 17, 2022 at 04:58:11PM +0900, Michael Paquier wrote:
>> Adding Robert in CC, as this has been added with bd807be.  I have
>> added an open item for now.
> 
> With the individual in CC, that's even better.

Three weeks later, ping.  Robert, could you look at this thread?
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-05-18 00:55   ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-09 01:48     ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
@ 2022-06-22 01:56       ` Michael Paquier <michael@paquier.xyz>
  2022-06-22 17:14         ` Re: amcheck is using a wrong macro to check compressed-ness Robert Haas <robertmhaas@gmail.com>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-06-22 01:56 UTC (permalink / raw)
  To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Robert Haas <robertmhaas@gmail.com>

On Thu, Jun 09, 2022 at 10:48:27AM +0900, Michael Paquier wrote:
> Three weeks later, ping.  Robert, could you look at this thread?

And again.  beta2 is planned to next week, and this is still an open
item.  I could look at that by myself, but I always tend to get easily
confused with all the VARATT macros when it comes to compressed blobs,
so it would take a bit of time.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-05-18 00:55   ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-09 01:48     ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-22 01:56       ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
@ 2022-06-22 17:14         ` Robert Haas <robertmhaas@gmail.com>
  2022-06-24 03:59           ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 7+ messages in thread

From: Robert Haas @ 2022-06-22 17:14 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

On Tue, Jun 21, 2022 at 9:56 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Jun 09, 2022 at 10:48:27AM +0900, Michael Paquier wrote:
> > Three weeks later, ping.  Robert, could you look at this thread?
>
> And again.  beta2 is planned to next week, and this is still an open
> item.  I could look at that by myself, but I always tend to get easily
> confused with all the VARATT macros when it comes to compressed blobs,
> so it would take a bit of time.

Oops, I missed this thread. I think the patch is correct, so I have
committed it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: amcheck is using a wrong macro to check compressed-ness
  2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
  2022-05-17 07:58 ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-05-18 00:55   ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-09 01:48     ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-22 01:56       ` Re: amcheck is using a wrong macro to check compressed-ness Michael Paquier <michael@paquier.xyz>
  2022-06-22 17:14         ` Re: amcheck is using a wrong macro to check compressed-ness Robert Haas <robertmhaas@gmail.com>
@ 2022-06-24 03:59           ` Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 7+ messages in thread

From: Michael Paquier @ 2022-06-24 03:59 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

On Wed, Jun 22, 2022 at 01:14:57PM -0400, Robert Haas wrote:
> Oops, I missed this thread. I think the patch is correct, so I have
> committed it.

Thanks, Robert.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2022-06-24 03:59 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-05-17 07:27 amcheck is using a wrong macro to check compressed-ness Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2022-05-17 07:58 ` Michael Paquier <michael@paquier.xyz>
2022-05-18 00:55   ` Michael Paquier <michael@paquier.xyz>
2022-06-09 01:48     ` Michael Paquier <michael@paquier.xyz>
2022-06-22 01:56       ` Michael Paquier <michael@paquier.xyz>
2022-06-22 17:14         ` Robert Haas <robertmhaas@gmail.com>
2022-06-24 03:59           ` Michael Paquier <michael@paquier.xyz>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox