On Mon, 22 Jul 2019 at 01:50, David Rowley wrote:
>
> On Mon, 22 Jul 2019 at 00:44, Andreas Seltenreich wrote:
> > sqlsmith triggers an assertion in this commit (3373c7155350):
> >
> > TRAP: FailedAssertion("!(rel->reloptkind == RELOPT_BASEREL)", File:
> > "equivclass.c", Line: 764)
>
> Thanks f
On Mon, 22 Jul 2019 at 00:44, Andreas Seltenreich wrote:
> sqlsmith triggers an assertion in this commit (3373c7155350):
>
> TRAP: FailedAssertion("!(rel->reloptkind == RELOPT_BASEREL)", File:
> "equivclass.c", Line: 764)
Thanks for the report.
It looks like this is caused by the join removal c
David Rowley writes:
> On Thu, 18 Jul 2019 at 19:24, David Rowley
> wrote:
>> Unless there's some objection, I'll be looking into pushing both 0001
>> and 0002 in a single commit in the next few days.
>
> I've pushed this after doing a bit of final tweaking.
sqlsmith triggers an assertion in th
On Thu, 18 Jul 2019 at 19:24, David Rowley wrote:
> Unless there's some objection, I'll be looking into pushing both 0001
> and 0002 in a single commit in the next few days.
I've pushed this after doing a bit of final tweaking.
After a re-read, I didn't really like all the code that rechecked th
On Sun, 16 Jun 2019 at 19:42, David Rowley wrote:
>
> I've rebased this on top of the current master. d25ea0127 conflicted
> with the old version.
... and again, per recent conflicting change in equivclass.c
I've also taken a fresh set of performance benchmarks since 1cff1b95a
has recently chang
On Sat, 25 May 2019 at 16:37, David Rowley wrote:
> The only problem I see is that you're not performing a bms_copy() of
> the parent's eclass_indexes in add_child_rel_equivalences(). You've
> written a comment that claims it's fine to just point to the parent's
> in:
>
> /*
> * The child is now m
On Fri, 22 Mar 2019 at 10:10, Tom Lane wrote:
>
> David Rowley writes:
> > [ eclass_indexes_v4.patch ]
>
> I still don't like this approach too much. I think we can fairly easily
> construct the eclass_indexes at a higher level instead of trying to
> manage them in add_eq_member, and after some
David Rowley writes:
> On Fri, 22 Mar 2019 at 10:10, Tom Lane wrote:
>> I'm unsure how hard we should push to get something like this into v12.
>> I'm concerned that its dependency on list_nth might result in performance
>> regressions in some cases; ...
> However, there's always a danger we fin
Thanks for having a hack at this.
On Fri, 22 Mar 2019 at 10:10, Tom Lane wrote:
> I'm unsure how hard we should push to get something like this into v12.
> I'm concerned that its dependency on list_nth might result in performance
> regressions in some cases; it's a lot easier to believe that this
David Rowley writes:
> [ eclass_indexes_v4.patch ]
I still don't like this approach too much. I think we can fairly easily
construct the eclass_indexes at a higher level instead of trying to
manage them in add_eq_member, and after some hacking I have the attached.
Some notes:
* To be sure of kn
On Mon, 18 Mar 2019 at 14:06, David Rowley wrote:
>
> On Mon, 18 Mar 2019 at 10:08, Tom Lane wrote:
> > If that doesn't work (because we need the index info sooner), maybe we
> > could consider never removing ECs from eq_classes, so that their indices
> > never change, then teaching most/all of t
Thanks for looking at this.
On Mon, 18 Mar 2019 at 10:08, Tom Lane wrote:
> I looked at this for a little bit. I'm on board with the basic idea
> of assigning integer indexes to the ECs and keeping bitmapsets of
> relevant EC indexes in RelOptInfos. However ... man, is that
> delete_eclass() th
David Rowley writes:
> [ eclass_indexes_v3.patch ]
I looked at this for a little bit. I'm on board with the basic idea
of assigning integer indexes to the ECs and keeping bitmapsets of
relevant EC indexes in RelOptInfos. However ... man, is that
delete_eclass() thing ugly. And slow, and fragil
On Sun, 10 Mar 2019 at 21:34, David Rowley wrote:
> I started looking at this again and I came up with the attached
> eclass_indexes_v2.patch.
The CF bot didn't like v2. It warned about an uninitialized variable.
My compiler didn't.
Here's v3, hopefully with that fixed.
--
David Rowley
On Sat, 9 Mar 2019 at 13:18, David Rowley wrote:
> This is something I'd like to look into for v13. I think it'll be
> much easier to do if we can get your List reimplementation patch in
> first. That's going to allow list_nth on PlannerInfo->eq_classes to
> work quickly and will remove the need
On Sat, 9 Mar 2019 at 08:05, Tom Lane wrote:
> Setting the CF entry to WOA for now. I wonder though if we should
> just push it out to v13 immediately --- are you intending to do more
> with it in the near future?
Thanks a lot for going over this and providing feedback. I put the
patch out ther
David Rowley writes:
> On Thu, 21 Feb 2019 at 15:00, Tom Lane wrote:
>> Anyway, I rebased the POC patch up to HEAD, just in case anyone
>> still wants to play with it.
> Cool. Thanks.
I haven't done any of the performance testing that this patch
clearly needs, but just in a quick look through t
On Thu, 21 Feb 2019 at 15:00, Tom Lane wrote:
> Pushed that one. I'm interested by the "POC" patch, but I agree
> that it'd take some research to show that it isn't a net negative
> for simple queries. It sounds like you're not really interested
> in pursuing that right now?
Thanks for pushing
David Rowley writes:
> Attaching the original patch again so the commitfest bot gets off my
> back about the other one not applying.
Pushed that one. I'm interested by the "POC" patch, but I agree
that it'd take some research to show that it isn't a net negative
for simple queries. It sounds li
On Tue, 5 Feb 2019 at 22:43, David Rowley wrote:
> So that this does not get lost, I've added an entry for the original
> patch for the March commitfest.
Attaching the original patch again so the commitfest bot gets off my
back about the other one not applying.
--
David Rowley
On Sat, 22 Dec 2018 at 09:28, David Rowley wrote:
>
> On Fri, 21 Dec 2018 at 06:44, Tom Lane wrote:
> > I was distressed to discover via perf that 69% of the runtime of this
> > test now goes into match_eclasses_to_foreign_key_col(). That seems
> > clearly unacceptable.
>
> Agreed. That's pretty
On Wed, 26 Dec 2018 at 09:50, Tomas Vondra wrote:
> Yeah, good questions. I think the simplest thing we could do is building
> them on the first access - that would at least ensure we don't build the
> index without accessing it at least once.
I think we first need to focus on what is back-patcha
On 12/25/18 3:48 AM, David Rowley wrote:
> On Tue, 25 Dec 2018 at 13:46, Tomas Vondra
> wrote:
>> I however observe failures on 4 regression test suites - inherit,
>> equivclass, partition_join and partition_prune (diff attached). That's a
>> bit surprising, because AFAICS the patch merely opti
On Tue, 25 Dec 2018 at 13:46, Tomas Vondra wrote:
> I however observe failures on 4 regression test suites - inherit,
> equivclass, partition_join and partition_prune (diff attached). That's a
> bit surprising, because AFAICS the patch merely optimizes the execution
> and should not change the pla
On 12/24/18 1:07 PM, David Rowley wrote:
> On Mon, 24 Dec 2018 at 09:38, David Rowley
> wrote:
>> Using the above idea, but rather than going to the trouble of storing
>> PlannerInfo->eq_classes as an array type list, if we build the array
>> on the fly inside match_foreign_keys_to_quals(), then
On Mon, 24 Dec 2018 at 09:38, David Rowley wrote:
> Using the above idea, but rather than going to the trouble of storing
> PlannerInfo->eq_classes as an array type list, if we build the array
> on the fly inside match_foreign_keys_to_quals(), then build a
> Bitmapset type index to mark which of t
On Sat, 22 Dec 2018 at 10:53, David Rowley wrote:
> Back in [1], I mentioned that I'd like to start moving away from our
> linked list based implementation of List and start using an array
> based version instead. If we had this we could easily further improve
> this code fkey matching code to not
On Sat, 22 Dec 2018 at 09:28, David Rowley wrote:
> Going by my profiler this drops match_eclasses_to_foreign_key_col()
> down to just 10% of total planner time for this query. The new
> bms_is_member() call is pretty hot inside that function though.
I should have said 28% instead of 10%. 10% is
On Fri, 21 Dec 2018 at 06:44, Tom Lane wrote:
> I was distressed to discover via perf that 69% of the runtime of this
> test now goes into match_eclasses_to_foreign_key_col(). That seems
> clearly unacceptable.
Agreed. That's pretty terrible.
I looked at this a bit and see that
match_eclasses_t
In connection with David Rowley's proposal to change bitmapset.c to use
64-bit words, I dug out an old test case I had for a complex-to-plan query
(attached). Andres Freund posted this to the lists perhaps ten years ago,
though I can't locate the original posting right now.
I was distressed to di
30 matches
Mail list logo
