RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Tuesday, June 15, 2021 10:01 PM Robert Haas wrote: > On Tue, Jun 15, 2021 at 7:05 AM Amit Kapila wrote: > > Yeah, dealing with partitioned tables is tricky. I think if we don't > > want to check upfront the parallel safety of all the partitions then > > the other option as discussed could be

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Tuesday, June 15, 2021 10:01 PM Robert Haas wrote: > On Tue, Jun 15, 2021 at 7:05 AM Amit Kapila wrote: > > Okay, but I think if we go with your suggested model where whenever > > there is a change in parallel-safety of any function, we need to send > > the new invalidation then I think it

RE: Skip partition tuple routing with constant partition key

Hi Amit-san From: Amit Langote > On Fri, Jun 4, 2021 at 6:05 PM Amit Langote > wrote: > > On Fri, Jun 4, 2021 at 4:38 PM Amit Langote > wrote: > > > On Thu, Jun 3, 2021 at 8:48 PM Amit Langote > wrote: > > > > On Tue, Jun 1, 2021 at 5:43 PM houzj.f...@f

RE: Unused function parameter in get_qual_from_partbound()

On Tuesday, June 8, 2021 7:30 PM David Rowley > On Tue, 8 Jun 2021 at 21:50, houzj.f...@fujitsu.com > wrote: > > I noticed that the first function parameter in > > get_qual_from_partbound(**Relation rel**, Relation parent, is not used in > > the > function. >

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Tuesday, June 8, 2021 10:51 PM Robert Haas > On Mon, Jun 7, 2021 at 11:33 PM Amit Kapila > wrote: > > Note the error is raised after applying the patch, without the patch, > > the above won't show any error (error message could be improved here). > > Such cases can lead to unpredictable

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Wednesday, June 16, 2021 11:27 AM houzj.f...@fujitsu.com wrote: > On Tuesday, June 15, 2021 10:01 PM Robert Haas wrote: > > Just to be clear here, I don't think it really matters what we *want* > > to do. I don't think it's reasonably *possible* to check all the > > p

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Tuesday, June 22, 2021 8:25 PM Amit Kapila wrote: > On Wed, Jun 16, 2021 at 8:57 AM houzj.f...@fujitsu.com > wrote: > > > > I think the check of partition could be even more complicated if we > > need to check the parallel safety of partition key expression. If

RE: Deadlock risk while inserting directly into partition?

On Wednesday, June 23, 2021 5:07 PM Amit Kapila wrote: > I noticed that while inserting directly into a partition table we compute the > PartitionCheckExpr by traversing all the parent partitions via > ExecPartitionCheck()->RelationGetPartitionQual()->generate_partition_qual(). > We take

Partition Check not updated when insert into a partition

Hi, When directly INSERT INTO partition, postgres will invoke ExecPartitionCheck which will execute its parent's and grandparent's partition constraint check. >From the code, the whole constraint check is saved in relcache::rd_partcheck. For a multi-level partition, for example: table 'A' is

RE: Partition Check not updated when insert into a partition

On Wednesday, June 23, 2021 12:16 PM I wrote: > When directly INSERT INTO partition, postgres will invoke ExecPartitionCheck > which will execute its parent's and grandparent's partition constraint check. > From the code, the whole constraint check is saved in relcache::rd_partcheck. > > For a

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Monday, June 21, 2021 11:23 PM Robert Haas wrote: > On Mon, Jun 21, 2021 at 12:56 AM Amit Kapila wrote: > > Yeah, the session in which we are doing Alter Function won't be able > > to lock it but it will wait for the AccessExclusiveLock on the rel to > > be released because it will also try

Unused function parameter in get_qual_from_partbound()

Hi, I noticed that the first function parameter in get_qual_from_partbound(**Relation rel**, Relation parent, is not used in the function. Is it better to remove it like the attached patch ? Best regards, houzj 0001-remove-unused-function-parameter-in-get_qual_from_pa.patch Description:

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

> > Sometimes this is actually quite useful. You might know that, while > > the function is in general volatile, it is immutable in the particular > > way that you are using it. Or, perhaps, you are using the volatile > > function incidentally and it doesn't affect the output of your > > function

RE: Parallel INSERT SELECT take 2

> > > > > So, users need to check count(*) for this to determine > > > > > parallel-safety? How about if we provide a wrapper function on > > > > > top of this function or a separate function that returns char to > > > > > indicate whether it is safe, unsafe, or restricted to perform a > > > > >

Re: Parallel scan with SubTransGetTopmostTransaction assert coredump

> To: Pengchengliu > Cc: Greg Nancarrow ; Andres Freund ; > PostgreSQL-development > Subject: Re: Re: Parallel scan with SubTransGetTopmostTransaction assert > coredump > I've also seen the reports of the same  > Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin)) with a subsequent >

RE: making update/delete of inheritance trees scale better

Hi After 86dc900, In " src/include/nodes/pathnodes.h ", I noticed that it uses the word " partitioned UPDATE " in the comment above struct RowIdentityVarInfo. But, it seems " inherited UPDATE " is used in the rest of places. Is it better to keep them consistent by using " inherited UPDATE " ?

RE: making update/delete of inheritance trees scale better

> On Mon, May 17, 2021 at 3:07 PM houzj.f...@fujitsu.com > wrote: > > > > Hi > > > > After 86dc900, In " src/include/nodes/pathnodes.h ", I noticed that it > > uses the word " partitioned UPDATE " in the comment above struct > RowIdentity

RE: Allow batched insert during cross-partition updates

> > Thanks! It looks good! > > Thanks for checking. I'll mark this as RfC. Hi, The patch cannot be applied to the latest head branch, it will be nice if you can rebase it. And when looking into the patch, I have some comments on it. 1) IIRC, After the commit c5b7ba4, the initialization of

RE: batch fdw insert bug (Postgres 14)

> I am testing new features in Postgres 14, and I found bug > EXPLAIN ANALYZE VERBOSE  for insert to FDW table with batch_size 1000 returns > --- > Insert on

RE: Enhanced error message to include hint messages for redundant options error

> > > > > > > Thanks! The v5 patch looks good to me. Let's see if all > > > > > > > agree on the goto duplicate_error approach which could reduce > the LOC by ~80. > > > > > > > > > > > > I think the "goto duplicate_error" approach looks good, it > > > > > > avoids duplicating the same error code

RE: Parallel INSERT SELECT take 2

> > > When the parallel safety of some of these objects is changed, it's costly > > > to > reflect it on the parallel safety of tables that depend on them. So, we > don't do > it. Instead, we provide a utility function > pg_get_parallel_safety('table_name') > that returns records of (objid,

Inaccurate error message when set fdw batch_size to 0

Hi, When testing the fdw batch insert, I found a possible issue. If I set the batch_size to 0 , it will throw an error: - CREATE FOREIGN TABLE test(a int, b varchar) SERVER testserver OPTIONS (table_name 'testlocal', batch_size '0'); ERROR: fetch_size requires a

RE: Enhanced error message to include hint messages for redundant options error

> > > Thanks! The v5 patch looks good to me. Let's see if all agree on the > > > goto duplicate_error approach which could reduce the LOC by ~80. > > > > I think the "goto duplicate_error" approach looks good, it avoids > > duplicating the same error code multiple times. > > Thanks. I will mark

Skip partition tuple routing with constant partition key

Hi, When loading some data into a partitioned table for testing purpose, I found even if I specified constant value for the partition key[1], it still do the tuple routing for each row. [1]- UPDATE partitioned set part_key = 2 , … INSERT into partitioned(part_key,

RE: Skip partition tuple routing with constant partition key

> > Hmm, does this seem common enough for the added complexity to be > worthwhile? > > I'd also like to know if there's some genuine use case for this. For testing > purposes does not seem to be quite a good enough reason. Thanks for the response. For some big data scenario, we sometimes

Fdw batch insert error out when set batch_size > 65535

Hi, When reading the code, I noticed some possible issue about fdw batch insert. When I set batch_size > 65535 and insert more than 65535 rows into foreign table, It will throw an error: For example: -- CREATE FOREIGN TABLE vzdalena_tabulka2(a int, b varchar)   SERVER omega_db

RE: Fdw batch insert error out when set batch_size > 65535

From: Bharath Rupireddy Sent: Friday, May 21, 2021 1:42 PM > On Fri, May 21, 2021 at 8:18 AM houzj.f...@fujitsu.com > wrote: > > Actually, I think if the (number of columns) * (number of rows) > > > 65535, then we can get this error. But, I think almost no one will set >

RE: Bug in query rewriter - hasModifyingCTE not getting set

From: Tom Lane > Greg Nancarrow writes: > > On Sun, Feb 7, 2021 at 10:03 AM Tom Lane wrote: > >> I think either the bit about rule_action is unnecessary, or most of > >> the code immediately above this is wrong, because it's only updating > >> flags in sub_action. Why do you think it's

RE: Skip partition tuple routing with constant partition key

From: Amit Langote Sent: Wednesday, May 19, 2021 9:17 PM > I gave a shot to implementing your idea and ended up with the attached PoC > patch, which does pass make check-world. > > I do see some speedup: > > -- creates a range-partitioned table with 1000 partitions create unlogged > table >

Fix typo: multiple tuple => tuples

Hi, I found a possible typo in the code comments of heap_multi_insert. - * heap_multi_insert - insert multiple tuple into a heap + * heap_multi_insert - insert multiple tuples into a heap Attaching a patch to fix it. Best regards, houzj 0001-fix-typo.patch Description:

RE: Parallel Inserts in CREATE TABLE AS

Hi Bharath-san, From: Bharath Rupireddy Sent: Friday, May 21, 2021 6:49 PM > > On Fri, May 21, 2021 at 3:46 PM Amit Kapila wrote: > > > > On Fri, Mar 19, 2021 at 11:02 AM Bharath Rupireddy > > wrote: > > > > > > On Wed, Jan 27, 2021 at 1:47 PM Bharath Rupireddy > > > wrote: > > > > > > > > >

RE: Fdw batch insert error out when set batch_size > 65535

From: Bharath Rupireddy Sent: Friday, May 21, 2021 5:03 PM > On Fri, May 21, 2021 at 1:19 PM houzj.f...@fujitsu.com > wrote: > > Attaching V2 patch. Please consider it for further review. > > Thanks for the patch. Some more comments: > > 1) Can f

Fixup some appendStringInfo and appendPQExpBuffer calls

Hi, In the latest HEAD branch, I found some places were using appendStringInfo/appendPQExpBuffer() when they could have been using appendStringInfoString/ appendPQExpBufferStr() instead. I think we'd better fix these places in case other developers will use these codes as a reference, though, it

RE: Skip partition tuple routing with constant partition key

Hi Amit-san From: Amit Langote Sent: Thursday, May 27, 2021 4:46 PM > Hou-san, > > On Thu, May 27, 2021 at 3:56 PM houzj.f...@fujitsu.com > wrote: > > From: Amit Langote > > Sent: Thursday, May 27, 2021 1:54 PM > > > On Thu, May 27, 2021 at 11:47 AM ho

RE: Skip partition tuple routing with constant partition key

From: Amit Langote Sent: Thursday, May 20, 2021 8:23 PM > > Hou-san, > > On Thu, May 20, 2021 at 7:35 PM houzj.f...@fujitsu.com > wrote: > > 2) Test expression in partition key > > > > create or replace function partition_func(i int) returns int as $$ &

RE: Skip partition tuple routing with constant partition key

From: houzj.f...@fujitsu.com Sent: Monday, May 24, 2021 3:58 PM > > From: Tsunakawa, Takayuki <mailto:tsunakawa.ta...@fujitsu.com> > Sent: Monday, May 24, 2021 3:34 PM > > > > From: mailto:houzj.f...@fujitsu.com <mailto:houzj.f...@fujitsu.com> > > > I

RE: Skip partition tuple routing with constant partition key

From: Tsunakawa, Takayuki Sent: Monday, May 24, 2021 3:34 PM > > From: houzj.f...@fujitsu.com > > I think this patch can solve the performance degradation of key > > expression after applying the [Save the last partition] patch. > > Besides, this could be a separate patc

RE: Parallel Inserts in CREATE TABLE AS

From: Bharath Rupireddy Sent: Thursday, May 27, 2021 3:41 PM > > On Thu, May 27, 2021 at 1:03 PM tsunakawa.ta...@fujitsu.com > wrote: > > > > From: houzj.f...@fujitsu.com > > > Although, the 4 workers case still has performance degradation > > > comp

RE: Parallel Inserts in CREATE TABLE AS

From: Bharath Rupireddy Sent: Thursday, May 27, 2021 10:07 PM > On Thu, May 27, 2021 at 9:53 AM Bharath Rupireddy > wrote: > > > One idea to find this out could be that we have three counters for > > > each worker which counts the number of times each worker extended > > > the relation in bulk,

RE: Parallel Inserts in CREATE TABLE AS

From: Tsunakawa, Takayuki/綱川 貴之 Sent: Friday, May 28, 2021 8:55 AM > To: 'Bharath Rupireddy' ; Hou, > Zhijie/侯 志杰 > Cc: Amit Kapila ; Tang, Haiying/唐 海英 > ; PostgreSQL-development > ; Zhihong Yu ; Luc > Vlaming ; Dilip Kumar ; > vignesh C > Subject: RE: Parallel Inserts in CREATE TABLE AS > >

RE: Fdw batch insert error out when set batch_size > 65535

From: Bharath Rupireddy Sent: Wednesday, May 26, 2021 9:56 PM > On Wed, May 26, 2021 at 6:36 PM Tomas Vondra > wrote: > > > > On 5/26/21 8:57 AM, Bharath Rupireddy wrote: > > > On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy > > > wrote: > > >>

RE: Parallel Inserts in CREATE TABLE AS

From: Bharath Rupireddy Sent: Wednesday, May 26, 2021 7:22 PM > Thanks for trying that out. > > Please see the code around the use_fsm flag in RelationGetBufferForTuple for > more understanding of the points below. > > What happens if FSM is skipped i.e. myState->ti_options = >

RE: Skip partition tuple routing with constant partition key

Hi amit-san From: Amit Langote Sent: Wednesday, May 26, 2021 9:38 AM > > Hou-san, > > On Wed, May 26, 2021 at 10:05 AM houzj.f...@fujitsu.com > wrote: > > > > Thanks for the explanation ! > > Yeah, we can get all the parent table info from Pa

RE: Skip partition tuple routing with constant partition key

Hi Amit-san From: Amit Langote Sent: Tuesday, May 25, 2021 10:06 PM > Hou-san, > > Thanks for the patch and It looks more compact than mine. > > > > After taking a quick look at the patch, I found a possible issue. > > Currently, the patch does not search the parent's partition key expression >

RE: Parallel Inserts in CREATE TABLE AS

From: Bharath Rupireddy Sent: Thursday, May 27, 2021 12:46 PM > On Thu, May 27, 2021 at 7:12 AM houzj.f...@fujitsu.com > wrote: > > I am afraid that the using the FSM seems not get a stable performance > > gain(at least on my machine), I will take a deep look into this

RE: Skip partition tuple routing with constant partition key

From: Amit Langote Sent: Thursday, May 27, 2021 1:54 PM > On Thu, May 27, 2021 at 11:47 AM houzj.f...@fujitsu.com > wrote: > > About teaching relcache about caching the target partition. > > > > David-san suggested cache the partidx in PartitionDesc. > > And it

RE: Parallel Inserts in CREATE TABLE AS

From: Bharath Rupireddy Sent: Thursday, May 27, 2021 2:59 PM > On Thu, May 27, 2021 at 12:19 PM houzj.f...@fujitsu.com > wrote: > > BTW, I checked my test results, I was testing INSERT INTO unlogged table. > > What do you mean by "testing INSERT INTO"? Is it that y

RE: Skip partition tuple routing with constant partition key

Hi Amit-san, From: Amit Langote Sent: Monday, May 24, 2021 4:27 PM > Hou-san, > > On Mon, May 24, 2021 at 10:31 AM houzj.f...@fujitsu.com > wrote: > > From: Amit Langote > > Sent: Thursday, May 20, 2021 8:23 PM > > > This one seems bit tough. Exec

RE: Added schema level support for publication.

On Wednesday, June 30, 2021 7:43 PM vignesh C wrote: > Thanks for reporting this issue, the attached v9 patch fixes this issue. This > also fixes the other issue you reported at [1]. Hi, I had a look at the patch, please consider following comments. (1) - if

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Sunday, July 4, 2021 1:44 PM Dilip Kumar wrote: > On Fri, Jul 2, 2021 at 8:16 PM Robert Haas wrote: > > > > On Wed, Jun 30, 2021 at 11:46 PM Greg Nancarrow > wrote: > > > I personally think "(b) provide an option to the user to specify > > > whether inserts can be parallelized on a relation"

RE: Added schema level support for publication.

On Thursday, July 8, 2021 11:47 AM houzj.f...@fujitsu.com wrote > On Wednesday, June 30, 2021 7:43 PM vignesh C > wrote: > > Thanks for reporting this issue, the attached v9 patch fixes this issue. > > This also > fixes the other issue you reported at [1]. >

RE: Parallel INSERT SELECT take 2

> > Based on above, we plan to move forward with the apporache 2) (declarative > idea). > > IIUC, the declarative behaviour idea attributes parallel > safe/unsafe/restricted > tags to each table with default being the unsafe. Does it mean for a parallel > unsafe table, no parallel selects,

RE: Parallel INSERT SELECT take 2

> > The parallel attributes in table means the parallel safety when user does > > some > data-modification operations on it. > > So, It only limit the use of parallel plan when using INSERT/UPDATE/DELETE. > > In that case, isn't it better to use the terminology "PARALLEL DML >

RE: Parallel INSERT SELECT take 2

> > Currently, index expression and predicate are stored in text format. > > We need to use stringToNode(expression/predicate) to parse it. > > Some committers think doing this twice does not look good, unless we > > found some ways to pass parsed info to the executor to avoid the second > parse.

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

> Thank you, fmgr_info() looks like the best place to do the parallel safety > check. > Having a quick look at its callers, I didn't find any concerning place (of > course, > we can't be relieved until the regression test succeeds.) Also, with > fmgr_info(), > we don't have to find other

RE: Parallel INSERT SELECT take 2

> BACKGROUND > > > We want to realize parallel INSERT SELECT in the following steps: > 1) INSERT + parallel SELECT > 2) Parallel INSERT + parallel SELECT > > Below are example use cases. We don't expect high concurrency or an empty > data source. > *

Fix redundant comments in fmgr.c

Hi, I found some possible redundant comments in fmgr.c 1. fmgr_symbol(Oid functionId, char **mod, char **fn) { HeapTuple procedureTuple; Form_pg_proc procedureStruct; bool isnull; Datum

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

> >>> I'm curious. The FmgrBuiltin struct includes the "strict" flag, so > >>> that would "lock down the value" of the strict flag, wouldn't it? > > >> It does, but that's much more directly a property of the function's C > >> code than parallel-safety is. > > > I'm not sure I agree with that,

RE: [HACKERS] logical decoding of two-phase transactions

> I have incorporated all your changes and additionally made few more changes > (a) got rid of LogicalRepBeginPrepareData and instead used > LogicalRepPreparedTxnData, (b) made a number of changes in comments and > docs, (c) ran pgindent, (d) modified tests to use standard wait_for_catch >

RE: Add Nullif case for eval_const_expressions_mutator

> + if (!has_nonconst_input) > + return ece_evaluate_expr(expr); > > That's not okay without a further check to see if the comparison function used > by the node is immutable. Compare ScalarArrayOpExpr, for instance. Thanks for

RE: fix typo in reorderbuffer.c

> > What about "Combo CID(s)", for Combo command ID? README.parallel uses > this term for example. Sorry for the late reply and yes, " Combo CID(s)" looks better. Attaching patch which replaces all styles "Combocid(s)" with " Combo CID(s)". Best regards, houzj

RE: Table refer leak in logical replication

> > > insert into test values (6); > > > > > > It seems an issue about reference leak. Anyone can fix this? > > > > It seems ExecGetTriggerResultRel will reopen the target table because it > cannot find an existing one. > > Storing the opened table in estate->es_opened_result_relations seems >

RE: Table refer leak in logical replication

> WARNING: relcache reference leak: relation "xxx" not closed. > > Example of the procedure: > --publisher-- > create table test (a int primary key); > create publication pub for table test; > > --subscriber-- > create table test (a int primary key); > create subscription sub

RE: Hybrid Hash/Nested Loop joins and caching results from subplans

> I've attached the updated patch. I'll let the CFbot grab this to ensure it's > happy with it before I go looking to push it again. Hi, I took a look into the patch and noticed some minor things. 1. + case T_ResultCache: + ptype = "ResultCache"; +

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

> > After some more on how to support parallel insert into fk relation. > > It seems we do not have a cheap way to implement this feature. > > > > In RI_FKey_check, Currently, postgres execute "select xx for key > > share" to check that foreign key exists in PK table. > > However "select for

RE: Parallel INSERT (INTO ... SELECT ...)

> I guess to have the finer granularity we'd have to go with > enable_parallel_insert, > which then would mean possibly having to later add enable_parallel_update, > should parallel update have similar potential overhead in the parallel-safety > checks (which to me, looks like it could, and

RE: Parallel Inserts in CREATE TABLE AS

> Seems like v22 patch was failing in cfbot for one of the unstable test cases. > Attaching v23 patch set with modification in 0003 and 0004 patches. No > changes to 0001 and 0002 patches. Hopefully cfbot will be happy with v23. > > Please consider v23 for further review. Hi, I was looking into

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

> To support parallel insert into FK relation. > There are two scenarios need attention. > 1) foreign key and primary key are on the same table(INSERT's target table). > (referenced and referencing are the same table) > 2) referenced and referencing table are both partition of INSERT's target >

RE: Parallel INSERT (INTO ... SELECT ...)

> > Attaching new version patch with this change. > > > > Thanks, the patch looks good to me. I have made some minor cosmetic > changes in the attached. I am planning to push this by tomorrow unless you or > others have any more comments or suggestions for this patch. Thanks for the review. I

RE: Parallel Inserts in CREATE TABLE AS

> > Seems like v22 patch was failing in cfbot for one of the unstable test > > cases. > > Attaching v23 patch set with modification in 0003 and 0004 patches. No > > changes to 0001 and 0002 patches. Hopefully cfbot will be happy with v23. > > > > Please consider v23 for further review. > Hi, >

Questions about CommandIsReadOnly

Hi hackers, When reading the code, I found that in function CommandIsReadOnly[1], "select for update/share" is defined as "not read only". [1]- if (pstmt->rowMarks != NIL) return false; /* SELECT FOR [KEY] UPDATE/SHARE */ - And from

RE: Parallel INSERT (INTO ... SELECT ...)

> > 2. Should we keep the default value of GUC to on or off? It is > > currently off. I am fine keeping it off for this release and we can > > always turn it on in the later releases if required. Having said that, > > I see the value in keeping it on because in many cases Insert ... > > Select

RE: Parallel INSERT (INTO ... SELECT ...)

> Since the "Parallel SELECT for INSERT" patch and related GUC/reloption patch > have been committed, I have now rebased and attached the rest of the original > patchset, > which includes: >- Additional tests for "Parallel SELECT for INSERT" >- Parallel INSERT functionality >- Test and

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

Hi, Thanks for the review. > I noticed some things on the first scan through: > > Patch 0001: > 1) Tidy up the comments a bit: > > Suggest the following update to part of the comments: Changed. > Patch 0002: > 1) The new max_parallel_hazard_context member "pk_rels" is not being > set (to >

RE: Parallel INSERT (INTO ... SELECT ...)

> > I noticed that some comments may need updated since we introduced > parallel insert in this patch. > > > > 1) src/backend/executor/execMain.c > > * Don't allow writes in parallel mode. Supporting UPDATE and > DELETE > > * would require (a) storing the combocid hash in shared

RE: should INSERT SELECT use a BulkInsertState?

Hi, About the 0002-patch [Check for volatile defaults]. I wonder if we can check the volatile default value by traversing "query->targetList" in planner. IMO, the column default expression was written into the targetList, and the current parallel-safety check travere the "query->targetList"

RE: Parallel Inserts in CREATE TABLE AS

> They are pretty simple though. I think someone can also check if the same > regression exists for parallel inserts in "INSERT INTO SELECT" > patch set as well for larger tuple sizes. Thanks for reminding. I did some performance tests for parallel inserts in " INSERT INTO SELECT " with the

RE: Parallel Inserts in CREATE TABLE AS

> > They are pretty simple though. I think someone can also check if the > > same regression exists for parallel inserts in "INSERT INTO SELECT" > > patch set as well for larger tuple sizes. > > Thanks for reminding. > I did some performance tests for parallel inserts in " INSERT INTO SELECT " >

RE: Parallel INSERT (INTO ... SELECT ...)

> On Thu, Mar 11, 2021 at 01:01:42PM +0000, houzj.f...@fujitsu.com wrote: > > > I guess to have the finer granularity we'd have to go with > > > enable_parallel_insert, which then would mean possibly having to > > > later add enable_parallel_update, should para

RE: Parallel INSERT (INTO ... SELECT ...)

> > The problem is that target_rel_trigger_max_parallel_hazard and its > > caller think they can use a relcache TriggerDesc field across other > > cache accesses, which they can't because the relcache doesn't > > guarantee that that won't move. > > > > One approach would be to add logic to

RE: Parallel INSERT (INTO ... SELECT ...)

> > If a table parameter value is set and the > > equivalent toast. parameter is not, the TOAST table > > will use the table's parameter value. > > -Specifying these parameters for partitioned tables is not supported, > > -but you may specify them for individual leaf

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

> Attaching the first version patch which avoid CCI in RI trigger when insert > into > referencing table. After some more on how to support parallel insert into fk relation. It seems we do not have a cheap way to implement this feature. Please see the explanation below: In RI_FKey_check,

RE: Parallel INSERT (INTO ... SELECT ...)

> On Fri, Mar 12, 2021 at 6:10 AM Justin Pryzby > wrote: > > Note also this CF entry > > https://commitfest.postgresql.org/32/2987/ > > | Allow setting parallel_workers on partitioned tables > > +/* > + * PartitionedOptions > + * Contents of rd_options for

RE: simplifying foreign key/RI checks

Hi amit, (sorry about not cc the hacker list) I have an issue about command id here. It's probably not directly related to your patch, so I am sorry if it bothers you. + /* +* Start the scan. To make the changes of the current command visible to +* the scan and for

RE: Parallel INSERT (INTO ... SELECT ...)

> I add some code to track the time spent in index operation. > From the results[1], we can see more workers will bring more cost in > _bt_search_insert() in each worker. > After debugged, the most cost part is the following: > - > /* drop the read lock on the page, then acquire

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

> > > I'm worried about having this dependency in RI check, because the > > > planner > > may allow parallel INSERT in these cases in the future. > > > > If we support parallel insert that can have other modifications in the > > future, I think we will also be able to share or increment command

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

Hi, > Why do we have to increment the command ID when the INSERT's target table > is a referenced table? Please refer to the explanation below. > > And INSERT command only have one target table, the modification on > > other tables can happen in the following cases. > > > > 1) has modifyingcte

RE: Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

> From the wiki[1], CCI is to let statements can not see the rows they modify. Sorry, a typo here "not". I meant CCI is to let statements can see the rows they modify. Best regards, houzj

Avoid CommandCounterIncrement in RI trigger when INSERT INTO referencing table

Hi hackers, Currently, postgres increments command id in ri trigger every time when inserting into a referencing table(fk relation). RI_FKey_check-> ri_PerformCheck->SPI_execute_snapshot-> _SPI_execute_plan-> CommandCounterIncrement It can be a block for supporting "parallel insert into a

RE: should INSERT SELECT use a BulkInsertState?

> > I am very interested in this patch, and I plan to do some experiments with > > the > patch. > > Can you please rebase the patch because it seems can not applied to the > master now. > > Thanks for your interest. > > I was sitting on a rebased version since the bulk FDW patch will cause >

RE: should INSERT SELECT use a BulkInsertState?

> > > I am very interested in this patch, and I plan to do some > > > experiments with the > > patch. > > > Can you please rebase the patch because it seems can not applied to > > > the > > master now. > > > > Thanks for your interest. > > > > I was sitting on a rebased version since the bulk FDW

RE: Parallel INSERT (INTO ... SELECT ...)

> > > > I've attached an updated set of patches with the suggested locking changes. > > > > Amit L, others, do let me know if you have still more comments on > 0001* patch or if you want to review it further? I took a look into the latest 0001 patch, and it looks good to me. Best regards, houzj

RE: A reloption for partitioned tables - parallel_workers

> > 4. Maybe it also doesn't make sense to consider the parent relation's > > parallel_workers if Parallel Append is disabled > > (enable_parallel_append = off). That's because with a simple > > (non-parallel) Append running under Gather, all launched parallel > > workers process the same

RE: Parallel INSERT (INTO ... SELECT ...)

> > It is quite possible what you are saying is correct but I feel that is > > not this patch's fault. So, won't it better to discuss this in a > > separate thread? > > > > Good use case but again, I think this can be done as a separate patch. > > Agreed. > I think even the current patch offers

RE: A reloption for partitioned tables - parallel_workers

> > It seems the patch does not include the code that get the > > parallel_workers from new struct " PartitionedTableRdOptions ", Did I miss > something ? > > Aren't the following hunks in the v2 patch what you meant? > > diff --git a/src/backend/access/common/reloptions.c >

RE: Table refer leak in logical replication

> > ... FWIW, I'd rather > > agree to use what has been proposed with es_opened_result_relations > > like TRUNCATE does rather than attempt to use ExecInitResultRelation() > > combined with potentially asymmetric calls to > > ExecCloseResultRelations(). > > Okay, how about the attached then? I

RE: [bug?] Missed parallel safety checks, and wrong parallel safety

> I think we've found a few existing problems with handling the parallel safety > of > functions while doing an experiment. Could I hear your opinions on what we > should do? I'd be willing to create and submit a patch to fix them. > > The experiment is to add a parallel safety check in

RE: Parallel INSERT (INTO ... SELECT ...)

> Posting a new version of the patches, with the following updates: > - Moved the update of glob->relationOIDs (i.e. addition of partition OIDs that > plan depends on, resulting from parallel-safety checks) from within > max_parallel_hazard() to set_plan_references(). > - Added an extra test for

RE: Determine parallel-safety of partition relations for Inserts

Hi, Attaching v7 patches with the changes: * rebase the code on the greg's latest parallel insert patch. Please consider it for further review. Best regards, houzj v7_0004-reloption-parallel_dml-test-and-doc.patch Description: v7_0004-reloption-parallel_dml-test-and-doc.patch

RE: Parallel INSERT SELECT take 2

> > BACKGROUND > > > > > > We want to realize parallel INSERT SELECT in the following steps: > > 1) INSERT + parallel SELECT > > 2) Parallel INSERT + parallel SELECT > > > > Below are example use cases. We don't expect high concurrency or an > > empty data

  1   2   3   4   5   >