Обсуждение: ECPG installcheck tests fail if PGDATABASE is set
Hi, I got a bit confused running installcheck-world and seeing ecpg failures like: [NO_PID]: ECPGconnect: opening database <DEFAULT> on <DEFAULT> port <DEFAULT> for user regress_ecpg_user2 [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: ECPGconnect: could not open database: FATAL: database "regress_ecpg_user2" does not exist - -[NO_PID]: sqlca: code: 0, state: 00000 [NO_PID]: ecpg_finish: connection main closed [NO_PID]: sqlca: code: 0, state: 00000 -[NO_PID]: raising sqlcode -402 on line 43: could not connect to database "<DEFAULT>" on line 43 -[NO_PID]: sqlca: code: -402, state: 08001 -[NO_PID]: raising sqlcode -220 on line 44: connection "main" does not exist on line 44 -[NO_PID]: sqlca: code: -220, state: 08003 [NO_PID]: ECPGconnect: opening database ecpg2_regression on <DEFAULT> port <DEFAULT> for user regress_ecpg_user1 A bit of pondering pointed me towards my environment's PGDATABASE=postgres being to blame. Unsetting that makes the test succeed. Do we consider that an unsupported configuration? It seems like a fairly reasonable thing to want to support imo. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> I got a bit confused running installcheck-world and seeing ecpg
> failures like:
> ...
> A bit of pondering pointed me towards my environment's
> PGDATABASE=postgres being to blame. Unsetting that makes the test
> succeed.
Hm ... pg_regress unsets PGDATABASE, along with the other related
environment variables, when it has a temp installation but not
when it doesn't. So what I don't understand is why your environment
doesn't also break every other regression test besides ecpg.
Perhaps they're all connecting to "postgres", but it's hard to
believe there wouldn't be conflicts if so.
regards, tom lane
On March 18, 2018 4:06:18 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Andres Freund <andres@anarazel.de> writes: >> I got a bit confused running installcheck-world and seeing ecpg >> failures like: >> ... >> A bit of pondering pointed me towards my environment's >> PGDATABASE=postgres being to blame. Unsetting that makes the test >> succeed. > >Hm ... pg_regress unsets PGDATABASE, along with the other related >environment variables, when it has a temp installation but not >when it doesn't. So what I don't understand is why your environment >doesn't also break every other regression test besides ecpg. >Perhaps they're all connecting to "postgres", but it's hard to >believe there wouldn't be conflicts if so. All the others specify a database. The issue with the ecpg test is that it doesn't for two test cases. The failure is causedby libpq guessing the db name from the user name (hence the error about a database with role in it), which doesn'thappen if libpq sees the environment variable. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> writes:
> On March 18, 2018 4:06:18 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hm ... pg_regress unsets PGDATABASE, along with the other related
>> environment variables, when it has a temp installation but not
>> when it doesn't. So what I don't understand is why your environment
>> doesn't also break every other regression test besides ecpg.
> All the others specify a database. The issue with the ecpg test is that
> it doesn't for two test cases.
Ah. Well, it doesn't seem unreasonable to want to test that case,
so I don't think "remove the test case" is the right answer.
Is it sane for pg_regress to unset PGDATABASE unconditionally? Not
sure, but if we're generally always specifying a value, maybe that's
OK.
regards, tom lane
Hi, On 2018-03-18 19:30:33 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On March 18, 2018 4:06:18 PM PDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> Hm ... pg_regress unsets PGDATABASE, along with the other related > >> environment variables, when it has a temp installation but not > >> when it doesn't. So what I don't understand is why your environment > >> doesn't also break every other regression test besides ecpg. > > > All the others specify a database. The issue with the ecpg test is that > > it doesn't for two test cases. > > Ah. Well, it doesn't seem unreasonable to want to test that case, > so I don't think "remove the test case" is the right answer. Right. > Is it sane for pg_regress to unset PGDATABASE unconditionally? Not > sure, but if we're generally always specifying a value, maybe that's > OK. I'm not sure either. I wonder whether we should just make ecpg's pg_regress invocation do so? That seems to be the way of least resistance ;) Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2018-03-18 19:30:33 -0400, Tom Lane wrote:
>> Is it sane for pg_regress to unset PGDATABASE unconditionally? Not
>> sure, but if we're generally always specifying a value, maybe that's
>> OK.
> I'm not sure either. I wonder whether we should just make ecpg's
> pg_regress invocation do so? That seems to be the way of least
> resistance ;)
Don't think I like ecpg's tests behaving differently in this respect
than the rest of them do; that seems like a recipe for unrecognized
security issues.
If nobody can think of a positive reason for pg_regress not to
unset PGDATABASE unconditionally, let's try that and see how it
goes.
regards, tom lane
On Sun, Mar 18, 2018 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Don't think I like ecpg's tests behaving differently in this respect > than the rest of them do; that seems like a recipe for unrecognized > security issues. > > If nobody can think of a positive reason for pg_regress not to > unset PGDATABASE unconditionally, let's try that and see how it > goes. It would be nice to get this fixed. Several people have been confused by it at this point. -- Peter Geoghegan
Peter Geoghegan <pg@bowt.ie> writes:
> On Sun, Mar 18, 2018 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Don't think I like ecpg's tests behaving differently in this respect
>> than the rest of them do; that seems like a recipe for unrecognized
>> security issues.
>>
>> If nobody can think of a positive reason for pg_regress not to
>> unset PGDATABASE unconditionally, let's try that and see how it
>> goes.
> It would be nice to get this fixed. Several people have been confused
> by it at this point.
I think I just forgot about this thread. Shall we change it in HEAD
and see what happens? Maybe backpatch, but not till after 12.0 is out.
regards, tom lane
On Fri, Sep 27, 2019 at 2:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I think I just forgot about this thread. Shall we change it in HEAD > and see what happens? Maybe backpatch, but not till after 12.0 is out. Please do. -- Peter Geoghegan
On 2019-09-27 14:43:00 -0700, Peter Geoghegan wrote: > On Fri, Sep 27, 2019 at 2:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I think I just forgot about this thread. Shall we change it in HEAD > > and see what happens? Maybe backpatch, but not till after 12.0 is out. > > Please do. +1
Peter Geoghegan <pg@bowt.ie> writes:
> On Fri, Sep 27, 2019 at 2:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think I just forgot about this thread. Shall we change it in HEAD
>> and see what happens? Maybe backpatch, but not till after 12.0 is out.
> Please do.
After looking closer at the code in pg_regress.c, I'm wondering a bit
about PGSERVICE. A setting for that might certainly bring in a value
for the database name, but I don't think we can just summarily unset it.
I don't plan to do anything about that right now, but conceivably it'd
bite people someday.
Another thing that looks a bit fishy here is that the set of variables
that pg_regress.c unsets is very much smaller than the set that libpq
reacts to --- we have added a ton of the latter without touching this
list (much less the three or four other places that duplicate it).
I wonder how problematic that is.
regards, tom lane
On Fri, Sep 27, 2019 at 3:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Please do. > > After looking closer at the code in pg_regress.c, I'm wondering a bit > about PGSERVICE. A setting for that might certainly bring in a value > for the database name, but I don't think we can just summarily unset it. > I don't plan to do anything about that right now, but conceivably it'd > bite people someday. At least there will be some breadcrumbs to follow in the archive. > Another thing that looks a bit fishy here is that the set of variables > that pg_regress.c unsets is very much smaller than the set that libpq > reacts to --- we have added a ton of the latter without touching this > list (much less the three or four other places that duplicate it). > I wonder how problematic that is. Only time will tell, I suspect. -- Peter Geoghegan