| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Subject: | Re: table AM option passing |
| Date: | 2026-04-01 06:33:28 |
| Message-ID: | 40E570EE-5A60-49D8-B8F7-2F8F2B7C8DFA@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Apr 1, 2026, at 00:10, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> Hi,
>
> On 2026-Mar-30, Álvaro Herrera wrote:
>
>> I just pushed the part of 0001 that modifies the API of table_insert and
>> its sibling functions. So here 0001 adds the options bitmask to
>> table_update and table_delete, which is important for REPACK; and 0002
>> is identical as before and makes the interface (IMO) a bit more
>> future-proof.
>
> FWIW I just posted 0001 as part of the repack series here [1]; 0002 is
> inessential, so I didn't; but here's both of them. I have drafted
> commit messages also.
>
> [1] https://postgr.es/m/202603311523.iqhng5ljkzpq@alvherre.pgsql
>
> --
> Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
> "Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
> sólo le suma el nuevo terror de la locura" (Perelandra, C.S. Lewis)
> <v4-0001-Give-options-parameter-to-table_delete-table_upda.patch><v4-0002-Change-heapam.c-to-obey-tableam.h-option-bits-dir.patch>
Looks like v4-0001/0002 are pure refactoring. A few small comments:
1 - 0001
For table_tuple_delete(), options is added and changingPart is removed, but the header comment should be updated to reflect the change.
2 - 0002
For table_tuple_update(), options is added, the header comment should be updated as well.
3 - 0002
Now TABLE_INSERT_SKIP_FSM, TABLE_INSERT_FROZEN, TABLE_INSERT_NO_LOGICAL belong to the same options word as HEAP_INSERT_SPECULATIVE, but they are still defined as:
```
#define TABLE_INSERT_SKIP_FSM 0x0002
#define TABLE_INSERT_FROZEN 0x0004
#define TABLE_INSERT_NO_LOGICAL 0x0008
```
Could it make sense to change them to the left-shift form?
4 - 0002
In heap_multi_insert(), heap_prepare_insert(), and heap_insert(), options is defined as uint32, but in RelationGetBufferForTuple() and raw_heap_insert() it is still defined as int. Would it make sense to take this opportunity to change all “options" to uint32 for consistency? Otherwise, if this is left for later, a trivial follow-up patch just to change int to uint32 may be harder to get processed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Lukas Fittl | 2026-04-01 06:33:50 | Re: pg_plan_advice |
| Previous Message | Ning Sun | 2026-04-01 06:32:17 | Add ParameterDescription message to libpq frontend long message types |