ase feel
free. I haven't managed to take more than a brief look at the v5
patch, but will try to look at it (or maybe the new version if you
post) more closely this week.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Mon, Aug 3, 2020 at 2:54 PM Amit Langote wrote:
> However, I noticed that having system columns like ctid, xmin, etc. in
> the RETURNING list is now broken and maybe irrepairably due to the
> approach we are taking in the patch. Let me show an example:
>
> drop table foo;
>
topped
working in PG 12 [1] for which I've proposed a workaround (attached).
Another is that we forgot in our patch to "materialize" the virtual
tuple after conversion, which means slot_getsysattr() can't find it to
access system columns like xmin, etc.
--
Amit Langote
Enterprise
Hi Andrey,
Thanks for updating the patch. I will try to take a look later.
On Wed, Jul 22, 2020 at 6:09 PM Andrey V. Lepikhov
wrote:
> On 7/16/20 2:14 PM, Amit Langote wrote:
> > * Why the "In" in these API names?
> >
> > + /* COPY a bulk of
On Sun, Jul 26, 2020 at 6:06 AM Tom Lane wrote:
> Amit Langote writes:
> > [ v7-0001-Improve-performance-of-binary-COPY-FROM-with-buff.patch ]
>
> Pushed with cosmetic changes.
Thanks for that.
> I'd always supposed that stdio does enough internal buffering that short
n's projection and the tuple in the source
partition's format, I wanted to make sure that any changes made by the
destination partition's triggers are reflected in the output.
PFA v2.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
v2-0001-Fix-a-bug-with-RETURNING-when-UPDATE-moves-tuple.patch
Description: Binary data
On Wed, Jul 15, 2020 at 1:06 PM vignesh C wrote:
> On Wed, Jul 15, 2020 at 8:03 AM Amit Langote wrote:
> > On Tue, Jul 14, 2020 at 10:23 PM vignesh C wrote:
> > > Rest of the changes looked fine to me. Also I noticed that
> > > commit message was missing in the p
> destination partition and can be replaced by simpler expression ?
Well, it's true that the function and the trigger have nothing to do
with the main bug, but it's often good to be sure that the bug-fix
isn't breaking cases where they are present and have visible effect.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Ctrl+C anymore, but that should be
fixable by adjusting PG_TRY() blocks.
* ResultRelInfo.UseBulkModifying should be ri_usesBulkModify for consistency.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
i
Hi Vignesh,
On Tue, Jul 14, 2020 at 10:23 PM vignesh C wrote:
> On Tue, Jul 14, 2020 at 11:19 AM Amit Langote wrote:
> > Sounds fine to me. Although CopyLoadRawBuf() does not seem to a
> > candidate for rigorous code optimization as it does not get called
> > that often.
&
On Tue, Jul 14, 2020 at 2:02 PM vignesh C wrote:
> On Tue, Jul 14, 2020 at 7:26 AM Amit Langote wrote:
> > In CopyLoadRawBuf(), we could also change the condition if
> > (cstate->raw_buf_index < cstate->raw_buf_len) to if (BUF_BYTES > 0),
> > which looks clearer.
raw_buf_index < cstate->raw_buf_len) to if (BUF_BYTES > 0),
which looks clearer.
Also, if we are going to use the macro more generally, let's make it
look less localized. For example, rename it to RAW_BUF_BYTES similar
to RAW_BUF_SIZE and place their definitions close
and a few other cosmetic changes.
> >
>
> CopyReadBinaryData() looks meaningful. +1.
Okay, thanks. Let's have a committer take a look at this then?
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Wed, Jul 1, 2020 at 6:56 PM Daniel Gustafsson wrote:
>
> > On 2 Mar 2020, at 06:08, Amit Langote wrote:
> >
> > On Mon, Mar 2, 2020 at 4:43 AM Tom Lane wrote:
> >> Amit Langote writes:
> >>> Rebased again.
> >>
> >> Seems to nee
he response [1]) that memcpy cost for this case may be
> negligible.
Actually, an extra memcpy is incurred on every call of
CopyReadFromRawBuf(), but I haven't seen it to be very problematic.
By the way, considering the rebase over cd22d3cdb9b, it seemed to me
that we needed to update the comments in CopyStateData struct
definition a bit more. While doing that, I realized
CopyReadFromRawBuf as a name for the new function might be misleading
as long as we are only using it for binary data. Maybe
CopyReadBinaryData is more appropriate? See attached v4 with these
and a few other cosmetic changes.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
CopyFrom-binary-buffering_v4.patch
Description: Binary data
Reading my own words, I think I must fix an ambiguity:
On Fri, Jul 10, 2020 at 3:23 PM Amit Langote wrote:
> So even if an AM's table_tuple_insert() itself doesn't populate the
> transaction info into the slot handed to it, maybe as an optimization,
> it does not sound entir
Hi Soumyadeep,
On Fri, Jul 10, 2020 at 2:56 AM Soumyadeep Chakraborty
wrote:
>
> Hey Amit,
>
> On Thu, Jul 9, 2020 at 12:16 AM Amit Langote wrote:
>
> > By the way, what happens today if you do INSERT INTO a_zedstore_table
> > ... RETURNING xmin? Do you get an erro
l try this on the next version of the patch.
>
> >
> > HEADpatched
> > foo58.5 6.5
> > foo10 14 10
> > foo20 25 18
> >
>
> Numbers might improve a bit, if we remove the extra function calls as
> stated above.
Here the numbers with the updated patch:
HEADpatched (v2)
foo5 8.56.1
foo10 149.4
foo20 25 16.7
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
CopyFrom-binary-buffering_v2.patch
Description: Binary data
Speaking of which, I hope that Kato-san has looked at functions
match_rowcompare_to_indexcol(), expand_indexqual_rowcompare(), etc. in
indxpath.c as starting points for the code to match RowCompares to
partition keys.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Jul 9, 2020 at 1:53 AM Soumyadeep Chakraborty
wrote:
> On Tue, Jul 7, 2020 at 7:17 PM Amit Langote wrote:
> > Ah, I see. You might've noticed that ExecInsert() only ever sees leaf
> > partitions, because tuple routing would've switched the result
> > rela
that allows row-wise comparison with partition-pruning.
That would be great, thank you.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Hi Soumyadeep,
On Wed, Jul 8, 2020 at 9:37 AM Soumyadeep Chakraborty
wrote:
> On Tue, Jul 7, 2020 at 7:18 AM Amit Langote wrote:
> > If I change this to return a "heap" slot for partitioned tables, just
> > like for foreign tables, the problem goes away (see the atta
ng() is of the correct type to begin with. We could
do what I suggest above or maybe find some other way.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
partitioned-table-heap-slot.patch
Description: Binary data
On Tue, Jul 7, 2020 at 4:28 PM Amit Langote wrote:
> The median times for the COPY FROM commands above, with and without
> the patch, are as follows:
>
> HEADpatched
> foo58.5 6.5
> foo10 14 10
> foo20 25 18
Sorry, I forgot
ou can or follow the
guidelines around long names:
+ int32 remainingbytesincurrdatablock = RAW_BUF_SIZE -
cstate->raw_buf_index;
Maybe, remaining_bytes would've sufficed here, because "in the current
data block" might be clear to most readers by looking at the
surrounding code.
* The above point also helps avoid long code lines that don't fit
within 78 characters, like these:
+ memcpy(&cstate->attribute_buf.data[0],
&cstate->raw_buf[cstate->raw_buf_index],
remainingbytesincurrdatablock);
+
+ if (CopyGetData(cstate,
&cstate->attribute_buf.data[remainingbytesincurrdatablock],
+ (fld_size - remainingbytesincurrdatablock),
(fld_size - remainingbytesincurrdatablock)) != (fld_size -
remainingbytesincurrdatablock))
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
CopyFrom-binary-buffering.patch
Description: Binary data
On Fri, Jun 26, 2020 at 9:36 PM Amit Langote wrote:
> I would like to discuss a refactoring patch that builds on top of the
> patches at [1] to address $subject.
I forgot to update a place in postgres_fdw causing one of its tests to crash.
Fixed in the attached updated version.
--
be
>> pushed down to the remote side by DirectModify. So, I'm not sure we
>> really need the bulk UPDATE/DELETE.
> That may not be true for a partitioned table whose partitions are foreign
> tables. Esp. given the work that Amit Langote is doing [1]. It really depends
> o
On Mon, Jun 29, 2020 at 10:39 AM David Rowley wrote:
>
> On Sat, 27 Jun 2020 at 00:36, Amit Langote wrote:
> > 2. ExecCheckRTPerms(): checks permissions of *all* partitions before
> > executing the plan tree, but maybe it's okay to check only the ones
> > that will
design, restrictions etc.
I think you may want to take a look this recent proposal by Andrey Lepikhov:
* [POC] Fast COPY FROM command for the table with foreign partitions *
https://www.postgresql.org/message-id/flat/3d0909dc-3691-a576-208a-90986e55489f%40postgrespro.ru
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Fri, Jun 26, 2020 at 9:36 PM Amit Langote wrote:
> I would like to discuss a refactoring patch that builds on top of the
> patches at [1] to address $subject.
I've added this to the next CF: https://commitfest.postgresql.org/28/2621/
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
that is), but maybe that's because the loop in
ExecInitModifyTable() mentioned above is still in the way.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
[1] https://commitfest.postgresql.org/28/2575/
[2]
https://www.postgresql.org/message-id/flat/CAKJS1f_kfRQ3ZpjQyHC7%3DPK9vrh
On Fri, Jun 12, 2020 at 9:22 PM Ashutosh Bapat
wrote:
> On Wed, Jun 3, 2020 at 12:44 PM Amit Langote wrote:
> > Are you saying that the planner should take into account the state of
> > the cursor specified in WHERE CURRENT OF to determine which of the
> > tables to scan
On Sun, Jun 14, 2020 at 4:23 PM Etsuro Fujita wrote:
> Hi Amit-san,
>
> On Thu, Jun 11, 2020 at 6:10 PM Amit Langote wrote:
> > Reproduction steps:
> >
> > create table foo (a int, b int) partition by list (a);
> > create table foo1 (c int, b int, a int);
>
onsider each with its own RETURNING list.
I think we should be computing the *source* relation's RETURNING list,
because only that one of the two can consume the plan tuple correctly.
Attached is a patch that fixes things to be that way.
By the way, the problem exists since PG 11 when UPDATE
On Thu, May 28, 2020 at 11:08 PM Ashutosh Bapat
wrote:
> On Wed, May 27, 2020 at 6:51 PM Amit Langote wrote:
> > So in Rajkumar's example, the cursor is declared as:
> >
> > CURSOR IS SELECT * FROM tbl WHERE c1< 5 FOR UPDATE;
> >
> > and the WHERE CURR
smarter with
binary or hashed search, as you are trying to do, might help us
somewhat, but would it be interesting to consider alternative designs
for the underlying abstraction? Sorry, I don't have any concrete
ideas to offer at the moment, but thought sharing this perspective of
the problem might inspire others whose may have some.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, May 28, 2020 at 1:36 PM amul sul wrote:
> On Wed, May 27, 2020 at 12:53 PM Amit Langote wrote:
>> I guess the workaround is to declare the cursor such that no
>> partitions/children are pruned/excluded.
>
> Disabling pruning as well -- at-least for the statement or
On Thu, May 28, 2020 at 1:36 PM amul sul wrote:
> On Wed, May 27, 2020 at 12:53 PM Amit Langote wrote:
>> Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
>> would fail even with traditional inheritance:
>>
>> drop table if exists p cascad
On Wed, May 27, 2020 at 9:11 PM Ashutosh Bapat
wrote:
> On Wed, May 27, 2020 at 12:53 PM Amit Langote wrote:
> > On Fri, May 22, 2020 at 9:09 PM amul sul wrote:
> > > I tried similar things on inherit partitioning as follow and that looks
> > > fine:
> > >
resent in a cursor from a
table that could be had it not been pruned/excluded? If it can do
that, then give an error for the former and return false for the
latter.
I guess the workaround is to declare the cursor such that no
partitions/children are pruned/excluded.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/message-id/468c85d9-540e-66a2-1dde-fec2b741e688%40lab.ntt.co.jp
On Thu, May 14, 2020 at 7:55 AM David Rowley wrote:
> On Wed, 13 May 2020 at 19:02, Amit Langote wrote:
> > > As for which ResultRelInfos to initialize, couldn't we just have the
> > > planner generate an OidList of all the ones that we could need.
> > > Basic
On Tue, May 12, 2020 at 10:57 PM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, May 12, 2020 at 5:25 AM Robert Haas wrote:
> >> Ah, that makes sense. If we can invent dummy columns on the parent
> >> rel, then most of what I was worrying about no longer seems ver
On Wed, May 13, 2020 at 8:52 AM David Rowley wrote:
> On Wed, 13 May 2020 at 00:54, Ashutosh Bapat
> wrote:
> >
> > On Mon, May 11, 2020 at 8:11 PM Amit Langote
> > wrote:
> > > > Per row overhead would be incurred for every row whereas the plan time
> &
On Tue, May 12, 2020 at 9:54 PM Ashutosh Bapat
wrote:
> On Mon, May 11, 2020 at 8:11 PM Amit Langote wrote:
> > > Per row overhead would be incurred for every row whereas the plan time
> > > overhead is one-time or in case of a prepared statement almost free.
> > >
nting
these child expressions, from there they go back into child
reltargets, so it would appear that our appendrel transformation code
must somehow be taught to deal with these dummy columns.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/docs/current/fdw-callbacks.html#FDW-CALLBACKS-UPDATE
"...If the extra expressions are more complex than simple Vars, they
must be run through eval_const_expressions before adding them to the
targetlist."
On Tue, May 12, 2020 at 8:51 AM Bruce Momjian wrote:
> On Fri, May 8, 2020 at 12:07:09PM +0900, Amit Langote wrote:
> > I have attached a patch with my suggestions above.
>
> OK, I slightly modified the wording of your first change, patch
> attached.
Thanks. I checked that
Hi Ashutosh,
Thanks for chiming in.
On Mon, May 11, 2020 at 9:58 PM Ashutosh Bapat
wrote:
> On Fri, May 8, 2020 at 7:03 PM Amit Langote wrote:
> > I do think that doing this would be worthwhile even if we may be
> > increasing ModifyTable's per-row overhead slight
on foo_1 (cost=0.00..35.50 rows=10 width=14)
Filter: (a = $1)
(9 rows)
While it's true that we will never have to actually update foo_2,
foo_3, and foo_4, ModifyTable still sets up its ResultRelInfos, which
ideally it shouldn't. Maybe we'll need to do something
On Fri, May 8, 2020 at 2:06 AM Bruce Momjian wrote:
> On Fri, May 8, 2020 at 12:32:16AM +0900, Amit Langote wrote:
> > c8434d64c implements a new feature whereby, to use partitionwise join,
> > partition bounds of the tables being joined no longer have to match
> > exactly.
On Thu, May 7, 2020 at 9:48 PM Bruce Momjian wrote:
> On Thu, May 7, 2020 at 12:06:33AM +0900, Amit Langote wrote:
> > +Author: Etsuro Fujita
> > +2020-04-08 [c8434d64c] Allow partitionwise joins in more cases.
> > +Author: Tom Lane
> > +2020-04-07 [981643dcd]
oid
uselessly scanning it) in more cases (Yuzuko Hosoya)
Sorry for the noise.
I think it's okay for her name to appear first even considering the
commits that only appear in PG 13, because my role was mainly
reviewing the work and perhaps posting an updated version of her
patch.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
+
+
+Improve cases where pruning of partitions can happen (Amit Langote,
Yuzuko Hosoya, Álvaro Herrera)
+
The following commit should be included with this item:
commit 489247b0e615592111226297a0564e11616361a5
Author: Alvaro Herrera
Date: Sun Aug 4 11:18:45 2019 -0400
Improve pruning of a d
that approach
also doesn't address the concern about when to generate the whole-tree
statistics. Because the linked thread mentions getting rid of the
whole-tree statistics altogether, there is no such concern if we go
its way. Although I do agree with Robert's assertion on that thread
that making every query on a parent a bit slower would not be a good
compromise.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Apr 23, 2020 at 7:37 PM David Rowley wrote:
> On Thu, 23 Apr 2020 at 02:37, Amit Langote wrote:
> > Regarding the patch, I had been assuming that the "pa" in
> > pa_subpaths_valid stands for "parallel append", so it using the
> > variable
oticed this when working on my patch to move this
code to a different location in that function, posted here:
https://www.postgresql.org/message-id/CA%2BHiwqEeU19iQgjN6HF1HTPU0L5%2BJxyS5CmxaOVGNXBAfUY06Q%40mail.gmail.com
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
d up in the referenced table by using
RI_FKey_check_ins(), a simpler query.
Ironically though, RI_Initial_Check() is to short-circuit the manual algorithm.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
parallel Append is enabled and allowed, it will be chosen
over a non-parallel Append as the partial path.
Regarding the patch, I had been assuming that the "pa" in
pa_subpaths_valid stands for "parallel append", so it using the
variable as is in the new code structure would be misleading. Maybe,
parallel_subpaths_valid?
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Hi David,
On Tue, Apr 21, 2020 at 12:03 PM David Rowley wrote:
> On Fri, 17 Apr 2020 at 19:08, Amit Langote wrote:
> > On Wed, Apr 15, 2020 at 4:18 PM David Rowley wrote:
> > > I'm a bit divided on what the correct fix is. If I blame Parallel
> > > Append for n
+ " WHERE tgname = t.tgname AND tgrelid = relid"
+ " AND tgparentid = 0) AS parent" :
+ " null AS parent"),
The server version number being compared against was missing a zero in
your patch.
Also, how about, for consistency, making the parent table labeling of
the trigger look similar to that for the foreign constraint, so
instead of:
Triggers:
trig BEFORE INSERT ON f11 FOR EACH ROW EXECUTE FUNCTION
trigfunc(), ON TABLE f1
how about:
Triggers:
TABLE "f1" TRIGGER "trig" BEFORE INSERT ON f11 FOR EACH ROW
EXECUTE FUNCTION trigfunc()
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Fri, Apr 17, 2020 at 10:23 PM Peter Eisentraut
wrote:
> On 2020-04-09 09:28, Amit Langote wrote:
> > While figuring this out, I thought the nearby code could be rearranged
> > a bit, especially to de-duplicate the code. Also, I think
> > get_rel_sync_entry() may be a be
d not more than needed. For
time-series partitioning, the same formula won't work, as you have
detailed in your comments. Is there any other partitioning pattern for
which the current formula won't work?
Considering that, how about having, say, a
autovacuum_analyze_partition_parent_freque
e problem is that we let an Append be nested, but don't
account for that in how partitioned_rels list it parent Append is
constructed. The top-level Append's partitioned_rels should not have
contained sub-partitioned table's RTI if it got its own Append. Maybe
if we want to make run-time prunin
On Tue, Apr 14, 2020 at 6:12 PM Andy Fan wrote:
> On Tue, Apr 14, 2020 at 4:58 PM Amit Langote wrote:
>> I am saying that instead of building index path with disabled cost,
>> just don't build it at all. A base rel will always have a sequetial
>> path, even th
On Tue, Apr 14, 2020 at 5:29 PM Andy Fan wrote:
> On Tue, Apr 14, 2020 at 3:40 PM Amit Langote wrote:
>> On Tue, Apr 14, 2020 at 4:13 PM Richard Guo wrote:
>> > On Tue, Apr 14, 2020 at 2:44 PM Amit Langote
>> > wrote:
>> >> Maybe I am missing someth
(a = 1)
Heap Fetches: 0
Buffers: shared hit=2
Planning Time: 0.102 ms
Buffers: shared hit=1
Execution Time: 0.047 ms
(7 rows)
It seems that 1 Buffer hit comes from get_relation_info() doing
_bt_getrootheight() for that index on foo.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Tue, Apr 14, 2020 at 4:13 PM Richard Guo wrote:
> On Tue, Apr 14, 2020 at 2:44 PM Amit Langote wrote:
>> Maybe I am missing something obvious, but is it intentional that
>> enable_indexscan is checked by cost_index(), that is, *after* creating
>> an index path? I
.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
the overlap
with an existing partition, which is better than now and seems helpful
enough.
Also, updated Alexandra's patch to incorporate Ashutosh's comment such
that we get the same output with ATTACH PARTITION commands too.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.co
m
ATExecCmd() rather than a ParseState.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
being attached,
so implementing the same behavior for trigger is going to have to be
almost as complex. Considering that #2 will be much simpler to
implement, but would be asymmetric with everything else.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Apr 9, 2020 at 4:14 PM Peter Eisentraut
wrote:
> On 2020-04-09 05:39, Amit Langote wrote:
> > sub_viaroot ERROR: number of columns (2601) exceeds limit (1664)
> > sub_viaroot CONTEXT: slot "sub_viaroot", output plugin "pgoutput", in
> > the
On Wed, Apr 8, 2020 at 11:07 PM Amit Langote wrote:
> On Wed, Apr 8, 2020 at 9:21 PM Peter Eisentraut
> wrote:
> > On 2020-04-08 13:16, Amit Langote wrote:
> > > prion seems to have failed:
> > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prio
On Wed, Apr 8, 2020 at 11:07 PM Amit Langote wrote:
> On Wed, Apr 8, 2020 at 9:21 PM Peter Eisentraut
> wrote:
> > I think this is because the END { } section in PostgresNode.pm shuts
> > down all running instances in immediate mode, which doesn't save
> > coverage
On Wed, Apr 8, 2020 at 9:21 PM Peter Eisentraut
wrote:
> On 2020-04-08 13:16, Amit Langote wrote:
> > On Wed, Apr 8, 2020 at 6:26 PM Peter Eisentraut
> > wrote:
> >> All committed.
> >>
> >> Thank you and everyone very much for working on this. I'
ave failed:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2020-04-08%2009%3A53%3A13
Also, still unsure why the coverage report for pgoutput.c changes not good:
https://coverage.postgresql.org/src/backend/replication/pgoutput/pgoutput.c.gcov.html
Will check.
--
Amit Lango
On Tue, Apr 7, 2020 at 8:52 PM David Rowley wrote:
> On Wed, 25 Mar 2020 at 15:37, Amit Langote wrote:
> > Actually, I was saying in that email that the update/delete planning
> > overhaul being talked about will make the entirety of the
> > functionality this patc
On Wed, Apr 8, 2020 at 1:22 AM Amit Langote wrote:
> On Tue, Apr 7, 2020 at 6:01 PM Peter Eisentraut
> wrote:
> > The descriptions of the new fields in RelationSyncEntry don't seem to
> > match the code accurately, or at least it's confusing.
> > replicate_as
On Wed, Apr 8, 2020 at 11:17 AM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, Apr 7, 2020 at 2:41 AM Tom Lane wrote:
> >> I *think* that it might be all right, because although what we
> >> are matching to is a user-written COALESCE() not an actual
> >&g
Thanks for the review.
On Tue, Apr 7, 2020 at 6:01 PM Peter Eisentraut
wrote:
> On 2020-04-07 08:44, Amit Langote wrote:
> > I updated the patch to make the following changes:
> >
> > * Rewrote the tests to match in style with those committed yesterday
> > * Ren
On Tue, Apr 7, 2020 at 12:04 AM Amit Langote wrote:
>
> On Mon, Apr 6, 2020 at 10:25 PM Peter Eisentraut
> wrote:
> > On 2020-04-03 16:25, Amit Langote wrote:
> > > On Fri, Apr 3, 2020 at 6:34 PM Amit Langote
> > > wrote:
> > >> I am checking te
On Tue, Apr 7, 2020 at 2:41 AM Tom Lane wrote:
> Amit Langote writes:
> > which does succeed in using partitionwise join. Please see attached
> > delta that applies on your v7 if that is what you'd rather have.
>
> I figured these queries were cheap enough that we
On Mon, Apr 6, 2020 at 11:09 PM Tom Lane wrote:
> Amit Langote writes:
> > Oops, I thought I copy-pasted 4-way full join test not this one, but
> > evidently didn't.
>
> Have you got such a query at hand? I wondered whether we shouldn't
> use a 4-way rather tha
On Mon, Apr 6, 2020 at 10:25 PM Peter Eisentraut
wrote:
> On 2020-04-03 16:25, Amit Langote wrote:
> > On Fri, Apr 3, 2020 at 6:34 PM Amit Langote wrote:
> >> I am checking test coverage at the moment and should have the patches
> >> ready by sometime later toda
On Mon, Apr 6, 2020 at 7:29 AM Tom Lane wrote:
> Amit Langote writes:
> > Okay, I tried that in the updated patch. I didn't try to come up with
> > examples that might break it though.
>
> I looked through this.
Thank you.
> * I think your logic for building the c
the patch is shown to be close to zero, which is a mystery to me,
because I can confirm by other means such as debugging elogs() to next
to the new code that the newly added tests do cover them.
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Sat, Apr 4, 2020 at 6:13 AM Tom Lane wrote:
> Amit Langote writes:
> > Updated patches attached.
>
> I looked through these and committed 0001+0002, with some further
> comment-polishing. However, I have no faith at all in 0003.
Thanks for the review.
> It is
>
On Fri, Apr 3, 2020 at 6:34 PM Amit Langote wrote:
> I am checking test coverage at the moment and should have the patches
> ready by sometime later today.
Attached updated patches.
I confirmed using a coverage build that all the new code in
logical/worker.c due to 0002 is now covered. Fo
On Fri, Apr 3, 2020 at 4:52 PM Petr Jelinek wrote:
> On 02/04/2020 14:23, Peter Eisentraut wrote:
> > On 2020-03-30 17:42, Amit Langote wrote:
> >> I have updated the comments in apply_handle_tuple_routing() (see 0002)
> >> to better explain what's going
I have updated the comments in apply_handle_tuple_routing() (see 0002)
to better explain what's going on with UPDATE handling. I also
rearranged the tests a bit for clarity.
Attached updated patches.
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
v1-0001-wor
On Thu, Mar 26, 2020 at 11:23 PM Amit Langote wrote:
> On Wed, Mar 25, 2020 at 9:29 PM Peter Eisentraut
> wrote:
> > On 2020-03-23 06:02, Amit Langote wrote:
> > > Okay, added some tests.
> > >
> > > Attached updated patches.
> >
> > I have com
On Wed, Mar 25, 2020 at 9:29 PM Peter Eisentraut
wrote:
> On 2020-03-23 06:02, Amit Langote wrote:
> > Okay, added some tests.
> >
> > Attached updated patches.
>
> I have committed the worker.c refactoring patch.
>
> "Add subscription support to replicat
chedPlanAllowsSimpleValidityCheck() and CachedPlanIsSimplyValid().
Maybe I am missing something, but could there not be just one
function, possibly using whether expr_simple_expr is set or not to
skip or do, resp., the checks that the former does?
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
g that's necessary.
What I did say in favor of this patch though is that it doesn not seem
that invasive, so maybe okay to get in for v13.
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Mar 19, 2020 at 11:18 PM Peter Eisentraut
wrote:
> On 2020-03-18 08:33, Amit Langote wrote:
> > By the way, I have rebased the patches, although maybe you've got your
> > own copies; attached.
>
> Looking through 0002 and 0003 now.
>
> The structure looks
Thank you Chris, Amit.
On Thu, Mar 19, 2020 at 1:46 PM Amit Kapila wrote:
> On Thu, Mar 19, 2020 at 3:55 AM Chris Bandy wrote:
> >
> >
> > Sorry for these troubles. Attached are patches created using `git
> > format-patch -n -v6` on master at 487e9861d0.
> >
>
> No problem. I have extracted you
Hi Pavel,
Sorry it took me a while to look at this.
On Tue, Feb 25, 2020 at 4:28 AM Pavel Stehule wrote:
> po 24. 2. 2020 v 18:56 odesílatel Pavel Stehule
> napsal:
>> But I found one issue - I don't know if this issue is related to your patch
>> or plpgsql_check.
>>
>> plpgsql_check try to c
On Thu, Mar 19, 2020 at 11:45 AM Fujii Masao
wrote:
> On 2020/03/19 11:32, Amit Langote wrote:
> > On Thu, Mar 19, 2020 at 11:24 AM Alvaro Herrera
> > wrote:
> >> On 2020-Mar-19, Amit Langote wrote:
> >>
> >>> Magnus' idea of checking the values
On Thu, Mar 19, 2020 at 11:24 AM Alvaro Herrera
wrote:
> On 2020-Mar-19, Amit Langote wrote:
>
> > Magnus' idea of checking the values in pg_stat_get_progress_info() to
> > determine whether to return NULL seems fine to me. We will need to
> > update the docum
Hello,
On Thu, Mar 19, 2020 at 1:47 AM Fujii Masao wrote:
> On 2020/03/19 1:22, Magnus Hagander wrote:
> > Would it perhaps be better to return NULL instead of 0 in the
> > statistics view if there is no data?
> >>>
> >>> Did you miss this comment, or not agree? :)
> >>
> >> Oh, I forgot
On Wed, Mar 18, 2020 at 8:16 PM Peter Eisentraut
wrote:
> On 2020-03-18 04:06, Amit Langote wrote:
> > + if (isnull || !remote_is_publishable)
> > + ereport(ERROR,
> > + (errmsg("table \"%s.%s\" on the publisher is not
> > publish
901 - 1000 of 2399 matches
Mail list logo
