Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-09 Thread Dilip Kumar
On Fri, Sep 9, 2016 at 6:51 PM, Tom Lane wrote: > Pushed with cosmetic adjustments --- mostly, I thought we needed some > comments about the topic. Okay, Thanks. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-09 Thread Tom Lane
Dilip Kumar writes: > Seems like a better option, done it this way.. > attaching the modified patch.. Pushed with cosmetic adjustments --- mostly, I thought we needed some comments about the topic. regards, tom lane -- Sent via pgsql-hackers

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-08 Thread Dilip Kumar
On Thu, Sep 8, 2016 at 7:21 PM, Tom Lane wrote: > We could make it work like that without breaking the ABI if we were > to add a NOERROR bit to the allowed "flags". However, after looking > around a bit I'm no longer convinced what I said above is a good idea. > In

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-08 Thread Tom Lane
Dilip Kumar writes: > On Thu, Sep 8, 2016 at 2:23 AM, Tom Lane wrote: >> In the case of record_in, it seems like it'd be easy enough to use >> lookup_rowtype_tupdesc_noerror() instead of lookup_rowtype_tupdesc() >> and then throw a user-facing error

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-08 Thread Dilip Kumar
On Thu, Sep 8, 2016 at 2:23 AM, Tom Lane wrote: > I really don't like this solution. Changing this one function, out of the > dozens just like it in lsyscache.c, seems utterly random and arbitrary. > > I'm actually not especially convinced that passing domain_in a random >

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-07 Thread Tom Lane
Dilip Kumar writes: > Basically this patch changes error at three places. > 1. getBaseTypeAndTypmod: This is being called from domain_in exposed > function (domain_in-> > domain_state_setup-> getBaseTypeAndTypmod). Though this function is > being called from many other

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-06 Thread Dilip Kumar
On Wed, Sep 7, 2016 at 8:52 AM, Haribabu Kommi wrote: > I reviewed and tested the patch. The changes are fine. > This patch provides better error message compared to earlier. > > Marked the patch as "Ready for committer" in commit-fest. Thanks for the review ! --

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-09-06 Thread Haribabu Kommi
On Fri, Aug 26, 2016 at 7:11 PM, Dilip Kumar wrote: > I have changed this in attached patch.. > I reviewed and tested the patch. The changes are fine. This patch provides better error message compared to earlier. Marked the patch as "Ready for committer" in commit-fest.

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-26 Thread Dilip Kumar
On Wed, Aug 17, 2016 at 6:10 PM, Robert Haas wrote: >> If you are making changes in plpgsql_validator(), then shouldn't we >> make changes in plperl_validator() or plpython_validator()? I see >> that you have made changes to function CheckFunctionValidatorAccess() >> which

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-17 Thread Robert Haas
On Wed, Aug 17, 2016 at 7:25 AM, Amit Kapila wrote: > On Wed, Aug 10, 2016 at 11:27 AM, Dilip Kumar wrote: >> On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar wrote: >>> This seems better, after checking at other places I found

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-17 Thread Amit Kapila
On Wed, Aug 10, 2016 at 11:27 AM, Dilip Kumar wrote: > On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar wrote: >> This seems better, after checking at other places I found that for >> invalid type we are using ERRCODE_UNDEFINED_OBJECT and for invalid >>

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-09 Thread Dilip Kumar
On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar wrote: > This seems better, after checking at other places I found that for > invalid type we are using ERRCODE_UNDEFINED_OBJECT and for invalid > functions we are using ERRCODE_UNDEFINED_FUNCTION. So I have done the > same way.

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-09 Thread Dilip Kumar
On Tue, Aug 9, 2016 at 6:49 PM, Amit Kapila wrote: > Okay, then how about ERRCODE_UNDEFINED_OBJECT? It seems to be used in > almost similar cases. This seems better, after checking at other places I found that for invalid type we are using ERRCODE_UNDEFINED_OBJECT and

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-09 Thread Amit Kapila
On Mon, Aug 8, 2016 at 6:00 PM, Dilip Kumar wrote: > On Mon, Aug 8, 2016 at 5:23 PM, Amit Kapila wrote: >> >> >> Did you consider to use ERRCODE_UNDEFINED_COLUMN with error messages >> like: "type %u does not exit" or "type id %u does not exit"?

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-08 Thread Dilip Kumar
On Mon, Aug 8, 2016 at 5:23 PM, Amit Kapila wrote: > > > > Your other options and the option you choose are same. > > > Sorry typo, I meant ERRCODE_INVALID_OBJECT_DEFINITION. > Did you consider to use ERRCODE_UNDEFINED_COLUMN with error messages > like: "type %u does

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-08 Thread Amit Kapila
On Mon, Aug 8, 2016 at 5:11 PM, Amit Kapila wrote: > On Mon, Aug 8, 2016 at 4:58 PM, Dilip Kumar wrote: >> >> On Wed, Aug 3, 2016 at 5:35 AM, Robert Haas wrote: >>> >>> I think that's just making life difficult. If nothing

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-08 Thread Amit Kapila
On Mon, Aug 8, 2016 at 4:58 PM, Dilip Kumar wrote: > > On Wed, Aug 3, 2016 at 5:35 AM, Robert Haas wrote: >> >> I think that's just making life difficult. If nothing else, sqlsmith >> hunts around for functions it can call that return internal

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-08 Thread Dilip Kumar
On Wed, Aug 3, 2016 at 5:35 AM, Robert Haas wrote: > I think that's just making life difficult. If nothing else, sqlsmith > hunts around for functions it can call that return internal errors, > and if we refuse to fix all of them to return user-facing errors, then > it's

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Peter Geoghegan
On Tue, Aug 2, 2016 at 5:05 PM, Robert Haas wrote: > I think that's just making life difficult. If nothing else, sqlsmith > hunts around for functions it can call that return internal errors, > and if we refuse to fix all of them to return user-facing errors, then > it's

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Robert Haas
On Tue, Aug 2, 2016 at 7:16 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Aug 2, 2016 at 6:03 AM, Dilip Kumar wrote: >>> There are many more such exposed functions, which can throw cache lookup >>> failure error if we pass

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Tom Lane
Robert Haas writes: > On Tue, Aug 2, 2016 at 6:03 AM, Dilip Kumar wrote: >> There are many more such exposed functions, which can throw cache lookup >> failure error if we pass wrong value. >> >> i.e. >> record_in >> domain_in >> fmgr_c_validator >>

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Robert Haas
On Tue, Aug 2, 2016 at 6:03 AM, Dilip Kumar wrote: > There are many more such exposed functions, which can throw cache lookup > failure error if we pass wrong value. > > i.e. > record_in > domain_in > fmgr_c_validator > plpgsql_validator > pg_procedure_is_visible > > Are we

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Dilip Kumar
On Tue, Aug 2, 2016 at 3:33 PM, Dilip Kumar wrote: > There are many more such exposed functions, which can throw cache lookup > failure error if we pass wrong value. > > i.e. > record_in > domain_in > fmgr_c_validator > edb_get_objecttypeheaddef > plpgsql_validator >

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-08-02 Thread Dilip Kumar
On Sat, Jul 30, 2016 at 4:27 AM, Michael Paquier wrote: > OK for me. Thanks for the commit. There are many more such exposed functions, which can throw cache lookup failure error if we pass wrong value. i.e. record_in domain_in fmgr_c_validator

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-07-29 Thread Michael Paquier
On Sat, Jul 30, 2016 at 1:17 AM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Jul 26, 2016 at 9:27 PM, Michael Paquier >> wrote: >>> While looking at the series of functions pg_get_*, I have noticed as >>> well that

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-07-29 Thread Tom Lane
Robert Haas writes: > On Tue, Jul 26, 2016 at 9:27 PM, Michael Paquier > wrote: >> While looking at the series of functions pg_get_*, I have noticed as >> well that pg_get_userbyid() returns "unknown (OID=%u)" when it does >> not know a user.

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-07-29 Thread Robert Haas
On Tue, Jul 26, 2016 at 9:27 PM, Michael Paquier wrote: > On Wed, Jul 27, 2016 at 5:11 AM, Robert Haas wrote: >> Committed with minor kibitizing: you don't need an "else" after a >> statement that transfers control out of the function. > >

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-07-26 Thread Michael Paquier
On Wed, Jul 27, 2016 at 5:11 AM, Robert Haas wrote: > Committed with minor kibitizing: you don't need an "else" after a > statement that transfers control out of the function. Thanks. Right, I forgot that. > Shouldn't > pg_get_function_arguments,

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-07-26 Thread Robert Haas
On Mon, Jun 6, 2016 at 3:30 AM, Michael Paquier wrote: > On Sun, Jun 5, 2016 at 11:16 PM, Tom Lane wrote: >> Michael Paquier writes: >>> Still, on back branches I think that it would be a good idea to have a >>> better

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-06 Thread Michael Paquier
On Sun, Jun 5, 2016 at 11:16 PM, Tom Lane wrote: > Michael Paquier writes: >> Still, on back branches I think that it would be a good idea to have a >> better error message for the ones that already throw an error, like >> "trigger with OID %u does

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-05 Thread Tom Lane
Michael Paquier writes: > Still, on back branches I think that it would be a good idea to have a > better error message for the ones that already throw an error, like > "trigger with OID %u does not exist". Switching from elog to ereport > would be a good idea, but

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-05 Thread Michael Paquier
On Sun, Jun 5, 2016 at 4:28 PM, Michael Paquier wrote: > On Sun, Jun 5, 2016 at 12:29 AM, Tom Lane wrote: >> Michael Paquier writes: >>> This is still failing: >>> =# select indexdef from pg_catalog.pg_indexes where

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-05 Thread Michael Paquier
On Sun, Jun 5, 2016 at 12:29 AM, Tom Lane wrote: > Michael Paquier writes: >> This is still failing: >> =# select indexdef from pg_catalog.pg_indexes where indexdef is not NULL; >> ERROR: XX000: cache lookup failed for index 2619 >> LOCATION:

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-04 Thread Tom Lane
Michael Paquier writes: > This is still failing: > =# select indexdef from pg_catalog.pg_indexes where indexdef is not NULL; > ERROR: XX000: cache lookup failed for index 2619 > LOCATION: pg_get_indexdef_worker, ruleutils.c:1054 > Do we want to improve at least the

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2016-06-04 Thread Michael Paquier
On Fri, Aug 7, 2015 at 9:21 AM, Tom Lane wrote: > Andreas Seltenreich writes: >> Tom Lane writes: >>> On 08/01/2015 05:59 PM, Tom Lane wrote: Well, I certainly think all of these represent bugs: 1 | ERROR: could not find pathkey item to sort >

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-06 Thread Tom Lane
Andreas Seltenreich seltenre...@gmx.de writes: Tom Lane writes: On 08/01/2015 05:59 PM, Tom Lane wrote: Well, I certainly think all of these represent bugs: 1 | ERROR: could not find pathkey item to sort Hmm ... I see no error with these queries as of today's HEAD or back-branch tips. I

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-05 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes: On 08/05/2015 02:24 AM, Tom Lane wrote: Piotr Stefaniak postg...@piotr-stefaniak.me writes: Set join_collapse_limit = 32 and you should see the error. Ah ... now I get that error on the smaller query, but the larger one (that you put in an

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-05 Thread Andreas Seltenreich
Tom Lane writes: On 08/01/2015 05:59 PM, Tom Lane wrote: Well, I certainly think all of these represent bugs: 1 | ERROR: could not find pathkey item to sort Hmm ... I see no error with these queries as of today's HEAD or back-branch tips. I surmise that this was triggered by one of the

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-04 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes: On 08/03/2015 09:18 PM, Tom Lane wrote: ... but I can't reproduce it on HEAD with either of these queries. Not clear why you're getting different results. I'm terribly sorry, but I didn't notice that postgresql.conf was modified... Set

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-04 Thread Ewan Higgs
From: Peter Geoghegan p...@heroku.com To: Andreas Seltenreich seltenre...@gmx.de Cc: Pg Hackers pgsql-hackers@postgresql.org Sent: Sunday, 2 August 2015, 10:39 Subject: Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c On Fri, Jul 31, 2015 at 5:56 PM, Andreas Seltenreich seltenre

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-03 Thread Andreas Seltenreich
Peter Geoghegan writes: On Fri, Jul 31, 2015 at 5:56 PM, Andreas Seltenreich seltenre...@gmx.de wrote: sqlsmith triggered the following assertion in master (c188204). Thanks for writing sqlsmith. It seems like a great tool. I wonder, are you just running the tool with assertions enabled

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-03 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes: How about this one? 1 ERROR: could not find RelOptInfo for given relids That would be a bug, for sure ... It's triggered on 13bba02271dce865cd20b6f49224889c73fed4e7 by this query and the attached one: ... but I can't reproduce it on

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-02 Thread Peter Geoghegan
On Fri, Jul 31, 2015 at 5:56 PM, Andreas Seltenreich seltenre...@gmx.de wrote: sqlsmith triggered the following assertion in master (c188204). Thanks for writing sqlsmith. It seems like a great tool. I wonder, are you just running the tool with assertions enabled when PostgreSQL is built? If

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-02 Thread Tom Lane
Piotr Stefaniak postg...@piotr-stefaniak.me writes: On 08/01/2015 05:59 PM, Tom Lane wrote: Well, I certainly think all of these represent bugs: 1 | ERROR: could not find pathkey item to sort sqlsmith was able to find these two queries that trigger the error on my machine: Hmm ... I see

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-01 Thread Andreas Seltenreich
Tom Lane writes: Well, I certainly think all of these represent bugs: [...] thanks for priorizing them. I'll try to digest them somewhat before posting. This one's pretty darn odd, because 2619 is pg_statistic and not an index at all: 4 | ERROR: cache lookup failed for index 2619

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-01 Thread Tom Lane
Andreas Seltenreich seltenre...@gmx.de writes: Tom Lane writes: What concerns me more is that what you're finding is only cases that trip an assertion sanity check. It seems likely that you're also managing to trigger other bugs with less drastic consequences, such as could not devise a

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-08-01 Thread Andreas Seltenreich
Tom Lane writes: What concerns me more is that what you're finding is only cases that trip an assertion sanity check. It seems likely that you're also managing to trigger other bugs with less drastic consequences, such as could not devise a query plan failures or just plain wrong answers.

Re: [HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-07-31 Thread Tom Lane
Andreas Seltenreich seltenre...@gmx.de writes: sqlsmith triggered the following assertion in master (c188204). TRAP: FailedAssertion(!(!bms_overlap(joinrelids, sjinfo-min_lefthand)), File: joinrels.c, Line: 500) Cool, I'll take a look. As usual, the query is against the regression

[HACKERS] [sqlsmith] Failed assertion in joinrels.c

2015-07-31 Thread Andreas Seltenreich
Hi, sqlsmith triggered the following assertion in master (c188204). TRAP: FailedAssertion(!(!bms_overlap(joinrelids, sjinfo-min_lefthand)), File: joinrels.c, Line: 500) As usual, the query is against the regression database. It is rather unwieldy… I wonder if I should stop working on new