On Tues, Sep 28, 2021 6:05 PM Amit Kapila wrote:
> On Tue, Sep 28, 2021 at 11:35 AM Masahiko Sawada
> wrote:
> >
> > On Tue, Sep 28, 2021 at 1:54 PM Amit Kapila
> wrote:
> > >
> > > >
> > > > Then, if, we proceed in this direction,
> > > > the place to implement those stats
> > > > would be on
> hi,
>
> Here we go, my first patch... solves
> https://www.postgresql.org/message-id/7d6fdc20-857c-4cbe-ae2e-c0ff9520
> e...@www.fastmail.com
>
Hi,
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+ static const relopt_parse_elt tab[] = {
+
Hi,
Attaching v6 patches with the changes:
* rebase the code on the greg's latest parallel insert patch.
* fix some code comment.
* add some test to cover the partitioned table.
Please consider it for further review.
Best regards,
Houzj
> > > It did have performance gain, but I think it's not huge enough to
> > > ignore the extra's index cost.
> > > What do you think ?
> >
> > Yes... as you suspect, I'm afraid the benefit from parallel bitmap
> > scan may not compensate for the loss of the parallel insert operation.
> >
> > The
> What are the results if disable the bitmap heap scan(Set enable_bitmapscan
> = off)? If that happens to be true, then we might also want to consider
> if in some way we can teach parallel insert to cost more in such cases.
> Another thing we can try is to integrate a parallel-insert patch with
> > >
> > > else if (IsA(node, Query))
> > > {
> > > Query *query = (Query *) node;
> > >
> > > /* SELECT FOR UPDATE/SHARE must be treated as unsafe */
> > > if (query->rowMarks != NULL)
> > > {
> > > context->max_hazard =
> > --
> > postgres=# select pg_size_pretty(pg_indexes_size('testscan_index'));
> > pg_size_pretty
> >
> > 4048 kB
> > (1 row)
> >
> > postgres=# select pg_size_pretty(pg_relation_size('testscan_index'));
> > pg_size_pretty
> >
> > 4768 kB
> > Till now, what I found is that:
> > With tang's conf, when doing parallel insert, the walrecord is more
> > than serial insert (IMO, this is the main reason why it has
> > performance degradation) See the attatchment for the plan info.
> >
> > I have tried alter the target table to unlogged
> > > postgres=# explain verbose insert into testscan select a from x
> > > where
> > > a<8 or (a%2=0 and a>19990);
> > > QUERY PLAN
> > >
>
> > > --
> > >
> > Did it actually use a parallel plan in your testing?
> > When I ran these tests with the Parallel INSERT patch applied, it did
> > not naturally choose a parallel plan for any of these cases.
>
> Yes, these cases pick parallel plan naturally on my test environment.
>
> postgres=# explain
> Posting an updated set of patches.
A minor comment about doc.
+
+Where the above target table features are determined to be, at worst,
+parallel-restricted, rather than parallel-unsafe, at least a parallel table
+scan may be used in the query plan for the INSERT
+statement.
> > >
> > > static Query *
> > > rewriteRuleAction(Query *parsetree,
> > > ...
> > > if (sub_action_ptr)
> > > + {
> > > *sub_action_ptr = sub_action;
> > > + rule_action->hasModifyingCTE |=
> >
> >
> > static Query *
> > rewriteRuleAction(Query *parsetree,
> > ...
> > if (sub_action_ptr)
> > + {
> > *sub_action_ptr = sub_action;
> > + rule_action->hasModifyingCTE |=
> parsetree->hasModifyingCTE;
> > +
> > I took a look into the hasModifyingCTE bugfix recently, and found a
> > possible bug case without the parallel insert patch.
> >
> > -
> > drop table if exists test_data1;
> > create table test_data1(a int, b int) ; insert into test_data1 select
> >
Hi,
I took a look into the hasModifyingCTE bugfix recently,
and found a possible bug case without the parallel insert patch.
-
drop table if exists test_data1;
create table test_data1(a int, b int) ;
insert into test_data1 select generate_series(1,1000),
> For v3_0003-reloption-parallel_dml-src.patch :
> + table_close(rel, NoLock);
> Since the rel would always be closed, it seems the return value from
> RelationGetParallelDML() can be assigned to a variable, followed by call to
> table_close(), then the return statement.
Thanks for the
Hi,
Attaching v5 patches with the changes:
* rebase the code on the greg's latest parallel insert patch
* fix some code style.
Please consider it for further review.
Best regards,
Houzj
v5_0004-reloption-parallel_dml-test-and-doc.patch
Description:
>
> I've had a further look at this, and this walker function is doing a lot
> of work recursing the parse tree, and I'm not sure that it reliably retrieves
> the information that we;re looking for, for all cases of different SQL
> queries. Unless it can be made much more efficient and specific
> > IMO, It seems more readable to extract all the check that we can do
> > before the safety-check and put them in the new function.
> >
> > Please consider it for further review.
> >
>
> I've updated your v2 patches and altered some comments and documentation
> changes (but made no code
Hi,
When developing the reloption patch, I noticed some issues in the patch.
1).
> - Reduce Insert parallel-safety checks required for some SQL, by noting
> that the subquery must operate on a relation (check for RTE_RELATION in
> subquery range-table)
+ foreach(lcSub,
Hi greg,
Thanks for the review !
> Personally, I think a table's "parallel_dml" option should be ON by default.
> It's annoying to have to separately enable it for each and every table being
> used, when I think the need to turn it selectively OFF should be fairly
> rare.
Yes, I agreed.
> Hi,
>
> Attatching v1 patches, introducing options which let user manually control
> whether to use parallel dml.
>
> About the patch:
> 1. add a new guc option: enable_parallel_dml (boolean) 2. add a new
> tableoption: parallel_dml (boolean)
>
>The default of both is off(false).
>
>
Hi,
Attatching v1 patches, introducing options which let user manually control
whether to use parallel dml.
About the patch:
1. add a new guc option: enable_parallel_dml (boolean)
2. add a new tableoption: parallel_dml (boolean)
The default of both is off(false).
User can set
> > Hi
> >
> > I have an issue about the parameter for DML.
> >
> > If we define the parameter as a tableoption.
> >
> > For a partitioned table, if we set the parent table's parallel_dml=on,
> > and set one of its partition parallel_dml=off, it seems we can not divide
> the plan for the separate
> From: Amit Kapila
> > Good question. I think if we choose to have a separate parameter for
> > DML, it can probably a boolean to just indicate whether to enable
> > parallel DML for a specified table and use the parallel_workers
> > specified in the table used in SELECT.
>
> Agreed.
Hi
I
> >> Combocid's ==>> Combocids
> >
> > Ping again, just in case it’s missed.
>
> There are in the comments "ComboCIDs", "combocids" and "ComboCids" on top
> of the existing Combocid's. Your patch introduces a fourth way of writing
> that. It may be better to just have one way to govern them
> I found a possible typo in reorderbuffer.c
>
> * has got a combocid. Combocid's are only valid for the duration
> of a
>
> Combocid's ==>> Combocids
>
> Attatching a small patch to correct it.
>
Ping again, just in case it’s missed.
Hi,
When testing the patch with the following kind of sql.
---
Insert into part_table select 1;
Insert into part_table select generate_series(1,1,1);
Insert into part_table select * from testfunc();
---
we usually use these sqls to initialize the table or for testing purpose.
Personally I
> I think we can allow parallel insert in this case, because the column value
> is determined according to the DEFAULT definition of the target table
> specified in the INSERT statement. This is described here:
>
> https://www.postgresql.org/docs/devel/sql-createtable.html
>
> "Defaults may be
Hi,
I have an issue of the check about column default expressions.
+ if (command_type == CMD_INSERT)
+ {
+ /*
+* Column default expressions for columns in the target-list are
+* already being checked for parallel-safety in the
+
Hi,
When reading the code of rel_max_parallel_hazard_for_modify in 0001.
I thought there are so many places call table_close().
Personally, It's a little confused to me.
Do you think it's better to do the table_open/close outside of
rel_max_parallel_hazard_for_modify ?
Like:
static bool
Hi,
After doing some test to cover the code path in the PATCH 0001.
I have some suggestions for the 0002 testcase.
(1)
+ /* Check parallel-safety of any expressions in the
partition key */
+ if (get_partition_col_attnum(pkey, i) == 0)
+
> > And there seems another solution for this:
> >
> > In the patch, We only use the { ii_Expressions , ii_NumIndexAttrs ,
> > ii_IndexAttrNumbers } from the IndexInfo, which seems can get from
> "Relation-> rd_index".
> >
> > Based on above, May be we do not need to call BuildIndexInfo to build
Hi
I took a look at v12-0001 patch, here are some comments:
1.
+ /*
+* Setup the context used in finding the max parallel-mode hazard.
+*/
+ Assert(initial_max_parallel_hazard == 0 ||
+ initial_max_parallel_hazard == PROPARALLEL_SAFE ||
+
> >
> > +
> > + index_oid_list = RelationGetIndexList(rel);
> > ...
> >
> > As memtioned in the comments of RelationGetIndexList:
> > * we return a copy of the list palloc'd in the caller's context. The
> > caller
> > * may list_free() the returned list after scanning it.
> >
> > Shall we
> > So I think we're saying that if the target table is a foreign table or
> > temporary table, it can be regarded as PARALLEL_RESTRICTED, right?
> >
>
> Yes
>
> IMO, PARALLEL_RESTRICTED currently enable parallel select but disable
> parallel insert.
> So, the INSERT only happen in leader worker
> > > Hi
> > >
> > > > > I may be wrong, and if I miss sth in previous mails, please give
> > > > > me some
> > > > hints.
> > > > > IMO, serial insertion with underlying parallel SELECT can be
> > > > > considered for foreign table or temporary table, as the
> > > > > insertions only
> > > >
> > > Attaching v15 patch set. Please consider it for further review.
> >
> > Hi
> >
> > I have some comments for the 0001 patch
> >
> > In v15-0001-postgres_fdw-function-to-discard-cached-connecti
> >
> > 1.
> > + If there is no open connection to the given foreign server,
> false
> > +
> Attaching v15 patch set. Please consider it for further review.
Hi
I have some comments for the 0001 patch
In v15-0001-postgres_fdw-function-to-discard-cached-connecti
1.
+ If there is no open connection to the given foreign server,
false
+ is returned. If no foreign server with
>
> Thanks for the feedback.
> Posting an updated set of patches. Changes are based on feedback, as detailed
> below:
Hi
It seems there are some previous comments[1][2] not addressed in current patch.
Just to make sure it's not missed.
[1]
Hi
Thanks for the review.
> It's a bit unfortunate now that between OpExpr, DistinctExpr, NullIfExpr,
> and to a lesser extent ScalarArrayOpExpr we will now have several different
> implementations of nearly the same thing, without any explanation why one
> approach was chosen here and another
> >>> +1 to add it after "dropped (Note )", how about as follows
> >>> with slight changes?
> >>>
> >>> dropped (Note that server name of an invalid connection can be NULL
> >>> if the server is dropped), and then such .
> >>
> >> Yes, I like this one. One question is; "should" or "is"
> We need to create the loopback3 with user mapping public, otherwise the
> test might become unstable as shown below. Note that loopback and
> loopback2 are not dropped in the test, so no problem with them.
>
> ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off'); DROP
> SERVER
> On Thu, Jan 14, 2021 at 5:36 PM Li Japin wrote
> > Do we really need to access PUBLICATIONRELMAP in this patch? What if
> > we just set it to false in the else condition of (if (publish &&
> > (relkind != RELKIND_PARTITIONED_TABLE || pub->pubviaroot)))
> >
> > Thank for you review. I agree with
Hi
I found a possible typo in reorderbuffer.c
* has got a combocid. Combocid's are only valid for the duration of a
Combocid's ==>> Combocids
Attatching a small patch to correct it.
Best regards,
houzj
0001-fix-typo.patch
Description: 0001-fix-typo.patch
> >>> Your observation seems correct to me, though I have not tried to
> >>> test your patch.
> >>
> >> +1 to apply, this looks correct and passes tests. Scanning around I
> >> +didn't see
> >> any other occurrences of the same pattern.
> >
> > Thanks. I am thinking to backpatch this even though
> I am not exactly sure of the concern. (If the extra info below does not
> help can you please describe your concern with more details).
>
> This [v14] patch code/feature is only referring to the immediate stopping
> of only the *** "tablesync" *** worker (if any) for any/each table being
>
Hi
In function fetch_table_list, it get the table names from publicer and return a
list of tablenames.
When append the name to the list, it use the following code:
**
nspname = TextDatumGetCString(slot_getattr(slot, 1, ));
Assert(!isnull);
relname =
> Also PSA some detailed logging evidence of some test scenarios involving
> Drop/AlterSubscription:
> + Test-20210112-AlterSubscriptionRefresh-ok.txt =
> AlterSubscription_refresh which successfully drops a tablesync slot
> + Test-20210112-AlterSubscriptionRefresh-warning.txt =
>
> > I think this patch should be about a tenth the size. Try modeling it
> > on the T_SubscriptingRef-etc case, ie, use ece_generic_processing and
> > then ece_evaluate_expr to cover the generic cases. OpExpr is common
> > enough to deserve specially optimized code, but NullIf isn't, so shorter
> > I notice that there are no Nullif case in eval_const_expression.
> > Since Nullif is similar to Opexpr and is easy to implement, I try add
> > this case in eval_const_expressions_mutator.
>
> I think this patch should be about a tenth the size. Try modeling it on
> the T_SubscriptingRef-etc
> Attaching v21 patch set, which has following changes:
> 1) 0001 - changed fpes->ins_cmd_type ==
> PARALLEL_INSERT_CMD_CREATE_TABLE_AS to fpes->ins_cmd_type !=
> PARALLEL_INSERT_CMD_UNDEF
> 2) 0002 - reworded the commit message.
> 3) 0003 - added cmin, xmin test case to one of the parallel insert
> PSA the v12 patch for the Tablesync Solution1.
>
> Differences from v11:
> + Added PG docs to mention the tablesync slot
> + Refactored tablesync slot drop (done by
> DropSubscription/process_syncing_tables_for_sync)
> + Fixed PG docs mentioning wrong state code
> + Fixed wrong code
> PSA the v11 patch for the Tablesync Solution1.
>
> Difference from v10:
> - Addresses several recent review comments.
> - pg_indent has been run
>
Hi
I took a look into the patch and have some comments.
1.
* So the state progression is always: INIT -> DATASYNC -> SYNCWAIT ->
- *
> >
> > + /* Okay to parallelize inserts, so mark it. */
> > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> > + ((DR_intorel *) dest)->is_parallel = true;
> >
> > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> > +
> > I think it makes sense.
> >
> > And if the check about ' ins_cmd == xxx1 || ins_cmd == xxx2' may be
> > used in some places, How about define a generic function with some comment
> to mention the purpose.
> >
> > An example in INSERT INTO SELECT patch:
> > +/*
> > + *
> > For v20-0001-Parallel-Inserts-in-CREATE-TABLE-AS.patch :
> >
> > ParallelInsCmdEstimate :
> >
> > + Assert(pcxt && ins_info &&
> > + (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS));
> > +
> > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> >
> > Sinc the if condition is
Hi
I found a possible typo in the comment of ReorderBufferProcessTXN().
* relmapper has no "historic" view, in contrast to normal
* the normal catalog during decoding. Thus repeated
Is there an extra ‘normal’ in the comment ?
in contrast to normal the normal catalog during decoding == >> in
Hi
>
> wrt v18-0002patch:
>
> It looks like this introduces a state machine that goes like:
> - starts at CTAS_PARALLEL_INS_UNDEF
> - possibly moves to CTAS_PARALLEL_INS_SELECT
> - CTAS_PARALLEL_INS_TUP_COST_CAN_IGN can be added
> - if both were added at some stage, we can
> Yeah without explain analyze we can not show whether the parallelism is
> picked in the test cases. What we could do is that we can add a plain RMV
> test case in write_parallel.sql after CMV so that at least we can be ensured
> that the parallelism will be picked because of the enforcement
Hi
> see new version in attachment.
I took a look into the patch, and have some comments.
1.
+ PG_FINALLY();
+ {
+ copy_fmstate = NULL; /* Detect problems */
I don't quite understand this comment,
does it means we want to detect something like Null reference ?
2.
+
Hi
> Yes this optimization can be done, I will handle this in the next patch
> set.
>
I have a suggestion for the parallel safety-check.
As designed, The leader does not participate in the insertion of data.
If User use (PARALLEL 1), there is only one worker process which will do the
> Thanks for taking a look at the patch.
>
> The intention of the patch is to just enable the parallel mode while planning
> the select part of the materialized view, but the insertions do happen in
> the leader backend itself. That way even if there's temporary tablespace
> gets created, we have
Hi
> > I may be wrong, and if I miss sth in previous mails, please give me some
> hints.
> > IMO, serial insertion with underlying parallel SELECT can be
> > considered for foreign table or temporary table, as the insertions only
> happened in the leader process.
> >
>
> I don't think we support
Hi
I have an issue about the parallel-safety checks.
If target table is foreign table or temporary table,
rel_max_parallel_hazard_for_modify will return PROPARALLEL_UNSAFE,
which not only disable parallel insert but also disable underlying parallel
SELECT.
+create temporary table temp_names
Hi
> Added this to commitfest, in case it is useful -
> https://commitfest.postgresql.org/31/2856/
I have an issue about the safety of enable parallel select.
I checked the [parallel insert into select] patch.
https://commitfest.postgresql.org/31/2844/
It seems parallel select is not allowed
Hi
> Posting an updated set of patches to address recent feedback:
>
> - Removed conditional-locking code used in parallel-safety checking code
> (Tsunakawa-san feedback). It turns out that for the problem test case, no
> parallel-safety checking should be occurring that locks relations because
Hi
The cfbost seems complains about the testcase:
Command exited with code 1
perl dumpregr.pl
=== $path ===\ndiff -w -U3
C:/projects/postgresql/src/test/regress/expected/write_parallel.out
C:/projects/postgresql/src/test/regress/results/write_parallel.out
---
Hi
I have an issue about the existing testcase.
"""
-- Test that alteration of server options causes reconnection SELECT c3, c4
FROM ft1 ORDER BY c3, c1 LIMIT 1; -- should work ALTER SERVER loopback OPTIONS
(SET dbname 'no such database'); SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT
1; --
Hi
> Discussion here is on the point - whether to show up the invalidated
> connections in the output of the new postgres_fdw_get_connections()
> function? If we were to show, then because of the solution we proposed for
> the connection leak problem in [1], will the invalidated entries be shown
> From: Hou, Zhijie [mailto:houzj.f...@cn.fujitsu.com]
> Sent: Tuesday, December 15, 2020 7:30 PM
> To: Bharath Rupireddy
> Cc: Amit Kapila ; Luc Vlaming ;
> PostgreSQL-development ; Zhihong Yu
> ; Dilip Kumar
> Subject: RE: Parallel Inserts in CREATE TABLE AS
>
>
> Thanks for the append use case.
>
> Here's my analysis on pushing parallel inserts down even in case the top
> node is Append.
>
> For union cases which need to remove duplicate tuples, we can't push the
> inserts or CTAS dest receiver down. If I'm not wrong, Append node is not
> doing
Hi
> Attaching v11 patch set. Please review it further.
Currently with the patch, we can allow parallel CTAS when topnode is Gather.
When top-node is Append and Gather is the sub-node of Append, I think we can
still enable
Parallel CTAS by pushing Parallel CTAS down to the sub-node Gather,
> IMO, let's not change the 1) behaviour to 3) with the patch. If agreed,
> I can do the following way in ExplainOneUtility and will add a comment on
> why we are doing this.
>
> if (es->analyze)
> (void) CheckRelExistenceInCTAS(ctas, true);
>
> Thoughts?
Agreed.
Just in case, I took
> Currently, for CTAS or CREATE MATERIALIZED VIEW(CMV) without if-not-exists
> clause, the existence of the relation gets checked during the execution
> of the select part and an error is thrown there.
> All the unnecessary rewrite and planning for the select part would have
> happened just to
Hi
+ allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
+ plannedstmt->parallelModeNeeded &&
+ plannedstmt->planTree &&
+ IsA(plannedstmt->planTree, Gather) &&
+
Hi
Since ba3e76c,
the optimizer call generate_useful_gather_paths instead of
generate_gather_paths() outside.
But I noticed that some comment still talking about generate_gather_paths not
generate_useful_gather_paths.
I think we should fix these comment, and I try to replace these "
> > I'm not quite sure how to address this. Can we not allow the planner
> > to consider that the select is for CTAS and check only after the
> > planning is done for the Gather node and other checks?
> >
>
> IIUC, you are saying that we should not influence the cost of gather node
> even when
> > 4.
> > A suggestion for CacheLineInfo.
> >
> > It use appendBinaryStringXXX to store the line in memory.
> > appendBinaryStringXXX will double the str memory when there is no enough
> spaces.
> >
> > How about call enlargeStringInfo in advance, if we already know the whole
> line size?
> > It
Hi
+ /*
+* Flag to let the planner know that the SELECT query is for CTAS. This
is
+* used to calculate the tuple transfer cost from workers to gather
node(in
+* case parallelism kicks in for the SELECT part of the CTAS), to zero
as
+* each worker will
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Thanks for the new patch, the patch LGTM and works as expected
Hi,
When test pg_terminate_backend_and_wait with parallel query.
I noticed that the function is not defined as parallel safe.
I am not very familiar with the standard about whether a function should be
parallel safe.
But I found the following function are all defined as parallel safe:
Hi,
-however only superusers can terminate superuser backends.
+however only superusers can terminate superuser backends. When no
+wait and timeout are
+provided, only SIGTERM is sent to the backend with the given process
+ID and false is returned
>
> I changed the status to 'wait on anthor'.
> The others of the patch LGTM,
> I think it can be changed to Ready for Committer again, when this comment
> is confirmed.
>
I am Sorry I forgot a possible typo comment.
+{ oid => '16386', descr => 'terminate a backend process and wait for it\'s
> > + ereport(WARNING,
> > + (errmsg("could not wait for the termination of the
> backend with PID %d within %ld milliseconds",
> > + pid, timeout)));
>
> > The code use %ld to print int64 type.
> > How about use INT64_FORMAT, which looks more
Hi
I take a look into the patch, and here some comments.
1.
+
+ ereport(WARNING,
+ (errmsg("could not wait for the termination of the
backend with PID %d within %ld milliseconds",
+ pid, timeout)));
+
The code use %ld to print
Hi,
In function plan_union_children(),
I found the lcons and list_delete_first here is easy to be replaced by lappend
and list_delete_last.
And I also found a previous commit do similar thing, so I try to improve this
one.
Previous commit:
d97b714a219959a50f9e7b37ded674f5132f93f3
Best
Hi
I look into the patch again and have some comments.
1.
+ Size oid_cmp_len = sizeof(Oid) * ind1->ncolumns;
+
+ return ind1->ncolumns == ind2->ncolumns &&
+ ind1->unique == ind2->unique &&
+ memcmp(ind1->indexkeys, ind2->indexkeys, sizeof(int) *
Hi,
I found IncrementalSortPath type is not analyzed in outNode.
It does not matter during runtime though.
Just for the convenience of debugging,
when debug by gdb, we can use "call pprint(incresortpath)" to list the node
info.
So I try to support IncrementalSortPath in outNode.
Best
Hi,
> > I took a deep look at the projection logic.
> > In most cases, you are right that Gather node does not need projection.
> >
> > In some rare cases, such as Subplan (or initplan I guess).
> > The projection will happen in Gather node.
> >
> > The example:
> >
> > Create table test(i int);
Hi ,
> On Thu, Nov 26, 2020 at 7:47 AM Hou, Zhijie
> wrote:
> >
> > Hi,
> >
> > I have an issue about the following code:
> >
> > econtext = node->ps.ps_ExprContext;
> > ResetExprContext(econte
Hi,
I have an issue about the following code:
econtext = node->ps.ps_ExprContext;
ResetExprContext(econtext);
+ if (ISCTAS(node->ps.intoclause))
+ {
+ ExecParallelInsertInCTAS(node);
+ return NULL;
+ }
/* If no projection
Hi,
I'm very interested in this feature,
and I'm looking at the patch, here are some comments.
1.
+ if (!TupIsNull(outerTupleSlot))
+ {
+ (void)
node->ps.dest->receiveSlot(outerTupleSlot, node->ps.dest);
+
Hi Vignesh,
I took a look at the v10 patch set. Here are some comments:
1.
+/*
+ * CheckExprParallelSafety
+ *
+ * Determine if where cluase and default expressions are parallel safe & do not
+ * have volatile expressions, return true if condition satisfies else return
+ * false.
+ */
'cluase'
Hi
I notice that there are no Nullif case in eval_const_expression.
Since Nullif is similar to Opexpr and is easy to implement,
I try add this case in eval_const_expressions_mutator.
Best regards,
houzj
0001-patch-eval-NULLIF.patch
Description: 0001-patch-eval-NULLIF.patch
Hi
>
> my $bytes = $ARGV[0];
> for(my $i = 0; $i < $bytes; $i+=8){
> print "longdata";
> }
> print "\n";
>
>
> postgres=# copy longdata from program 'perl /tmp/longdata.pl 1'
> with (parallel 2);
>
> This gets stuck forever (or at least I didn't have the patience to wait
Hi
(sorry forget to cc the hacklist)
> Improve psql \df to choose functions by their arguments
I think this is useful.
I found some comments in the patch.
1.
> * Abbreviations of common types is permitted (because who really likes
> to type out "character varying"?), so the above could also
Hi
I found some possible typo in procarray.c and xlogreader.h
- * For VACUUM separate horizons (used to to decide which deleted tuples must
+ * For VACUUM separate horizons (used to decide which deleted tuples must
- * Callers supply a page_read callback if they want to to call
+ *
Hi
I found some issue in v9-0002
1.
+
+ elog(DEBUG1, "[Worker] Processing - line position:%d, block:%d,
unprocessed lines:%d, offset:%d, line size:%d",
+write_pos, lineInfo->first_block,
+pg_atomic_read_u32(_blk_ptr->unprocessed_line_parts),
+
Hi
I found the comment of function get_attgenerated(Oid relid, AttrNumber attnum)
seems wrong.
It seems the function is given the attribute number not the name.
/*
* get_attgenerated
*
- * Given the relation id and the attribute name,
+ * Given the relation id and
1 - 100 of 121 matches
Mail list logo
