Hi,
On 2018-11-23 17:41:26 +1300, David Rowley wrote:
> Ideally, the locking code would realise we already hold a stronger
> lock and skip the lock, but I don't see how that's realistically
> possible without probing the hash table for all stronger lock types
> first, which would likely damage the
On Fri, 5 Apr 2019 at 08:26, Tom Lane wrote:
> Pushed with some minor tweaking, mostly comments.
Thanks for tweaking and pushing this.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
David Rowley writes:
> Wrong patch. Here's what I meant to send.
Pushed with some minor tweaking, mostly comments.
Some notes:
* We now have a general convention that queries always take the same lock
type on indexes as on their parent tables, but that convention is not
respected by index DDL.
On Fri, 5 Apr 2019 at 02:22, David Rowley wrote:
> v2 attached.
Wrong patch. Here's what I meant to send.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
use_rellockmode_for_indexes_too_v3.patch
Description: Binary dat
On Thu, 4 Apr 2019 at 15:35, Amit Langote wrote:
> BTW, get_actual_variable_range is static to selfuncs.c and other public
> functions that are entry point to get_actual_variable_range's
> functionality appear to require having built a RelOptInfo first, which
> means (even for third party code) ha
On 2019/04/04 11:13, David Rowley wrote:
> On Thu, 4 Apr 2019 at 15:01, Amit Langote
> wrote:
>> Sorry, I didn't understand why it wouldn't be OK to pass NoLock to
>> index_open, for example, here:
>>
>> @@ -5191,7 +5191,14 @@ get_actual_variable_range(PlannerInfo *root,
>> VariableStatData *vard
On Thu, 4 Apr 2019 at 15:01, Amit Langote wrote:
> Sorry, I didn't understand why it wouldn't be OK to pass NoLock to
> index_open, for example, here:
>
> @@ -5191,7 +5191,14 @@ get_actual_variable_range(PlannerInfo *root,
> VariableStatData *vardata,
> * necessarily on th
Hi David,
Thanks for updating the patch.
On 2019/04/04 9:14, David Rowley wrote:
> I also looked over other index_open() calls in the planner and found a
> bunch of places in selfuncs.c that we open an index to grab some
> information then close it again releasing the lock. At this stage
> get_r
On Wed, 3 Apr 2019 at 06:03, Tom Lane wrote:
>
> David Rowley writes:
> > On Thu, 14 Mar 2019 at 21:52, Amit Langote
> > Maybe you're right about being able to use rellockmode for indexes,
> > but I assume that we lowered the lock level for indexes for some
> > reason, and this would reverse that
David Rowley writes:
> On Thu, 14 Mar 2019 at 21:52, Amit Langote
> wrote:
>> If the correct lock is taken in both cases by the current code, then maybe
>> there's no need to change anything?
> I'm aiming to fix the problem described by Tom when he started this
> thread. It's pretty easy to get
On Thu, 14 Mar 2019 at 21:52, Amit Langote
wrote:
> If the correct lock is taken in both cases by the current code, then maybe
> there's no need to change anything? What does idxlockmode improve about
> the existing situation? One thing I can imagine it improves is that we
> don't need the poten
On 2019/03/14 7:12, David Rowley wrote:
> Thanks for having a look at this.
>
> On Wed, 13 Mar 2019 at 22:45, Amit Langote
> wrote:
>> I have one question about the relation between idxlockmode and
>> rellockmode? From skimming the patch, it appears that they're almost
>> always set to the same
Thanks for having a look at this.
On Wed, 13 Mar 2019 at 22:45, Amit Langote
wrote:
> I have one question about the relation between idxlockmode and
> rellockmode? From skimming the patch, it appears that they're almost
> always set to the same value. If so, why not use rellockmode for index
>
Hi David,
On 2019/03/13 10:38, David Rowley wrote:
> I had another go at this patch and fixed the problem by just setting
> the idxlockmode inside the planner just before the call to
> expand_inherited_tables(). This allows the lockmode to be copied into
> child RTEs.
I have one question about t
On Wed, 13 Mar 2019 at 14:55, Amit Langote
wrote:
> Did you miss ri_RangeTableIndex? It's the range table index of the result
> relation for which a given ResultRelInfo is created.
I did indeed. I'll hold off modifying the patch in favour of seeing
what other people think about what should be do
(this is not a reply to your full proposal, just something I thought to
point out)
On 2019/03/13 10:38, David Rowley wrote:
> i.e don't open the indexes for DELETEs. I had ideas that maybe this
> could be changed to check the idxlockmode and open the indexes if it's
> above AccessSharedLock. The
On Tue, 19 Feb 2019 at 12:13, David Rowley wrote:
>
> On Tue, 12 Feb 2019 at 09:58, Julien Rouhaud wrote:
> >
> > On Mon, Feb 11, 2019 at 5:32 AM David Rowley
> > wrote:
> > > 1. Adding a new field to RangeTblEntry to indicate the operation type
> > > that's being performed on the relation; or
>
On Tue, 12 Feb 2019 at 09:58, Julien Rouhaud wrote:
>
> On Mon, Feb 11, 2019 at 5:32 AM David Rowley
> wrote:
> > 1. Adding a new field to RangeTblEntry to indicate the operation type
> > that's being performed on the relation; or
> > 2. Adding a Bitmapset field to PlannerGlobal that sets the rta
On Mon, Feb 11, 2019 at 5:32 AM David Rowley
wrote:
>
> On Mon, 11 Feb 2019 at 01:22, Julien Rouhaud wrote:
> > The patch is quite straightforward, so I don't have general comments
> > on it. However, I think that the idxlockmode initialization is
> > problematic: you're using the statement's co
On Mon, 11 Feb 2019 at 01:22, Julien Rouhaud wrote:
> The patch is quite straightforward, so I don't have general comments
> on it. However, I think that the idxlockmode initialization is
> problematic: you're using the statement's commandType so this doesn't
> work with CTE. For instance, with
Hi,
On Tue, Feb 5, 2019 at 5:16 AM David Rowley
wrote:
>
> I've changed a couple of things:
>
> 1. Changed nodeBitmapIndexscan.c now properly uses the RangeTblEntry's
> idxlockmode field.
> 2. Renamed a few variables in finalize_lockmodes().
>
> I'm keen to get some feedback if we should go about
On Wed, 28 Nov 2018 at 01:55, David Rowley wrote:
> If this looks like a good path to go in, then I can produce something
> a bit more finished. I'm just a bit unsure when exactly I can do that
> as I'm on leave and have other commitments to take care of.
This patch is still on my list, so I had
On Fri, Nov 23, 2018 at 9:55 PM Tom Lane wrote:
>
> Thinking more about this, the problem I noted previously about two of
> these solutions not working if the index scan node is not physically
> underneath the ModifyTable node actually applies to all three :-(.
> It's a slightly different issue fo
On Tue, 27 Nov 2018 at 19:00, Amit Langote
wrote:
> On 2018/11/27 6:19, David Rowley wrote:
> > On Mon, 26 Nov 2018 at 18:57, Amit Langote
> >> That's an interesting point. Although, considering the concerns that Tom
> >> raised about the same index relation being locked such that lock-strength
>
On 2018/11/27 6:19, David Rowley wrote:
> On Mon, 26 Nov 2018 at 18:57, Amit Langote
> wrote:
>> On 2018/11/26 14:25, David Rowley wrote:
>>> I'm making efforts to delay per-partition work even further in the
>>> executor, for example locking of the per-partition result relations
>>> until after r
On Mon, 26 Nov 2018 at 18:57, Amit Langote
wrote:
> On 2018/11/26 14:25, David Rowley wrote:
> > I'm making efforts to delay per-partition work even further in the
> > executor, for example locking of the per-partition result relations
> > until after run-time pruning would be a significant win fo
On 2018/11/26 14:25, David Rowley wrote:
> On Mon, 26 Nov 2018 at 17:37, Amit Langote
> wrote:
>> On 2018/11/24 1:25, Tom Lane wrote:
>>> I'm beginning to think that postponing target-index locking to
>>> ExecInitModifyTable was a damfool idea and we should undo it.
>>
>> +1
>>
>> Also as already
On Mon, 26 Nov 2018 at 17:37, Amit Langote
wrote:
> On 2018/11/24 1:25, Tom Lane wrote:
> > I'm beginning to think that postponing target-index locking to
> > ExecInitModifyTable was a damfool idea and we should undo it.
>
> +1
>
> Also as already proposed, InitPlan should lock result relation ind
Sorry for jumping in late on this.
On 2018/11/24 1:25, Tom Lane wrote:
> David Rowley writes:
> Thinking more about this, the problem I noted previously about two of
> these solutions not working if the index scan node is not physically
> underneath the ModifyTable node actually applies to all th
On Sat, 24 Nov 2018 at 05:25, Tom Lane wrote:
> Now, after more thought, I believe that it's probably impossible
> to have a plan tree in which ExecRelationIsTargetRelation would
> return true at an indexscan node that's not underneath the ModifyTable
> node. What *is* possible is that we have su
David Rowley writes:
> On Thu, 8 Nov 2018 at 13:14, Tom Lane wrote:
>> There are several things we might do to fix this:
>>
>> 1. Drop the "operation != CMD_DELETE" condition from the above-quoted bit
>> in ExecInitModifyTable. We might be forced into that someday anyway if
>> we want to suppor
On Thu, 8 Nov 2018 at 13:14, Tom Lane wrote:
>
> I discovered that it's possible to trigger relation_open's new assertion
> about having a lock on the relation by the simple expedient of running
> the core regression tests with plan_cache_mode = force_generic_plan.
> There are several things we
I discovered that it's possible to trigger relation_open's new assertion
about having a lock on the relation by the simple expedient of running
the core regression tests with plan_cache_mode = force_generic_plan.
(This doesn't give me a terribly warm feeling about how thoroughly we've
exercised the
33 matches
Mail list logo
