Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#2264 closed patch (fixed)

Fix postgis_restore.pl to upgrade from PostGIS installed in a dedicated schema

Reported by: frost242 Owned by: strk
Priority: medium Milestone: PostGIS 2.1.0
Component: build Version: 2.0.x
Keywords: Cc:

Description

Hello,

One of our customers has PostGIS installed a dedicated schema in his database. We had to fix postgis_restore.pl to get rid of multiple errors during the restore.

You can find an attached patch to postgis_restore.pl to add a new option -s schemaname to be able to tell the script in witch schema is PostGIS located.

It may need some rework, but at least it works for our client.

Regards, Thomas

Attachments (3)

postgis_restore.pl.patch (1.4 KB ) - added by frost242 11 years ago.
Patch to add -s schemaname to postgis_restore.pl
postgis_restore.pl.2.patch (2.3 KB ) - added by frost242 11 years ago.
patch updated per comments.
postgis_restore.pl.in.patch (1.8 KB ) - added by frost242 11 years ago.
Another revision of the patch

Download all attachments as: .zip

Change History (14)

by frost242, 11 years ago

Attachment: postgis_restore.pl.patch added

Patch to add -s schemaname to postgis_restore.pl

comment:1 by robe, 11 years ago

Component: postgisbuild/upgrade/install
Milestone: PostGIS 2.0.4
Owner: changed from pramsey to strk
Type: enhancementpatch

comment:2 by strk, 11 years ago

I don't trust those "SET search_schema" statements. Would have no problem if absence of -s switch would omit those lines.

by frost242, 11 years ago

Attachment: postgis_restore.pl.2.patch added

patch updated per comments.

comment:3 by frost242, 11 years ago

Please find an updated version of this patch to address your comment. The schema name is also now protected by double-quotes.

Regards, Thomas Reiss

comment:4 by strk, 11 years ago

A couple more tweaks:

  • It is postgis_restore.pl.in that needs to be patched
  • Please avoid introducing a dependency on GetoptLong, isn't really needed in this case

comment:5 by frost242, 11 years ago

Sorry, I missed to patch the appropriate file.

About GetOpt::Long, do you mean to avoid using any helper module and parse @ARGV or is it possible to rely on the lighter GetOpt::Std ?

comment:6 by strk, 11 years ago

I mean to parse @ARGV directly, like it's done for -v, while shift() check what it is :)

by frost242, 11 years ago

Attachment: postgis_restore.pl.in.patch added

Another revision of the patch

comment:7 by frost242, 11 years ago

Comments adressed in the attached patch.

I used the same construct as for -v, that means the options must be passed in a defined order: -v always before -s. I hope it's OK this way.

comment:8 by strk, 11 years ago

Resolution: fixed
Status: newclosed

Better, I improved the option parsing so you can specify in arbitrary order as long as it is before the dump filename. It's in trunk @ r11285

comment:9 by strk, 11 years ago

Milestone: PostGIS 2.0.4PostGIS 2.1.0
Resolution: fixed
Status: closedreopened

Actually I'm reopening because the code still lacks adding -s semantic to Usage output (see bottom line of the output, it tells you about -v but not about -s).

A note in the manual may also be nice to have.

Actually I'm also unclear myself about what the switch does. It looks like restoring into a database in which postgis is installed in a custom schema, is that correct ? But how does it deal with a _dump_ having postgis in a custom schema ? Is it still able to drop that one ?

comment:10 by strk, 11 years ago

Resolution: fixed
Status: reopenedclosed

r11296 adds to usage string. Will not touch the postgis manual as this new switch seems partial to me (some of the signatures to be skipped from the dump have "public" schema name hard-coded, thus restoring a dump in which postgis is in a separate schema would fail to skip those functions, even with the -s switch).

Anyway, considering this complete.

comment:11 by frost242, 11 years ago

Thanks !

Note: See TracTickets for help on using tickets.