From: | "David E(dot) Wheeler" <david(at)kineticode(dot)com> |
---|---|
To: | Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> |
Cc: | PostgreSQL-development Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Review: Extensions Patch |
Date: | 2010-12-08 21:00:07 |
Message-ID: | 0B50B023-FB1A-4B2E-8670-065430D4D33B@kineticode.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Dec 8, 2010, at 12:42 PM, Dimitri Fontaine wrote:
> Kineticode Billing <david(at)kineticode(dot)com> writes:
>> No, it's not. There are no unit tests at all. You can call the contrib
>> modules and their tests acceptance tests, but that's not the same
>> thing.
>
> Ok, I need some more guidance here. All contrib extension (there are 38
> of them) are using the CREATE EXTENSION command and checking the result
> with the pg_regress framework. What are we missing?
unit tests. You add a bunch of functions. You need to test those functions.
> I can see about adding DROP EXTENSION for all the tests, but that's
> about it.
If you add that, you'll also need something to CREATE EXTENSION with, eh? And also, tests to make sure that WITH SCHEMA works properly (however that shakes out).
>> Okay, keep the installed control files. But don't make me distribute
>> them unless absolutely necessary.
>
> Yes you have to distribute them, that's necessary. Sorry about that.
I don't see why. Most of them are dead simple and could easily be Makefile variables.
>> Sure. But you're mandating one version even if you have multiple
>> extensions. That's the potentially confusing part.
>
> I see how confusing it is, because what you say ain't true. You can
> always put different version numbers in the control file and even skip
> the rule to produce the .control from the .control.in by providing the
> .control directly. That's just a facility here.
I see, okay.
>> What? I don't follow what you're saying.
>
> You're complaining that a single EXTVERSION applied to more than one
> extension's control file is confusing. What if we had EXTCOMMENT and
> EXTRELOCATABLE in there too? What exactly are you expecting the Makefile
> to look like?
Mostly these will all have only one setting. In more complex cases perhaps one *would* be required to distribute a control file.
>> In contrib. You seem to forget that there are a lot of third-party
>> extensions out there already.
>
> True. That's still not the common case, and it's still covered the same
> way as before, you need to restart to attach to shared memory.
Okay.
>> I would much rather retain that warning -- everyone should know about
>> it -- and somehow convince SPI to be much less verbose in reporting
>> issues. It should specify where the error came from (which query) and
>> what the error actually is.
>
> The problem is much more complex than that and could well kill the patch
> if we insist on fixing it as part of the extension's work, v1. The
> problem is exposing more internals of the SQL parser into SPI so that
> you can send a bunch of queries in an explicit way. Mind you, the
> firsts version of the patch had something like that in there, but that
> wouldn't have supported this use case. I've been told to simply use SPI
> there.
I agree that SPI should be fixed in a different project/patch. Go with what you've got, it will just highlight the problem with SPI more.
Best,
David
From | Date | Subject | |
---|---|---|---|
Next Message | Oleg Bartunov | 2010-12-08 21:15:53 | Re: plperlu problem with utf8 |
Previous Message | Greg Smith | 2010-12-08 20:58:25 | Re: [COMMITTERS] pgsql: Optimize commit_siblings in two ways to improve group commit. |