Обсуждение: Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

Поиск
Список
Период
Сортировка

Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Heikki Linnakangas
Дата:
On 12.10.2011 10:58, Stefan Kaltenbrunner wrote:
> On 10/12/2011 09:53 AM, Martin Pitt wrote:
>> Hello all,
>>
>> In https://launchpad.net/bugs/835502 it was reported that the 9.1.1
>> contrib *.sql files contain the token "MODULE_PATHNAME", which is
>> unknown:
>>
>>    psql test<  /usr/share/postgresql/9.1/extension/intarray--1.0.sql
>>
>> This fails with a ton of errors about the file "MODULE_PATHNAME" not
>> existing.
>>
>> When I replace this with "$libdir/_int", it works:
>>
>>    sed 's!MODULE_PATHNAME!$libdir/_int!g' /usr/share/postgresql/9.1/extension/intarray--1.0.sql | psql test
>>
>> Is that something I'm doing wrong in the packaging, or should the
>> contrib Makefiles be fixed to do this substitution?
>>
>> It doesn't only affect intarray, but pretty much all *.sql files.
>
> uh - the reason is that contrib is now packaged as extensions and that
> you are supposed to use "CREATE EXTENSION intarray;" on the SQL level
> instead of manually loading sql-scripts through psql.

9.1 has been out for only a couple of months, and we've seen a lot of 
people trying to do that already. In hindsight, we probably should've 
chosen a different filename extension for those files, to make it clear 
that you can't just run them in psql. It's too late for that, but a 
comment at the top of the .sql files would be good:

--- a/contrib/intarray/intarray--1.0.sql
+++ b/contrib/intarray/intarray--1.0.sql
@@ -1,4 +1,8 @@
-/* contrib/intarray/intarray--1.0.sql */
+/*
+ * contrib/intarray/intarray--1.0.sql
+ *
+ * Script file to be run by CREATE EXTENSION.
+ */
 -- -- Create the user-defined type for the 1-D integer arrays (_int4)

The people trying to run these files with psql look inside the file when 
they get the error, so mentioning "CREATE EXTENSION" should give a hint 
on what to do.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Magnus Hagander
Дата:
On Wed, Oct 12, 2011 at 10:39, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> On 12.10.2011 10:58, Stefan Kaltenbrunner wrote:
>>
>> On 10/12/2011 09:53 AM, Martin Pitt wrote:
>>>
>>> Hello all,
>>>
>>> In https://launchpad.net/bugs/835502 it was reported that the 9.1.1
>>> contrib *.sql files contain the token "MODULE_PATHNAME", which is
>>> unknown:
>>>
>>>   psql test<  /usr/share/postgresql/9.1/extension/intarray--1.0.sql
>>>
>>> This fails with a ton of errors about the file "MODULE_PATHNAME" not
>>> existing.
>>>
>>> When I replace this with "$libdir/_int", it works:
>>>
>>>   sed 's!MODULE_PATHNAME!$libdir/_int!g'
>>> /usr/share/postgresql/9.1/extension/intarray--1.0.sql | psql test
>>>
>>> Is that something I'm doing wrong in the packaging, or should the
>>> contrib Makefiles be fixed to do this substitution?
>>>
>>> It doesn't only affect intarray, but pretty much all *.sql files.
>>
>> uh - the reason is that contrib is now packaged as extensions and that
>> you are supposed to use "CREATE EXTENSION intarray;" on the SQL level
>> instead of manually loading sql-scripts through psql.
>
> 9.1 has been out for only a couple of months, and we've seen a lot of people
> trying to do that already. In hindsight, we probably should've chosen a
> different filename extension for those files, to make it clear that you
> can't just run them in psql. It's too late for that, but a comment at the
> top of the .sql files would be good:
>
> --- a/contrib/intarray/intarray--1.0.sql
> +++ b/contrib/intarray/intarray--1.0.sql
> @@ -1,4 +1,8 @@
> -/* contrib/intarray/intarray--1.0.sql */
> +/*
> + * contrib/intarray/intarray--1.0.sql
> + *
> + * Script file to be run by CREATE EXTENSION.
> + */
>
>  --
>  -- Create the user-defined type for the 1-D integer arrays (_int4)
>
> The people trying to run these files with psql look inside the file when
> they get the error, so mentioning "CREATE EXTENSION" should give a hint on
> what to do.

Hmm. is there some way that we could make it do something that would
affect only psql? I guess not, because any kind of \-command would
actually break the CREATE EXTENSION running of the script, right?

But it would be useful to be able to inject something there that psql
would notice but the backend would ignore...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Andrew Dunstan
Дата:

On 10/12/2011 04:39 AM, Heikki Linnakangas wrote:
> On 12.10.2011 10:58, Stefan Kaltenbrunner wrote:
>> On 10/12/2011 09:53 AM, Martin Pitt wrote:
>>> Hello all,
>>>
>>> In https://launchpad.net/bugs/835502 it was reported that the 9.1.1
>>> contrib *.sql files contain the token "MODULE_PATHNAME", which is
>>> unknown:
>>>
>>>    psql test<  /usr/share/postgresql/9.1/extension/intarray--1.0.sql
>>>
>>> This fails with a ton of errors about the file "MODULE_PATHNAME" not
>>> existing.
>>>
>>> When I replace this with "$libdir/_int", it works:
>>>
>>>    sed 's!MODULE_PATHNAME!$libdir/_int!g' 
>>> /usr/share/postgresql/9.1/extension/intarray--1.0.sql | psql test
>>>
>>> Is that something I'm doing wrong in the packaging, or should the
>>> contrib Makefiles be fixed to do this substitution?
>>>
>>> It doesn't only affect intarray, but pretty much all *.sql files.
>>
>> uh - the reason is that contrib is now packaged as extensions and that
>> you are supposed to use "CREATE EXTENSION intarray;" on the SQL level
>> instead of manually loading sql-scripts through psql.
>
> 9.1 has been out for only a couple of months, and we've seen a lot of 
> people trying to do that already. In hindsight, we probably should've 
> chosen a different filename extension for those files, to make it 
> clear that you can't just run them in psql. It's too late for that, 
> but a comment at the top of the .sql files would be good:
>
>

I've made this mistake myself in an unthinking moment. I suggest that we 
deprecate calling them something.sql and add code ASAP providing for 
some other suffix ( .xtn ?) with legacy support for falling back to 
.sql. I'd almost be inclined to backpatch it while there are so few 
third party extensions out there.

cheers

andrew




Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Robert Haas
Дата:
On Wed, Oct 12, 2011 at 4:42 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> 9.1 has been out for only a couple of months, and we've seen a lot of people
>> trying to do that already. In hindsight, we probably should've chosen a
>> different filename extension for those files, to make it clear that you
>> can't just run them in psql. It's too late for that, but a comment at the
>> top of the .sql files would be good:
>>
>> --- a/contrib/intarray/intarray--1.0.sql
>> +++ b/contrib/intarray/intarray--1.0.sql
>> @@ -1,4 +1,8 @@
>> -/* contrib/intarray/intarray--1.0.sql */
>> +/*
>> + * contrib/intarray/intarray--1.0.sql
>> + *
>> + * Script file to be run by CREATE EXTENSION.
>> + */
>>
>>  --
>>  -- Create the user-defined type for the 1-D integer arrays (_int4)
>>
>> The people trying to run these files with psql look inside the file when
>> they get the error, so mentioning "CREATE EXTENSION" should give a hint on
>> what to do.
>
> Hmm. is there some way that we could make it do something that would
> affect only psql? I guess not, because any kind of \-command would
> actually break the CREATE EXTENSION running of the script, right?
>
> But it would be useful to be able to inject something there that psql
> would notice but the backend would ignore...

We could do that, but I think Heikki's idea of adding a comment would
help a lot.  When people try to run the file through psql and it fails
due to the MODULE_PATHNAME stuff, the next thing they're probably
going to do is look at the file contents.  If they see something there
telling them to use CREATE EXTENSION, that will help.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> We could do that, but I think Heikki's idea of adding a comment would
> help a lot.

+1.  Simple, easy, should help significantly.

Also, I disagree with the position that the files "aren't SQL files".
Sure they are.   You'd want them treated as SQL by your editor, for
example.  So changing the extension is just wrong.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Andrew Dunstan
Дата:

On 10/12/2011 10:12 AM, Tom Lane wrote:
> Robert Haas<robertmhaas@gmail.com>  writes:
>> We could do that, but I think Heikki's idea of adding a comment would
>> help a lot.
> +1.  Simple, easy, should help significantly.
>
> Also, I disagree with the position that the files "aren't SQL files".
> Sure they are.   You'd want them treated as SQL by your editor, for
> example.  So changing the extension is just wrong.
>

*shrug* ok. Another thought I had was to have the file raise an error 
and have that filtered out by the extension mechanism. But I'm not sure 
if it's worth the trouble.

cheers

andrew


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Magnus Hagander
Дата:
On Wed, Oct 12, 2011 at 16:31, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 10/12/2011 10:12 AM, Tom Lane wrote:
>>
>> Robert Haas<robertmhaas@gmail.com>  writes:
>>>
>>> We could do that, but I think Heikki's idea of adding a comment would
>>> help a lot.
>>
>> +1.  Simple, easy, should help significantly.
>>
>> Also, I disagree with the position that the files "aren't SQL files".
>> Sure they are.   You'd want them treated as SQL by your editor, for
>> example.  So changing the extension is just wrong.
>>
>
> *shrug* ok. Another thought I had was to have the file raise an error and
> have that filtered out by the extension mechanism. But I'm not sure if it's
> worth the trouble.

How about adding something like
-- \\psql_hates_this
-- rest of comment

and then at least have new versions of psql find that and stop
processing the file with a more useful error at that point? Or maybe
that's overengineering..

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Andrew Dunstan <andrew@dunslane.net> writes:
> *shrug* ok. Another thought I had was to have the file raise an error 
> and have that filtered out by the extension mechanism. But I'm not sure 
> if it's worth the trouble.

Hmm ...

\echo You should use CREATE EXTENSION foo to load this file!

and teach CREATE EXTENSION to drop any line beginning with \echo?
The latter part seems easy enough, but I'm not quite sure about the
wording or placement of the \echo command.  Putting it at the top
feels natural but the message might scroll offscreen due to errors...
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Heikki Linnakangas
Дата:
On 12.10.2011 17:33, Magnus Hagander wrote:
> How about adding something like
> -- \\psql_hates_this
> -- rest of comment
>
> and then at least have new versions of psql find that and stop
> processing the file with a more useful error at that point? Or maybe
> that's overengineering..

Overengineering IMHO. Besides, if a psql poison comment like that 
exists, then we'd have to be careful not to emit one elsewhere. Think 
pg_dump, if someone puts that comment in a function body...

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Aidan Van Dyk
Дата:
On Wed, Oct 12, 2011 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> *shrug* ok. Another thought I had was to have the file raise an error
>> and have that filtered out by the extension mechanism. But I'm not sure
>> if it's worth the trouble.
>
> Hmm ...
>
> \echo You should use CREATE EXTENSION foo to load this file!
>
> and teach CREATE EXTENSION to drop any line beginning with \echo?
> The latter part seems easy enough, but I'm not quite sure about the
> wording or placement of the \echo command.  Putting it at the top
> feels natural but the message might scroll offscreen due to errors...

Decorate them with a marker like:  \extension <name> <version>

And make the CREATE EXTENSION skip (or verify) it?

It will make psql stop on the \extension command.

a.


--
Aidan Van Dyk                                             Create like a god,
aidan@highrise.ca                                       command like a king,
http://www.highrise.ca/                                   work like a slave.


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Aidan Van Dyk <aidan@highrise.ca> writes:
> On Wed, Oct 12, 2011 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmm ...
>> \echo You should use CREATE EXTENSION foo to load this file!

> Decorate them with a marker like:
>    \extension <name> <version>
> And make the CREATE EXTENSION skip (or verify) it?
> It will make psql stop on the \extension command.

No, the point is not to stop or fail, it is to print out an unmistakable
user instruction.  Otherwise we'll still be getting "cube.sql failed to
load for me" bug reports.  So I think \echo is entirely sufficient,
and we should not rely on psql features that aren't there yet.  Ideally
this should do what we want even in older psql releases.  \echo has been
there at least since 7.0.

It strikes me that we could get rid of the error message clutter
I worried about before if we coded like this:
/* contrib/foo--1.0.sql */
\echo Use "CREATE EXTENSION foo" to load this file. \quit
... SQL commands here ...

The forced \quit is a bit unfriendly maybe but it will get the job done.
And again, this isn't making any assumptions about which psql version
you're using.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> On 12.10.2011 17:33, Magnus Hagander wrote:
>> How about adding something like
>> -- \\psql_hates_this
>> -- rest of comment
>> 
>> and then at least have new versions of psql find that and stop
>> processing the file with a more useful error at that point? Or maybe
>> that's overengineering..

> Overengineering IMHO. Besides, if a psql poison comment like that 
> exists, then we'd have to be careful not to emit one elsewhere. Think 
> pg_dump, if someone puts that comment in a function body...

Well, it can't be a comment, but what about a real psql command?
See my suggestion of using \echo.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Andrew Dunstan
Дата:

On 10/12/2011 11:15 AM, Tom Lane wrote:
>
>     \echo Use "CREATE EXTENSION foo" to load this file. \quit
>
>     

+1 for this.

cheers

andrew


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Robert Haas
Дата:
On Wed, Oct 12, 2011 at 11:24 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
>>> Overengineering IMHO. Besides, if a psql poison comment like that
>>> exists, then we'd have to be careful not to emit one elsewhere. Think
>>> pg_dump, if someone puts that comment in a function body...
>>
>> Well, it can't be a comment, but what about a real psql command?
>> See my suggestion of using \echo.
>
> Frankly I think a comment is sufficient. We can make it more complicated
> later if people are still confused.

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Heikki Linnakangas
Дата:
On 12.10.2011 18:20, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> On 12.10.2011 17:33, Magnus Hagander wrote:
>>> How about adding something like
>>> -- \\psql_hates_this
>>> -- rest of comment
>>>
>>> and then at least have new versions of psql find that and stop
>>> processing the file with a more useful error at that point? Or maybe
>>> that's overengineering..
>
>> Overengineering IMHO. Besides, if a psql poison comment like that
>> exists, then we'd have to be careful not to emit one elsewhere. Think
>> pg_dump, if someone puts that comment in a function body...
>
> Well, it can't be a comment, but what about a real psql command?
> See my suggestion of using \echo.

Frankly I think a comment is sufficient. We can make it more complicated 
later if people are still confused.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> On 12.10.2011 18:20, Tom Lane wrote:
>> Well, it can't be a comment, but what about a real psql command?
>> See my suggestion of using \echo.

> Frankly I think a comment is sufficient. We can make it more complicated 
> later if people are still confused.

The thing is that this will be the third time we've gone back to try to
make it more apparent that you should use CREATE EXTENSION, and I no
longer believe that mere documentation is really going to get the job
done.  Putting in a comment will only stop the bug reports from people
who bother to examine the script contents before filing a report, but
the kind of folks who don't read the release notes probably won't do
that either.  In fact, if we just put in a comment, I confidently
predict we'll be coming back to revisit this issue again in future.

The only thing the \echo approach will cost us is a few more lines of C
code in execute_extension_script(), and I think it's more than worth
that given the evident scope of the problem.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Magnus Hagander
Дата:
On Wed, Oct 12, 2011 at 17:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> On 12.10.2011 18:20, Tom Lane wrote:
>>> Well, it can't be a comment, but what about a real psql command?
>>> See my suggestion of using \echo.
>
>> Frankly I think a comment is sufficient. We can make it more complicated
>> later if people are still confused.
>
> The thing is that this will be the third time we've gone back to try to
> make it more apparent that you should use CREATE EXTENSION, and I no
> longer believe that mere documentation is really going to get the job
> done.  Putting in a comment will only stop the bug reports from people
> who bother to examine the script contents before filing a report, but
> the kind of folks who don't read the release notes probably won't do
> that either.  In fact, if we just put in a comment, I confidently
> predict we'll be coming back to revisit this issue again in future.

That's exactly my concern - I strongly doubt those not bothering to
read that even for a major release, aren't going to review the source
of the SQL scrpit either.


> The only thing the \echo approach will cost us is a few more lines of C
> code in execute_extension_script(), and I think it's more than worth
> that given the evident scope of the problem.

+1.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Oct 12, 2011 at 17:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The only thing the \echo approach will cost us is a few more lines of C
>> code in execute_extension_script(), and I think it's more than worth
>> that given the evident scope of the problem.

> +1.

PFA, a sample patch for this --- I've only bothered to change one script
file here, but will of course do the rest if there are no further
objections.  The technique actually works even better than I expected,
because of the seemingly nowhere documented fact that \quit in a script
file doesn't terminate psql, only processing of the script.  So what
I get is

regression=# \i ~/postgres/share/extension/cube--1.0.sql
Use "CREATE EXTENSION cube" to load this file.
regression=#

which is about as good as one could hope for.

(Looks like a patch to the psql docs is upcoming, too.)

            regards, tom lane


diff --git a/contrib/cube/cube--1.0.sql b/contrib/cube/cube--1.0.sql
index ee9febe005315bf13e93a9ef216a7411fc13a445..0d259c0969d9df4a00f160bf1fc00407273e2b1d 100644
*** a/contrib/cube/cube--1.0.sql
--- b/contrib/cube/cube--1.0.sql
***************
*** 1,5 ****
--- 1,7 ----
  /* contrib/cube/cube--1.0.sql */

+ \echo Use "CREATE EXTENSION cube" to load this file. \quit
+
  -- Create the user-defined type for N-dimensional boxes

  CREATE FUNCTION cube_in(cstring)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 3af15dd38bb7044ec6f733787cad6897b204ccd3..ba1e2c45cd97c89265912d338a98f52bf48ea4b0 100644
*** a/src/backend/commands/extension.c
--- b/src/backend/commands/extension.c
***************
*** 33,38 ****
--- 33,39 ----
  #include "catalog/indexing.h"
  #include "catalog/namespace.h"
  #include "catalog/objectaccess.h"
+ #include "catalog/pg_collation.h"
  #include "catalog/pg_depend.h"
  #include "catalog/pg_extension.h"
  #include "catalog/pg_namespace.h"
*************** execute_extension_script(Oid extensionOi
*** 855,880 ****
      CurrentExtensionObject = extensionOid;
      PG_TRY();
      {
!         char       *sql = read_extension_script_file(control, filename);

          /*
           * If it's not relocatable, substitute the target schema name for
           * occcurrences of @extschema@.
           *
!          * For a relocatable extension, we just run the script as-is. There
!          * cannot be any need for @extschema@, else it wouldn't be
!          * relocatable.
           */
          if (!control->relocatable)
          {
              const char *qSchemaName = quote_identifier(schemaName);

!             sql = text_to_cstring(
!                                   DatumGetTextPP(
!                                             DirectFunctionCall3(replace_text,
!                                                     CStringGetTextDatum(sql),
!                                           CStringGetTextDatum("@extschema@"),
!                                          CStringGetTextDatum(qSchemaName))));
          }

          /*
--- 856,894 ----
      CurrentExtensionObject = extensionOid;
      PG_TRY();
      {
!         char       *c_sql = read_extension_script_file(control, filename);
!         Datum        t_sql;
!
!         /* We use various functions that want to operate on text datums */
!         t_sql = CStringGetTextDatum(c_sql);
!
!         /*
!          * Reduce any lines beginning with "\echo" to empty.  This allows
!          * scripts to contain messages telling people not to run them via
!          * psql, which has been found to be necessary due to old habits.
!          */
!         t_sql = DirectFunctionCall4Coll(textregexreplace,
!                                         C_COLLATION_OID,
!                                         t_sql,
!                                         CStringGetTextDatum("^\\\\echo.*$"),
!                                         CStringGetTextDatum(""),
!                                         CStringGetTextDatum("ng"));

          /*
           * If it's not relocatable, substitute the target schema name for
           * occcurrences of @extschema@.
           *
!          * For a relocatable extension, we needn't do this.  There cannot be
!          * any need for @extschema@, else it wouldn't be relocatable.
           */
          if (!control->relocatable)
          {
              const char *qSchemaName = quote_identifier(schemaName);

!             t_sql = DirectFunctionCall3(replace_text,
!                                         t_sql,
!                                         CStringGetTextDatum("@extschema@"),
!                                         CStringGetTextDatum(qSchemaName));
          }

          /*
*************** execute_extension_script(Oid extensionOi
*** 883,897 ****
           */
          if (control->module_pathname)
          {
!             sql = text_to_cstring(
!                                   DatumGetTextPP(
!                                             DirectFunctionCall3(replace_text,
!                                                     CStringGetTextDatum(sql),
!                                       CStringGetTextDatum("MODULE_PATHNAME"),
!                             CStringGetTextDatum(control->module_pathname))));
          }

!         execute_sql_string(sql, filename);
      }
      PG_CATCH();
      {
--- 897,912 ----
           */
          if (control->module_pathname)
          {
!             t_sql = DirectFunctionCall3(replace_text,
!                                         t_sql,
!                                         CStringGetTextDatum("MODULE_PATHNAME"),
!                             CStringGetTextDatum(control->module_pathname));
          }

!         /* And now back to C string */
!         c_sql = text_to_cstring(DatumGetTextPP(t_sql));
!
!         execute_sql_string(c_sql, filename);
      }
      PG_CATCH();
      {

Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Magnus Hagander
Дата:
On Wed, Oct 12, 2011 at 19:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Oct 12, 2011 at 17:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The only thing the \echo approach will cost us is a few more lines of C
>>> code in execute_extension_script(), and I think it's more than worth
>>> that given the evident scope of the problem.
>
>> +1.
>
> PFA, a sample patch for this --- I've only bothered to change one script
> file here, but will of course do the rest if there are no further
> objections.  The technique actually works even better than I expected,
> because of the seemingly nowhere documented fact that \quit in a script
> file doesn't terminate psql, only processing of the script.  So what
> I get is
>
> regression=# \i ~/postgres/share/extension/cube--1.0.sql
> Use "CREATE EXTENSION cube" to load this file.
> regression=#
>
> which is about as good as one could hope for.

Looks great to me.

I guess the failure scenario is if someone has an extension from 9.1.2
and tries to load it into 9.1.1 or earlier, in which case they will
get a syntax error or somehing when trying to run the CREATE EXTENSION
command, right? I doubt that's something worth dealing with - it's a
lot less likely to happen.

We might want to document this for other third-party extension
developers to use as a trick as well?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Oct 12, 2011 at 19:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> PFA, a sample patch for this --- I've only bothered to change one script
>> file here, but will of course do the rest if there are no further
>> objections.

> I guess the failure scenario is if someone has an extension from 9.1.2
> and tries to load it into 9.1.1 or earlier, in which case they will
> get a syntax error or somehing when trying to run the CREATE EXTENSION
> command, right? I doubt that's something worth dealing with - it's a
> lot less likely to happen.

Hmm, yeah, you're right.  But it doesn't seem like a big problem to me,
certainly not as big as the problem we're trying to solve.

> We might want to document this for other third-party extension
> developers to use as a trick as well?

Yes, I will add something to the docs.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Andrew Dunstan
Дата:

On 10/12/2011 02:21 PM, Magnus Hagander wrote:
> On Wed, Oct 12, 2011 at 19:36, Tom Lane<tgl@sss.pgh.pa.us>  wrote:
>>
>> regression=# \i ~/postgres/share/extension/cube--1.0.sql
>> Use "CREATE EXTENSION cube" to load this file.
>> regression=#
>>
>> which is about as good as one could hope for.
> Looks great to me.

Yes, me too.

> I guess the failure scenario is if someone has an extension from 9.1.2
> and tries to load it into 9.1.1 or earlier, in which case they will
> get a syntax error or somehing when trying to run the CREATE EXTENSION
> command, right? I doubt that's something worth dealing with - it's a
> lot less likely to happen.
>

As long as we are going to apply it for 9.1 and not wait for 9.2 I don't 
think there will be much problem. I think this is one of the rare cases 
where we should apply a change to the stable release.

cheers

andrew


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Tom Lane
Дата:
Andrew Dunstan <andrew@dunslane.net> writes:
> On 10/12/2011 02:21 PM, Magnus Hagander wrote:
>> I guess the failure scenario is if someone has an extension from 9.1.2
>> and tries to load it into 9.1.1 or earlier, in which case they will
>> get a syntax error or somehing when trying to run the CREATE EXTENSION
>> command, right? I doubt that's something worth dealing with - it's a
>> lot less likely to happen.

> As long as we are going to apply it for 9.1 and not wait for 9.2 I don't 
> think there will be much problem. I think this is one of the rare cases 
> where we should apply a change to the stable release.

By 9.2 doing this would be rather pointless, likely.  Also, the earlier
we get it in the easier it will be for third-party devs to rely on it
working.
        regards, tom lane


Re: [BUGS] *.sql contrib files contain unresolvable MODULE_PATHNAME

От
Dimitri Fontaine
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> regression=# \i ~/postgres/share/extension/cube--1.0.sql
> Use "CREATE EXTENSION cube" to load this file.
> regression=# 

Great work, thank you!
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support