Re: [HACKERS] Review: create extension default_full_version

2013-02-22 Thread Dimitri Fontaine
Ibrar Ahmed ibrar.ah...@gmail.com writes:
 * In case we have hstore--1.3.sql file and want to install that file, but
 failed because of default_full_version.

That's now fixed, please see the Extension Templates patch at

  http://www.postgresql.org/message-id/m21uc8l4j8@2ndquadrant.fr

Where you will even find regression tests for that problem.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Review: create extension default_full_version

2012-12-11 Thread Ibrar Ahmed
Now it works in most of the cases, here is one more point about the patch.

* In case we have hstore--1.3.sql file and want to install that file, but
failed because of default_full_version.

No default_full_version specified
---
postgres=# CREATE EXTENSION hstore version '1.3';
CREATE EXTENSION

default_full_version = 1.2

postgres=# CREATE EXTENSION hstore version '1.3';
ERROR:  could not stat file
/usr/local/pgsql/share/extension/hstore--1.2.sql: No such file or
directory

If we don't want to address this issue at least we should give some
meaningful error message.



On Tue, Dec 4, 2012 at 4:46 PM, Ibrar Ahmed ibrar.ah...@gmail.com wrote:



 On Tue, Dec 4, 2012 at 7:54 PM, Dimitri Fontaine 
 dimi...@2ndquadrant.frwrote:

 Ibrar Ahmed ibrar.ah...@gmail.com writes:
  I am still getting the same error message.

 With the attached patch (v2), it works well:

 create extension hstore version '1.2' from 'unpackaged';
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--unpackaged--1.0.sql'
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.1--1.2.sql'
 CREATE EXTENSION

 You have to remember that the spelling FROM 'unpackaged version' really
 means that we have previously installed a loose version of the
 extension (just \i hstore.sql) and want to apply the upgrade path from
 there.

 We can't have FROM meaning the same thing as default_full_version.


 I know.



  With default_full_version = '1.1'
  
  postgres=# CREATE EXTENSION hstore version '1.3' from '1.1';
  DEBUG:  execute_extension_script:
  '/usr/local/pgsql/share/extension/hstore--1.1.sql'
  DEBUG:  execute_extension_script:
  '/usr/local/pgsql/share/extension/hstore--1.1--1.3.sql'
  *ERROR:  could not stat file
  /usr/local/pgsql/share/extension/hstore--1.1--1.3.sql: No such file or
  directory*

 That's nonetheless a bug and is fixed now:

 -   if (pcontrol-default_full_version)
 +   if (pcontrol-default_full_version  !unpackaged)


 Thanks, I will look at this again in detail.



 See attached.

 Regards,
 --
 Dimitri Fontaine
 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support




 --
 Ibrar Ahmed
 EnterpriseDB   http://www.enterprisedb.com





-- 
Ibrar Ahmed
EnterpriseDB   http://www.enterprisedb.com


Re: [HACKERS] Review: create extension default_full_version

2012-12-04 Thread Ibrar Ahmed
On Mon, Dec 3, 2012 at 11:05 PM, Dimitri Fontaine dimi...@2ndquadrant.frwrote:

 Hi,

 Thanks for your very good review!

 Ibrar Ahmed ibrar.ah...@gmail.com writes:
  I looked at the discussion for this patch and the patch itself. Here
  are my comments and observations about the patch.
  What I got from the discussion is that the patch tries to implement a
  mechanism to install extension from series of SQL scripts from
  base/full version e.g. if a user wants to create an extension 1.1,
  system should run v1.0 script followed by 1.0--1.1 script. In that
  case we need to know about the base or full version which in the above
  case is v1.0. So the patch added a defualt_full_version option in
  extension control file.

 Exactly, that was an idea from Robert and I implemented it quite
 quickly. Too quickly as we can see from your testing report.

  Here are my comments about the patch
 
  * Note: Patch does not apply cleanly on latest code base. You probably
  need to re-base the code

 Done. The thing is that meanwhile another solution to the main problem
 has been found: drop support for installing hstore 1.0. Attached patch
 fixes the problem by reinstalling hstore--1.0.sql and re-enabling this
 version, and removing the hstore--1.1.sql file now that it's enough to
 just have hstore--1.0--1.1.sql to install directly (and by default) the
 newer version.

 I think we will have to decide about taking only the mechanism or both
 the mechanism and the actual change for the hstore contrib.


  * This is a user visible change so documentation change is required here.

 Added coverage of the new parameter.

  * Also, You need to update the comment, because this code is now
  handling default_full_version as well.
 
/*
 * Determine the (unpackaged) version to update from, if any, and
 then
 * figure out what sequence of update scripts we need to apply.
 */
if ((d_old_version  d_old_version-arg) ||
 pcontrol-default_full_version)

 Done. I also fixed the bugs you reported here. Here's an edited version
 of the new (fixed) output:


 dim=# set client_min_messages to debug1;

 dim=# create extension hstore version '1.0';
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
 WARNING:  = is deprecated as an operator name
 CREATE EXTENSION

 dim=# create extension hstore version '1.1';
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
 WARNING:  = is deprecated as an operator name
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
 CREATE EXTENSION

 dim=# create extension hstore;
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
 WARNING:  = is deprecated as an operator name
 DETAIL:  This name may be disallowed altogether in future versions of
 PostgreSQL.
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
 CREATE EXTENSION

  postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';
  WARNING:  /usr/local/pgsql/share/extension/hstore--1.0--1.1.sql
  WARNING:  /usr/local/pgsql/share/extension/hstore--1.1--1.2.sql
  WARNING:  /usr/local/pgsql/share/extension/hstore--1.2--1.3.sql
  CREATE EXTENSION

 I liked your idea of extending the reporting about what files are used,
 but of course we can't keep that at the WARNING level, so I made that
 logging DEBUG1 in the attached patch.

  postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';

 Please try that case again, I believe it's fixed in the attached.


I am still getting the same error message.

Without default_full_version


postgres=# CREATE EXTENSION hstore version '1.3' from '1.1';
DEBUG:  execute_extension_script:
'/usr/local/pgsql/share/extension/hstore--1.1--1.2.sql'
DEBUG:  execute_extension_script:
'/usr/local/pgsql/share/extension/hstore--1.2--1.3.sql'
CREATE EXTENSION



With default_full_version = '1.1'

postgres=# CREATE EXTENSION hstore version '1.3' from '1.1';
DEBUG:  execute_extension_script:
'/usr/local/pgsql/share/extension/hstore--1.1.sql'
DEBUG:  execute_extension_script:
'/usr/local/pgsql/share/extension/hstore--1.1--1.3.sql'
*ERROR:  could not stat file
/usr/local/pgsql/share/extension/hstore--1.1--1.3.sql: No such file or
directory*


I think there is an issue in this area of the code

if (pcontrol-default_full_version)
{
execute_extension_script(extensionOid, control,
 -- 1.1.sql
 NULL, oldVersionName,
 requiredSchemas,
 schemaName, schemaOid);

ApplyExtensionUpdates(extensionOid, pcontrol,
   -- 1.1--1.3.sql (wrong)
  oldVersionName, updateVersions);


The first statement is executing 1.1.sql  because oldVersionName = 1.1.
Keep in mind that versionName = 1.2 and updateVersions list contain only
version name 1.3. So in case of  default_full_version you are ignoring *
versionName* which is *1.2* that 

Re: [HACKERS] Review: create extension default_full_version

2012-12-04 Thread Dimitri Fontaine
Ibrar Ahmed ibrar.ah...@gmail.com writes:
 I am still getting the same error message.

With the attached patch (v2), it works well:

create extension hstore version '1.2' from 'unpackaged';
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--unpackaged--1.0.sql'
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.1--1.2.sql'
CREATE EXTENSION

You have to remember that the spelling FROM 'unpackaged version' really
means that we have previously installed a loose version of the
extension (just \i hstore.sql) and want to apply the upgrade path from
there.

We can't have FROM meaning the same thing as default_full_version.

 With default_full_version = '1.1'
 
 postgres=# CREATE EXTENSION hstore version '1.3' from '1.1';
 DEBUG:  execute_extension_script:
 '/usr/local/pgsql/share/extension/hstore--1.1.sql'
 DEBUG:  execute_extension_script:
 '/usr/local/pgsql/share/extension/hstore--1.1--1.3.sql'
 *ERROR:  could not stat file
 /usr/local/pgsql/share/extension/hstore--1.1--1.3.sql: No such file or
 directory*

That's nonetheless a bug and is fixed now:

-   if (pcontrol-default_full_version)
+   if (pcontrol-default_full_version  !unpackaged)


See attached.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

*** a/contrib/hstore/Makefile
--- b/contrib/hstore/Makefile
***
*** 5,11  OBJS = hstore_io.o hstore_op.o hstore_gist.o hstore_gin.o hstore_compat.o \
  	crc32.o
  
  EXTENSION = hstore
! DATA = hstore--1.1.sql hstore--1.0--1.1.sql hstore--unpackaged--1.0.sql
  
  REGRESS = hstore
  
--- 5,11 
  	crc32.o
  
  EXTENSION = hstore
! DATA = hstore--1.0.sql hstore--1.0--1.1.sql hstore--unpackaged--1.0.sql
  
  REGRESS = hstore
  
*** /dev/null
--- b/contrib/hstore/hstore--1.0.sql
***
*** 0 
--- 1,530 
+ /* contrib/hstore/hstore--1.0.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use CREATE EXTENSION hstore to load this file. \quit
+ 
+ CREATE TYPE hstore;
+ 
+ CREATE FUNCTION hstore_in(cstring)
+ RETURNS hstore
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_out(hstore)
+ RETURNS cstring
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_recv(internal)
+ RETURNS hstore
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_send(hstore)
+ RETURNS bytea
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE TYPE hstore (
+ INTERNALLENGTH = -1,
+ INPUT = hstore_in,
+ OUTPUT = hstore_out,
+ RECEIVE = hstore_recv,
+ SEND = hstore_send,
+ STORAGE = extended
+ );
+ 
+ CREATE FUNCTION hstore_version_diag(hstore)
+ RETURNS integer
+ AS 'MODULE_PATHNAME','hstore_version_diag'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION fetchval(hstore,text)
+ RETURNS text
+ AS 'MODULE_PATHNAME','hstore_fetchval'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE OPERATOR - (
+ 	LEFTARG = hstore,
+ 	RIGHTARG = text,
+ 	PROCEDURE = fetchval
+ );
+ 
+ CREATE FUNCTION slice_array(hstore,text[])
+ RETURNS text[]
+ AS 'MODULE_PATHNAME','hstore_slice_to_array'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE OPERATOR - (
+ 	LEFTARG = hstore,
+ 	RIGHTARG = text[],
+ 	PROCEDURE = slice_array
+ );
+ 
+ CREATE FUNCTION slice(hstore,text[])
+ RETURNS hstore
+ AS 'MODULE_PATHNAME','hstore_slice_to_hstore'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION isexists(hstore,text)
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_exists'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION exist(hstore,text)
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_exists'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE OPERATOR ? (
+ 	LEFTARG = hstore,
+ 	RIGHTARG = text,
+ 	PROCEDURE = exist,
+ 	RESTRICT = contsel,
+ 	JOIN = contjoinsel
+ );
+ 
+ CREATE FUNCTION exists_any(hstore,text[])
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_exists_any'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE OPERATOR ?| (
+ 	LEFTARG = hstore,
+ 	RIGHTARG = text[],
+ 	PROCEDURE = exists_any,
+ 	RESTRICT = contsel,
+ 	JOIN = contjoinsel
+ );
+ 
+ CREATE FUNCTION exists_all(hstore,text[])
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_exists_all'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE OPERATOR ? (
+ 	LEFTARG = hstore,
+ 	RIGHTARG = text[],
+ 	PROCEDURE = exists_all,
+ 	RESTRICT = contsel,
+ 	JOIN = contjoinsel
+ );
+ 
+ CREATE FUNCTION isdefined(hstore,text)
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_defined'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION defined(hstore,text)
+ RETURNS bool
+ AS 'MODULE_PATHNAME','hstore_defined'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION delete(hstore,text)
+ RETURNS hstore
+ AS 'MODULE_PATHNAME','hstore_delete'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION 

Re: [HACKERS] Review: create extension default_full_version

2012-12-04 Thread Ibrar Ahmed
On Tue, Dec 4, 2012 at 7:54 PM, Dimitri Fontaine dimi...@2ndquadrant.frwrote:

 Ibrar Ahmed ibrar.ah...@gmail.com writes:
  I am still getting the same error message.

 With the attached patch (v2), it works well:

 create extension hstore version '1.2' from 'unpackaged';
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--unpackaged--1.0.sql'
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
 DEBUG:  execute_extension_script:
 '/Users/dim/pgsql/ddl/share/extension/hstore--1.1--1.2.sql'
 CREATE EXTENSION

 You have to remember that the spelling FROM 'unpackaged version' really
 means that we have previously installed a loose version of the
 extension (just \i hstore.sql) and want to apply the upgrade path from
 there.

 We can't have FROM meaning the same thing as default_full_version.


I know.



  With default_full_version = '1.1'
  
  postgres=# CREATE EXTENSION hstore version '1.3' from '1.1';
  DEBUG:  execute_extension_script:
  '/usr/local/pgsql/share/extension/hstore--1.1.sql'
  DEBUG:  execute_extension_script:
  '/usr/local/pgsql/share/extension/hstore--1.1--1.3.sql'
  *ERROR:  could not stat file
  /usr/local/pgsql/share/extension/hstore--1.1--1.3.sql: No such file or
  directory*

 That's nonetheless a bug and is fixed now:

 -   if (pcontrol-default_full_version)
 +   if (pcontrol-default_full_version  !unpackaged)


 Thanks, I will look at this again in detail.



 See attached.

 Regards,
 --
 Dimitri Fontaine
 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support




-- 
Ibrar Ahmed
EnterpriseDB   http://www.enterprisedb.com


Re: [HACKERS] Review: create extension default_full_version

2012-12-03 Thread Dimitri Fontaine
Hi,

Thanks for your very good review!

Ibrar Ahmed ibrar.ah...@gmail.com writes:
 I looked at the discussion for this patch and the patch itself. Here
 are my comments and observations about the patch.
 What I got from the discussion is that the patch tries to implement a
 mechanism to install extension from series of SQL scripts from
 base/full version e.g. if a user wants to create an extension 1.1,
 system should run v1.0 script followed by 1.0--1.1 script. In that
 case we need to know about the base or full version which in the above
 case is v1.0. So the patch added a defualt_full_version option in
 extension control file.

Exactly, that was an idea from Robert and I implemented it quite
quickly. Too quickly as we can see from your testing report.

 Here are my comments about the patch

 * Note: Patch does not apply cleanly on latest code base. You probably
 need to re-base the code

Done. The thing is that meanwhile another solution to the main problem
has been found: drop support for installing hstore 1.0. Attached patch
fixes the problem by reinstalling hstore--1.0.sql and re-enabling this
version, and removing the hstore--1.1.sql file now that it's enough to
just have hstore--1.0--1.1.sql to install directly (and by default) the
newer version.

I think we will have to decide about taking only the mechanism or both
the mechanism and the actual change for the hstore contrib.


 * This is a user visible change so documentation change is required here.

Added coverage of the new parameter.

 * Also, You need to update the comment, because this code is now
 handling default_full_version as well.

   /*
* Determine the (unpackaged) version to update from, if any, and then
* figure out what sequence of update scripts we need to apply.
*/
   if ((d_old_version  d_old_version-arg) || 
 pcontrol-default_full_version)

Done. I also fixed the bugs you reported here. Here's an edited version
of the new (fixed) output:


dim=# set client_min_messages to debug1;

dim=# create extension hstore version '1.0';
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
WARNING:  = is deprecated as an operator name
CREATE EXTENSION

dim=# create extension hstore version '1.1';
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
WARNING:  = is deprecated as an operator name
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
CREATE EXTENSION

dim=# create extension hstore;
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0.sql'
WARNING:  = is deprecated as an operator name
DETAIL:  This name may be disallowed altogether in future versions of 
PostgreSQL.
DEBUG:  execute_extension_script: 
'/Users/dim/pgsql/ddl/share/extension/hstore--1.0--1.1.sql'
CREATE EXTENSION

 postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';
 WARNING:  /usr/local/pgsql/share/extension/hstore--1.0--1.1.sql
 WARNING:  /usr/local/pgsql/share/extension/hstore--1.1--1.2.sql
 WARNING:  /usr/local/pgsql/share/extension/hstore--1.2--1.3.sql
 CREATE EXTENSION

I liked your idea of extending the reporting about what files are used,
but of course we can't keep that at the WARNING level, so I made that
logging DEBUG1 in the attached patch.

 postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';

Please try that case again, I believe it's fixed in the attached.

 - hstore regression is also failing.

That's because it doesn't cope anymore with the operator = warning, and
I left it this way because we have to decide about shipping hstore 1.0
once we have this patch in.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

*** a/contrib/hstore/Makefile
--- b/contrib/hstore/Makefile
***
*** 5,11  OBJS = hstore_io.o hstore_op.o hstore_gist.o hstore_gin.o hstore_compat.o \
  	crc32.o
  
  EXTENSION = hstore
! DATA = hstore--1.1.sql hstore--1.0--1.1.sql hstore--unpackaged--1.0.sql
  
  REGRESS = hstore
  
--- 5,11 
  	crc32.o
  
  EXTENSION = hstore
! DATA = hstore--1.0.sql hstore--1.0--1.1.sql hstore--unpackaged--1.0.sql
  
  REGRESS = hstore
  
*** /dev/null
--- b/contrib/hstore/hstore--1.0.sql
***
*** 0 
--- 1,530 
+ /* contrib/hstore/hstore--1.0.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use CREATE EXTENSION hstore to load this file. \quit
+ 
+ CREATE TYPE hstore;
+ 
+ CREATE FUNCTION hstore_in(cstring)
+ RETURNS hstore
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_out(hstore)
+ RETURNS cstring
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_recv(internal)
+ RETURNS hstore
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE FUNCTION hstore_send(hstore)
+ RETURNS bytea
+ AS 'MODULE_PATHNAME'
+ LANGUAGE C STRICT IMMUTABLE;
+ 
+ CREATE TYPE 

[HACKERS] Review: create extension default_full_version

2012-11-30 Thread Ibrar Ahmed
Hi,

I looked at the discussion for this patch and the patch itself. Here
are my comments and observations about the patch.
What I got from the discussion is that the patch tries to implement a
mechanism to install extension from series of SQL scripts from
base/full version e.g. if a user wants to create an extension 1.1,
system should run v1.0 script followed by 1.0--1.1 script. In that
case we need to know about the base or full version which in the above
case is v1.0. So the patch added a defualt_full_version option in
extension control file.

Here are my comments about the patch

* Note: Patch does not apply cleanly on latest code base. You probably
need to re-base the code

ibrar@ibrar-laptop:~/work/postgresql$ patch -p1
extension-default-full-version.v0.patch
patching file contrib/hstore/Makefile
Hunk #1 FAILED at 5.
1 out of 1 hunk FAILED -- saving rejects to file contrib/hstore/Makefile.rej
patching file contrib/hstore/hstore--1.1.sql
patching file contrib/hstore/hstore.control
patching file src/backend/commands/extension.c
Hunk #1 succeeded at 68 (offset 2 lines).
Hunk #2 succeeded at 508 (offset 2 lines).
Hunk #3 succeeded at 1295 (offset 2 lines).
Hunk #4 succeeded at 1316 (offset 2 lines).
Hunk #5 succeeded at 1473 (offset 3 lines).


* This is a user visible change so documentation change is required here.

* Also, You need to update the comment, because this code is now
handling default_full_version as well.

/*
 * Determine the (unpackaged) version to update from, if any, and then
 * figure out what sequence of update scripts we need to apply.
 */
if ((d_old_version  d_old_version-arg) || 
pcontrol-default_full_version)


* In case the default_full_version and VERSION in SQL are same then
we are getting a misleading error message i.e.

comment = 'data type for storing sets of (key, value) pairs'
default_version = '1.1'
default_full_version = '1.0'
module_pathname = '$libdir/hstore'
relocatable = true

postgres=# create EXTENSION hstore version '1.0';
ERROR:  FROM version must be different from installation target version 1.0

Error message is complaining about FROM clause which is not used in
the query.  I think EXTENSION creation should not be blocked in case
default_full_version and VERSION are same. But if we want to block
that; error message should be meaningful.

* I noticed another issue with the patch.

In case we do not specify the default_full_version in control file
then this is the sequence of sql scripts.

postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';
WARNING:  /usr/local/pgsql/share/extension/hstore--1.0--1.1.sql
WARNING:  /usr/local/pgsql/share/extension/hstore--1.1--1.2.sql
WARNING:  /usr/local/pgsql/share/extension/hstore--1.2--1.3.sql
CREATE EXTENSION


But in case default_full_version = 1.0, then we are getting  ERROR:
could not stat file... error message.

postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';
WARNING:  SCRIPT = /usr/local/pgsql/share/extension/hstore--1.0.sql
WARNING:  SCRIPT =
/usr/local/pgsql/share/extension/hstore--1.0--1.2.sql
   ---  Why not 1.0--1.1
ERROR:  could not stat file
/usr/local/pgsql/share/extension/hstore--1.0--1.2.sql: No such file
or directory

This is because of missing version number i.e. first we're executing
1.0 followed by 1.0--1.2 not 1.0--1.1 but I think following should be
the right sequence


postgres=# CREATE EXTENSION hstore version '1.3' from '1.0';
WARNING:  /usr/local/pgsql/share/extension/hstore--1.0.sql
WARNING:  /usr/local/pgsql/share/extension/hstore--1.0--1.1.sql
WARNING:  /usr/local/pgsql/share/extension/hstore--1.1--1.2.sql
WARNING:  /usr/local/pgsql/share/extension/hstore--1.2--1.3.sql
CREATE EXTENSION

PS: I modified the code to get this result.



- IMHO there should be an SQL option along with the default_full_version; like.

postgres=# create EXTENSION hstore VERSION '1.1' FULL_VERSION '1.0';


- hstore regression is also failing.


---

Ibrar Ahmed