[HACKERS] Location of PG_CATALOG_VERSION

2017-05-21 Thread Vicky Vergara
Hello all


The postgreSQL version is needed internally in order to make the code work 
because for example the type of funcctx->max_calls  changed on 9.6



uint64_t result_count = 0;


...


#if PGSQL_VERSION > 95
funcctx->max_calls = result_count;
#else
funcctx->max_calls = (uint32_t)result_count;
#endif


PGSQL_VERSION is a result of a manipulation of the version found using 
FindPostgres.cmake (which uses pg_config --version)

>From this message: 
>https://www.postgresql.org/message-id/1585.1472410329%40sss.pgh.pa.us




I deduced that in the code I can use

PG_CATALOG_VERSION


I made the following experiment:


#define STRINGIFY(s) XSTRINGIFY(s)
#define XSTRINGIFY(s) #s

#pragma message ("The value PGSQL_VERSION: " STRINGIFY(PGSQL_VERSION))
#ifdef PG_CATALOG_VERSION
#pragma message ("The value PG_CATALOG_VERSION: " STRINGIFY(PG_CATALOG_VERSION))
#endif



I have this result:

note: #pragma message: The value PGSQL_VERSION: 93

So PG_CATALOG_VERSION is not defined, then I went to see the doxygen page to 
find out which file I have to include to get the definition.
But PG_CATALOG_VERSION its not there.

So, what am I missing?

I have no problem on doing more manipulations to get a value that I can use in 
the #if PGSQL_VERSION > 95 comparison (like 100 for postgreSQL 10betaX) but if 
PG_CATALOG_VERSION is considered the best thing to look at, where can I find it?

Vicky










Re: [HACKERS] Instead of DROP function use UPDATE pg_proc in an upgrade extension script

2017-04-04 Thread Vicky Vergara
Thanks,


> It is not safe due views - that are saved in post analyze form.


What is post analyze form? any link that you can give me to read about it?


Thanks


De: Pavel Stehule <pavel.steh...@gmail.com>
Enviado: lunes, 3 de abril de 2017 11:21 p. m.
Para: Vicky Vergara
Cc: pgsql-hackers@postgresql.org
Asunto: Re: [HACKERS] Instead of DROP function use UPDATE pg_proc in an upgrade 
extension script



2017-04-04 6:17 GMT+02:00 Vicky Vergara 
<vicky_verg...@hotmail.com<mailto:vicky_verg...@hotmail.com>>:


Hello,


When creating an extension upgrade sql script, because the function does not 
have the same parameter names and/or parameters type and/or the result types 
changes, there is the need to drop the function because otherwise the CREATE OR 
REPLACE of the new signature will fail.


So for example:

having the following function:


SELECT proallargtypes, proargmodes, proargnames FROM pg_proc WHERE
proallargtypes = '{25,20,20,16,23,23,20,20}'   AND proname = 
'pgr_edgedisjointpaths';
-[ RECORD 1 
]--+-
proallargtypes | {25,20,20,16,23,23,20,20}
proargmodes| {i,i,i,i,o,o,o,o}
proargnames| {"","","",directed,seq,path_seq,node,edge}


When adding extra OUT parameters, because the result types () change, the 
function needs a DROP:


-- Row type defined by OUT parameters is different

 ALTER EXTENSION pgrouting DROP FUNCTION 
pgr_edgedisjointpaths(text,bigint,bigint,boolean);

 DROP FUNCTION IF EXISTS pgr_edgedisjointpaths(text,bigint,bigint,boolean);


but doing that, objects that depend on the function. like a view, get dropped 
when using CASCADE in the ALTER extension, and functions that use the 
pgr_edgedisjointpaths internally don't get dropped.


So, I must say that I experimented: instead of doing the drop, I made:


UPDATE pg_proc SET

  proallargtypes = 
'{25,20,20,16,23,23,23,20,20,701,701}',

  proargmodes = '{i,i,i,i,o,o,o,o,o,o,o}',

   proargnames = 
'{"","","","directed","seq","path_id","path_seq","node","edge","cost","agg_cost"}'

 WHERE proallargtypes = '{25,20,20,16,23,23,20,20}'   AND proname = 
'pgr_edgedisjointpaths';


And CASCADE was not needed, and the view remained intact.


So, I want to know how "safe" can you consider the second method, and what kind 
of other objects do I need to test besides views.

It is not safe due views - that are saved in post analyze form.

Regards

Pavel

My plan, is to use the second method:

- when the current names of the OUT parameters don't change, and there is an 
additional OUT parameter

- when the current names of the IN parameters don't change, and there is an 
additional IN parameter with a default value


Thanks


Vicky Vergara









Re: [HACKERS] Instead of DROP function use UPDATE pg_proc in an upgrade extension script

2017-04-04 Thread Vicky Vergara
Thanks,

you answered so fast that I know I am stepping into dangerous grounds.

But I would like to know more about your experience.

Any links that you can give me to read about the code and/or issues regarding 
the ip4r experience?


Vicky




De: Andrew Gierth <and...@tao11.riddles.org.uk>
Enviado: lunes, 3 de abril de 2017 11:28 p. m.
Para: Vicky Vergara
Cc: pgsql-hackers@postgresql.org
Asunto: Re: [HACKERS] Instead of DROP function use UPDATE pg_proc in an upgrade 
extension script

>>>>> "Vicky" == Vicky Vergara <vicky_verg...@hotmail.com> writes:

 Vicky> UPDATE pg_proc SET [...]

 Vicky> So, I want to know how "safe" can you consider the second
 Vicky> method, and what kind of other objects do I need to test besides
 Vicky> views.

Speaking from personal experience (I did this in the upgrade script for
ip4r, in a much simpler case than yours, and broke it badly), it's not
at all safe.

--
Andrew (irc:RhodiumToad)


[HACKERS] Instead of DROP function use UPDATE pg_proc in an upgrade extension script

2017-04-03 Thread Vicky Vergara

Hello,


When creating an extension upgrade sql script, because the function does not 
have the same parameter names and/or parameters type and/or the result types 
changes, there is the need to drop the function because otherwise the CREATE OR 
REPLACE of the new signature will fail.


So for example:

having the following function:


SELECT proallargtypes, proargmodes, proargnames FROM pg_proc WHERE
proallargtypes = '{25,20,20,16,23,23,20,20}'   AND proname = 
'pgr_edgedisjointpaths';
-[ RECORD 1 
]--+-
proallargtypes | {25,20,20,16,23,23,20,20}
proargmodes| {i,i,i,i,o,o,o,o}
proargnames| {"","","",directed,seq,path_seq,node,edge}


When adding extra OUT parameters, because the result types () change, the 
function needs a DROP:


-- Row type defined by OUT parameters is different

 ALTER EXTENSION pgrouting DROP FUNCTION 
pgr_edgedisjointpaths(text,bigint,bigint,boolean);

 DROP FUNCTION IF EXISTS pgr_edgedisjointpaths(text,bigint,bigint,boolean);


but doing that, objects that depend on the function. like a view, get dropped 
when using CASCADE in the ALTER extension, and functions that use the 
pgr_edgedisjointpaths internally don't get dropped.


So, I must say that I experimented: instead of doing the drop, I made:


UPDATE pg_proc SET

  proallargtypes = 
'{25,20,20,16,23,23,23,20,20,701,701}',

  proargmodes = '{i,i,i,i,o,o,o,o,o,o,o}',

   proargnames = 
'{"","","","directed","seq","path_id","path_seq","node","edge","cost","agg_cost"}'

 WHERE proallargtypes = '{25,20,20,16,23,23,20,20}'   AND proname = 
'pgr_edgedisjointpaths';


And CASCADE was not needed, and the view remained intact.


So, I want to know how "safe" can you consider the second method, and what kind 
of other objects do I need to test besides views.

My plan, is to use the second method:

- when the current names of the OUT parameters don't change, and there is an 
additional OUT parameter

- when the current names of the IN parameters don't change, and there is an 
additional IN parameter with a default value


Thanks


Vicky Vergara








RV: [HACKERS] Compilation warning on 9.5

2016-11-07 Thread Vicky Vergara



Hello,

Posting an update to this issue (which by the way also shows up on 9.6)

Maybe this information is useful for extension developers (that have all the 
warnings flags on while developing using GNUC)


By wrapping the files as follows, any warnings generated by the postgres's 
header files are ignored.


#ifdef __GNUC__
#pragma GCC diagnostic ignored "-pedantic"
#endif

#include "postgres.h"

#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif


#ifdef __GNUC__
#pragma GCC diagnostic ignored "-Wsign-conversion"
#pragma GCC diagnostic ignored "-Wunused-parameter"
#endif

#include "executor/spi.h"

#ifdef __GNUC__
#pragma GCC diagnostic pop
#pragma GCC diagnostic pop
#endif



#ifdef __GNUC__
#pragma GCC diagnostic ignored "-Wunused-parameter"
#endif

#include "funcapi.h"

#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif


Vicky




De: Tom Lane <t...@sss.pgh.pa.us>
Enviado: viernes, 12 de febrero de 2016 01:45 p. m.
Para: Vicky Vergara
Cc: pgsql-hackers@postgresql.org
Asunto: Re: [HACKERS] Compilation warning on 9.5

Vicky Vergara <vicky_verg...@hotmail.com> writes:
> I wonder if -std=gnu99 is the correct standard to include postgres.h etc. in 
> 9.5
> because that standard (and all the flags I am using to generate pgrouting 
> code without warnings)
> catches the following catches warnings of type conversions on some postgresql 
> included files.

It's not -std=gnu99 that's causing those messages, it's -pedantic and
-Wconversion respectively.

> /usr/include/postgresql/9.5/server/c.h:298:9: warning: ISO C does not support 
> '__int128' type [-pedantic]
> /usr/include/postgresql/9.5/server/c.h:299:18: warning: ISO C does not 
> support '__int128' type [-pedantic]

We're not going to do anything about this one; certainly we won't stop
using int128 where it's available, and there isn't any other apparent way
to suppress the warning.  I doubt that -pedantic is a useful switch in
practice, and this seems to be a particularly unhelpful bit of pedantry.
Consider dropping that flag.

> /usr/include/postgresql/9.5/server/port/atomics/generic.h: In function 
> 'pg_atomic_add_fetch_u32_impl':
> /usr/include/postgresql/9.5/server/port/atomics/generic.h:238:2: warning: 
> conversion to 'uint32' from 'int32' may change the sign of the result 
> [-Wsign-conversion]

According to the gcc manual, inserting explicit casts would silence these;
is that worth doing?  I doubt anyone cares about making all our code
-Wconversion clean, but maybe making the headers clean would be worth
the trouble.

regards, tom lane


[HACKERS] Compilation warning on 9.5

2016-02-12 Thread Vicky Vergara
Hello:

I am a pgRouting developer.

In my CmakeLists.txt I use this flags:
set(CMAKE_C_FLAGS   "${CMAKE_C_FLAGS}   -std=gnu99 -fPIC -O2 -g -Wall 
-Wconversion -pedantic -fmax-errors=10  -Wmissing-prototypes -frounding-math")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x -fPIC -O2 -g -Wconversion 
-Wall -pedantic -fmax-errors=10 -Wextra  -frounding-math -Wno-deprecated")


for testing I use travis CI framework and test pgRouting using postgresql 9.1 
to 9.5
I follow the instructions for including:
#include "postgres.h"
#include "executor/spi.h"
#include "funcapi.h"
#include "catalog/pg_type.h"
#if PGSQL_VERSION > 92
#include "access/htup_details.h"
#endif
#include "fmgr.h"


I wonder if -std=gnu99 is the correct standard to include postgres.h etc. in 
9.5 
because that standard (and all the flags I am using to generate pgrouting code 
without warnings)
catches the following catches warnings of type conversions on some postgresql 
included files.
This doesn't happen on postgresql 9.1 to 9.4

for example:

In file included from /usr/include/postgresql/9.5/server/postgres.h:47:0,
 from 
/home/travis/build/pgRouting/pgrouting/src/dijkstra/src/many_to_many_dijkstra.c:31:
/usr/include/postgresql/9.5/server/c.h:298:9: warning: ISO C does not support 
‘__int128’ type [-pedantic]
/usr/include/postgresql/9.5/server/c.h:299:18: warning: ISO C does not support 
‘__int128’ type [-pedantic]




In file included from /usr/include/postgresql/9.5/server/port/atomics.h:119:0,
 from /usr/include/postgresql/9.5/server/storage/lwlock.h:19,
 from /usr/include/postgresql/9.5/server/storage/lock.h:18,
 from /usr/include/postgresql/9.5/server/access/genam.h:20,
 from /usr/include/postgresql/9.5/server/nodes/execnodes.h:17,
 from /usr/include/postgresql/9.5/server/executor/execdesc.h:18,
 from /usr/include/postgresql/9.5/server/utils/portal.h:50,
 from /usr/include/postgresql/9.5/server/executor/spi.h:18,
 from 
/home/travis/build/pgRouting/pgrouting/src/dijkstra/src/many_to_many_dijkstra.c:32:
/usr/include/postgresql/9.5/server/port/atomics/generic.h: In function 
‘pg_atomic_add_fetch_u32_impl’:
/usr/include/postgresql/9.5/server/port/atomics/generic.h:238:2: warning: 
conversion to ‘uint32’ from ‘int32’ may change the sign of the result 
[-Wsign-conversion]
/usr/include/postgresql/9.5/server/port/atomics/generic.h: In function 
‘pg_atomic_sub_fetch_u32_impl’:
/usr/include/postgresql/9.5/server/port/atomics/generic.h:247:2: warning: 
conversion to ‘uint32’ from ‘int32’ may change the sign of the result 
[-Wsign-conversion]
/usr/include/postgresql/9.5/server/port/atomics/generic.h: In function 
‘pg_atomic_add_fetch_u64_impl’:
/usr/include/postgresql/9.5/server/port/atomics/generic.h:372:2: warning: 
conversion to ‘long unsigned int’ from ‘int64’ may change the sign of the 
result [-Wsign-conversion]
/usr/include/postgresql/9.5/server/port/atomics/generic.h: In function 
‘pg_atomic_sub_fetch_u64_impl’:
/usr/include/postgresql/9.5/server/port/atomics/generic.h:381:2: warning: 
conversion to ‘long unsigned int’ from ‘int64’ may change the sign of the 
result [-Wsign-conversion]
of course, I can't go and modify  generic.h, c.h which are included when I 
include postgres.h or spi.h, or any of the files
that I include that are of the postgresql project.

I already posted in this mailing list 
http://www.postgresql.org/message-id/bay177-w104ec0b93c9fc5453b04cd8a...@phx.gbl
But talking with my co-developer Steve Woodbri, he suggested this mailing list.


you can see a full travis test here:
https://travis-ci.org/pgRouting/pgrouting/builds/108791787
(I have my own warnings which I am fixing and are very visible from 9.1 to 9.4)

Note1: when pgRouting gets released, all those flags:
-Wall -Wconversion -pedantic -fmax-errors=10  -Wmissing-prototypes
will be removed, and of course those warnings won't show up.




Thanks
Vicky Vergara