l_change(toptxn)) &&
> - rbtxn_is_serialized(txn))
> + rbtxn_is_serialized(txn) &&
> + rbtxn_has_streamable_change(txn))
> ReorderBufferStreamTXN(rb, toptxn);
> }
You are right we need this in ReorderBufferProcessPartialChange(
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 largest transaction, but
> > in
> > add
ase for
> this patch. I think before this patch if the size of DDL changes in a
> transaction exceeds logical_decoding_work_mem, the empty streams will
> be output in the plugin but after this patch, there won't be any such
> stream.
Yes, we can do that, I will make these two changes.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Dec 5, 2022 at 9:21 AM Dilip Kumar wrote:
>
> On Mon, Dec 5, 2022 at 8:59 AM Amit Kapila wrote:
> >
> > On Sun, Dec 4, 2022 at 5:14 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Saturday, December 3, 2022 7:37 PM Amit Kapila
> >
On Wed, Dec 7, 2022 at 9:28 AM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 11:55 AM shiy.f...@fujitsu.com
> wrote:
> >
> > On Mon, Dec 5, 2022 6:57 PM Amit Kapila wrote:
> > >
> > > On Mon, Dec 5, 2022 at 3:41 PM Dilip Kumar wrote:
> > > >
XX = true/false
> logical_replication_dev_YYY = true/false
> logical_replication_dev_ZZZ = true/false
>
Even I agree that usability wise keeping them independent is better.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
uot;force_stream" then it will either stream or serialize but I
guess we do not want to force serialize based on this parameter.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
logical slots.
+ */
+if (SlotIsPhysical(slot))
+PG_RETURN_VOID();
+}
+
pgstat_reset_replslot_counter(target);
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Apr 22, 2021 at 7:52 AM Masahiko Sawada wrote:
>
> On Wed, Apr 21, 2021 at 4:44 PM Dilip Kumar wrote:
> >
> > On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada
> > wrote:
> >
> > >
> > > I've attached the patch. In addition to the
to be on the safer side. While on it, I
> think we can also replace BLCKSZ with PageGetPageSize(page).
>
> Attaching a small patch. Thoughts?
+1 for changing to MAXALIGN(sizeof(BTPageOpaqueData)).
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
uples at once - but it might be material in some
> cases, and it's definitely material if you don't have enough disk
> space left for it to complete without error.
Is it a good idea to always perform an I/O after collecting the dead
TID's or there should be an option where t
em to happen. (In my
> case, there are 8 publications and the failure always happened).
>
> The scripts and the log are attached. It took me about 4 minutes to run
> the script on my machine.
> Please contact me if you need more specific info for the problem.
Thanks for reporting. I will look into it.
> --
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Apr 26, 2021 at 1:26 PM Dilip Kumar wrote:
>
> On Mon, 26 Apr 2021 at 12:45 PM, tanghy.f...@fujitsu.com
> wrote:
>>
>> Hi
>>
>> I think I may found a bug when using streaming in logical replication. Could
>> anyone please take a look at this?
rd\" option." or "The \"password\" option is specified more
> than once. Remove all the duplicates.", then it makes sense to use
> errhint.
I agree this should be errdetail.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Apr 26, 2021 at 6:59 PM Amit Kapila wrote:
>
> On Mon, Apr 26, 2021 at 5:55 PM Dilip Kumar wrote:
> >
> > I am able to reproduce this and I think I have done the initial
> > investigation.
> >
> > The cause of the issue is that, this transaction has
On Mon, Apr 26, 2021 at 7:52 PM Dilip Kumar wrote:
>
> On Mon, Apr 26, 2021 at 6:59 PM Amit Kapila wrote:
> >
> > On Mon, Apr 26, 2021 at 5:55 PM Dilip Kumar wrote:
> > >
> > > I am able to reproduce this and I think I have done the initial
> > >
On Tue, Apr 27, 2021 at 11:43 AM Amit Kapila wrote:
>
> On Mon, Apr 26, 2021 at 7:52 PM Dilip Kumar wrote:
> >
> > On Mon, Apr 26, 2021 at 6:59 PM Amit Kapila wrote:
> > >
> > > On Mon, Apr 26, 2021 at 5:55 PM Dilip Kumar wrote:
> > > >
> &
hat am I missing if anything?
Got your point, yeah this will only have top transactions so we can
use this. I will change this in the next patch. In fact we can put
an assert that it should not be an sub transaction?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Apr 27, 2021 at 12:55 PM Amit Kapila wrote:
>
> On Tue, Apr 27, 2021 at 12:22 PM Dilip Kumar wrote:
> >
> > On Tue, Apr 27, 2021 at 12:05 PM Amit Kapila
> > wrote:
> > > > > Can't we use 'txns_by_base_snapshot_lsn&#
or prepared transaction (the same check as we have for
> setting curtxn) or we can check if CheckXidAlive is a valid
> transaction id. What do you think?
I think a check based on CheckXidAlive looks good to me. This will
protect against if a similar error is raised from any other path as
you mentioned above.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Apr 28, 2021 at 12:25 PM tanghy.f...@fujitsu.com
wrote:
>
> > I have modified the patch based on the above comments.
>
> Thanks for your patch.
> I tested again after applying your patch and the problem is fixed.
Thanks for confirming.
--
Regards,
Dilip Kumar
On Wed, Apr 28, 2021 at 1:02 PM Dilip Kumar wrote:
>
> On Wed, Apr 28, 2021 at 12:25 PM tanghy.f...@fujitsu.com
> wrote:
> >
> > > I have modified the patch based on the above comments.
> >
> > Thanks for your patch.
> > I tested again after ap
On Thu, Apr 29, 2021 at 12:09 PM tanghy.f...@fujitsu.com
wrote:
>
>
> On Thursday, April 29, 2021 3:18 PM, Dilip Kumar wrote
>
> >I tried to think about how to write a test case for this scenario, but
> >I think it will not be possible to generate an automated test cas
On Thu, Apr 29, 2021 at 5:24 PM Amit Kapila wrote:
>
> On Tue, Apr 27, 2021 at 5:18 PM Dilip Kumar wrote:
> >
> > I have modified the patch based on the above comments.
> >
>
> The patch looks good to me. I have slightly modified the comments and
> commit me
signing defel to
that?
If the goal is to just improve the error message then you can simply
use defel->defname?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 30, 2021 at 10:43 AM Bharath Rupireddy
wrote:
>
> On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar wrote:
> > In this function, we already have the "defel" variable then I do not
> > understand why you are using one extra variable and assigning defel to
>
On Fri, Apr 30, 2021 at 11:09 AM Bharath Rupireddy
wrote:
>
> On Fri, Apr 30, 2021 at 10:51 AM Dilip Kumar wrote:
> >
> > On Fri, Apr 30, 2021 at 10:43 AM Bharath Rupireddy
> > wrote:
> > >
> > > On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar
> &g
ul in
> debugging. Thoughts?
>
It makes sense to include xid in the debug message, earlier I thought
that will it be a good idea to also print the toplevel_xid. But I
think it is for debug purposes and only we have the xid we can fetch
the other parameters so maybe adding xid is good eno
sically
why to process an extra statement ( *volatility_item = defel;) if we
have already decided to error.
if (is_procedure)
is_procedure_error = true;
else if (*volatility_item)
is_duplicate_error = true;
else
*volatility_item = defel;
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 30, 2021 at 3:01 PM Amit Kapila wrote:
>
> On Wed, Apr 28, 2021 at 11:03 AM Dilip Kumar wrote:
> >
> > On Wed, Apr 28, 2021 at 11:00 AM Amit Kapila
> > wrote:
> > >
> >
> > The idea I have is to additionally check that we are decoding
he issue.
[1]
https://www.postgresql.org/message-id/os0pr01mb611340cbd300a7c4fd6b6101fb...@os0pr01mb6113.jpnprd01.prod.outlook.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, May 1, 2021 at 10:43 AM Bharath Rupireddy
wrote:
>
> On Fri, Apr 30, 2021 at 2:49 PM Dilip Kumar wrote:
> > Looking into this again, why not as shown below? IMHO, this way the
> > code will be logically the same as it was before the patch, basically
> > why to pr
h and let you know.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Now, in this case, this comment is true right because we are not
supposed to truncate tbl_fk_sub on the subscriber side and this should
error out.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
sh,On Mon, May 3, 2021 at 12:37 PM Bharath Rupireddy
wrote:
>
> On Mon, May 3, 2021 at 11:59 AM Dilip Kumar wrote:
> >
> > On Mon, May 3, 2021 at 10:42 AM Bharath Rupireddy
> > wrote:
> > >
> > > Hi,
> > >
> > > In apply_handle_trunc
have posted the same in the v5
> > patch posted in my earlier mail.
>
> 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.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, May 3, 2021 at 10:48 AM Dilip Kumar wrote:
>
> On Sun, May 2, 2021 at 10:04 PM vignesh C wrote:
> >
> > Thanks for the comments.
> > The Attached patch has the fixes for the same.
>
> I was reviewing the documentation part, I think in the below
https://www.postgresql.org/message-id/E1lNKw9-0008DT-1L%40gemulon.postgresql.org
[2]
https://www.postgresql.org/message-id/CAFiTN-s7fno8pGwfK7jwSf7uNaVhPZ38C3LAcF%2B%3DWHu7jNvy7g%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
WIP-0001-Toast-compression-method-options.
On Tue, Mar 2, 2021 at 3:14 PM Dilip Kumar wrote:
> =
> ee994272ca50f70b53074f0febaec97e28f83c4e
> Author: Heikki Linnakangas 2013-01-03 14:11:58
> Committer: Heikki Linnakangas 2013-01-03 14:11:58
>
> Delay reading timeline history file until it
would be empty while we're
> > waiting for the next file to be ready to be archived.
>
> I think that's right.
If we agree with your above point that it only needs to scan the full
directory when it has to wait for the next file to be ready then
making a separate directory for .done file can improve a lot because
the directory will be empty so scanning will not be very costly.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
with
> the
> + compression methods of the target columns.
> +
> + The supported compression
> methods are pglz and lz4.
>lz4 is available only if
> --with-lz4
>was used when building PostgreSQL.
Your documentation looks fine to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
e then the column compression method and the default
compression method will be same so the dump will not generate an extra
command of type ALTER TABLE... SET COMPRESSION.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
WAL segment.
> > What am I missing?
>
> It's not true now, but my proposal would make it true.
Okay, got it. Thanks.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, May 5, 2021 at 11:02 AM Dilip Kumar wrote:
>
> On Wed, May 5, 2021 at 12:06 AM Robert Haas wrote:
> >
>
> > > There are no tests in pg_dump to make sure that some ALTER
> > > MATERIALIZED VIEW or ALTER TABLE commands are generated when the
> >
On Wed, May 5, 2021 at 4:00 PM Michael Paquier wrote:
>
> On Wed, May 05, 2021 at 01:41:03PM +0530, Dilip Kumar wrote:
> > I think we already have such test cases at least through pg_upgrade.
> > Basically, if you see in compression.sql we are not dropping the table
> >
el
> binding, I think that you could use something like
> check_pg_config("#define HAVE_LIBLZ4 1") and use its result to decide
> which tests to skip or not.
Thanks, Robert and Michael for your input. I will try to understand
how it is done in the example shared by you and come u
ut the buffer
lock order because now we will have to hold the lock on the TID fork
buffer as well as the heap buffer so that we don't create any
deadlock. And there is also a possibility of holding the lock on
multiple TID fork buffers, which will depend upon how many tid we have
pruned.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 6, 2021 at 9:01 AM Amit Kapila wrote:
>
> On Fri, Apr 30, 2021 at 7:43 PM Dilip Kumar wrote:
> >
> > So, I
> > > used the other approach which led to the attached.
> >
> > The patch looks fine to me.
> >
>
> Thanks, pushed!
Tha
On Thu, May 6, 2021 at 10:45 AM Dilip Kumar wrote:
I noticed that the error code for invalid compression method is not
perfect, basically when we pass the invalid compression method during
CREATE/ALTER table that time we give
ERRCODE_FEATURE_NOT_SUPPORTED. I think the correct error code is
t we can dodge that requirement
> > here, but I have reservations about departing from the usual locking
> > pattern. It's easier to reason about the behavior when everybody
> > follows the same set of rules.
>
> Yes, agreed. I was thinking of replaying WAL, not writing WAL.
Right, I was pointing to while writing the WAL.
> --
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ion can easily be reproduced by adding a delay before the buffer
> lock and invoking autovacuums frequently:
>
Your analysis seems correct to me that before we acquire the page the
someone can modify it. So this assertion is not correct.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, May 3, 2021 at 6:27 PM Dilip Kumar wrote:
>
> We have already pushed the configurable lz4 toast compression code[1].
> In the custom compression thread, we were already having the patch to
> support the compression method options[2]. But the design for the
> base patc
On Thu, May 6, 2021 at 5:42 PM Michael Paquier wrote:
>
> On Thu, May 06, 2021 at 10:45:53AM +0530, Dilip Kumar wrote:
> > Thanks, Robert and Michael for your input. I will try to understand
> > how it is done in the example shared by you and come up with the test
> > on
On Fri, May 7, 2021 at 8:23 AM Kyotaro Horiguchi
wrote:
>
> At Tue, 4 May 2021 17:41:06 +0530, Dilip Kumar wrote
> in
> Could you please fix the test script so that it causes your issue
> correctly? And/or elaborate a bit more?
>
> The attached first file is the debug
report(ERROR,
+ (errmsg("could not connect to the publisher: %s", err)));
Instead of using global wrconn, I think you should use a local variable?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, May 7, 2021 at 2:33 PM Kyotaro Horiguchi
wrote:
>
> Thanks.
>
>
> At Fri, 7 May 2021 11:04:53 +0530, Dilip Kumar wrote
> in
> > On Fri, May 7, 2021 at 8:23 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Tue, 4 May 2021 17:41:06 +05
On Fri, May 7, 2021 at 5:38 PM Bharath Rupireddy
wrote:
>
> On Fri, May 7, 2021 at 11:50 AM Dilip Kumar wrote:
> >
> > On Thu, May 6, 2021 at 7:22 PM vignesh C wrote:
> > >
> >
> > Some comments:
> > 1.
> > I don't see any change in p
to set that. I think the documentation should
clearly say the impact of setting this to true.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
this thread again.
Thanks!
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
eger value” ?
>
> I also found fetch_size has the similar issue, attaching a patch to fix this.
Yes, it should be a positive integer, so your change makes sense.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
hanges. I will send the other patches soon, after
removing the doc part which you have already included here.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, May 8, 2021 at 2:08 PM Michael Paquier wrote:
>
> On Thu, May 06, 2021 at 09:33:53PM +0530, Dilip Kumar wrote:
> > On Thu, May 6, 2021 at 5:42 PM Michael Paquier wrote:
> > >
> > > On Thu, May 06, 2021 at 10:45:53AM +0530, Dilip Kumar wrote:
> > >
teps.
>
> Now, the \help command is for commands, which is a different thing as the
> command in that case is DELETE not DELETE FROM, even if you will have to
> follow
> your DELETE with a FROM.
I agree with Julien. But, I also agree with the consistency point
from Tang. So maybe we can fix the insert and add INSERT INTO in the
tab completion?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
tputs objects that are not parallel safe.
> >
>
> 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 DML operation
> on the table?
Such wrapper function make sense.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, May 10, 2021 at 11:27 AM Michael Paquier wrote:
>
> On Sat, May 08, 2021 at 08:19:03PM +0530, Dilip Kumar wrote:
> > I have fixed some of them, I could not write the test cases where we
> > have to ensure that 'ALTER TABLE COMPRESSION' is not appearing in the
&
WAL, this patch is required for that to
work.
=
>
> I believe the 004_timeline_switch.pl detects your issue. And the
> attached change fixes it.
I think this fix looks better to me, but I will think more about it
and give my feedback. Thanks for quickly coming up with the
repro
pattern as every other ScanData skey[n] code.
> >
> > Please search PG source code for "ScanData skey[1];" - there are
> > dozens of precedents where other people felt the same as me for
> > declaring single keys.
>
> AFAICT there are 73 occurences vs 62 of the "Scandata skey;". I don't think
> there's a huge consensus for one over the other.
I think both Scandata skey[1]; and Scandata skey; are used. But IMHO
using Scandata skey; looks better.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
true for the vacuum no?
I will try to do a more detailed review.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Doc said.
> CREATE POLICY
> CREATE [ OR REPLACE ] RULE
> CREATE [ OR REPLACE ] TRIGGER
> ALTER DEFAULT PRIVILEGES
>
> After applying the patch, the tap-tests for psql is passed.
> Please be free to tell me anything insufficient you found in my fix. Thanks.
LGTM.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
t least this will give people an option to test LZ4 on
> Windows. Perhaps this will require some adjustments, but let's see if
> that's necessary when that comes up.
Thanks, make sense.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ritten the WAL in
XLogAcceptWrites but later if we could not set the state to
WALPROHIBIT_STATE_READ_WRITE? Then maybe we can inform all the
backend first but before setting the state to
WALPROHIBIT_STATE_READ_WRITE, we can call XLogAcceptWrites?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 11, 2021 at 1:42 PM Kyotaro Horiguchi
wrote:
>
> At Mon, 10 May 2021 14:27:21 +0530, Dilip Kumar wrote
> in
> > On Mon, May 10, 2021 at 2:05 PM Kyotaro Horiguchi
> > wrote:
> >
> > > I thought that the reason using receiveTLI instead of
> >
... USING"
> is not as good as it is now.(I guess that's why the comment was added). And
> for now, IMHO, we can remove the comment directly.
But your patch is doing nothing to add the implementation for DELETE..
USING. Basically, the tab completion support for DELETEUSING is
still pending right?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
d memory state is changed to
WALPROHIBIT_STATE_READ_WRITE?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 11, 2021 at 3:03 PM tanghy.f...@fujitsu.com
wrote:
>
> On Tuesday, May 11, 2021 5:44 PM, Dilip Kumar wrote:
> >But your patch is doing nothing to add the implementation for DELETE..
> >USING. Basically, the tab completion support for DELETEUSING is
> >st
On Tue, May 11, 2021 at 3:38 PM Amul Sul wrote:
>
> On Tue, May 11, 2021 at 2:26 PM Dilip Kumar wrote:
> >
> > On Tue, May 11, 2021 at 2:16 PM Amul Sul wrote:
> >
> > > I get why you think that, I wasn't very precise in briefing the problem.
> > >
On Tue, May 11, 2021 at 4:50 PM Amul Sul wrote:
>
> On Tue, May 11, 2021 at 4:13 PM Dilip Kumar wrote:
> > I might be missing something, but assume the behavior should be like this
> >
> > 1. If the state is getting changed from WALPROHIBIT_STATE_READ_WRITE
> >
On Tue, May 11, 2021 at 6:56 PM Amul Sul wrote:
>
> On Tue, May 11, 2021 at 6:48 PM Dilip Kumar wrote:
> >
> > On Tue, May 11, 2021 at 4:50 PM Amul Sul wrote:
> > >
> > > On Tue, May 11, 2021 at 4:13 PM Dilip Kumar wrote:
> > > > I might be miss
rform vacuum in parallel */
> if (parallel_workers <= 0)
Yes it should if (parallel_workers == 0)
> 8) Is it still true that if parallel workers are specified as 0 the
> parallelism will not be picked up?
> From the docs: This feature is known as parallel vacuum. To disable
on't need separate "if checks" whether the
XLogAcceptWrites() is called or not, instead we can just rely on the
state, if it is #4 then we have to call XLogAcceptWrites(). If so
then I think it's okay to have an additional state, just wanted to
know what idea you had in mind?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, 12 May 2021 at 1:43 PM, Pavel Borisov
wrote:
> ср, 12 мая 2021 г. в 11:09, Dilip Kumar :
>
>> While testing something on spgist I found that at certain point while
>> inserting in spgist it is going for doPickSplit, but even after split
>> is is not able to find a
On Wed, May 12, 2021 at 2:21 PM Pavel Borisov wrote:
>>
>> PFA v1 patch. Does this help?
>
> I've made a mistake in attributes count in v1. PFA v2
V2 works. Thanks for fixing this quickly, I think you can add a
comment for the new error condition you added.
-
ow whether we can reduce
+ * index tuple size by suffixing its key part or we will go into the
+ * endless loop on pick-split (in case included columns data is big enough
INCLUDEd -> why you have used a mixed case here?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
v4
Okay, that makes sense.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
if (!oldpublist)
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > errmsg("subscription must contain at least one
> > publication"),
> > errdetail("Dropping all the publications from a
> > subscription is not supported")));
>
> Or how about just errmsg("cannot drop all the publications of the
> subscriber \"%s\"", subname) without any error detail?
IMHO, this message without errdetail looks much better.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 13, 2021 at 2:26 AM Robert Haas wrote:
>
> On Wed, May 12, 2021 at 1:39 AM Dilip Kumar wrote:
> > Your idea makes sense, but IMHO, if we are first writing
> > XLogAcceptWrites() and then pushing out the barrier, then I don't
> > understand the meanin
now IMHO, we don't need to keep the
system in recovery until pg_prohibit_wal(false) is called, right?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
hose pid you have passed,
the connected client to that backend might not have superuser
privileges and if you use elevel LOG then that message will be sent to
that connected client as well and I don't think that is secure. So
can we use LOG_SERVER_ONLY so that we can prevent
it from sending to the client.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 13, 2021 at 2:54 PM Amul Sul wrote:
>
> On Thu, May 13, 2021 at 12:36 PM Dilip Kumar wrote:
> >
> > On Wed, May 12, 2021 at 5:55 PM Amul Sul wrote:
> > >
> >
> > Thanks for the updated patch, while going through I noticed this comment.
> >
On Thu, May 13, 2021 at 3:06 PM Bharath Rupireddy
wrote:
>
> On Thu, May 13, 2021 at 2:57 PM Bharath Rupireddy
> wrote:
> > On Thu, May 13, 2021 at 2:44 PM Dilip Kumar wrote:
> > > +1 for the idea. I did not read the complete patch but while reading
> > > thro
>
Yes, that was my exact point, that in this particular code log with
LOG_SERVER_ONLY.
Like this.
/* Log immediately if dictated by log_statement */
if (check_log_statement(parsetree_list))
{
ereport(LOG_SERVER_ONLY,
.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 13, 2021 at 5:15 PM Bharath Rupireddy
wrote:
>
> On Thu, May 13, 2021 at 5:14 PM Dilip Kumar wrote:
> >
> > On Thu, May 13, 2021 at 4:16 PM Bharath Rupireddy
> > wrote:
> > >
> > > I'm saying that - currently, queries are logged wit
On Thu, May 13, 2021 at 5:18 PM Dilip Kumar wrote:
>
> On Thu, May 13, 2021 at 5:15 PM Bharath Rupireddy
> wrote:
> >
> > On Thu, May 13, 2021 at 5:14 PM Dilip Kumar wrote:
> > >
> > > On Thu, May 13, 2021 at 4:16 PM Bharath Rupireddy
> > > wro
ory(recoveryTargetTLI);
-
- /*
- * If the location of the checkpoint record is not on the expected
- * timeline in the history of the requested timeline, we cannot proceed:
- * the backup is not part of the history of the requested timeline.
- */
- if (tliOfPointInHistory(ControlFile->checkPoint, expectedTLEs) !=
- ControlFile->checkPointCopy.ThisTimeLineID)
- {
=
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
thing there yet.
- pg_log_error("parallel vacuum degree must be a non-negative integer");
+ pg_log_error("parallel workers for vacuum must be greater than or
equal to zero");
exit(1);
[1]
https://www.postgresql.org/message-id/os0pr01mb5716415335a06b489f1b3a8194...@os0pr0
ng back-branches
> > unfixed is not great.
>
> Hm. Index bloat is not something to totally ignore, though, so I'm
> not sure what the best cutoff is.
>
> Anyway, here is a patch set teased apart into committable bites,
> and with your other points addr
sm they are
using) on the remote server and what is the vacuuming need for that
database. So maybe garbage collection should be controlled locally by
the DBA on that server.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
l if we directly fix this one line to initialize
"expectedTLEs" from "recoveryTargetTLI" then it will expose to the
same problem this commit tried to fix.
[2]
if (tliOfPointInHistory(ControlFile->checkPoint, expectedTLEs) !=
ControlFile->checkPointCopy.ThisTimeLineID)
{
error()
}
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 13, 2021 at 2:56 PM Dilip Kumar wrote:
>
> Great thanks. I will review the remaining patch soon.
I have reviewed v28-0003, and I have some comments on this.
===
@@ -126,9 +127,14 @@ XLogBeginInsert(void)
Assert(mainrdata_last == (XLogRecData *) &mainrdata_head);
f history
was missing.
> The fix caused the issue for the case (1).
Basically, before this commit expectedTLEs and recoveryTargetTLI were
in always in sync which this patch broke.
> The proposed fix fixes the case (1), caused by the commit.
Right, I agree with the fix. So fix should be j
901 - 1000 of 1722 matches
Mail list logo
