pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18244: Corruption in indexes involving whole-row expressions
8+ messages / 5 participants
[nested] [flat]

* BUG #18244: Corruption in indexes involving whole-row expressions
@ 2023-12-12 19:28 PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  0 siblings, 1 reply; 8+ messages in thread

From: PG Bug reporting form @ 2023-12-12 19:28 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: nik@postgres.ai

The following bug has been logged on the website:

Bug reference:      18244
Logged by:          Nikolay Samokhvalov
Email address:      nik@postgres.ai
PostgreSQL version: 16.1
Operating system:   any
Description:        

nik=# create index on t1 (hash_record(t1));
CREATE INDEX

Such an index can easily be corrupted, e.g.:

nik=#  alter table t1 add column yo int default -1;
ALTER TABLE

Or if we just drop a column:

nik=# alter table t_n drop column c2;
ALTER TABLE
nik=# \d t_n
                Table "public.t_n"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 c1     | integer |           |          |
Indexes:
    "t_n_hash_record_idx" btree (hash_record(t_n.*))

nik=# select * from t_n where hash_record(t_n) = hash_record(row(1, -1));
 c1
----
  1
(1 row)

nik=# select * from t_n where hash_record(t_n) = hash_record(row(1));
 c1
----
(0 rows)

Or if index is unique:

nik=# select * from t_n;
 c4
----
 -1
 -1
 -1
(3 rows)

nik=# \d t_n
                   Table "public.t_n"
 Column |  Type   | Collation | Nullable |    Default
--------+---------+-----------+----------+---------------
 c4     | integer |           |          | '-1'::integer
Indexes:
    "t_n_hash_record_idx" UNIQUE, btree (hash_record(t_n.*))

nik=# reindex index t_n_hash_record_idx;
ERROR:  could not create unique index "t_n_hash_record_idx"
DETAIL:  Key (hash_record(t_n.*))=(385747274) is duplicated.

Proposal: prohibit the use of whole-row expression – as it is already done
for generated columns and produce a similar error ("cannot use whole-row
variable in column generation expression")



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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
@ 2023-12-12 22:53 ` Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Laurenz Albe @ 2023-12-12 22:53 UTC (permalink / raw)
  To: nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Tue, 2023-12-12 at 19:28 +0000, PG Bug reporting form wrote:
> nik=# create index on t1 (hash_record(t1));
> 
> Such an index can easily be corrupted, e.g.:
> 
> nik=#  alter table t1 add column yo int default -1;
>
> Proposal: prohibit the use of whole-row expression – as it is already done
> for generated columns and produce a similar error ("cannot use whole-row
> variable in column generation expression")

I reported that bug before:
https://www.postgresql.org/message-id/flat/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel%40cybertec...

I think that your proposal is good, it seems that nobody cared enough
to implement it.  If we forbid that, one thing to consider is pg_upgrade:
it needs a new check.

Yours,
Laurenz Albe





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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
@ 2023-12-13 01:38   ` Thomas Munro <thomas.munro@gmail.com>
  2023-12-13 03:55     ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  0 siblings, 1 reply; 8+ messages in thread

From: Thomas Munro @ 2023-12-13 01:38 UTC (permalink / raw)
  To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Wed, Dec 13, 2023 at 11:53 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> On Tue, 2023-12-12 at 19:28 +0000, PG Bug reporting form wrote:
> > nik=# create index on t1 (hash_record(t1));
> >
> > Such an index can easily be corrupted, e.g.:
> >
> > nik=#  alter table t1 add column yo int default -1;
> >
> > Proposal: prohibit the use of whole-row expression – as it is already done
> > for generated columns and produce a similar error ("cannot use whole-row
> > variable in column generation expression")
>
> I reported that bug before:
> https://www.postgresql.org/message-id/flat/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel%40cybertec...

FTR I also posted a repro for another variation of that problem.  I
think it's slightly more general that, it not just whole-row
expressions (eg index on <table_name>), it's row types generally:

https://www.postgresql.org/message-id/CA%2BhUKGKb4SB%2BqQ-vAVomxAvJY6um%2B5URyq2D0vv10g7mbYZ1Ww%40ma...





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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
@ 2023-12-13 03:55     ` Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 04:04       ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Laurenz Albe @ 2023-12-13 03:55 UTC (permalink / raw)
  To: Thomas Munro <thomas.munro@gmail.com>; +Cc: nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Wed, 2023-12-13 at 14:38 +1300, Thomas Munro wrote:
> On Wed, Dec 13, 2023 at 11:53 AM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> > On Tue, 2023-12-12 at 19:28 +0000, PG Bug reporting form wrote:
> > > nik=# create index on t1 (hash_record(t1));
> > > 
> > > Such an index can easily be corrupted, e.g.:
> > > 
> > > nik=#  alter table t1 add column yo int default -1;
> > > 
> > > Proposal: prohibit the use of whole-row expression – as it is already done
> > > for generated columns and produce a similar error ("cannot use whole-row
> > > variable in column generation expression")
> > 
> > I reported that bug before:
> > https://www.postgresql.org/message-id/flat/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel%40cybertec...
> 
> FTR I also posted a repro for another variation of that problem.  I
> think it's slightly more general that, it not just whole-row
> expressions (eg index on <table_name>), it's row types generally:
> 
> https://www.postgresql.org/message-id/CA%2BhUKGKb4SB%2BqQ-vAVomxAvJY6um%2B5URyq2D0vv10g7mbYZ1Ww%40ma...

Yes, and that makes it difficult to decide on the correct path.

Should we check all places where the type is used before allowing
ALTER TYPE?  Sounds complicated and error-prone.

Should we forbid composite types in index declarations?  Sounds posssible,
but very restrictive.  We might as well forbid composite types in table
definitions.  That would be a good thing in my opinion, but it would cause
a compatibility headache.

Perhaps the best thing would be to add a warning to chapter 8.16.4 that
it is "best practice" (and conforming to the first normal form) not to
use composite types in column definitions, and that altering a composite
type used in a table definition can lead to consistency problems.

Yours,
Laurenz Albe





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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
  2023-12-13 03:55     ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
@ 2023-12-13 04:04       ` Tom Lane <tgl@sss.pgh.pa.us>
  2023-12-14 05:43         ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2023-12-13 04:04 UTC (permalink / raw)
  To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Thomas Munro <thomas.munro@gmail.com>; nik@postgres.ai; pgsql-bugs@lists.postgresql.org

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> Should we forbid composite types in index declarations?  Sounds posssible,
> but very restrictive.

That would make a lot of people very sad, I fear.

I think a minimum answer could be to document that you might need to
REINDEX such indexes after a change in the composite type.  We could
perhaps try to be more proactive, say by marking such indexes invalid.
I think though that that would have deadlock problems as well as race
conditions.  Perhaps best to leave it at "reindex when necessary",
especially in view of the tiny number of reports to date.

			regards, tom lane





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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
  2023-12-13 03:55     ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 04:04       ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-12-14 05:43         ` Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-14 08:27           ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Junwang Zhao <zhjwpku@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Laurenz Albe @ 2023-12-14 05:43 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Tue, 2023-12-12 at 23:04 -0500, Tom Lane wrote:
> Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > Should we forbid composite types in index declarations?  Sounds posssible,
> > but very restrictive.
> 
> That would make a lot of people very sad, I fear.
> 
> I think a minimum answer could be to document that you might need to
> REINDEX such indexes after a change in the composite type.  We could
> perhaps try to be more proactive, say by marking such indexes invalid.
> I think though that that would have deadlock problems as well as race
> conditions.  Perhaps best to leave it at "reindex when necessary",
> especially in view of the tiny number of reports to date.

I agree.  How about the attached documentation patch?

Yours,
Laurenz Albe

Attachments:

0001-Document-the-dangers-of-composite-type-columns.patch (text/x-patch, 1.4K) download From 2399b71203168f9905236380379ed3cadd6d3f4c Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.albe@cybertec.at> Date: Thu, 14 Dec 2023 06:41:18 +0100 Subject: [PATCH] Document the dangers of composite type columns Table columns of a composite type are usually not a good idea, as they violate the first normal form. Moreover, if you create an index on a composite type and later use ALTER TYPE to modify the type, the index can get corrupted. Author: Laurenz Albe Discussion: https://postgr.es/m/18244-8a7897de2acd5a08%40postgresql.org --- doc/src/sgml/rowtypes.sgml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/src/sgml/rowtypes.sgml b/doc/src/sgml/rowtypes.sgml index 4d86f97c03..6a3beaf7d0 100644 --- a/doc/src/sgml/rowtypes.sgml +++ b/doc/src/sgml/rowtypes.sgml @@ -67,6 +67,16 @@ SELECT price_extension(item, 10) FROM on_hand; </para> + <note> + <para> + While you can use composite types in column definitions, there is usually no + benefit in doing so. Also, if you create indexes on composite type columns, + beware of using <command>ALTER TYPE</command> to modify the type definition: + doing that could break the requirement that index expressions must be + immutable and might render the index corrupted. + </para> + </note> + <para> Whenever you create a table, a composite type is also automatically created, with the same name as the table, to represent the table's -- 2.43.0
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
  2023-12-13 03:55     ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 04:04       ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Tom Lane <tgl@sss.pgh.pa.us>
  2023-12-14 05:43         ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
@ 2023-12-14 08:27           ` Junwang Zhao <zhjwpku@gmail.com>
  2023-12-14 08:44             ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  0 siblings, 1 reply; 8+ messages in thread

From: Junwang Zhao @ 2023-12-14 08:27 UTC (permalink / raw)
  To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Thu, Dec 14, 2023 at 1:43 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> On Tue, 2023-12-12 at 23:04 -0500, Tom Lane wrote:
> > Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > > Should we forbid composite types in index declarations?  Sounds posssible,
> > > but very restrictive.
> >
> > That would make a lot of people very sad, I fear.
> >
> > I think a minimum answer could be to document that you might need to
> > REINDEX such indexes after a change in the composite type.  We could
> > perhaps try to be more proactive, say by marking such indexes invalid.
> > I think though that that would have deadlock problems as well as race
> > conditions.  Perhaps best to leave it at "reindex when necessary",
> > especially in view of the tiny number of reports to date.
>
> I agree.  How about the attached documentation patch?
>
+   While you can use composite types in column definitions, there is usually no
+   benefit in doing so.

Is this true? If yes, we'd better update this to
https://wiki.postgresql.org/wiki/Don%27t_Do_This.

> Yours,
> Laurenz Albe



-- 
Regards
Junwang Zhao





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

* Re: BUG #18244: Corruption in indexes involving whole-row expressions
  2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
  2023-12-12 22:53 ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 01:38   ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Thomas Munro <thomas.munro@gmail.com>
  2023-12-13 03:55     ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-13 04:04       ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Tom Lane <tgl@sss.pgh.pa.us>
  2023-12-14 05:43         ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Laurenz Albe <laurenz.albe@cybertec.at>
  2023-12-14 08:27           ` Re: BUG #18244: Corruption in indexes involving whole-row expressions Junwang Zhao <zhjwpku@gmail.com>
@ 2023-12-14 08:44             ` Laurenz Albe <laurenz.albe@cybertec.at>
  0 siblings, 0 replies; 8+ messages in thread

From: Laurenz Albe @ 2023-12-14 08:44 UTC (permalink / raw)
  To: Junwang Zhao <zhjwpku@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; nik@postgres.ai; pgsql-bugs@lists.postgresql.org

On Thu, 2023-12-14 at 16:27 +0800, Junwang Zhao wrote:
> +   While you can use composite types in column definitions, there is usually no
> +   benefit in doing so.
> 
> Is this true? If yes, we'd better update this to
> https://wiki.postgresql.org/wiki/Don%27t_Do_This.

I cannot think of a use case for composite type columns that wouldn't be
better served by directly declaring the columns of the composite key
in the table definition.  And it makes accessing the values more complicated.

So yes, I'd be on board with a "don't do this" entry.

Yours,
Laurenz Albe





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


end of thread, other threads:[~2023-12-14 08:44 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-12-12 19:28 BUG #18244: Corruption in indexes involving whole-row expressions PG Bug reporting form <noreply@postgresql.org>
2023-12-12 22:53 ` Laurenz Albe <laurenz.albe@cybertec.at>
2023-12-13 01:38   ` Thomas Munro <thomas.munro@gmail.com>
2023-12-13 03:55     ` Laurenz Albe <laurenz.albe@cybertec.at>
2023-12-13 04:04       ` Tom Lane <tgl@sss.pgh.pa.us>
2023-12-14 05:43         ` Laurenz Albe <laurenz.albe@cybertec.at>
2023-12-14 08:27           ` Junwang Zhao <zhjwpku@gmail.com>
2023-12-14 08:44             ` Laurenz Albe <laurenz.albe@cybertec.at>

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