Re: Adding lfirst_node (was Re: [HACKERS] [sqlsmith] Planner crash on foreign table join)

2017-04-10 Thread Andres Freund
On 2017-04-10 12:20:16 -0400, Tom Lane wrote: > Barring objections, I'll push this shortly. +1, to just about all of it -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Adding lfirst_node (was Re: [HACKERS] [sqlsmith] Planner crash on foreign table join)

2017-04-10 Thread Tom Lane
I wrote: > Andrew Gierth writes: >> In a discussion with Andres on the hash grouping sets review thread, I >> proposed that we should have something of the form >> #define lfirst_node(_type_, l) (castNode(_type_,lfirst(l))) > That seems like a fairly good idea. A

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-10 Thread Tom Lane
Andrew Gierth writes: > In the discussion with Andres the same point came up for palloc, for > which I suggested we add something along the lines of: > #define palloc_object(_type_) (_type_ *) palloc(sizeof(_type_)) > #define palloc_array(_type_, n) (_type_ *)

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Andrew Gierth
> "Thomas" == Thomas Munro writes: >> SomeType *x = (SomeType *) lfirst(l); >> >> (in my code I tend to omit the (SomeType *), which I dislike because >> it adds no real protection) Thomas> Just BTW, without that cast it's not compilable as C++, so I'm

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Thomas Munro
On Sun, Apr 9, 2017 at 8:27 AM, Andrew Gierth wrote: > foreach(l, blah) > { > SomeType *x = (SomeType *) lfirst(l); > > (in my code I tend to omit the (SomeType *), which I dislike because it > adds no real protection) Just BTW, without that cast it's

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Andreas Seltenreich
Tom Lane writes: > I made the attached quick-hack patch, and found that check-world > passes just fine with it. That's not complete proof that we have > no other bugs of this ilk, but it definitely supports the idea > that we don't really need to add the overhead. I'll just put this > in the

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Mark Dilger
> On Apr 8, 2017, at 7:41 PM, Mark Dilger wrote: > > >> On Apr 8, 2017, at 7:35 PM, Tom Lane wrote: >> >> Mark Dilger writes: >>> This is very near where the original crash reported in this thread was >>> crashing,

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Mark Dilger
> On Apr 8, 2017, at 7:35 PM, Tom Lane wrote: > > Mark Dilger writes: >> This is very near where the original crash reported in this thread was >> crashing, probably only >> different due to the extra lines of Assert that were added. Am I missing

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
Mark Dilger writes: > This is very near where the original crash reported in this thread was > crashing, probably only > different due to the extra lines of Assert that were added. Am I missing > some portion of the > fix that you are testing? I have only applied the

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Mark Dilger
> On Apr 8, 2017, at 6:48 PM, Mark Dilger wrote: > > >> On Apr 8, 2017, at 6:38 PM, Mark Dilger wrote: >> >> >>> On Apr 8, 2017, at 5:13 PM, Tom Lane wrote: >>> >>> I wrote: Robert Haas

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Mark Dilger
> On Apr 8, 2017, at 6:38 PM, Mark Dilger wrote: > > >> On Apr 8, 2017, at 5:13 PM, Tom Lane wrote: >> >> I wrote: >>> Robert Haas writes: On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: I

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Mark Dilger
> On Apr 8, 2017, at 5:13 PM, Tom Lane wrote: > > I wrote: >> Robert Haas writes: >>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: >>> I think it's pretty dubious to change this, honestly. Just because it >>> would have caught

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
I wrote: > Robert Haas writes: >> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: >> I think it's pretty dubious to change this, honestly. Just because it >> would have caught this one bug doesn't make it an especially valuable >> thing in general.

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Andres Freund
On 2017-04-08 17:20:28 -0400, Tom Lane wrote: > Robert Haas writes: > > On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: > >> This makes me wonder whether we were being penny-wise and pound-foolish > >> by not making Bitmapsets be a kind of Node, so that

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
Robert Haas writes: > On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: >> This makes me wonder whether we were being penny-wise and pound-foolish >> by not making Bitmapsets be a kind of Node, so that there could be IsA >> assertions in the bitmapset.c

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> Experimentation shows that actually, the standard regression tests > Tom> provide dozens of opportunities for find_relation_from_clauses to > Tom> fail on non-RestrictInfo input.

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Robert Haas
On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane wrote: > This makes me wonder whether we were being penny-wise and pound-foolish > by not making Bitmapsets be a kind of Node, so that there could be IsA > assertions in the bitmapset.c routines, as there are for Lists. Most >

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Andrew Gierth
> "Tom" == Tom Lane writes: Tom> Experimentation shows that actually, the standard regression tests Tom> provide dozens of opportunities for find_relation_from_clauses to Tom> fail on non-RestrictInfo input. However, it lacks any IsA check, In a discussion with

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
I wrote: > Andrew Gierth writes: >> Commit ac2b095088 assumes that clauselist_selectivity is being passed a >> list of RelOptInfo, but postgres_fdw is passing it a list of bare >> clauses. One of them is wrong :-) > It's a bit scary that apparently none of the

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Tom Lane
Andrew Gierth writes: > "Andreas" == Andreas Seltenreich writes: > Andreas> testing master at f0e44021df with a loopback postgres_fdw > Andreas> installed, I see lots of crashes on queries joining foreign > Andreas> tables with various

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-08 Thread Andrew Gierth
> "Andreas" == Andreas Seltenreich writes: Andreas> Hi, Andreas> testing master at f0e44021df with a loopback postgres_fdw Andreas> installed, I see lots of crashes on queries joining foreign Andreas> tables with various expressions. Below is a reduced recipe