re not many new dead tuples in
the heap (total dead tuple based on the statistic - existing items in
the conveyor belt) then we should conditionally skip the heap scanning
(first pass) and directly jump to the index vacuuming for some or all
the indexes based on the index size bloat.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 25, 2022 at 10:45 PM Peter Geoghegan wrote:
>
> On Fri, Feb 25, 2022 at 5:06 AM Dilip Kumar wrote:
> > Based on this discussion, IIUC, we are saying that now we will do the
> > lazy_scan_heap every time like we are doing now. And we will
> > conditionally s
allowed values for strategy
> types along with an error message. This will be helpful for the users.
>
I will fix these two comments while posting the next version.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Mar 1, 2022 at 5:15 PM Dilip Kumar wrote:
> On Tue, Feb 22, 2022 at 8:27 PM Ashutosh Sharma
> wrote:
>
>> I'm not sure about the current status, but found it while playing
>> around with the latest changes a bit, so thought of sharing it here.
>>
that right? and If I set only_local=off then
it will create the infinite loop again? So how are we protecting
against this case?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
between
node2 to node1? I mean instead of making this a boolean flag that
whether we want local change or remote change, can't we control the
changes based on the origin id? Such that node1 will get the local
changes of node3 but with using the same subscription it will get
changes from node3 which are originated from node2 but it will not
receive the changes which are originated from node1.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Mar 7, 2022 at 3:01 PM Amit Kapila wrote:
>
> On Mon, Mar 7, 2022 at 1:11 PM Dilip Kumar wrote:
> >
> > On Mon, Mar 7, 2022 at 10:15 AM Amit Kapila wrote:
> > > > > I haven't yet gone through the patch, but I have a question about the
> > >
the OID isn't too big,
> though. We should ereport() before we get to this point if we somehow
> run out of values, but it might be nice to have a check here as a
> backup.
Yeah, we could do that, I will do that in the next version. Thanks.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
orig_attr->tai_colflags |= TOASTCOL_INCOMPRESSIBLE;
Do we need to set TOASTCOL_INCOMPRESSIBLE while trying to externalize
it, the comment say "ignore on subsequent compression passes"
but after this will there be more compression passes? If we need to
set this TOASTCOL_INCOMPRESSIBLE then comment should explain this.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
so
we can not completely deny that it knows nothing about the copy?
And the fact that it's got an argument of type
> copy_relation_storage and the argument name is copy_storage and the
> value is sometimes RelationCopyStorageis a terminological muddle, too.
> So I feel like perhaps this needs more thought.
One option is that we can duplicate this loop in dbcommand.c as well
like we are having it already in tablecmds.c and heapam_handler.c?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
y)
I think we should also write the test cases for create database
strategy. But I do not see any test case for create database for
testing the existing options. So I am wondering whether we should add
the test case only for the new option we are providing or we should
create a separate path
we can proceed as usual. */
+if (!VARATT_IS_COMPRESSED(DatumGetPointer(*value)))
+toast_tuple_externalize(ttc, attribute, options);
I think we don't need "Sanity check:" here, the remaining part of the
comment is fine.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
;
>
> We can probably club together above two if-checks.
Done
> ==
>
> +
> + strategy
> +
> +
> + This is used for copying the database directory. Currently, we have
> + two strategies the WAL_LOG and the
> + FILE_C
On Thu, Mar 10, 2022 at 10:18 PM Robert Haas wrote:
>
> On Thu, Mar 10, 2022 at 6:02 AM Dilip Kumar wrote:
> > I have completely changed the logic for this refactoring. Basically,
> > write_relmap_file(), is already having parameters to control whether
> > to write wa
On Fri, Mar 11, 2022 at 11:52 AM Dilip Kumar wrote:
>
> On Thu, Mar 10, 2022 at 10:18 PM Robert Haas wrote:
> >
> > On Thu, Mar 10, 2022 at 6:02 AM Dilip Kumar wrote:
> > > I have completely changed the logic for this refactoring. Basically,
> > > wr
stand is that we need to drop database buffers and remove
pending sync requests because we are deleting underlying files and if
we do not do that in some extreme cases then there is no need to drop
the buffers or remove the pending sync request and the worst
consequences would be waste of
from pg_class where relpersistence='t';
> count
> ---
> 1
> (1 row)
I think this is not a right example to show the problem, no? Because
you are showing the pg_class entry and the pg_class is not a temp
relation so even if we avoid copying the temp rel
le creation I think the exact
data insertion on the subscriber side will be taken care of by the
insert WALs no?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, Mar 12, 2022 at 11:06 AM Dilip Kumar wrote:
>
> In fact while we are rewriting the relation during vacuum full that
> time also we are calling log_newpage() under RelationCopyStorage() and
> during standby if it gets promoted we will be having some buffers in
> the buffe
el lock function. Do you still think we should
change?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
'll probably go through and do a pass over the comments once you post
> the next version of this. There seems to be work needed in a bunch of
> places, but it probably makes more sense for me to go through and
> adjust the things that seem to need it rather than listing a bunch of
>
rue only in case of a shared map file (when dbOid is
> > Invalid and tblspcOid is globaltablespace_oid) or am I missing
> > something?
>
> *facepalm*
>
> Good catch, sorry that I'm slow on the uptake today.
>
> v3 attached.
Thanks Ashutosh and Robert. Other pact
On Fri, Mar 18, 2022 at 1:44 AM Robert Haas wrote:
>
> On Wed, Mar 16, 2022 at 12:53 AM Dilip Kumar wrote:
> > Thanks Ashutosh and Robert. Other pacthes cleanly applied on this
> > patch still generated a new version so that we can find all patches
> > together. Th
On Sun, Mar 20, 2022 at 12:03 AM Robert Haas wrote:
>
> On Fri, Mar 18, 2022 at 12:39 AM Dilip Kumar wrote:
> > > One question that occurred to me when looking this over is whether, or
> > > why, it's safe against concurrent smgr invalidations.
> >
> &
E followed by insertions) either from the parsetree or the
> WALs. I’ll have to dig more on this.
I agree that this is a bit tricky, anyway I will also put more thoughts on this.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Mar 21, 2022 at 7:07 PM Robert Haas wrote:
>
> On Sun, Mar 20, 2022 at 1:34 AM Dilip Kumar wrote:
> > I thought that way because IIUC, when we are locking the database
> > tuple we are ensuring that we are calling
> > ReceiveSharedInvalidMess
On Mon, Mar 21, 2022 at 8:29 PM Dilip Kumar wrote:
>
> On Mon, Mar 21, 2022 at 7:07 PM Robert Haas wrote:
> >
> > On Sun, Mar 20, 2022 at 1:34 AM Dilip Kumar wrote:
> > > I thought that way because IIUC, when we are locking the database
> > > tuple
On Mon, Mar 21, 2022 at 11:53 PM Robert Haas wrote:
>
> On Mon, Mar 21, 2022 at 11:21 AM Dilip Kumar wrote:
> > I tried to debug the case but I realized that somehow
> > CHECK_FOR_INTERRUPTS() is not calling the
> > AcceptInvalidationMessages() and I could not find th
s_build#L347
I have marked it rejected.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Mar 22, 2022 at 10:28 AM Dilip Kumar wrote:
>
>
> I think this make sense. I haven't changed the original patch as you
> told you were improving on some comments, so in order to avoid
> conflict I have created this add on patch.
>
In my previous patch mistakenly I
On Tue, Mar 22, 2022 at 8:53 PM Robert Haas wrote:
>
> On Tue, Mar 22, 2022 at 5:00 AM Dilip Kumar wrote:
> > In my previous patch mistakenly I used src_dboid instead of
> > dest_dboid. Fixed in this version. For destination db I have used
> > lock mode as AccessShar
using the wal log strategy we do not remove the database
directory, because an abort transaction will take care of removing the
relation file. But then in failure case we will leave the orphaned
database directory with version file and the relmap file. Another
option is to do the redundant cleanup as we are doing now. Any other
options?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
also curious to see how much those parameters become
> relevant with more compression options possible with toast values.
Agree with you, it doesn't make much sense here to add an extra user
visible configuration parameter, where it is not really clear for
users how to really configure it to get the best out of it.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ill always be under the threshold. So maybe the
chunk difference threshold can be a function based on the total
numbers of chunks required for the data, maybe a logarithmic function
so that the threshold grows slowly along with the base data size.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Mar 23, 2022 at 5:54 PM Robert Haas wrote:
>
> On Wed, Mar 23, 2022 at 4:42 AM Dilip Kumar wrote:
> > Okay this is an interesting point. So one option is that in case of
> > failure while using the wal log strategy we do not remove the database
> > dir
On Wed, Mar 23, 2022 at 7:03 PM Robert Haas wrote:
>
> On Wed, Mar 23, 2022 at 9:19 AM Dilip Kumar wrote:
> > I think directly using smgrcreate() is a better idea instead of first
> > registering and then unregistering it. I have made that change in
> > the attached p
On Wed, Mar 23, 2022 at 9:13 PM Andres Freund wrote:
>
> Hi,
>
> On 2022-03-23 18:49:11 +0530, Dilip Kumar wrote:
> > I think directly using smgrcreate() is a better idea instead of first
> > registering and then unregistering it. I have made that change in
> > t
On Wed, Mar 23, 2022 at 9:05 PM Dilip Kumar wrote:
>
> On Wed, Mar 23, 2022 at 7:03 PM Robert Haas wrote:
> >
> > On Wed, Mar 23, 2022 at 9:19 AM Dilip Kumar wrote:
> > > I think directly using smgrcreate() is a better idea instead of first
> > > registering
On Wed, Mar 23, 2022 at 9:25 PM Dilip Kumar wrote:
>
> On Wed, Mar 23, 2022 at 9:13 PM Andres Freund wrote:
> >
> > Hi,
> >
> > On 2022-03-23 18:49:11 +0530, Dilip Kumar wrote:
> > > I think directly using smgrcreate() is a better idea instead of first
>
On Wed, Mar 23, 2022 at 10:37 PM Andres Freund wrote:
>
> Hi,
>
> On 2022-03-23 22:29:40 +0530, Dilip Kumar wrote:
> > I could not see any reason for it to fail, and I could not reproduce
> > it either. Is it possible to access the server log for this cfbot
> >
tion to stream which has no changes. I can try
> to fix the code here such that we don't mark the transaction as
> streamed unless we have streamed at least one change but I don't see
> how it is related to this particular test failure.
Yeah, this can be improved but as you mentioned that we never select
an empty transaction for streaming so this case should not occur. I
will perform some testing/review around this and report.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
amhandler=lz4handler, amtype=c.
> The core code must of course know how to instantiate an AM of type
> 'c' and what to use it for.
>
> https://postgr.es/m/20171213151818.75a20...@postgrespro.ru
I have changed this, I agree that using the access method for creating
compression
one of the built-in compression
methods and if we are altering the compression method or we are doing
INSERT INTO SELECT to the target field having a different compression
method then we should not rewrite/decompress those tuples. Basically,
I mean to say that the built-in compression methods can always be
treated as PRESERVE because those can not be dropped.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Dec 1, 2020 at 4:50 PM Dilip Kumar wrote:
>
> On Sat, Nov 21, 2020 at 3:50 AM Robert Haas wrote:
>
> While working on this comment I have doubts.
>
> > I wonder in passing about TOAST tables and materialized views, which
> > are the other things that have st
On Tue, Dec 1, 2020 at 11:38 AM Dilip Kumar wrote:
>
> On Mon, Nov 30, 2020 at 6:49 PM Amit Kapila wrote:
> >
> > On Mon, Nov 30, 2020 at 3:14 AM Noah Misch wrote:
> > >
> > > On Mon, Sep 07, 2020 at 12:00:41PM +0530, Amit Kapila wrote:
> > > >
might not be the best idea. Instead, you can write some
common functions and we can call them from different places. So if
something is very much common and will not vary based on the storage
type we can keep it outside the tableam interface however we can move
them into some common functio
state.
>
> I have also improved the comments atop ReorderBufferResetTXN to
> mention the case when we need to continue streaming after getting an
> error.
>
> Attached patch for the above changes.
>
> Thoughts?
LGTM.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Dec 1, 2020 at 9:15 PM Dilip Kumar wrote:
>
> On Tue, Dec 1, 2020 at 4:50 PM Dilip Kumar wrote:
> >
> > On Sat, Nov 21, 2020 at 3:50 AM Robert Haas wrote:
> >
> > While working on this comment I have doubts.
> >
> > > I wonder in passing abo
the
query->isForCTAS flag then why we are calling this again?
——
---
+*/
+ if (!(root->parse->isForCTAS &&
+ root->query_level == 1))
+ run_cost += parallel_tuple_cost * path->path.rows;
>From this check, it appeared that the lower level gather will also get
influenced by this, consider this
-> NLJ
-> Gather
-> Parallel Seq Scan
-> Index Scan
This condition is only checking that it should be a top-level query
and it should be under CTAS then this will impact all the gather nodes
as shown in the above example.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
> if (limit_needed(root->parse))
> if (root->parse->sortClause)
> if (root->parse->distinctClause)
> if (root->parse->hasWindowFuncs)
> if (root->parse->groupClause || root->parse->groupingSets ||
> root->parse->hasAggs || root->root->hasHavingQual)
>
Yeah, and as I pointed earlier, along with this we also need to
consider that the RelOptInfo must be the final target(top level rel).
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
_UPDATE)
+ return PROPARALLEL_UNSAFE;
I understand that we can now allow updates from the worker, but what
is the problem if we allow the parallel select even if there is an
update in the leader?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Dec 9, 2020 at 4:03 PM Amit Kapila wrote:
>
> On Wed, Dec 9, 2020 at 2:38 PM Dilip Kumar wrote:
> >
> > On Wed, Dec 9, 2020 at 10:11 AM Greg Nancarrow wrote:
> > >
> > > On Wed, Dec 9, 2020 at 1:35 AM vignesh C wrote:
> > > >
> >
On Wed, 9 Dec 2020 at 5:41 PM, Amit Kapila wrote:
> On Wed, Dec 9, 2020 at 4:18 PM Dilip Kumar wrote:
> >
> > On Wed, Dec 9, 2020 at 4:03 PM Amit Kapila
> wrote:
> > >
> > > On Wed, Dec 9, 2020 at 2:38 PM Dilip Kumar
> wrote:
> > > >
>
On Mon, Nov 30, 2020 at 2:40 PM Dilip Kumar wrote:
>
> On Mon, Nov 30, 2020 at 12:17 PM Yugo NAGATA wrote:
>
> Thanks for looking into this.
>
> > On Thu, 22 Oct 2020 20:36:48 +0530
> > Dilip Kumar wrote:
> >
> > > On Thu, Oct 22, 2020 at 7:50 PM Dilip
ing the SELECT to be executed in parallel so why
we would need the transaction Id for that. I agree that we would need
this once we try to perform the Insert also from the worker in the
remaining patches.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Dec 10, 2020 at 1:50 PM Greg Nancarrow wrote:
>
> On Thu, Dec 10, 2020 at 5:25 PM Dilip Kumar wrote:
> >
> >
> > /*
> > + * PrepareParallelMode
> > + *
> > + * Prepare for entering parallel mode, based on command-type.
> > + */
> &
essage-id/cahut+puemk4so8ogzxc_ftzpkga8uc-y5qi-krqhsy_p0i3...@mail.gmail.com
> > [3] -
> > https://www.postgresql.org/message-id/CAA4eK1KFsjf6x-S7b0dJLvEL3tcn9x-voBJiFoGsccyH5xgDzQ%40mail.gmail.com
> > [4] -
> > https://www.postgresql.org/message-id/CAA4eK1Ld9XaLoTZ
sA(plannedstmt->planTree, Gather) is false ?
>
> I did some test but did not find a case like that.
>
This seems like an extra check. Apart from that if we combine 0001
and 0002 there should be an additional protection so that it should
not happen that in cost_gather
On Thu, Dec 10, 2020 at 5:00 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 10, 2020 at 4:49 PM Dilip Kumar wrote:
> > > + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
> > > +
On Mon, Dec 14, 2020 at 4:06 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 10, 2020 at 7:20 PM Bharath Rupireddy
> wrote:
> > On Thu, Dec 10, 2020 at 5:19 PM Dilip Kumar wrote:
> > > > > > + allow = ps && IsA(ps, Gath
I kept the 0001 patch(so far) such that
> it doesn't have the code to influence the planner to consider parallel
> tuple cost as 0. It works on the plan whatever gets generated and
> decides to allow parallel inserts or not. And in the 0002 patch, I
> added the code for influencing the planner to consider parallel tuple
> cost as 0. Maybe we can have a 0003 patch for tests alone.
Yeah, that makes sense and it will be easy for the review.
> Once we are okay with the above analysis and use cases, we can
> incorporate the Append changes to respective patches.
>
> Hope that's okay.
Make sense to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Dec 9, 2020 at 5:37 PM Dilip Kumar wrote:
>
> On Sat, Nov 21, 2020 at 3:50 AM Robert Haas wrote:
> >
> > On Wed, Nov 11, 2020 at 9:39 AM Dilip Kumar wrote:
> > > There were a few problems in this rebased version, basically, the
> > > compression optio
would be
better that in the first part we parallelize the plan where gather
node on top. I see that we have already worked on the patch where the
append node is on top so I would suggest that we can keep that part in
a separate patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
tting
each page from the bitmaphash we need to acquire the lock and this also
might generate a lot of lock contention but we want to try the POC and
check the performance. In fact I have already implemented the POC and
results aren't great. But I am still experimenting with it to see whether
t
-> Parallel Seq Scan on table1 (cost=0.00..9.17 rows=417 width=4)
>
> IMO, showing Insert under Gather makes sense if the query is INSERT
> INTO SELECT as it's in the other patch [1]. Since here it is a CTAS
> query, so having Create under Gather looks fine to me. This way we can
> also distinguish the EXPLAINs of parallel inserts in INSERT INTO
> SELECT and CTAS.
I don't think that is a problem because now also if we EXPLAIN CTAS it
will appear like we are executing the select query because that is
what we are planning for only the select part. So now if we are
including the INSERT in the planning and pushing the insert under the
gather then it will make more sense to show INSERT instead of showing
CREATE. Let's see what others think.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
come back to it later once all other
> parts of the patch are good.
>
> > And also, some might wonder that Create under Gather means that each
> > parallel worker is creating the table, it's actually not the creation
> > of the table that's parallelized but it's insertion. If required, we
> > can clarify it in CTAS docs with a sample EXPLAIN. I have not yet
> > added docs related to allowing parallel inserts in CTAS. Shall I add a
> > para saying when parallel inserts can be picked and how the sample
> > EXPLAIN looks? Thoughts?
> >
>
> Yeah, I don't see any problem with it, and maybe we can move Explain
> related code to a separate patch. The reason is we don't display DDL
> part without parallelism and this might need a separate discussion.
>
This makes sense to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
an
plan must know that it is returning a 0 rows?
8.
+char *intoclause_space = shm_toc_allocate(pcxt->toc,
+ intoclause_len);
+memcpy(intoclause_space, intoclausestr, intoclause_len);
+shm_toc_insert(pcxt->toc, PARALLEL_KEY_INTO_CLAUSE, intoclause_space);
One blank line between variable declaration and next code segment,
take care at other places as well.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sun, Dec 27, 2020 at 2:20 PM Bharath Rupireddy
wrote:
>
> On Sat, Dec 26, 2020 at 11:11 AM Dilip Kumar wrote:
> > I have reviewed part of v15-0001 patch, I have a few comments, I will
> > continue to review this.
>
> Thanks a lot.
>
> > 1.
> > Why is th
On Sun, Dec 27, 2020 at 12:40 PM Andrey Borodin wrote:
>
>
>
> > 25 дек. 2020 г., в 14:34, Dilip Kumar написал(а):
> >
> >
> >
> >
> >
> >
> >
>
> Maybe add Lz4\Zlib WAL FPI compression on top of this patchset? I'
On Mon, Dec 28, 2020 at 10:45 AM Dilip Kumar wrote:
>
> On Sun, Dec 27, 2020 at 2:20 PM Bharath Rupireddy
> wrote:
> >
> > On Sat, Dec 26, 2020 at 11:11 AM Dilip Kumar wrote:
> > > I have reviewed part of v15-0001 patch, I have a few comments, I will
On Wed, 30 Dec 2020 at 10:47 AM, Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> On Wed, Dec 30, 2020 at 10:32 AM Dilip Kumar
> wrote:
> > I have completed reviewing 0001, I don't have more comments, just one
> > question. Soon I will
On Wed, Dec 30, 2020 at 10:49 AM Dilip Kumar wrote:
>
> On Wed, 30 Dec 2020 at 10:47 AM, Bharath Rupireddy
> wrote:
>>
>> On Wed, Dec 30, 2020 at 10:32 AM Dilip Kumar wrote:
>> > I have completed reviewing 0001, I don't have more comments, just one
>
arallelInsertState to
> +void
> +SetParallelInsertState(QueryDesc *queryDesc)
>
> Change IsParallelInsertionAllowedInCTAS to
>
> +bool
> +IsParallelInsertionAllowed(ParallelInsertKind pinskind, IntoClause *into)
> +{
>
> Thoughts?
>
I haven’t thought about these structures yet but yeah making them
generic will be good.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
t; + int32 *acceleration = palloc(sizeof(int32));
> +
> + /* initialize with the default acceleration */
> + *acceleration = 1;
> +
> + if (list_length(options) > 0)
> + {
> + ListCell*lc;
> +
> + foreach(lc, options)
> + {
> + DefElem*def = (DefElem *) lfirst(lc);
> +
> + if (strcmp(def->defname, "acceleration") == 0)
> + *acceleration = pg_atoi(defGetString(def),
> sizeof(int32), 0);
>
> Don't you need to say "else: error: unknown compression option" ?
>
> + /*
> +* Compression option must be only valid if we are updating the
> compression
> +* method.
> +*/
> + Assert(DatumGetPointer(acoptions) == NULL ||
> OidIsValid(newcompression));
> +
>
> should say "need be valid only if .."
>
Thanks for the review, I will work on these and respond along with the
updated patches.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
dd_u64_impl,
> > pg_atomic_compare_exchange_u64_impl, pg_atomic_init_u64_impl and their
> > u32 counterparts use pass the parameter as volatile pg_atomic_uint64 *ptr.
But in your case, I do not understand the intention that where do you
think that the compiler can optimize it and read the old value?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ome code that is generic for CTAS and INSERT INTO
SELECT *, So is it
possible to take out that common code to a separate base patch? Later
both CTAS and INSERT INTO SELECT * can expand
that for their usage.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jan 6, 2021 at 11:26 AM Bharath Rupireddy
wrote:
>
> On Wed, Jan 6, 2021 at 10:17 AM Dilip Kumar wrote:
> >
> > On Wed, Jan 6, 2021 at 9:23 AM Bharath Rupireddy
> > wrote:
> > >
> >
> > +/*
> > + * List the commands here for which para
heckpoint->CreateCheckPoint) along with the checkpointer
> process. Having said that, I'm not quite sure whether it can get
> called at the same time from a backend(may be with checkpoint;
> command) and the checkpointer process.
If it is a standalone backend then there will be no postmaster and no
other process i.e. no checkpoint process also.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ng like below?
> > CREATE PUBLICATION production_publication FOR ALL TABLES SCHEMA
> > production WITH skip = marketing, accounts, sales; --> meaning is,
> > replicate all the tables in the schema production except marketing,
> > accounts, sales tables.
>
On Thu, Feb 11, 2021 at 8:17 PM Robert Haas wrote:
>
> On Thu, Feb 11, 2021 at 7:36 AM Dilip Kumar wrote:
> > W.R.T the attached patch, In HeapTupleHeaderGetDatum, we don't even
> > attempt to detoast if there is no external field in the tuple, in POC
> > I have
text compression pglz with (level 1) );
Thanks for the review and patch for HIDE_COMPRESSAM, I will merge
this into the main patch. And work on other comments after fixing the
issue related to compressed data in composite types.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, Feb 13, 2021 at 8:14 PM Dilip Kumar wrote:
>
> On Thu, Feb 11, 2021 at 8:17 PM Robert Haas wrote:
> >
> > On Thu, Feb 11, 2021 at 7:36 AM Dilip Kumar wrote:
> > > W.R.T the attached patch, In HeapTupleHeaderGetDatum, we don't even
> > > attempt
On Thu, Feb 11, 2021 at 1:37 AM Robert Haas wrote:
>
> On Wed, Feb 10, 2021 at 9:52 AM Dilip Kumar wrote:
> > [ new patches ]
>
> I think that in both varattrib_4b and toast_internals.h it would be
> better to pick a less generic field name. In toast_internals.h it's
to fetch 1 chunk either way, but I
> think we should make it correct.
Fixed
> TOAST_COMPRESS_SET_SIZE_AND_METHOD() could Assert something about cm_method.
While replying to the comments, I realised that I have missed it. I
will fix it in the next version.
> Small delta patch with a few other suggested changes attached.
Merged
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, Feb 20, 2021 at 2:51 AM Robert Haas wrote:
>
> On Fri, Feb 19, 2021 at 11:12 AM Dilip Kumar wrote:
> I think that these performance tests aren't really exercising the
> expanded-record stuff, just the ExecEvalRow changes. We need to test
> that test case, and I ten
On Sat, Feb 20, 2021 at 11:04 AM Dilip Kumar wrote:
> Even after the above case, we might say it is still not a problem for
> this patch because even though t2 doesn't have a direct relationship
> with lz4 but it has an indirect relationship with lz4 via t1. So I
> think thi
On Sat, Feb 20, 2021 at 11:04 AM Dilip Kumar wrote:
>
> On Sat, Feb 20, 2021 at 2:51 AM Robert Haas wrote:
> >
> > On Fri, Feb 19, 2021 at 11:12 AM Dilip Kumar wrote:
> > I think that these performance tests aren't really exercising the
> > expanded-record
On Fri, Feb 12, 2021 at 3:26 AM Robert Haas wrote:
>
> On Thu, Feb 11, 2021 at 6:07 AM Dilip Kumar wrote:
> > > Thanks for the patch. I tested the new function and it works as
> > > expected. I have no further comments on the v13 patch.
> >
> > Thanks for the
On Wed, Feb 24, 2021 at 12:39 PM Kyotaro Horiguchi
wrote:
>
> At Tue, 23 Feb 2021 12:03:32 +0530, Dilip Kumar wrote
> in
> > On Fri, Feb 12, 2021 at 3:26 AM Robert Haas wrote:
> > > There might be some more to say here, but those are things I notice on
> > > a
On Wed, Feb 24, 2021 at 2:26 PM Kyotaro Horiguchi
wrote:
>
> At Wed, 24 Feb 2021 13:15:27 +0530, Dilip Kumar wrote
> in
> > On Wed, Feb 24, 2021 at 12:39 PM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Tue, 23 Feb 2021 12:03:32 +0530, Dilip Kumar
> &g
On Wed, Feb 24, 2021 at 3:25 PM Dilip Kumar wrote:
>
> > > > The reason for the checkpoint is to move to "paused" state in a
> > > > reasonable time. I think we need to mention that reason rather than
> > > > what is done her
nd it didn't seem
> stable I push that a bit strongly but it actually consists of only
> three values and not likely to get further values. So I don't insist
> on the style so strongly here.
>
Changed as per the suggestion.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www
ther comment on this.
>
> I'll wait for a day before marking this RfC in case anyone have
> further comments.
Okay.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 26, 2021 at 8:10 PM Dilip Kumar wrote:
>
> On Sun, Feb 21, 2021 at 5:33 PM Dilip Kumar wrote:
> >
>
> Based on offlist discussion with Robert, I have done further analysis
> of the composite type data. So the Idea is that I have analyzed all
> the callers of
.sql:\set HIDE_COMPRESSAM false
> src/test/regress/sql/compression.sql:\set HIDE_COMPRESSAM false
>
> Whereas the patch I sent had at the end:
>
> +\set HIDE_COMPRESSAM on
>
> ("on" is the default when run under pg_regress)
I will fix this.
[1]
https://docs.unrealengine.com/en-US/API/Runtime/Core/Compression/LZ4_decompress_safe_partial/index.html
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 26, 2021 at 1:33 PM Kyotaro Horiguchi
wrote:
>
> At Thu, 25 Feb 2021 13:22:53 +0530, Dilip Kumar wrote
> in
> > On Thu, Feb 25, 2021 at 12:42 PM Kyotaro Horiguchi
> > wrote:
> > > The latest version applies (almost) cleanly to the current master and
&
ast_compression_oid = false;
> The false assignment was copied from namespace.c: baseSearchPathValid.
I will fix this.
So as of now, we can make this patch such that it is enough to work
with the built-in method and later we can add another enhancement
patch that can work with the custom compression methods.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
w i.e. below1.8.3 (
in this release the slicing issue was fixed) then we can call the full
decompression routine from the slicing function.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Mar 1, 2021 at 8:53 PM Dilip Kumar wrote:
>
> Now, I think the only pending thing is related to the expandedrecord,
> basically, currently, we have detoasted the compressed filed only in
> expanded_record_set_field_internal function. I am still not
> completely sure that f
On Sat, Jan 23, 2021 at 10:06 AM Dilip Kumar wrote:
>
> But I am afraid that whether this adjustment (setting based on
> receiveTLI) is done based on some analysis or part of some bug fix. I
> will try to find the history of this and maybe based on that we can
> make a better de
1 - 100 of 1664 matches
Mail list logo
