RE: Parallel INSERT SELECT take 2

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Greg Nancarrow <gregn4422(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: RE: Parallel INSERT SELECT take 2
Date: 2021-05-31 05:34:09
Message-ID: OS0PR01MB5716F50BCC5722D17F721990943F9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: Greg Nancarrow <gregn4422(at)gmail(dot)com>
Sent: Friday, May 28, 2021 4:42 PM
> On Mon, May 24, 2021 at 3:15 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> >
> > Thanks for the comments and your descriptions looks good.
> > Attaching v5 patchset with all these changes.
> >
>
> A few other minor things I noticed:
>
> (1) error message wording when declaring a table SAFE for parallel DML
>
> src/backend/commands/tablecmds.c
>
> Since data modification for the RELKIND_FOREIGN_TABLE and
> RELPERSISTENCE_TEMP types are allowed in the parallel-restricted case (i.e.
> leader may modify in parallel mode) I'm thinking it may be better to use
> wording like:
>
> "cannot support foreign or temporary table data modification by parallel
> workers"
>
> instead of
>
> "cannot support parallel data modification on a foreign or temporary table"
>
> There are TWO places where this error message is used.
>
> (What do you think?)

I think your change looks good.
I used your msg in the latest patchset.

> (2) Minor formatting issue
>
> src/backend/optimizer/util/clauses.c
>
> static safety_object *make_safety_object(Oid objid, Oid classid, char
> proparallel)
>
> should be:
>
> static safety_object *
> make_safety_object(Oid objid, Oid classid, char proparallel)

Changed.

> (3) Minor formatting issue
>
> src/backend/utils/cache/typcache.c
>
>
> List *GetDomainConstraints(Oid type_id)
>
> should be:
>
> List *
> GetDomainConstraints(Oid type_id)

Changed.

Attaching v6 patchset.
And I registered it in CF https://commitfest.postgresql.org/33/3143/,
comments are welcome.

Best regards,
houzj

Attachment Content-Type Size
v6-0004-regression-test-and-doc-updates.patch application/octet-stream 42.7 KB
v6-0001-CREATE-ALTER-TABLE-PARALLEL-DML.patch application/octet-stream 37.4 KB
v6-0002-parallel-SELECT-for-INSERT.patch application/octet-stream 9.8 KB
v6-0003-get-parallel-safety-functions.patch application/octet-stream 28.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2021-05-31 05:46:22 Re: Regarding the necessity of RelationGetNumberOfBlocks for every rescan / bitmap heap scan.
Previous Message Dean Gibson (DB Administrator) 2021-05-31 05:24:00 Re: AWS forcing PG upgrade from v9.6 a disaster