On Thur, Jan 20, 2022 7:25 PM Amit Kapila wrote:
>
> On Thu, Jan 20, 2022 at 6:42 AM houzj.f...@fujitsu.com
> wrote:
> >
> > Attach the V68 patch set which addressed the above comments and changes.
> >
>
> Few comments and suggestions:
> ==
On Monday, January 24, 2022 5:36 AM Peter Smith
>
> FYI - I noticed the cfbot is reporting a failed test case [1] for the latest
> v69 patch
> set.
>
> [21:09:32.183] # Failed test 'check replicated inserts on subscriber'
> [21:09:32.183] # at t/025_rep_changes_for_schema.pl line 202.
> [21:09:
On Monday, January 24, 2022 4:38 PM Peter Smith wrote:
>
> Thanks for all the patches!
>
> Here are my review comments for v69-0001
>
> ~~~
>
> 1. src/backend/executor/execReplication.c CheckCmdReplicaIdentity call to
> RelationBuildPublicationDesc
>
> + /*
> + * It is only safe to execute U
On Saturday, January 29, 2022 8:31 AM Andres Freund wrote:
>
> Hi,
>
> Are there any recent performance evaluations of the overhead of row filters? I
> think it'd be good to get some numbers comparing:
Thanks for looking at the patch! Will test it.
> 1) $workload with master
> 2) $workload wit
> On Saturday, January 29, 2022 8:31 AM Andres Freund
> wrote:
> >
> > Hi,
> >
> > Are there any recent performance evaluations of the overhead of row
> > filters? I think it'd be good to get some numbers comparing:
>
> Thanks for looking at the patch! Will test it.
>
> > > case REORDE
On Saturday, November 5, 2022 1:43 PM Amit Kapila
>
> On Fri, Nov 4, 2022 at 7:35 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Friday, November 4, 2022 4:07 PM Amit Kapila
> wrote:
> > >
> > > On Thu, Nov 3, 2022 at 6:36 PM houzj.f...@fujitsu.com
&g
On Monday, November 7, 2022 7:43 PM Kuroda, Hayato/黒田 隼人
wrote:
>
> Dear Hou,
>
> The followings are my comments. I want to consider the patch more, but I sent
> it once.
Thanks for the comments.
>
> ===
> worker.c
>
> 01. typedef enum TransApplyAction
>
> ```
> /*
> * What action to take
On Monday, November 7, 2022 4:17 PM Peter Smith
>
> Here are my review comments for v42-0001
Thanks for the comments.
> ==
>
> 28. handle_streamed_transaction
>
> static bool
> handle_streamed_transaction(LogicalRepMsgType action, StringInfo s) {
> - TransactionId xid;
> + TransactionId
On Tuesday, November 8, 2022 7:50 PM Amit Kapila
wrote:
> On Mon, Nov 7, 2022 at 6:49 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Friday, November 4, 2022 7:45 PM Amit Kapila
> wrote:
> > > 3.
> > > apply_handle_stream_start(StringI
On Monday, November 7, 2022 6:18 PM Masahiko Sawada
wrote:
>
> On Thu, Nov 3, 2022 at 10:06 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Wednesday, November 2, 2022 10:50 AM Masahiko Sawada
> wrote:
> > >
> > > On Mon, Oct 24, 2022 at 8:42 PM Masahi
On Friday, November 18, 2022 8:36 AM Masahiko Sawada
wrote:
>
> Here are review comments on v47-0001 and v47-0002 patches:
Thanks for the comments!
> When the parallel apply worker exited, I got the following server log.
> I think this log is not appropriate since the worker was not terminated
On Monday, November 21, 2022 2:26 PM Peter Smith wrote:
> On Fri, Nov 18, 2022 at 6:03 PM Peter Smith
> wrote:
> >
> > Here are some review comments for v47-0001
> >
> > (This review is a WIP - I will post more comments for this patch next
> > week)
> >
>
> Here are the rest of my comments for
On Tuesday, November 22, 2022 2:49 PM Hayato Kuroda (Fujitsu)
>
> Dear Nathan,
>
> > I think you are correct. I did it this way in v2. I've also moved
> > the bulk of the logic to logical/worker.c.
>
> Thanks for updating! It becomes better. Further comments:
>
> 01. AlterSubscription()
>
Hi,
When doing some other related work, I noticed that when decoding the COMMIT
record via SnapBuildCommitTxn()-> SnapBuildDistributeNewCatalogSnapshot() we
will add a new snapshot to all transactions including the one being
decoded(just committed one).
But since we've already done required modi
On Friday, November 25, 2022 10:54 AM Peter Smith wrote:
>
> Here are some review comments for v51-0001.
Thanks for the comments!
> ==
>
> .../replication/logical/applyparallelworker.c
>
> 1. General - Error messages, get_worker_name()
>
> I previously wrote a comment to ask if the get_wo
On Tuesday, November 22, 2022 9:53 PM Kuroda, Hayato
wroteL
>
> Thanks for updating the patch!
> I tested the case whether the deadlock caused by foreign key constraint could
> be detected, and it worked well.
>
> Followings are my review comments. They are basically related with 0001, but
> so
On Mon, November 28, 2022 15:19 PM Peter Smith wrote:
> Here are some review comments for patch v51-0002
Thanks for your comments!
> ==
>
> 1.
>
> GENERAL - terminology: spool/serialize and data/changes/message
>
> The terminology seems to be used at random. IMO it might be worthwhile
>
On Tuesday, November 29, 2022 12:08 PM Dilip Kumar
wrote:
Hi,
>
> On Mon, Nov 28, 2022 at 3:19 PM Dilip Kumar wrote:
> >
> > On Mon, Nov 28, 2022 at 1:46 PM shiy.f...@fujitsu.com
> > wrote:
> > >
> > > Thanks for your patch.
> > >
> > > I saw that the patch added a check when selecting large
On Thursday, December 1, 2022 3:58 PM Masahiko Sawada
wrote:
>
> On Wed, Nov 30, 2022 at 10:51 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Wednesday, November 30, 2022 9:41 PM houzj.f...@fujitsu.com
> wrote:
> > >
> > > On Tuesday, November 2
On Friday, December 2, 2022 7:27 PM Kuroda, Hayato/黒田 隼人
wroteL
>
> Dear Hou,
>
> Thanks for making the patch. Followings are my comments for v54-0003 and
> 0004.
Thanks for the comments!
>
> 0003
>
> pa_free_worker()
>
> + /* Unlink any files that were needed to serialize partial ch
On Friday, December 2, 2022 4:59 PM Peter Smith wrote:
>
> Here are my review comments for patch v54-0001.
Thanks for the comments!
> ==
>
> FILE: .../replication/logical/applyparallelworker.c
>
> 1b.
>
> + *
> + * This file contains routines that are intended to support setting up,
> +
On Saturday, December 3, 2022 7:37 PM Amit Kapila
wrote:
>
> On Tue, Nov 29, 2022 at 12:23 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Tuesday, November 29, 2022 12:08 PM Dilip Kumar
> wrote:
> >
> > I have few comments about the patch.
> >
>
On Tue, Dec 6, 2022 7:57 AM Peter Smith wrote:
> Here are my review comments for patch v55-0002
Thansk for your comments.
> ==
>
> .../replication/logical/applyparallelworker.c
>
> 1. pa_can_start
>
> @@ -276,9 +278,9 @@ pa_can_start(TransactionId xid)
> /*
> * Don't start a new paral
On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada
wrote:
>
> On Mon, Dec 5, 2022 at 1:29 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Sunday, December 4, 2022 7:17 PM houzj.f...@fujitsu.com
>
> > >
> > > Thursday, December 1, 2022 8:40 PM Amit
On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada
wrote:
>
> On Mon, Dec 5, 2022 at 1:29 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Sunday, December 4, 2022 7:17 PM houzj.f...@fujitsu.com
>
> > >
> > > Thursday, December 1, 2022 8:40 PM Amit
> > ... 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 de
> 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 Functio
> 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 plac
> 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.
> * Data
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 prosrcattr
> > 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
> > 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, insert
> > 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
> SAFE/UNSAFE/RESTRIC
> > 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.
>
> >>> 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, bu
> > 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
mt
> 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 public.vzdalen
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 non-nega
> > > 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 th
> > > > > > > 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 m
> > > 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, cl
> > 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 a
> > > > > 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
> > > > > DM
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 " ?
B
> 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
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, ...)
> > 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 transfe
> 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
> c
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 necessar
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
> f
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
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
>
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 $$
&
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
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
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
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:
> > > >
> > >
> >
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 fmst
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
> r
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: 000
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 =
> TABLE_INSERT_SKIP
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 Partition
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:
> > >>
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 to
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 will
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
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
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
>
> F
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, t
On Monday, January 23, 2023 11:17 AM Amit Kapila
wrote:
>
> On Fri, Jan 20, 2023 at 11:48 AM Masahiko Sawada
> wrote:
> >
> > >
> > > Yet another way is to use the existing parameter logical_decode_mode
> > > [1]. If the value of logical_decoding_mode is 'immediate', then we
> > > can immediate
On Monday, January 23, 2023 8:51 AM Peter Smith wrote:
>
> Here are my review comments for patch v4-0001
> ==
> Commit message
>
> 2.
>
> The problem is when there is a DDL in a transaction that generates lots of
> temporary data due to rewrite rules, these temporary data will not be
> pro
On Monday, January 23, 2023 3:13 AM Tom Lane wrote:
Hi,
>
> Nathan Bossart writes:
> > On Tue, Jan 10, 2023 at 10:59:14AM +0530, Amit Kapila wrote:
> >> I haven't looked in detail but isn't it better to explain somewhere
> >> in the comments that it achieves to rate limit the restart of worker
On Tuesday, January 24, 2023 3:19 PM Peter Smith wrote:
>
> Here are some review comments for v86-0002
>
> ==
> Commit message
>
> 1.
> Use the use the existing developer option logical_replication_mode to test the
> parallel apply of large transaction on subscriber.
>
> ~
>
> Typo “Use t
On Tuesday, January 24, 2023 11:43 AM Peter Smith wrote:
>
> Here are my review comments for patch v86-0001.
Thanks for your comments.
>
>
> ==
> Commit message
>
> 2.
> Since we may extend the developer option logical_decoding_mode to to test the
> parallel apply of large transaction o
On Monday, January 23, 2023 8:34 PM Kuroda, Hayato wrote:
>
> Followings are my comments.
Thanks for your comments.
>
> 1. guc_tables.c
>
> ```
> static const struct config_enum_entry logical_decoding_mode_options[] = {
> - {"buffered", LOGICAL_DECODING_MODE_BUFFERED, false},
> -
On Tuesday, January 24, 2023 8:47 PM Hou, Zhijie wrote:
>
> On Tuesday, January 24, 2023 3:19 PM Peter Smith
> wrote:
> >
> > Here are some review comments for v86-0002
> >
Sorry, the patch set was somehow attached twice. Here is the correct new version
patch set which addressed all comments so
On Wednesday, January 25, 2023 7:30 AM Peter Smith
wrote:
>
> Here are my review comments for patch v87-0002.
Thanks for your comments.
> ==
> doc/src/sgml/config.sgml
>
> 1.
>
> -Allows streaming or serializing changes immediately in
> logical decoding.
> The al
On Wednesday, January 25, 2023 7:26 PM Amit Kapila
>
> On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com
> wrote:
> >
> > Attach the new patch.
> >
>
> I think the patch missed to handle the case of non-transactional messages
> which
> was previously getting handled. I have tried to addre
On Friday, January 27, 2023 8:16 PM Amit Kapila
>
> On Fri, Jan 27, 2023 at 3:45 PM vignesh C wrote:
> >
> > On Mon, 23 Jan 2023 at 10:52, Amit Kapila
> wrote:
> > >
> > > IIRC, this is done to prevent concurrent drops of origin drop say by
> > > exposed API pg_replication_origin_drop(). See th
On Monday, January 30, 2023 12:13 PM Peter Smith wrote:
>
> Here are my review comments for v88-0002.
Thanks for your comments.
>
> ==
> General
>
> 1.
> The test cases are checking the log content but they are not checking for
> debug logs or untranslated elogs -- they are expecting a no
On Thursday, January 26, 2023 11:37 AM Kuroda, Hayato/黒田 隼人
wrote:
>
> Followings are comments.
Thanks for the comments.
> In this test the rollback-prepared seems not to be executed. This is because
> serializations are finished while handling PREPARE message and the final
> state of transact
On Monday, January 30, 2023 2:32 PM Amit Kapila wrote:
>
> On Mon, Jan 30, 2023 at 9:20 AM vignesh C wrote:
> >
> > On Sat, 28 Jan 2023 at 11:26, Amit Kapila wrote:
> > >
> > > One thing that looks a bit odd is that we will anyway have a similar
> > > check in replorigin_drop_guts() which is a
On Monday, January 30, 2023 10:20 PM Masahiko Sawada
wrote:
>
>
> I have one comment on v89 patch:
>
> + /*
> +* Using 'immediate' mode returns false to cause a switch to
> +* PARTIAL_SERIALIZE mode so that the remaining changes will
> be serialized.
> +*/
> +
On Tuesday, January 31, 2023 8:23 AM Peter Smith wrote:
>
> On Mon, Jan 30, 2023 at 5:23 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Monday, January 30, 2023 12:13 PM Peter Smith
> wrote:
> > >
> > > Here are my review comments for
On Tuesday, January 31, 2023 10:49 PM Tom Lane wrote:
Hi,
> Amit Kapila writes:
> > On Tue, Jan 31, 2023 at 4:25 AM Tom Lane wrote:
> >> Hmph. I generally think that options defined like this (it's a
> >> boolean, except it isn't) are a bad idea, and would prefer to see
> >> that API rethough
On Tuesday, January 31, 2023 1:07 AM vignesh C wrote:
> On Mon, 30 Jan 2023 at 17:30, vignesh C wrote:
> >
> > On Mon, 30 Jan 2023 at 13:00, houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Monday, January 30, 2023 2:32 PM Amit Kapila
> wrote:
>
On Friday, February 3, 2023 11:04 AM Amit Kapila
wrote:
>
> On Thu, Feb 2, 2023 at 4:52 AM Peter Smith
> wrote:
> >
> > Some minor review comments for v91-0001
> >
>
> Pushed this yesterday after addressing your comments!
Thanks for pushing.
Currently, we have two remaining patches which we
On Thursday, February 2, 2023 7:21 PM Amit Kapila
wrote:
>
> On Thu, Feb 2, 2023 at 12:05 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Tuesday, January 31, 2023 1:07 AM vignesh C
> wrote:
> > > On Mon, 30 Jan 2023 at 17:30, vignesh C wrote:
> > >
Hi,
while reading the code, I noticed that in pa_send_data() we set wait event
to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while sending the
message to the queue. Because this state is used in multiple places, user might
not be able to distinguish what they are waiting for. So It seems we'd
On Monday, February 6, 2023 6:34 PM Kuroda, Hayato
wrote:
> > while reading the code, I noticed that in pa_send_data() we set wait
> > event to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while
> sending
> > the message to the queue. Because this state is used in multiple
> > places, user migh
On Tuesday, February 7, 2023 12:12 PM Peter Smith wrote:
> On Fri, Feb 3, 2023 at 6:58 PM houzj.f...@fujitsu.com
> wrote:
> >
> ...
> > > Right, I think that case could be addressed by Tom's patch to some
> > > extent but I am thinking we should also try
On Tuesday, February 7, 2023 11:17 AM Amit Kapila
wrote:
>
> On Mon, Feb 6, 2023 at 3:43 PM houzj.f...@fujitsu.com
> wrote:
> >
> > while reading the code, I noticed that in pa_send_data() we set wait
> > event to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE whil
On Tuesday, February 14, 2023 9:44 AM Peter Smith wrote:
>
> FYI - the latest patch cannot be applied.
>
Thanks for reporting. I will post a rebased patch after fixing some of the
comments raised so far(in a day or so).
Best regards,
Hou zj
On Wednesday, February 15, 2023 10:34 AM Amit Kapila
wrote:
>
> On Tue, Feb 14, 2023 at 7:45 PM Masahiko Sawada
> wrote:
> >
> > On Tue, Feb 14, 2023 at 3:58 PM Peter Smith
> wrote:
> > >
> > > On Tue, Feb 14, 2023 at 5:04 PM Amit Kapila
> wrote:
> > > >
> > > > On Fri, Feb 10, 2023 at 8:56 A
On Wednesday, February 15, 2023 5:51 PM Amit Kapila
wrote:
>
> On Wed, Feb 15, 2023 at 2:02 PM Alvaro Herrera
> wrote:
> >
> > On 2023-Feb-15, Peter Smith wrote:
> >
> > > On Thu, Feb 9, 2023 at 8:55 PM Ajin Cherian wrote:
> > > >
> > > > On Fri, Feb 3, 2023 at 11:41 AM Peter Smith
> wrote:
>
On Wed, Feb 15, 2023 at 13:57 PM Amit Kapila wrote:
> On Fri, Feb 10, 2023 at 8:23 PM Masahiko Sawada
>
> wrote:
> >
> > On Thu, Feb 9, 2023 at 6:55 PM Ajin Cherian wrote:
> > >
> > (v67)
> >
> > I have some questions about adding the infrastructure for DDL deparsing.
> >
> > Apart from the cha
On Monday, September 26, 2022 4:57 PM Amit Kapila
>
> On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera
> wrote:
> >
> > On 2022-Sep-26, Amit Kapila wrote:
> >
> > > On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera
> wrote:
> >
> > > > ERROR: cannot use column list for relation "%s.%s" in publicati
On Monday, September 26, 2022 5:03 PM houzj.f...@fujitsu.com wrote:
>
> On Monday, September 26, 2022 4:57 PM Amit Kapila
>
> >
> > On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera
> >
> > wrote:
> > >
> > > On 2022-Sep-26, Amit Kapila wrote:
&
On Tuesday, September 27, 2022 3:21 PM Alvaro Herrera
wrote:
>
> On 2022-Sep-27, Kyotaro Horiguchi wrote:
>
> > By the way, this is not an issue caused by the proposed patch, I see
> > the following message in the patch.
> >
> > -errdetail("Column list cannot
On Monday, September 26, 2022 6:58 PM Amit Kapila
wrote:
>
> On Mon, Sep 26, 2022 at 8:41 AM wangw.f...@fujitsu.com
> wrote:
> >
> > On Thur, Sep 22, 2022 at 18:12 PM Amit Kapila
> wrote:
> >
> > > 3.
> > > ApplyWorkerMain()
> > > {
> > > ...
> > > ...
> > > +
> > > + if (server_version >= 160
201 - 300 of 482 matches
Mail list logo