| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | cca5507 <cca5507(at)qq(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Do not lock tables in get_tables_to_repack |
| Date: | 2026-07-10 14:22:04 |
| Message-ID: | alD9l-XlCuu3eUEe@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-Jul-08, cca5507 wrote:
> - classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid));
> + classtup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(index->indrelid));
>
> Do we really need to copy it? We hold a refcount on it so it won't be freed
> until we release it. Otherwise LGTM.
Yeah, I guess it doesn't matter. I have removed the copy and updated
some comments. I also realized that there are some places where we
weren't dealing correctly with the possibility that the relation goes
away, or is replaced with something different, so I added that too.
While looking at it I also realized that get_tables_to_repack_partitioned
is likewise not careful enough about it: we do IndexGetRelation(, false)
which fails hard if the pg_index tuple cannot be found, which is the
wrong thing to do.
At the same time, it's annoying that half of the code that clearly
belongs in that routine is actually in ExecRepack(). I moved that to
where it rightfully belongs. (The only somewhat annoying thing is that
we have to NULL-out the Relation pointer after returning; but that's not
*too* bad IMO.)
Any opinions on this?
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"No me acuerdo, pero no es cierto. No es cierto, y si fuera cierto,
no me acuerdo." (Augusto Pinochet a una corte de justicia)
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-change-get_tables_to_repack_partitioned.patch | text/x-diff | 6.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-07-10 14:41:38 | Re: use of SPI by postgresImportForeignStatistics |
| Previous Message | Tom Lane | 2026-07-10 14:08:05 | Re: use of SPI by postgresImportForeignStatistics |