Hi,
sqlsmith triggered an assertion with the following MERGE statement
against the regression database. Testing was done with master at
039eb6e92f. Backtrace below.
regards,
Andreas
MERGE INTO public.pagg_tab_ml_p3 as target_0
USING public.hash_i4_heap as ref_0
ON target_0.b = ref_0.seqno
WHEN
On Fri, Apr 6, 2018 at 1:30 AM, Andres Freund wrote:
> Hi,
>
> On 2018-04-05 11:31:48 +0530, Pavan Deolasee wrote:
> > > +/*-
> > >
> > > + *
> > > + * nodeMerge.c
> > > + * routines to handle Merge nodes relating to the M
On 5 April 2018 at 21:00, Andres Freund wrote:
> And if somebody cleans it up Simon will
> complain that things are needlessly being destabilized (hello xlog.c
> with it's 1000+ LOC functions).
Taking this comment as a special point... with other points addressed
separately.
...If anybody wants
On 2018/04/06 5:00, Andres Freund wrote:
> On 2018-04-05 11:31:48 +0530, Pavan Deolasee wrote:
>>> + /*
>>> +* If there are not WHEN MATCHED actions, we are done.
>>> +*/
>>> + if (mergeMatchedActionStates == NIL)
>>> + return true;
>>>
>>> Maybe I'm confused, but why is mergeMatc
On Fri, Apr 6, 2018 at 1:30 AM, Andres Freund wrote:
>
>
> My impression is that this simply shouldn't be going through
> nodeModifyTuple, but be it's own nodeMerge node. The trigger handling
> would need to be abstraced into execTrigger.c or such to avoid
> duplication. We're now going from nod
Hi,
On 2018-04-05 11:31:48 +0530, Pavan Deolasee wrote:
> > +/*-
> >
> > + *
> > + * nodeMerge.c
> > + * routines to handle Merge nodes relating to the MERGE command
> >
> > Isn't this file misnamed and it should be execMer
On 5 April 2018 at 17:07, Alvaro Herrera wrote:
> Simon Riggs wrote:
>> On 5 April 2018 at 16:09, Alvaro Herrera wrote:
>> > Quick item: parse_clause.h fails cpluspluscheck because it has a C++
>> > keyword as a function argument name:
>> >
>> > ./src/include/parser/parse_clause.h:26:14: error: e
Simon Riggs wrote:
> On 5 April 2018 at 16:09, Alvaro Herrera wrote:
> > Quick item: parse_clause.h fails cpluspluscheck because it has a C++
> > keyword as a function argument name:
> >
> > ./src/include/parser/parse_clause.h:26:14: error: expected ‘,’ or ‘...’
> > before ‘namespace’
> >
On 5 April 2018 at 16:09, Alvaro Herrera wrote:
> Quick item: parse_clause.h fails cpluspluscheck because it has a C++
> keyword as a function argument name:
>
> ./src/include/parser/parse_clause.h:26:14: error: expected ‘,’ or ‘...’
> before ‘namespace’
>List **namespace);
>
Quick item: parse_clause.h fails cpluspluscheck because it has a C++
keyword as a function argument name:
./src/include/parser/parse_clause.h:26:14: error: expected ‘,’ or ‘...’ before
‘namespace’
List **namespace);
^
--
Álvaro Herrerahttps://www.2nd
On 5 April 2018 at 13:18, Teodor Sigaev wrote:
>> The variable would become unused in non-assert builds. I see that. But
>> simply removing it is not a solution and I don't think the code will compile
>> that way. We should either rewrite that assertion or put it inside a #ifdef
>> ASSERT_CHECKING
On 5 April 2018 at 13:19, Jesper Pedersen wrote:
> Hi,
>
> On 04/05/2018 08:04 AM, Simon Riggs wrote:
>>
>> On 5 April 2018 at 12:56, Jesper Pedersen
>> wrote:
>>>
>>> Updated for non-assert build.
>>
>>
>> Thanks, pushed. Sorry to have you wait til v3
>>
>
> That patch was a but rushed, and cut
Hi,
On 04/05/2018 08:04 AM, Simon Riggs wrote:
On 5 April 2018 at 12:56, Jesper Pedersen wrote:
Updated for non-assert build.
Thanks, pushed. Sorry to have you wait til v3
That patch was a but rushed, and cut off too much.
As attached.
Best regards,
Jesper
diff --git a/src/backend/exec
The variable would become unused in non-assert builds. I see that. But simply
removing it is not a solution and I don't think the code will compile that way.
We should either rewrite that assertion or put it inside a #ifdef
ASSERT_CHECKING block or simple remove that assertion because we already
On 5 April 2018 at 12:56, Jesper Pedersen wrote:
> Hi,
>
> On 04/05/2018 07:48 AM, Simon Riggs wrote:
>>>
>>> Updated version due to latest refactoring.
>>
>>
>> Thanks for your input. Removing that seems to prevent compilation.
>>
>> Did something change in between?
>>
>
> Updated for non-assert
Hi,
On 04/05/2018 07:48 AM, Simon Riggs wrote:
Updated version due to latest refactoring.
Thanks for your input. Removing that seems to prevent compilation.
Did something change in between?
Updated for non-assert build.
Best regards,
Jesper
diff --git a/src/backend/executor/execMerge.c b
On Thu, Apr 5, 2018 at 5:08 PM, Jesper Pedersen
wrote:
> Hi Simon and Paven,
>
> On 04/04/2018 08:46 AM, Jesper Pedersen wrote:
>
>> On 03/30/2018 07:10 AM, Simon Riggs wrote:
>>
>>> No problems found, but moving proposed commit to 2 April pm
>>>
>>>
>> There is a warning for this, as attached.
>
On 5 April 2018 at 12:38, Jesper Pedersen wrote:
> Hi Simon and Paven,
>
> On 04/04/2018 08:46 AM, Jesper Pedersen wrote:
>>
>> On 03/30/2018 07:10 AM, Simon Riggs wrote:
>>>
>>> No problems found, but moving proposed commit to 2 April pm
>>>
>>
>> There is a warning for this, as attached.
>>
>
>
Hi Simon and Paven,
On 04/04/2018 08:46 AM, Jesper Pedersen wrote:
On 03/30/2018 07:10 AM, Simon Riggs wrote:
No problems found, but moving proposed commit to 2 April pm
There is a warning for this, as attached.
Updated version due to latest refactoring.
Best regards,
Jesper
diff --gi
On 5 April 2018 at 07:01, Pavan Deolasee wrote:
>> +/*
>> + * Given OID of the partition leaf, return the index of the leaf in the
>> + * partition hierarchy.
>> + */
>> +int
>> +ExecFindPartitionByOid(PartitionTupleRouting *proute, Oid partoid)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < pro
On Tue, Apr 3, 2018 at 7:48 AM, Andres Freund wrote:
>
>
> @@ -3828,8 +3846,14 @@ struct AfterTriggersTableData
> boolbefore_trig_done; /* did we already queue BS triggers? */
> boolafter_trig_done;/* did we already queue AS triggers? */
> AfterTriggerEventList a
Hi Simon,
On 03/30/2018 07:10 AM, Simon Riggs wrote:
No problems found, but moving proposed commit to 2 April pm
There is a warning for this, as attached.
Best regards,
Jesper
diff --git a/src/backend/executor/nodeMerge.c b/src/backend/executor/nodeMerge.c
index 0e0d0795d4..9aee073a94 10064
Hi,
On 2018-04-02 19:40:12 -0700, Peter Geoghegan wrote:
> > So what happens if there's a concurrent insertion of a potentially
> > matching tuple?
>
> It's not a special case. In all likelihood, you get a dup violation.
> This is a behavior that I argued for from an early stage.
Right. I think
On Mon, Apr 2, 2018 at 8:11 PM, Pavan Deolasee wrote:
> Honestly I don't think Peter ever raised concerns about the join, though I
> could be missing early discussions when I wasn't paying attention. It's
> there from day 1. Peter raised concerns about the two RTE stuff which was
> necessitated wh
On Tue, Apr 3, 2018 at 8:31 AM, Robert Haas wrote:
> On Mon, Apr 2, 2018 at 10:40 PM, Peter Geoghegan wrote:
> > On Mon, Apr 2, 2018 at 7:18 PM, Andres Freund
> wrote:
> >> I did a scan through this, as I hadn't been able to keep with the thread
> >> previously. Sorry if some of the things ment
On Mon, Apr 2, 2018 at 10:40 PM, Peter Geoghegan wrote:
> On Mon, Apr 2, 2018 at 7:18 PM, Andres Freund wrote:
>> I did a scan through this, as I hadn't been able to keep with the thread
>> previously. Sorry if some of the things mentioned here have been
>> discussed previously. I am just reading
On Mon, Apr 2, 2018 at 7:18 PM, Andres Freund wrote:
> I did a scan through this, as I hadn't been able to keep with the thread
> previously. Sorry if some of the things mentioned here have been
> discussed previously. I am just reading through the patch in its own
> order, so please excuse if the
Hi,
On 2018-03-30 12:10:04 +0100, Simon Riggs wrote:
> On 29 March 2018 at 10:50, Simon Riggs wrote:
> > On 28 March 2018 at 12:00, Pavan Deolasee wrote:
> >
> >> v27 attached, though review changes are in
> >> the add-on 0005 patch.
> >
> > This all looks good now, thanks for making all of thos
On 29 March 2018 at 10:50, Simon Riggs wrote:
> On 28 March 2018 at 12:00, Pavan Deolasee wrote:
>
>> v27 attached, though review changes are in
>> the add-on 0005 patch.
>
> This all looks good now, thanks for making all of those changes.
>
> I propose [v27 patch1+patch3+patch5] as the initial c
On 28 March 2018 at 12:00, Pavan Deolasee wrote:
> v27 attached, though review changes are in
> the add-on 0005 patch.
This all looks good now, thanks for making all of those changes.
I propose [v27 patch1+patch3+patch5] as the initial commit candidate
for MERGE, with other patches following la
On 29 March 2018 at 07:37, Pavan Deolasee wrote:
>
>
> On Tue, Mar 27, 2018 at 5:00 PM, Simon Riggs wrote:
>>
>>
>> In terms of further performance optimization, if there is just one
>> WHEN AND condition and no unconditional WHEN clauses then we can add
>> the WHEN AND easily to the join query.
On Tue, Mar 27, 2018 at 5:00 PM, Simon Riggs wrote:
>
> In terms of further performance optimization, if there is just one
> WHEN AND condition and no unconditional WHEN clauses then we can add
> the WHEN AND easily to the join query.
>
> That seems like an easy thing to do for PG11
>
>
I think w
On Wed, Mar 28, 2018 at 8:28 AM, Peter Geoghegan wrote:
> On Tue, Mar 27, 2018 at 2:28 AM, Pavan Deolasee
> wrote:
> > (Version 26)
>
> I have some feedback on this version:
>
> * ExecMergeMatched() needs to determine tuple lock mode for
> EvalPlanQual() in a way that's based on how everything e
On Tue, Mar 27, 2018 at 2:28 AM, Pavan Deolasee
wrote:
> (Version 26)
I have some feedback on this version:
* ExecMergeMatched() needs to determine tuple lock mode for
EvalPlanQual() in a way that's based on how everything else works;
it's not okay to just use LockTupleExclusive in all cases. Th
On Tue, Mar 27, 2018 at 1:15 AM, Simon Riggs wrote:
> Accepted, the only question is whether it affects UPDATE as well cos
> it looks like it should.
If you mean an UPDATE FROM self-join, then I suppose that it does, in
a very limited way. The difference is that there are no hard-coded
assumption
On 27 March 2018 at 11:46, Simon Riggs wrote:
> On 27 March 2018 at 10:31, Pavan Deolasee wrote:
>
>> Fixed in v26.
>
> More comments on v26
In terms of further performance optimization, if there is just one
WHEN AND condition and no unconditional WHEN clauses then we can add
the WHEN AND easily
On 27 March 2018 at 10:31, Pavan Deolasee wrote:
> Fixed in v26.
More comments on v26
* Change errmsg “Ensure that not more than one source rows match any
one target row”
should be
“Ensure that not more than one source row matches any one target row”
* I think we need an example in the docs sh
On 27 March 2018 at 10:28, Pavan Deolasee wrote:
>> 1. In ExecMergeMatched() we have a line of code that does this...
>>
>> if (TransactionIdIsCurrentTransactionId(hufd.xmax))
>> then errcode(ERRCODE_CARDINALITY_VIOLATION)
>>
>> I notice this is correct, but too strong. It should be possible to r
On Tue, Mar 27, 2018 at 1:54 PM, Simon Riggs wrote:
> On 26 March 2018 at 17:06, Simon Riggs wrote:
> > On 26 March 2018 at 15:39, Pavan Deolasee
> wrote:
> >
>
> > That's all I can see so far.
>
> * change comment “once to” to “once” in src/include/nodes/execnodes.h
> * change comment “and to
On 26 March 2018 at 17:06, Simon Riggs wrote:
> On 26 March 2018 at 15:39, Pavan Deolasee wrote:
>
> That's all I can see so far.
* change comment “once to” to “once” in src/include/nodes/execnodes.h
* change comment “and to run” to “and once to run”
* change “result relation” to “target relati
On 26 March 2018 at 23:10, Peter Geoghegan wrote:
> On Mon, Mar 26, 2018 at 12:17 PM, Simon Riggs wrote:
>>> As far as I
>>> know, the proposed MERGE patch has that issue an existing DML commands
>>> don't; but someone else may have better information.
>>
>> I will look deeper and report back.
>
On Mon, Mar 26, 2018 at 12:17 PM, Simon Riggs wrote:
>> As far as I
>> know, the proposed MERGE patch has that issue an existing DML commands
>> don't; but someone else may have better information.
>
> I will look deeper and report back.
It's quite clear that the problem exists with the MERGE pat
On 26 March 2018 at 17:52, Robert Haas wrote:
> On Mon, Mar 26, 2018 at 12:16 PM, Simon Riggs wrote:
>> On 26 March 2018 at 16:09, Robert Haas wrote:
>>> On Mon, Mar 26, 2018 at 5:53 AM, Simon Riggs wrote:
Since we now have MVCC catalog scans, all the name lookups are
performed using
On Mon, Mar 26, 2018 at 12:16 PM, Simon Riggs wrote:
> On 26 March 2018 at 16:09, Robert Haas wrote:
>> On Mon, Mar 26, 2018 at 5:53 AM, Simon Riggs wrote:
>>> Since we now have MVCC catalog scans, all the name lookups are
>>> performed using the same snapshot so in the above scenario the newly
On 26 March 2018 at 16:09, Robert Haas wrote:
> On Mon, Mar 26, 2018 at 5:53 AM, Simon Riggs wrote:
>> Since we now have MVCC catalog scans, all the name lookups are
>> performed using the same snapshot so in the above scenario the newly
>> created object would be invisible to the second name loo
On 26 March 2018 at 15:39, Pavan Deolasee wrote:
> reviewer
1. In ExecMergeMatched() we have a line of code that does this...
if (TransactionIdIsCurrentTransactionId(hufd.xmax))
then errcode(ERRCODE_CARDINALITY_VIOLATION)
I notice this is correct, but too strong. It should be possible to run
a
On Mon, Mar 26, 2018 at 5:53 AM, Simon Riggs wrote:
> Since we now have MVCC catalog scans, all the name lookups are
> performed using the same snapshot so in the above scenario the newly
> created object would be invisible to the second name lookup.
That's not true, because each lookup would be
On 26 March 2018 at 15:39, Pavan Deolasee wrote:
> Now that ON CONFLICT patch is in, here are rebased patches. The second patch
> is to add support for CTE (thanks Peter).
>
> Apart from rebase, the following things are fixed/improved:
>
> - Added test cases for column level privileges as suggest
On 24 March 2018 at 12:27, Robert Haas wrote:
> On Sat, Mar 24, 2018 at 8:16 AM, Robert Haas wrote:
>> On Thu, Mar 22, 2018 at 7:13 PM, Peter Geoghegan wrote:
>>> While I think this this particular HINT buglet is pretty harmless, I
>>> continue to be concerned about the unintended consequences o
On Sat, Mar 24, 2018 at 5:27 AM, Robert Haas wrote:
> If it's possible to identify the two OIDs that are supposed to match
> and cross-check that the OIDs are the same, then we could just bomb
> out with an error if they aren't. That's not lovely, and is basically
> a hack, but it's possible that
On Fri, Mar 23, 2018 at 11:52 PM, Pavan Deolasee
wrote:
> A quick gdb tracing shows that the CTE itself is assigned plan_id 1 and the
> SubPlan then gets plan_id 2. I can investigate further, but given that we
> see a similar behaviour with regular UPDATE, I don't think it's worth.
Clearly I jump
On Sat, Mar 24, 2018 at 8:16 AM, Robert Haas wrote:
> On Thu, Mar 22, 2018 at 7:13 PM, Peter Geoghegan wrote:
>> While I think this this particular HINT buglet is pretty harmless, I
>> continue to be concerned about the unintended consequences of having
>> multiple RTEs for MERGE's target table.
On 24 March 2018 at 12:16, Robert Haas wrote:
> On Thu, Mar 22, 2018 at 7:13 PM, Peter Geoghegan wrote:
>> While I think this this particular HINT buglet is pretty harmless, I
>> continue to be concerned about the unintended consequences of having
>> multiple RTEs for MERGE's target table. Each R
On Thu, Mar 22, 2018 at 7:13 PM, Peter Geoghegan wrote:
> While I think this this particular HINT buglet is pretty harmless, I
> continue to be concerned about the unintended consequences of having
> multiple RTEs for MERGE's target table. Each RTE comes from a
> different lookup path -- the first
On Sat, Mar 24, 2018 at 1:36 AM, Peter Geoghegan wrote:
>
> Fair enough. Attached patch shows what I'm on about. This should be
> applied on top of 0001_merge_v23e_onconflict_work.patch +
> 0002_merge_v23e_main.patch. I'm not expecting an authorship credit for
> posting this patch.
>
Thanks for
On Fri, Mar 23, 2018 at 11:15 PM, Peter Geoghegan wrote:
> I agree that this is very similar, as far as the RTEs go. What is
> dissimilar is the fact that there is hard-coded knowledge of both
> through parsing, planning, and execution. It's everything, taken
> together.
>
> ResultRelInfo has a ri
On Fri, Mar 23, 2018 at 4:56 AM, Pavan Deolasee
wrote:
> postgres=# insert into target as t select sid from source s join target t on
> t.ttid = s.sid;
> ERROR: column t.ttid does not exist
> LINE 1: ...rget as t select sid from source join target t on t.ttid = s...
>
On Fri, Mar 23, 2018 at 6:55 AM, Simon Riggs wrote:
> Peter, if you have the code and you consider it important that this
> subfeature is in PostgreSQL, why not post the code so we can commit
> it?
Fair enough. Attached patch shows what I'm on about. This should be
applied on top of 0001_merge_v2
On 23 March 2018 at 11:26, Pavan Deolasee wrote:
>
>
> On Fri, Mar 23, 2018 at 6:45 AM, Peter Geoghegan wrote:
>>
>> On Thu, Mar 22, 2018 at 6:02 PM, Alvaro Herrera
>> wrote:
>> > Incremental development is a good thing. Trying to do everything in a
>> > single commit is great when time is infi
On 2018/03/23 20:07, Pavan Deolasee wrote:
> On Fri, Mar 23, 2018 at 12:57 PM, Amit Langote wrote:
>> Also, it seems that the delta patch I sent in the last email didn't
>> contain all the changes I had to make. It didn't contain, for example,
>> replacing adjust_and_expand_inherited_tlist() with
On Fri, Mar 23, 2018 at 4:43 AM, Peter Geoghegan wrote:
> On Thu, Mar 22, 2018 at 11:42 AM, Pavan Deolasee
> wrote:
> > A slightly improved version attached.
>
> You still need to remove this change:
>
> > diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
> > index a4574cd533..dbfb5
On Fri, Mar 23, 2018 at 6:45 AM, Peter Geoghegan wrote:
> On Thu, Mar 22, 2018 at 6:02 PM, Alvaro Herrera
> wrote:
> > Incremental development is a good thing. Trying to do everything in a
> > single commit is great when time is infinite or even merely very long,
> > but if you run out of it, w
On 2018/03/23 13:57, Pavan Deolasee wrote:
> On Fri, Mar 23, 2018 at 10:00 AM, Amit Langote wrote:
>> I managed to apply it by ignoring the errors, but couldn't get make check
>> to pass; attached regressions.diffs if you want to take a look.
>
> Thanks. Are you sure you're using a clean repo? I su
On Fri, Mar 23, 2018 at 10:00 AM, Amit Langote <
langote_amit...@lab.ntt.co.jp> wrote:
> On 2018/03/23 3:42, Pavan Deolasee wrote:
> > A slightly improved version attached. Apart from doc cleanup based on
> > earlier feedback, fixed one assertion failure based on Rahila's report.
> > This was happ
On 2018/03/23 3:42, Pavan Deolasee wrote:
> A slightly improved version attached. Apart from doc cleanup based on
> earlier feedback, fixed one assertion failure based on Rahila's report.
> This was happening when target relation is referenced in the source
> subquery. Fixed that and added a test c
On Thu, Mar 22, 2018 at 6:02 PM, Alvaro Herrera wrote:
> Incremental development is a good thing. Trying to do everything in a
> single commit is great when time is infinite or even merely very long,
> but if you run out of it, which I'm sure is common, leaving some things
> out that can be reaso
Peter Geoghegan wrote:
> Pavan hasn't added support for referencing CTEs, which other database
> systems with MERGE have. I think that it ought to be quite doable. It
> didn't take me long to get it working myself, but there wasn't follow
> through on that (I could have posted the patch, which loo
On Thu, Mar 22, 2018 at 1:06 AM, Simon Riggs wrote:
> I'm now proposing that I move to commit this, following my own final
> review, on Tues 27 Mar in 5 days time, giving time for cleanup of
> related issues.
>
> If there are any items you believe are still open, please say so now
> or mention any
On Thu, Mar 22, 2018 at 12:59 AM, Simon Riggs wrote:
>> The point here is that we're primarily talking about two whole tables.
>> That deserves such prominent placement, as that suggests where users
>> might really find MERGE useful, but without being too prescriptive.
>
> The information I have i
On Thu, Mar 22, 2018 at 11:42 AM, Pavan Deolasee
wrote:
> A slightly improved version attached.
You still need to remove this change:
> diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
> index a4574cd533..dbfb5d2a1a 100644
> --- a/src/include/miscadmin.h
> +++ b/src/include/miscadm
On Thu, Mar 22, 2018 at 1:15 AM, Peter Geoghegan wrote:
>
> >
> >
> > No, it did not. We only support VALUES clause with INSERT action.
>
> But can't you have a subselect in the VALUES()? Support for subselects
> seems like a totally distinct thing to the restriction that only a
> (single row) VA
On 21 March 2018 at 12:23, Pavan Deolasee wrote:
> Fixed
Looks like that this completes all outstanding items with the MERGE code.
I'm now proposing that I move to commit this, following my own final
review, on Tues 27 Mar in 5 days time, giving time for cleanup of
related issues.
If there are
On 21 March 2018 at 19:45, Peter Geoghegan wrote:
>>> * We never actually get around to saying that MERGE is good with bulk
>>> loading, ETL, and so on. I think that we should remark on that in
>>> passing.
>>
>>
>> Suggestion?
>
> How about adding this sentence after "MERGE ... a task that would
On Wed, Mar 21, 2018 at 5:23 AM, Pavan Deolasee
wrote:
>> * The docs say "A condition cannot contain subqueries, set returning
>> functions, nor can it contain window or aggregate functions". Thought
>> it can now?
>
>
> Yes, it now supports sub-queries. What about set-returning, aggregates etc?
On Thu, Mar 8, 2018 at 6:59 PM, Peter Geoghegan wrote:
> Phew! Thanks for your patience and perseverance. I do have more
> feedback on the docs lined up, but that isn't so important right now.
I want to get this feedback on the docs to you now. This is a little
raw, as it's taken from notes made
On Mon, Mar 12, 2018 at 5:43 PM, Pavan Deolasee
wrote:
>
>
> On Sun, Mar 11, 2018 at 11:18 AM, Peter Geoghegan wrote:
>
>>
>>
>> As you know, there is an ON CONFLICT DO UPDATE + partitioning patch in
>> the works from Alvaro. In your explanation about that approach that
>> you cited, you wondere
Hi Stephen,
On Fri, Mar 16, 2018 at 7:28 AM, Stephen Frost wrote:
> Greetings Pavan, all,
>
> * Pavan Deolasee (pavan.deola...@gmail.com) wrote:
> > On 9 March 2018 at 08:29, Peter Geoghegan wrote:
> > > My #1 concern has become RLS, and
> > > perhaps only because I haven't studied it in enou
Greetings Pavan, all,
* Pavan Deolasee (pavan.deola...@gmail.com) wrote:
> On 9 March 2018 at 08:29, Peter Geoghegan wrote:
> > My #1 concern has become RLS, and
> > perhaps only because I haven't studied it in enough detail.
>
> Sure. I've done what I thought is the right thing to do, but plea
On Sun, Mar 11, 2018 at 11:18 AM, Peter Geoghegan wrote:
>
> It sounds like we should try to thoroughly understand why these
> duplicates arose. Did you actually call EvalPlanQualSetPlan() for all
> subplans at the time?
>
>
The reason for duplicates or even wrong answers is quite simple. The way
On Sat, Mar 10, 2018 at 9:22 PM, Pavan Deolasee
wrote:
> Ok. I will look at it. I think it shouldn't be too difficult and the
> original restriction was mostly a fallout of expecting CHECK constraint
> style expressions there.
Good, thanks.
> Ok. OVERRIDING is done. I think we can support ruleut
On Sun, Mar 11, 2018 at 9:27 AM, Peter Geoghegan wrote:
>
> >>
> >> We're talking about the scantuple here. It's not like excluded.*.
>
> I often care about things like system columns not because of the
> user-visible functionality, but because it reassures me that the
> design is robust.
>
>
Ok.
On Fri, Mar 9, 2018 at 6:55 AM, Pavan Deolasee wrote:
>> * Is this actually needed at all?:
>>
>> > + /* In MERGE when and condition, no system column is allowed */
>> > + if (pstate->p_expr_kind == EXPR_KIND_MERGE_WHEN_AND &&
>> > + attnum < InvalidAttrNumber &&
>> > +
Please don't forget to remove the xlog.c and miscadmin.h hunks from the
patch.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 8, 2018 at 4:54 AM, Pavan Deolasee wrote:
> Thanks for the feedback. I've greatly refactored the code based on your
> comments and I too like the resultant code. What we have now have
> essentially is: two functions:
I really like how ExecMerge() now mostly just consists of this code
On Thu, Mar 8, 2018 at 3:29 PM, Tomas Vondra
wrote:
> The reason why the patch tried to prevent that is because the SQL
> standard says this (p. 1176 of SQL 2016):
>
> 15) The immediately contained in a ,
> the immediately contained in a clause>, and the immediately contained in a when not ma
On 03/08/2018 11:46 PM, Peter Geoghegan wrote:
> On Thu, Mar 8, 2018 at 6:52 AM, Robert Haas wrote:
>>> I removed this code since it was wrong. We might want to add some basic
>>> checks for existence of volatile functions in the WHEN or SET clauses. But I
>>> agree, it's no different than regul
On Thu, Mar 8, 2018 at 6:52 AM, Robert Haas wrote:
>> I removed this code since it was wrong. We might want to add some basic
>> checks for existence of volatile functions in the WHEN or SET clauses. But I
>> agree, it's no different than regular UPDATEs. So may be not a big deal.
>
> I just caugh
On Thu, Mar 8, 2018 at 7:54 AM, Pavan Deolasee wrote:
>> > + /*
>> > +* SQL Standard says that WHEN AND conditions must not
>> > +* write to the database, so check we haven't written
>> > +* any WAL during the test. Very sensible that is, since
>> > +
On Mon, Mar 5, 2018 at 3:02 AM, Pavan Deolasee wrote:
>> Again, I have to ask: is such an UPDATE actually meaningfully
>> different from a concurrent DELETE + INSERT? If so, why is a special
>> error better than a dup violation, or maybe even having the INSERT
>> (and whole MERGE statement) succee
On 02/06/2018 03:40 PM, Tomas Vondra wrote:
>
>
> I plan to go through the patch and this thread over the couple of
> days, and summarize what the current status is (or my understanding
> of it). That is (a) what are the missing pieces, (b) why are they
> missing, (c) how we plan to address them
On Thu, Mar 1, 2018 at 4:33 AM, Pavan Deolasee wrote:
> What if the updated tuple fails the join qual with respect to the current
> tuple from the source relation but it now matches some other tuple from the
> source relation? I described this case in one of the earlier emails too. In
> this case,
On Thu, Mar 1, 2018 at 3:50 AM, Peter Geoghegan wrote:
> On Fri, Feb 9, 2018 at 6:36 AM, Robert Haas wrote:
> > Here's my $0.02: I think that new concurrency errors thrown by the
> > merge code itself deserve strict scrutiny and can survive only if they
> > have a compelling justification, but i
On Fri, Feb 9, 2018 at 6:36 AM, Robert Haas wrote:
> Here's my $0.02: I think that new concurrency errors thrown by the
> merge code itself deserve strict scrutiny and can survive only if they
> have a compelling justification, but if the merge code happens to
> choose an action that leads to a co
On Wed, Feb 28, 2018 at 8:53 AM, Robert Haas wrote:
> On Tue, Feb 27, 2018 at 5:07 PM, Peter Geoghegan wrote:
>> I now feel like Simon's suggestion of throwing an error in corner
>> cases isn't so bad. It still seems like we could do better, but the
>> more I think about it, the less that seems l
On Tue, Feb 27, 2018 at 5:07 PM, Peter Geoghegan wrote:
> I now feel like Simon's suggestion of throwing an error in corner
> cases isn't so bad. It still seems like we could do better, but the
> more I think about it, the less that seems like a cop-out. My reasons
> are:
I still think we really
On Tue, Feb 27, 2018 at 10:19 AM, Pavan Deolasee
wrote:
>> I attach a rough example of this, that uses plpgsql.
>
> Thanks for writing the sample code. I understand you probably don't mean to
> suggest that we need to mimic the behaviour of the plpgsql code and the
> semantics offered by MERGE wou
On Fri, Feb 16, 2018 at 6:37 AM, Peter Geoghegan wrote:
>
> ISTM that a MERGE isn't really a thing that replaces 2 or 3 other DML
> statements, at least in most cases. It's more like a replacement for
> procedural code with an outer join, with an INSERT, UPDATE or DELETE
> that affects zero or on
Pavan Deolasee writes:
> Thanks for doing those tests. I've just sent v16a version of the patch and
> I think it fixes the issues reported so far. Can you please recheck? Please
> let me know if there are other issues detected by sqlsmith or otherwise.
I re-did the testing with merge_v16a applied
On Sun, Feb 11, 2018 at 11:09 PM, Pavan Deolasee
wrote:
> On Fri, Feb 9, 2018 at 6:53 AM, Peter Geoghegan wrote:
>> My concern is mostly just that MERGE manages to behave in a way that
>> actually "provides a single SQL statement that can conditionally
>> INSERT, UPDATE or DELETE rows, a task tha
Hi Andreas,
On Sun, Feb 4, 2018 at 5:45 PM, Andreas Seltenreich
wrote:
>
>
> Attached are testcases that trigger the assertions when run against an
> empty database instead of the one left behind by make installcheck. The
> "unrecognized node type" one appears to be a known issue according to
>
1 - 100 of 177 matches
Mail list logo
