Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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.

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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.

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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: >> > >> >

Re: [HACKERS] MERGE SQL Statement for PG11

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 ‘...’ > >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 Herrera

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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?

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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; >> +

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 >>

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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()

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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.

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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. >> >>

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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,

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 >>>

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 >>

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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.

Re: [HACKERS] MERGE SQL Statement for PG11

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. > >

Re: [HACKERS] MERGE SQL Statement for PG11

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... >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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. > >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 <

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 > >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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 >

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

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

Re: [HACKERS] MERGE SQL Statement for PG11

Greetings Pavan, * Pavan Deolasee (pavan.deola...@gmail.com) wrote: > On Tue, Feb 6, 2018 at 3:37 PM, Stephen Frost wrote: > > Coming out of that, my understanding is that Simon is planning to have a > > patch which implements RLS and partitioning (though the query plans for

Re: [HACKERS] MERGE SQL Statement for PG11

Hi Stephen, On Tue, Feb 6, 2018 at 3:37 PM, Stephen Frost wrote: > > > Coming out of that, my understanding is that Simon is planning to have a > patch which implements RLS and partitioning (though the query plans for > partitioning may be sub-par and not ideal) as part of

Re: [HACKERS] MERGE SQL Statement for PG11

On Fri, Feb 9, 2018 at 8:06 PM, Robert Haas wrote: > > > On that basis, of the options I listed in > http://postgr.es/m/CA+TgmoZDL-caukHkWet7sr7sqr0-e2T91+ > devhqen5sfqsm...@mail.gmail.com > I like #1 least. > > I also dislike #4 from that list for the reasons stated

Re: [HACKERS] MERGE SQL Statement for PG11

On Fri, Feb 9, 2018 at 6:53 AM, Peter Geoghegan wrote: > On Wed, Feb 7, 2018 at 7:51 PM, Pavan Deolasee > wrote: > > I understand getting EPQ semantics right is very important. Can you > please > > (once again) summarise your thoughts on what you think is

Re: [HACKERS] MERGE SQL Statement for PG11

Hi, On 02/07/2018 10:24 AM, Pavan Deolasee wrote: > > ... > > Here is v15 of the patch. > I've been looking at this version of the patch, mostly to educate myself before attempting to write the "status summary". One bit that I don't quite understand is GetXactWALBytes(). It pretty much just

Re: [HACKERS] MERGE SQL Statement for PG11

On Wed, Feb 7, 2018 at 7:51 PM, Pavan Deolasee wrote: > I understand getting EPQ semantics right is very important. Can you please > (once again) summarise your thoughts on what you think is the *most* > appropriate behaviour? I can then think how much efforts might be

Re: [HACKERS] MERGE SQL Statement for PG11

On Wed, Feb 7, 2018 at 10:52 PM, Peter Geoghegan wrote: > > > Apparently there is a pending patch to do better there - the commit > message of 2f178441 refers to this. > > Thanks. Will look at it. > > The revised version also supports subqueries in SET targetlist as well as > >

  1   2   >