Re: REVIEW: Extensions support for pg_dump - Mailing list pgsql-hackers

From Anssi Kääriäinen
Subject Re: REVIEW: Extensions support for pg_dump
Date
Msg-id 4D3558C7.1080704@thl.fi
Whole thread Raw
In response to Re: REVIEW: Extensions support for pg_dump  (Dimitri Fontaine <dimitri@2ndQuadrant.fr>)
Responses Re: REVIEW: Extensions support for pg_dump  (Dimitri Fontaine <dimitri@2ndQuadrant.fr>)
List pgsql-hackers
On 01/17/2011 06:53 PM, Dimitri Fontaine wrote:

>> Usability review:
>>
>> The patch implements a way to create extensions. While the patch is labeled
>> extensions support for pg_dump, it actually implements more. It implements a
>> new way to package and install extension, and changes contrib extensions to
>> use the new way.
> Well, all there's in the patch is infrastructure to be able to issue
> those single lines in your dump :)
Is this supposed to be used mainly by contrib and PGXN extensions? When 
I saw the documentation, I immediately thought that this is a nice way 
to package my application's stored procedures. If this is not one of the 
intended usages, it should be documented. I can see that this could be 
problematic when updating PostgreSQL and when recovering from backups.

When recovering from backup, you need to have the locally created 
extension available. But it might be that the extension files are lost 
when the system went down in flames. Now, the backup is unusable 
(right?) until extension files can be recovered from source control or 
where ever they might be stored. This is why I suggested having multiple 
locations for the extensions. It would be easy to backup locally created 
extensions if those were in a single directory. All in all, I have a 
nervous feeling that somebody someday will have an unusable dump because 
they used this feature, but do not have the extension files available...

Also, this part of documentation:

The goal of using extensions is so that <application>pg_dump</> knows
not to dump all the object definitions into your backups, but rather
issue a single <xref linkend="SQL-CREATEEXTENSION"> command.
From that, it is not entirely clear to me why this is actually wanted 
in PostgreSQL. I suppose this will make dump/restore easier to use. But 
from that paragraph, I get the feeling the only advantage is that your 
dump will be smaller. And I still have a feeling this implements more. 
Not that it is a bad thing at all.
> It used to work this way with \i, obviously.  Should the extension patch
> care about that and how?  Do we want to RESET search_path or to manually
> manage it like we do for the client_min_messages and log_min_messages?
It was unintuitive to me to have search_path changed by SQL command as a 
side effect. When using \i, not so much.
>> When trying to load extensions which contain identical signatured functions,
>> if the loading will succeed is dependent on the search path. If there is a
>> conflicting function in search path (first extension loaded to schema
>> public), then the second extension load will fail. But if the order is
>> reversed (first extension loaded to schema foo which is not in search path),
>> then the second extension can be loaded to the public schema.
> Well I think that's an expected limitation here.  In the future we might
> want to add suport for inter-extension dependencies and conflicts, but
> we're certainly not there yet.
OK with this as is. It is just a bit surprising that you can create the 
extensions in one order but not in another.
>> There is no validation for the extension names in share/contrib/. It is
>> possible to have extensions control files named ".control", ".name.control",
>> "name''.control" etc. While it is stupid to name them so, it might be better
>> to give an explicit warning / error in these cases. It is of course possible
>> to use these extensions.
> I don't have a strong opinion here, will wait for some votes.
Yeah, this is not a big problem in practice. Just don't name them like 
that. And if you do, you will find out soon enough that you made a 
mistake. By the way, in my comment above "It is of course possible to 
use these extensions." -> "It is of course NOT possible ...".
>> I haven't had time to review the pg_dump part of the patch yet, will do that
>> next (tomorrow). I hope it is OK to post a partial review...
>  From here, it's very good!  Will you continue from the git repository,
> where the fixes are available, or do you want a v26 already?
It is easy for me to continue from the Git repo. I will next continue 
doing the pg_dump part of the review. I hope I have time to complete 
that today.
 - Anssi



pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Confusing comment in TransactionIdIsInProgress
Next
From: Susanne Ebrecht
Date:
Subject: Re: Determining client_encoding from client locale