Dear Fujii-san,
> Isn't it a very special case where many FDWs use their own user timeouts?
> Could
> you tell me the assumption that you're thinking, especially how many FDWs are
> working?
I came up with the case like star schema, which postgres database connects data
store.
If each dbms are
Dear Horiguchi-san, Fujii-san,
> > I understood here as removing following mechanism from core:
> >
> > * disable timeout at end of tx.
> > * skip if held off or read commands
>
> I think we're on the same page. Anyway query cancel interrupt is
> ignored while rading input.
>
> > > - If an exist
Dear Fujii-san,
> cfbot is reporting that the 0002 patch fails to be applied cleanly. Could you
> update
> the patch?
> http://cfbot.cputube.org/patch_37_3388.log
Thanks for reporting and sorry for inconvenience.
I repo was not latest version. Attached can be applied to 52e4f0c
Best Regards,
Ha
Cfbot is still angry because of missing PGDLLIMPORT, so attached.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
v12_0001_add_checking_infrastracture.patch
Description: v12_0001_add_checking_infrastracture.patch
v12_0002_add_health_check.patch
Description: v12_0002_add_health_check.patch
v12_0
Dear Fujii-san,
Thank you for your quick reviewing! I attached new version.
I found previous patches have wrong name. Sorry.
> The connection check timer is re-scheduled repeatedly even while the backend
> is
> in idle state or is running a local transaction that doesn't access to any
> foreign
Dear Wang,
Thank you for teaching some backgrounds about the patch.
> According to our discussion, we need to send keepalive messages to subscriber
> when skipping changes.
> One approach is that **for each skipped change**, we try to send keepalive
> message by calculating whether a timeout will
Dear Wang,
> Attached a new patch that addresses following improvements I have got so far
> as
> comments:
> 1. Consider other changes that need to be skipped(truncate, DDL and function
> calls pg_logical_emit_message). [suggestion by Ajin, Amit]
> (Add a new function SendKeepaliveIfNecessary for
Hi Vignesh,
> In logical replication, currently Walsender sends the data that is
> generated locally and the data that are replicated from other
> instances. This results in infinite recursion in circular logical
> replication setup.
Thank you for good explanation. I understand that this fix can
Dear Wang,
> Attach the new patch. [suggestion by Kuroda-San]
> 1. Fix the typo.
> 2. Improve comment style.
> 3. Fix missing consideration.
> 4. Add comments to clarifies above functions and function calls.
Thank you for updating the patch! I confirmed they were fixed.
```
Hi Hackers,
> It's not happy, but I'm not sure about a good solution. I made a timer
> reschedule
> if connection lost had detected. But if queries in the transaction are quite
> short,
> catching SIGINT may be fail.
Attached uses another way: sets pending flags again if DoingCommandRead is tru
Dear Wang,
> Some codes were added in ReorderBufferProcessTXN() for treating DDL,
My mailer went wrong, so I'll put comments again. Sorry.
Some codes were added in ReorderBufferProcessTXN() for treating DDL,
but I doubted updating accept_writes is needed.
IMU, the parameter is read by OutputPlug
Dear Peter,
> > So, why does the patch use syntax option 1?
IMU it might be useful for the following case.
Assuming that multi-master configuration with node1, node2.
Node1 has a publication pub1 and a subscription sub2, node2 has pub2 and sub1.
From that situation, please consider that new nod
Dear Vignesh,
> I felt changing only_local option might be useful for the user while
> modifying the subscription like setting it with a different set of
> publications. Changes for this are included in the v2 patch attached
> at [1].
+1, thanks. I'll post if I notice something to say.
> Shall w
Dear Wang,
Thank you for updating the patch! Good self-reviewing.
> And I looked into the function WalSndUpdateProgress. I found function
> WalSndUpdateProgress try to record the time of some message(by function
> LagTrackerWrite) sent to subscriber, such as in function pgoutput_commit_txn.
Yeah
Dear Wang,
Thank you for updating!
> > Do we need adding a test for them? I think it can be added to 100_bugs.pl.
> > Actually I tried to send PoC, but it does not finish to implement that.
> > I'll send if it is done.
> I'm not sure if it is worth it.
> Because the reproduced test of this bug mi
Hi Vegnesh,
While considering about second problem, I was very confusing about it.
I'm happy if you answer my question.
> To handle this if user has specified only_local option, we could throw
> a warning or error out while creating subscription in this case, we
> could have a column srreplicated
Dear Vignesh,
Thank you for updating your patch!
> Let's consider an existing Multi master logical replication setup
> between Node1 and Node2 that is created using the following steps:
> a) Node1 - Publication publishing employee table - pub1
> b) Node2 - Subscription subscribing from publicatio
Dear Vignesh,
> Thanks for kind explanation.
> I read above and your doc in 0002, and I put some comments.
I forgot a comment about 0002 doc.
5. create_subscription.sgml - about your example
Three possibilities were listed in the doc,
but I was not sure about b) case.
In the situation Node1 and
Dear Hackers,
In the previous discussion [1], we noticed that ECPG cannot accept IPv6
connection string, it means the following statement does not work well:
EXEC SQL CONNECT TO 'tcp:postgresql://::1/postgres';
This is caused because colons are gotten entangled in the ECPGconnect(),
and Wang su
Dear Wang,
Thank you for giving comments!
I forgot to write that parse functions imitates libpq's functios,
but you understood that immediately. Genius!
> So, I think parse_options() is not need to be refactored.
OK.
> I think we can use the message as same as the message in fe-connect.c:
> > l
Sorry for sending again.
> * Only an SQL literal or a host variable is acceptable.
> I understand we should support other notations, but now hacking.
I tried to add support notation. Now unquoted string can be used.
In the flex file, IPv6 string is parsed with the square bracket, it means
the f
Dear Hackers,
I reviewed for myself and fixed something:
* refactor parse_options(), same as conninfo_uri_parse_params() in libpq
Skipping blanks is needed in this functions because ecpg precompiler add
additional blanks
between option parameters. I did not fix precompiler because of the
co
Dear Miyake-san,
I agree your suggestions and I think this patch is basically good.
I put some comments:
* When the following line is input, the error message is not happy.
I think output should be " \sleep command argument must be an integer...".
\sleep foo
-> pgbench: fatal: test.sql:5: unre
Dear Miyake-san,
> I'm not sure but I think this is caused because `my_command->argv[2]`
> becomes "foo".
Sorry, I missed some lines in your patch. Please ignore this analysis.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Miyake-san,
Thank you for updating the patch.
> When argc == 2, pgbench assumes that (1) argv[1] is just a number (e.g.
> \sleep 10) or (2) argv[1] is a pair of a number and a time unit (e.g.
> \sleep 10ms).
I see.
> So I fixed the problem as follows:
> 1) When argv[1] starts with a numb
Dear Fujii-san, Miyake-san
> Isn't it better to accept even negative sleep time like currently pgbench
> does?
> Otherwise we always need to check the variable is a positive integer
> (for example, using \if command) when using it as the sleep time in \sleep
> command. That seems inconvenient.
B
Dear Ikeda-san
I think the idea is good.
I read the patch and other sources, and I found process_startup_packet_die also
execute _exit(1).
I think they can be combined into one function and moved to interrupt.c, but
some important comments might be removed. How do you think?
Best Regards,
Hay
Dear Fabien;
> The current implementation is too simple. If nthreads >= 2 and connection
> fails in the one thread,
> the other one will wait forever.
I attached the very preliminary patch for solving the problem.
Even if threads fail to connect, the others can go through the barrier.
But I thin
Dear Fabien,
> Indeed. I scanned the file but did not find other places that needed
> updating.
> Yes.
> Not sure either. I'm not for having too many braces anyway, so I removed
> them.
I checked your fixes and I think it's OK.
Finally, please move the doc fixes to patch B in order to separ
Dear Fabien,
> Usually I run many pgbench through scripts, so I'm probably not there to
> check a lone stderr failure at the beginning if performance figures are
> actually reported.
> I can remove the line, but I strongly believe that reporting performance
> figures if some client connection f
Dear Fabien,
> and this will wait till its time comes. In the mean time, I think that you
> should put the patch status as you see fit, independently of the other
> patch: it seems unlikely that they would be committed together, and I'll
> have to merge the remaining one anyway.
OK. I found t
Dear Li,
I read your patch, and I think the documentation is too simple to avoid all
problems.
(I think if some connection pooling is used, the same problem will occur.)
Could you add some explanations in the doc file? I made an example:
```
Note that this values should be set to zero if you us
Dear Li,
> Thanks for your advice! Attached v4.
I confirmed it. OK.
> @@ -30,6 +30,7 @@ typedef enum TimeoutId
> STANDBY_DEADLOCK_TIMEOUT,
> STANDBY_TIMEOUT,
> STANDBY_LOCK_TIMEOUT,
> + IDLE_SESSION_TIMEOUT,
> IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
> /* First user
Dear Li, David,
> Additionally, using postgres_fdw within the server doesn't cause issues,
> its using postgres_fdw and the remote server having this setting set to zero
> that causes a problem.
I didn't know the fact that postgres_fdw can use within the server... Thanks.
I read optimize-setiti
Dear Li,
> I’m not familiar with the system interrupt, however,
> the sigalrm_due_at is subsutitue between HOLD_INTERRUPTS()
> and RESUM_INTERRUPTS(), so I think it cannot be interrupted.
> The following comments comes from miscadmin.h.
Right, but how about before HOLD_INTERRUPTS()?
If so, only c
Dear Li,
> Yeah, it might be occurred. Any suggestions to fix it?
Oops.. I forgot putting my suggestion. Sorry.
How about substituting sigalrm_delivered to true in the reschedule_timeouts()?
Maybe this processing looks strange, so some comments should be put too.
Here is an example:
```diff
@@
Dear Li,
> Thanks for your suggestion. Attached!
I prefer your comments:-).
I think this patch is mostly good.
I looked whole the codes again and I found the following comment in the
PostgresMain():
```c
/*
* (5) turn off the idle-in-transaction timeout
Dear Li,
> Thanks! Add the comment for idle-session timeout.
I confirmed it. OK.
I don't have any comments anymore. If no one has,
I will change the status few days later.
Other comments or suggestions to him?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
No one have any comments, patch tester says OK, and I think this works well.
I changed status to "Ready for Committer."
Hayato Kuroda
FUJITSU LIMITED
-Original Message-----
From: kuroda.hay...@fujitsu.com
Sent: Friday, November 20, 2020 11:05 AM
To: 'japin'
Cc: David G
Dear Heikki,
I'm also interested in this patch, but it cannot be applied to the current
HEAD...
$ git apply ~/v2-0001-Make-resowners-more-easily-extensible.patch
error: patch failed: src/common/cryptohash_openssl.c:57
error: src/common/cryptohash_openssl.c: patch does not apply
error: patch fai
Dear Tom,
> So I propose to change the new ERRCODE_IDLE_SESSION_TIMEOUT to be in
> class 57 and call it good.
I agreed your suggestion and I confirmed your commit.
Thanks!
Hayato Kuroda
FUJITSU LIMITED
Dear Heikki,
Thank you for rebasing it, I confirmed it can be applied.
I will check the source.
Now I put the very elementary comment.
ResourceOwnerEnlarge(), ResourceOwnerRemember(), and ResourceOwnerForget()
are exported routines.
They should put below L418.
Best regards,
Hayato Kuroda
FUJITSU
Hi,
I put some comments.
Throughout, some components don’t have helper functions.
(e.g. catcache has ResOwnerReleaseCatCache, but tupdesc doesn't.)
I think it should be unified.
[catcache.c]
> +/* support for catcache refcount management */
> +static inline void
> +ResourceOwnerEnlargeCatCacheRe
Dear Heikki,
> Hmm. ResOwnerReleaseTupleDesc() does exist, those functions are needed
> for the callbacks. I think you meant the wrappers around
> ResourceOwnerRemember and ResourceOwnerForget, like
> ResourceOwnerRememberCatCacheRef(). I admit those are not fully
> consistent: I didn't bother
Dear Tsunakawa-san, Iwata-san,
> * Is setlinebuf() available on Windows? Maybe you should use setvbuf()
> instead.
Yeah, cfbot2021 throws errors:
https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.124922
```
src/interfaces/libpq/fe-connect.c(6776): warning C4013: 'setlineb
Dear Heikki,
I apologize for sending again.
> I will check another ResourceOwnerEnlarge() if I have a time.
I checked all ResourceOwnerEnlarge() types after applying patch 0001.
(searched by "grep -rI ResourceOwnerEnlarge")
No problem was found.
Hayato Kuroda
FUJITSU LIMITED
Dear Heikki,
I tested in the same situation, and I confirmed that almost same results are
returned.
(results are at the end of the e-mail)
You think that these results are acceptable
because resowners own many resources(more than 64) in general
and it's mainly faster in such a situation, isn't i
Dear Ikeda-san,
This patch cannot be applied to the HEAD, but anyway I put a comment.
```
+ /*
+* Measure i/o timing to fsync WAL data.
+*
+* The wal receiver skip to collect it to avoid performance degradation
of standy servers.
+* If sync_method doesn't ha
Dear Ikeda-san,
Thank you for updating the patch. This can be applied to master, and
can be used on my RHEL7.
wal_write_time and wal_sync_time increase normally :-).
```
postgres=# select * from pg_stat_wal;
-[ RECORD 1 ]+--
wal_records | 121781
wal_fpi
Dear Ikeda-san,
> I checked columns' descriptions of other views.
> There are "Number of xxx", "Total number of xxx", "Total amount of time
> that xxx" and "Total time spent xxx".
Right.
> Since the "time" is used for showing spending time, not count,
> I'll change it to "Total number of WAL da
Dear Hackers,
I know I'm asking a big favor, but could you review(and commit) the patch?
The status has become RFC last Nov., but no one checked this after that.
Maybe Meskes is quite busy and have no time to review it.
The main part of the patch is about 200 lines(It means not so long), and mayb
Dear Wang,
> the value tmp2 will always be NULL, the unix-socket path will be ignored.
I confirmed it, you're right.
> I have fixed this problem, the patch attached.
It looks good to me:-).
> I will try to fix this problem later, but it seems a little difficult to add
> some lex/bison file ru
Dear Horiguchi-san,
My response crossed in the e-mail with yours. Sorry.
> FWIW, directly embedding /unixsocket/path syntax in a URL is broken in
> the view of URI. It is the reason why the current connection URI takes
> the way shown above. So I think we want to remove that code rather
> than to
Dear Wang, Tom
> I think add the bison rule is a little difficult because in PG13 windows can
> also support unix-socket,
> In your patch:
> > dir_name: '/' dir_name{ $$ = make2_str(mm_strdup("/"), $2); }
> > | ecpg_ident{ $$ = $1; }
> >;
> Windows will remains wrong(I'm not sure ecpg on windows
Dear Wang, Horiguchi-san,
> > How about the attached?
>
> I think, this patch is good.
I agree. The backward compatibility is violated in the doc, but maybe no one
take care.
> Maybe we can create a new thread to talk about how ecpg support ipv6
+1
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Fujii-san,
Thank you for reviewing! I'll post the latest version.
> Currently StringInfo variable is defined in connect_pg_server() and passed to
> parse_pgfdw_appname(). But there is no strong reason why the variable needs to
> be defined connect_pg_server(). Right? If so how about refactor
Dear Horiguchi-san
Thank you for giving comments! I attached new patches.
> > + if (appname == NULL)
> > + appname = "[unknown]";
> > + if (username == NULL || *username
> == '\0')
> > +
Dear Fujii-san
Thank you for quick reviewing! I attached new ones.
> Regarding 0001 patch, IMO it's better to explain that
> postgres_fdw.application_name can be any string of any length and contain even
> non-ASCII characters, unlike application_name. So how about using the
> following
> descri
Dear Fujii-san,
I apologize for sending bad patches...
I confirmed that it can be built and passed a test.
> Could you tell me why you added new paragraph into the middle of existing
> paragraph? This change caused the following error when compiling the docs.
>
> postgres-fdw.sgml:952: parser er
Dear Kato-san,
Thank you for giving comments! And sorry for late reply.
I rebased my patches.
> Even for local-only transaction, I thought it useless to execute
> CallCheckingRemoteServersCallbacks() and enable_timeout_after(). Can I
> make it so that it determines outside whether it contains SQL
Dear Fujii-san,
Thank you for updating! I read your patches and I have
only one comment.
> if (strcmp(keywords[i], "application_name") == 0 &&
> values[i] != NULL && *(values[i]) != '\0')
I'm not sure but do we have a case that values[i] become
Dear Fujii-san,
> Thanks for the review! At first I pushed 0001 patch.
I found your commit. Thanks!
> BTW, 0002 patch adds the regression test that checks
> pg_stat_activity.application_name. But three months before, we added the
> similar
> test when introducing postgres_fdw.application_name G
Dear Fujii-san,
Sorry I forgot replying your messages.
> >>if (strcmp(keywords[i], "application_name") == 0 &&
> >>values[i] != NULL && *(values[i]) != '\0')
> >
> > I'm not sure but do we have a case that values[i] becomes NULL
> > even if keywords
Dear Gunnar,
> postgres=# CREATE TABLE test (i int, lz4 text COMPRESSION lz4, std text);
> postgres=# INSERT INTO test SELECT
> i,repeat(md5(i::text),100),repeat(md5(i::text),100) FROM
> generate_series(0,10) x(i);
> postgres=# SELECT * FROM pg_stat_toast WHERE schemaname = 'public';
> -[ REC
Dear Gunnar,
> The attached v0.3 includes
> * columns "storagemethod" and "compressmethod" added as per Hayato
> Kuroda's suggestion
I prefer your implementation that referring another system view.
> * gathering timing information
Here is a minor comment:
I'm not sure when we should start to
Dear Fujii-san, Horiguchi-san,
I confirmed that the feature was committed but reverted the test.
Now I'm checking buildfarm.
But anyway I want to say thank you for your contribution!
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Fujii-san,
> Attached is the patch that adds the regression test for
> postgres_fdw.application_name. Could you review this?
>
> As Horiguchi-san suggested at [1], I split the test into two, and then
> tweaked them
> as follows.
>
> 1. Set application_name option of a server option to 'fdw
Dear Sawada-san,
> Good idea. But the application_name is actually truncated to 63
> characters (NAMEDATALEN - 1)? If so, we need to do substring(... for
> 63) instead.
Yeah, the parameter will be truncated as one less than NAMEDATALEN:
```
max_identifier_length (integer)
Reports the maximum ide
Dear Fujii-san,
Thank you for updating the patch.
I understand that you don't want to change the current specification.
```diff
+ if (usec == 0)
+ {
+ char *c = var;
+
+ /* Skip sign */
+ if (*c ==
Dear Ikeda-san,
I confirmed new patch and no problem was found. Thanks.
(I'm not a native English speaker, so I cannot check your comments correctly,
sorry)
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Fujii-san,
I confirmed your patch and some parse functions, and I agree
the check condition in evaluateSleep() is correct.
No problem is found.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Andrei,
I think the idea is good because the pg_stat_statements_info view cannot
distinguish
whether the specific statement is deallocated or not.
But multiple calling of GetCurrentTimestamp() may cause some performance issues.
How about adding a configuration parameter for controlling this
Dear Andrei,
> Certaily I was thinking about this. And I've taken an advice of Teodor
> Sigaev - a much more expirienced developer than me. It seems that
> GetCurrentTimestamp() is fast enough for our purpose and we won't call
> it too often - only on new statement entry allocation.
OK.
> Howeve
Dear Andrei,
> I think, yes, version of pg_stat_statements is need to be updated. Is
> it will be 1.10 in PG15?
I think you are right.
According to [1] we can bump up the version per one PG major version,
and any features are not committed yet for 15.
[1]: https://www.postgresql.org/message-id/
Dear Horiguchi-san, Fujii-san,
Perfect work... Thank you for replying and analyzing!
> A. "^-?[0-9]+.*" : returns valid padding. p goes after the last digit.
> B. "^[^0-9-].*" : padding = 0, p doesn't advance.
> C. "^-[^0-9].*" : padding = 0, p advances by 1 byte.
> D. "^-" : padd
> Does isdigit() understand multi-byte character correctly? The arguments
> of isdigit() is just a unsigned char, and this is 1byte.
> Hence I thought that they cannot distinguish 'ー'.
Sorry, but I referred some wrong doc. Please ignore here...
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Horiguchi-san,
> I'm not sure. All of it is a if-story. z/OS's isdigit is defined as
> "Test for a decimal digit, as defined in the digit locale source file
> and in the digit class of the LC_CTYPE category of the current
> locale.", which I read it as "it accepts multibyte strings" but of
>
Dear Hackers,
I want to propose the feature that checks the health of foreign servers.
As a first step I want to add an infrastructure for periodical checking to
PostgreSQL core.
Currently this is the WIP, it does not contain docs.
## Background
Currently there is no way to check the status o
Dear Fujii-san,
Thank you for giving comments! I attached new patches.
> On second thought, do we really need padding support for application_name
> in postgres_fdw? I just thought that when I read the description
> "Padding can be useful to aid human readability in log files." in the docs
> abou
Dear Horiguchi-san, Fujii-san
Thank you for discussing!
> Isn't it enough to perform this assertion check only once
> at the top of parse_pgfdw_appname()?
Fixed.
> If possible, I'd like to see this change as a separate patch
> and commt it first because this is the description for
> the existin
Dear Fujita-san,
I love your proposal because it will remove a bottleneck
for PostgreSQL build-in sharding.
I read your patch briefly and I think basically it's good.
Currently I have only one comment.
In your patch, postgres_fdw sends a COMMIT command to all entries in the hash
table
and wait
Dear Kato-san,
Thank you for your interest!
> > I also want you to review the postgres_fdw part,
> > but I think it should not be attached because cfbot cannot understand
> > such a dependency
> > and will throw build error. Do you know how to deal with them in this
> > case?
>
> I don't know ho
Dear Kato-san,
Thank you for reviewing!
> Thank you for sending the patches!
> I confirmed that they can be compiled and tested successfully on CentOS
> 8.
Thanks!
> + {
> + {"remote_servers_connection_check_interval", PGC_USERSET,
> CONN_AUTH_SETTINGS,
> + g
Dear Kato-san,
I found the missing space, and I added a test.
I'm very happy if you review this.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
v03_0001_add_checking_infrastracture.patch
Description: v03_0001_add_checking_infrastracture.patch
<>
Dear Zhihong,
Thank you for giving comments! I'll post new patches later.
> +#define HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS()
> (CheckingRemoteServersHoldoffCount++)
>
> The macro contains only one operation. Can the macro be removed (with
> `CheckingRemoteServersHoldoffCount++` inlined) ?
Dear Hackers,
I attached new version that is based from Zhihong's comments.
And I newly attached a doc patch. I think the infrastructure part is no longer
WIP.
Could you review them?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
<>
v04_0001_add_checking_infrastracture.patch
Description: v04_0
Dear ALL,
I want to report and consult about DECLARE STATEMENT.
This feature, committed last February, has some bugs.
* This is not thread-independent.
* If some cursors are declared for the same SQL identifier,
only one cursor you declared at last is enabled.
* This syntax does not have oracl
Dear all,
Hi, thank you for replying.
> It seems far too late to be considering any major rewrite for v12;
> If a solid case can be made that ECPG's DECLARE STATEMENT was done
> wrong, we'd be better off to just revert the feature out of v12
> and try again, under less time pressure, for v13.
Dear Peter,
I want to complement about another purpose.
This is that declaring an SQL identifier.
In the oracle (and maybe DB2), the following example is not allowed:
...
EXEC SQL DECLARE cursor CURSOR FOR stmt;
EXEC SQL PREPARE stmt FOR "SELECT ..."
...
T
Dear hackers,
As declared last month, I propose again the new ECPG grammar, DECLARE STATEMENT.
This had been committed once, but it removed from PG12 because of
some problems.
In this mail, I want to report some problems that previous implementation has,
produce a new solution, and attach a WIP p
Dear Amit,
> It seems by mistake you have removed the changes from pgoutput_message
> and pgoutput_truncate functions. I have added those back.
> Additionally, I made a few other changes: (a) moved the function
> UpdateProgress to pgoutput.c as it is not used outside it, (b) change
> the new param
Dear Wang-san,
Thank you for updating!
...but it also cannot be applied to current HEAD
because of the commit 923def9a533.
Your patch seems to conflict the adding an argument of
logicalrep_write_insert().
It allows specifying columns to publish by skipping some columns in
logicalrep_write_tuple
Dear Amit, Wang,
> > I think maybe we do not need to deal with this use case.
> > The maximum number of table columns allowed by PG is 1600
> > (macro MaxHeapAttributeNumber), and after loop through all columns in the
> > function logicalrep_write_tuple, the function OutputPluginWrite will be
> >
Dear Peter,
> FYI, here is a test script that is using the current patch (v6) to
> demonstrate a way to share table data between different numbers of
> nodes (up to 5 of them here).
Thanks for sharing your script! It's very helpful for us.
While reading your script, however, I had a question abo
Dear Laurenz,
Thank you for your interest in our works!
> I am missing a discussion how replication conflicts are handled to
> prevent replication from breaking or the databases from drifting apart.
Actually we don't have plans for developing the feature that avoids conflict.
We think that it sh
Dear Zhihong,
Thank you for reviewing! And I have to apologize for the late.
I'll post new patchset later.
> + UnregisterAllCheckingRemoteServersCallback();
>
> UnregisterAllCheckingRemoteServersCallback
> -> UnregisterAllCheckingRemoteServersCallbacks
Will fix.
> + CheckingRemoteServe
Dear Fujii-san,
Thank you for your interest! I'll post new version within several days.
> > Yeah, remote-checking timeout will be enable even ifa local transaction is
> opened.
> > In my understanding, postgres cannot distinguish whether opening
> > transactions
> > are using only local object o
Dear Fujii-san, Zhihong,
I attached the latest version.
The biggest change is that callbacks are no longer un-registered at the end of
transactions.
FDW developer must enable or disable timeout instead, via new APIs.
The timer will be turned on when:
* new GUC is >= 0, and
* anyone calls TryEnab
Dear Fujii-san,
Thank you for reviewing! I attached the latest version.
> When more than one FDWs are used, even if one FDW calls this function to
> disable the timeout, its remote-server-check-callback can still be called. Is
> this
> OK?
> Please imagine the case where two FDWs are used and th
Dear Fujii-san,
Thank you for good suggestions.
> This logic sounds complicated to me. I'm afraid that FDW developers may a bit
> easily misunderstand the logic and make the bug in their FDW.
> Isn't it simpler to just disable the timeout in core whenever the transaction
> ends
> whether committ
1 - 100 of 207 matches
Mail list logo
