Re: [HACKERS] Error trying to compile a simple C trigger

[Asif Naeem]

It seems that compiler is complain about Relation structure, can you
please try adding the following in trigtest.c i.e.

#include utils/rel.h

Best Regards,
Asif Naeem

On Tue, Mar 20, 2012 at 3:53 PM, Marco Nenciarini 
marco.nenciar...@2ndquadrant.it wrote:

 I was trying to compile orafce on the current master and it yield
 an error at line

  tupdesc = trigdata-tg_relation-rd_att;

 alert.c: In function ‘dbms_alert_defered_signal’:
 alert.c:839:33: error: dereferencing pointer to incomplete type
 make: *** [alert.o] Error 1

 I've also tried the example at

 http://www.postgresql.org/docs/devel/static/trigger-example.html

 and the result is exactly the same.

 trigtest.c: In function ‘trigf’:
 trigtest.c:44:36: error: dereferencing pointer to incomplete type
 make: *** [trigtest.o] Error 1

 Regards,
 Marco

 --
 Marco Nenciarini - 2ndQuadrant Italy
 PostgreSQL Training, Services and Support
 marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it



 --
 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] pg_ctl idempotent option

[Asif Naeem]

I am working on reviewing the patch. Patch apply without warning/error on
master branch. My findings are as following i.e.

1. Behavior change in pg_ctl return value i.e.
*
*
* Server already running*

a. Without Patch

inst asif$ ./bin/pg_ctl -D data_test/ -l data_test.log start

pg_ctl: another server might be running; trying to start server anyway

server starting

inst asif$ echo $?

0


b. With Patch


inst_pg_ctl_idempotent_option asif$ ./bin/pg_ctl -D data_test/ -l
 data_test.log start
 pg_ctl: another server might be running
 inst_pg_ctl_idempotent_option asif$ echo $?
 1


2. -w option seems not working for start as per documentation, it should
return 0.

*Starting already running server with -I -w option*


inst_pg_ctl_idempotent_option asif$ ./bin/pg_ctl -D data_test/ -l
 data_test.log -I -w start

pg_ctl: another server might be running; trying to start server anyway

waiting for server to start

pg_ctl: this data directory appears to be running a pre-existing postmaster

 stopped waiting

pg_ctl: could not start server

Examine the log output.

inst_pg_ctl_idempotent_option asif$ echo $?

1


3. I believe postmaster (DAEMON=$prefix/bin/postmaster) is not going to
accept -I option as mentioned in the patch i.e.

contrib/start-scripts/linux

  su - $PGUSER -c $DAEMON -I -D '$PGDATA'  $PGLOG 2


Rest of the patch changes looks good to me. Thanks.

Best Regards,
Asif Naeem

On Thu, Jan 24, 2013 at 6:06 PM, Bruce Momjian br...@momjian.us wrote:

 On Thu, Jan 24, 2013 at 09:05:59AM +0530, Ashutosh Bapat wrote:
  I agree, answering the question, whether the particular attempt of
  starting a server succeeded or not, will need the current behaviour.
  Now, question is which of these behaviours should be default?

 That would work.  pg_upgrade knows the cluster version at that point and
 can use the proper flag.

 --
   Bruce Momjian  br...@momjian.ushttp://momjian.us
   EnterpriseDB http://enterprisedb.com

   + It's impossible for everything to be true. +


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

[HACKERS] plpython crash (PG 92)

[Asif Naeem]

Hi,

PFA test case. It used simple select statement to retrieve data via
plpython. It crashes latest pg 9.2 with the following stack trace i.e.

#0  0x0073021f in pfree ()
 #1  0x7fa74b632f7a in PLy_result_dealloc () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #2  0x7fa74b2c710b in iter_iternext (iterator=0x1ad7150) at
 Objects/iterobject.c:74
 #3  0x7fa74b2934db in PyIter_Next (iter=0x1b3c5f0) at
 Objects/abstract.c:3107
 #4  0x7fa74b630245 in PLy_exec_function () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #5  0x7fa74b630c57 in plpython_call_handler () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #6  0x00583907 in ExecMakeFunctionResult ()
 #7  0x0057f146 in ExecProject ()
 #8  0x00596740 in ExecResult ()
 #9  0x0057e708 in ExecProcNode ()
 #10 0x0057d582 in standard_ExecutorRun ()
 #11 0x0064f477 in PortalRunSelect ()
 #12 0x00650778 in PortalRun ()
 #13 0x0064ceca in exec_simple_query ()
 #14 0x0064ddc7 in PostgresMain ()
 #15 0x0060bdd9 in ServerLoop ()
 #16 0x0060e9d7 in PostmasterMain ()
 #17 0x005ad360 in main ()


Apparently it is being crashed because of invalid related pointer value of
pfree() *header-context-methods-free_p. It is reproducible with latest
version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks.

Best Regards,
Muhammad Asif Naeem


plpython_crash.sql
Description: Binary data

-- 
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] plpython crash (PG 92)

[Asif Naeem]

FYI,
I have observed this crash on Linux64. Thanks.

Best Regards,
Muhammad Asif Naeem

On Thu, Apr 26, 2012 at 5:32 PM, Asif Naeem asif.na...@enterprisedb.comwrote:

 Hi,

 PFA test case. It used simple select statement to retrieve data via
 plpython. It crashes latest pg 9.2 with the following stack trace i.e.

 #0  0x0073021f in pfree ()
 #1  0x7fa74b632f7a in PLy_result_dealloc () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #2  0x7fa74b2c710b in iter_iternext (iterator=0x1ad7150) at
 Objects/iterobject.c:74
 #3  0x7fa74b2934db in PyIter_Next (iter=0x1b3c5f0) at
 Objects/abstract.c:3107
 #4  0x7fa74b630245 in PLy_exec_function () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #5  0x7fa74b630c57 in plpython_call_handler () from
 /home/masif/work/postgresql/postgresql/inst/lib/plpython2.so
 #6  0x00583907 in ExecMakeFunctionResult ()
 #7  0x0057f146 in ExecProject ()
 #8  0x00596740 in ExecResult ()
 #9  0x0057e708 in ExecProcNode ()
 #10 0x0057d582 in standard_ExecutorRun ()
 #11 0x0064f477 in PortalRunSelect ()
 #12 0x00650778 in PortalRun ()
 #13 0x0064ceca in exec_simple_query ()
 #14 0x0064ddc7 in PostgresMain ()
 #15 0x0060bdd9 in ServerLoop ()
 #16 0x0060e9d7 in PostmasterMain ()
 #17 0x005ad360 in main ()


 Apparently it is being crashed because of invalid related pointer value of
 pfree() *header-context-methods-free_p. It is reproducible with latest
 version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks.

 Best Regards,
 Muhammad Asif Naeem

[HACKERS] pgdump tar bug (PG 9.2)

[Asif Naeem]

Hi,

With the following test case pgdump creates a corrupt tar file i.e.

CREATE DATABASE dump_test;
 \c dump_test
 CREATE TABLE test_table1 (int1 int);
 INSERT INTO test_table1 (SELECT * FROM generate_series(1, 1000));
 \! pg_dump -F t -f dump_test.tar dump_test


Debugging shows that pg_dump tries to fopen tar file with w option that
corrupts already opened archive file i.e.

_CloseArchive() - RestoreArchive() - SetOutput() - fopen(filename,
 PG_BINARY_W);


man fopen

 ...
 ...
 w
 Truncate file to zero length or create text file for writing. The
 stream is positioned at the beginning of the file.


This issue is caused by addition of the following code in function
_CloseArchive() i.e.

 memcpy(ropt, AH-ropt, sizeof(RestoreOptions));


It was intruduced by recent patch is as following i.e.

 commit 4317e0246c645f60c39e6572644cff1cb03b4c65
 Author: Tom Lane t...@sss.pgh.pa.us
 Date:   Tue May 29 23:22:14 2012 -0400
 Rewrite --section option to decouple it from --schema-only/--data-only.


PFA patch. Thanks.

Best Regards,
Muhammad Asif Naeem


pg_backup_tar.c.patch.pg
Description: Binary data

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

[HACKERS] plpython issue with Win64 (PG 9.2)

[Asif Naeem]

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

CREATE PROCEDURAL LANGUAGE 'plpython3u';
 CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
   RETURNS integer
 AS $$
   if a  b:
 return a
   return b
 $$ LANGUAGE plpython3u;
 SELECT pymax(1, 2);


Server exit with the following exception i.e.

 Unhandled exception at 0x777a3483 in postgres.exe: 0xC0FD: Stack
overflow.

  plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth)  Line 174 C
  plpython3.dll!PLy_elog(int elevel, const char * fmt, ...)  Line 67
  C
  plpython3.dll!PLyUnicode_AsString(_object * unicode)  Line 96 C
  plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth)  Line 176 + 0x8 bytes C
  plpython3.dll!PLy_elog(int elevel, const char * fmt, ...)  Line 67
  C
  plpython3.dll!PLyUnicode_AsString(_object * unicode)  Line 96 C
  ...
  ...
  plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth)  Line 176 + 0x8 bytes C
  plpython3.dll!PLy_elog(int elevel, const char * fmt, ...)  Line 67
  C
  plpython3.dll!PLyUnicode_AsString(_object * unicode)  Line 96 C
  plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth)  Line 176 + 0x8 bytes C

Dbserver get stuck in the following call loop i.e.
... PLy_elog() - PLy_traceback() - PLyUnicode_AsString() -
PLyUnicode_Bytes() - PLy_elog() ...

I think primary reason that trigger this issue is when Function
PLyUnicode_Bytes() calls PyUnicode_AsEncodedString( ,WIN1252 /*Server
encoding*/, )  it fails with null. I built latest pg 9.2 source code with
python 3.2.2.3 by using Visual Studio 2010. Thanks.

Best Regards,
Muhammad Asif Naeem

Re: [HACKERS] plpython issue with Win64 (PG 9.2)

[Asif Naeem]

Thank you. Please do let me know once fix check-in. I will test it and
share feedback with you. Thanks.

Best Regards,
Asif Naeem

On Fri, Jun 29, 2012 at 3:36 AM, Jan Urbański wulc...@wulczer.org wrote:

 On 27/06/12 13:57, Jan Urbański wrote:

 On 27/06/12 11:51, Asif Naeem wrote:

 Hi,

 On Windows 7 64bit, plpython is causing server crash with the following
 test case i.e.

 CREATE PROCEDURAL LANGUAGE 'plpython3u';

 CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
 RETURNS integer
 AS $$
 if a b:
 return a
 return b
 $$ LANGUAGE plpython3u;
 SELECT pymax(1, 2);



 I think primary reason that trigger this issue is when Function
 PLyUnicode_Bytes() calls PyUnicode_AsEncodedString( ,WIN1252 /*Server
 encoding*/, )  it fails with null. I built latest pg 9.2 source code
 with
 python 3.2.2.3 by using Visual Studio 2010. Thanks.


 I'll try to reproduce this on Linux, which should be possible given the
 results of your investigation.


 Your analysis is correct, I managed to reproduce this by injecting

 serverenc = win1252;

 into PLyUnicode_Bytes. The comment in that function says that Python
 understands all PostgreSQL encoding names except for SQL_ASCII, but that's
 not really true. In your case GetDatabaseEncodingName() returns WIN1252
 and Python accepts CP125.

 I'm wondering how this should be fixed. Just by adding more special cases
 in PLyUnicode_Bytes?

 Even if we add a switch statement that would convert PG_WIN1250 into
 CP1250, Python can still raise an exception when encoding (for various
 reasons). How about replacing the PLy_elog there with just an elog? This
 loses traceback support and the Python exception message, which could be
 helpful for debugging (something like invalid character foo for encoding
 cp1250). OTOH, I'm uneasy about invoking the entire PLy_elog machinery
 from a function that's as low-level as PLyUnicode_Bytes.

 Lastly, we map SQL_ASCII to ascii which is arguably wrong. The function
 is supposed to return bytes in the server encoding, and under SQL_ASCII
 that probably means we can return anything (ie. use any encoding we deem
 useful). Using ascii as the Python codec name will raise an error on
 anything that has the high bit set.

 So: I'd add code to translate WINxxx into CPxxx when choosing the Python
 to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII
 case alone, as there were no complaints and people using SQL_ASCII are
 asking for it anyway.

 Cheers,
 Jan

Re: [HACKERS] plpython issue with Win64 (PG 9.2)

[Asif Naeem]

 Patch attached. Asif, could you try a few things on a CP1252 database?

First verify if your original test case now works and then try this:


I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.


 create function enctest() returns text as $$
   return b'tr\xc3\xb3spido'.decode('**utf-8')
 $$ language plpython3u;

 select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');


create function enctest() returns text as $$
  return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
 enctest  |   encode
--+
 tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.

Best Regards,
Muhammad Asif Naeem

Re: [HACKERS] [PATCH] Add use of asprintf()

[Asif Naeem]

Hi,

I did put some time review the patch, please see my findings below i.e.

1. It seems that you have used strdup() on multiple places in the patch,
e.g. in the below code snippet is it going to lead crash if newp-ident is
NULL because of strdup() failure ?

static EPlan *
 find_plan(char *ident, EPlan **eplan, int *nplans)
 {
 ...
  newp-ident = strdup(ident);
 ...
 }


2. Can we rely on return value of asprintf() instead of recompute length as
strlen(cmdbuf) ?

  if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 
 0)
return fail_lo_xact(\\lo_import, own_transaction);
   bufptr = cmdbuf + strlen(cmdbuf);


3. There seems naming convention for functions like pfree (that seems
similar to free()), pstrdup etc; but psprintf seems different than sprintf.
Can't we use pg_asprintf instead in the patch, as it seems that both
(psprintf and pg_asprintf) provide almost same functionality ?

4. It seems that ret  0 need to be changed to rc  0 in the following
code i.e.

int
 pg_asprintf(char **ret, const char *format, ...)
 {
  va_list  ap;
  int   rc;
  va_start(ap, format);
  rc = vasprintf(ret, format, ap);
  va_end(ap);
  if (ret  0)
  {
   fprintf(stderr, _(out of memory\n));
   exit(EXIT_FAILURE);
  }
  return rc;
 }


5. It seems that in the previously implementation, error handling was not
there, is it appropriate here that if there is issue in duplicating
environment, quit the application ? i.e.

/*
  * Handy subroutine for setting an environment variable var to val
  */
 static void
 doputenv(const char *var, const char *val)
 {
  char*s;
  pg_asprintf(s, %s=%s, var, val);
  putenv(s);
 }



Please do let me know if I missed something or more info is required. Thank
you.

Regards,
Muhammad Asif Naeem


On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote:

 Peter Eisentraut wrote:

  The attached patch should speak for itself.

 Yeah, it's a very nice cleanup.

  I have supplied a few variants:
 
  - asprintf() is the standard function, supplied by libpgport if
  necessary.
 
  - pg_asprintf() is asprintf() with automatic error handling (like
  pg_malloc(), etc.)
 
  - psprintf() is the same idea but with palloc.

 Looks good to me, except that pg_asprintf seems to be checking ret
 instead of rc.

 Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
 I don't see that we use the integer return value anywhere.  Callers
 interested in the return value can use asprintf directly (and you have
 already inserted callers that do nonstandard things using direct
 asprintf).

 --
 Álvaro Herrerahttp://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


 --
 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] [PATCH] Add use of asprintf()

[Asif Naeem]

On Tue, Sep 17, 2013 at 3:13 PM, Asif Naeem anaeem...@gmail.com wrote:

 Hi,

 I did put some time review the patch, please see my findings below i.e.

 1. It seems that you have used strdup() on multiple places in the patch,
 e.g. in the below code snippet is it going to lead crash if newp-ident is
 NULL because of strdup() failure ?

 static EPlan *
 find_plan(char *ident, EPlan **eplan, int *nplans)
 {
 ...
  newp-ident = strdup(ident);
 ...
 }


 2. Can we rely on return value of asprintf() instead of recompute length
 as strlen(cmdbuf) ?

   if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 
 0)
return fail_lo_xact(\\lo_import, own_transaction);
   bufptr = cmdbuf + strlen(cmdbuf);


 3. There seems naming convention for functions like pfree (that seems
 similar to free()), pstrdup etc; but psprintf seems different than sprintf.
 Can't we use pg_asprintf instead in the patch, as it seems that both
 (psprintf and pg_asprintf) provide almost same functionality ?

 4. It seems that ret  0 need to be changed to rc  0 in the following
 code i.e.

 int
 pg_asprintf(char **ret, const char *format, ...)
 {
  va_list  ap;
  int   rc;
  va_start(ap, format);
  rc = vasprintf(ret, format, ap);
  va_end(ap);
  if (ret  0)
  {
   fprintf(stderr, _(out of memory\n));
   exit(EXIT_FAILURE);
  }
  return rc;
 }


It seems this point is already mentioned by Alvaro. Thanks.



 5. It seems that in the previously implementation, error handling was not
 there, is it appropriate here that if there is issue in duplicating
 environment, quit the application ? i.e.

 /*
  * Handy subroutine for setting an environment variable var to val
  */
 static void
 doputenv(const char *var, const char *val)
 {
  char*s;
  pg_asprintf(s, %s=%s, var, val);
  putenv(s);
 }



 Please do let me know if I missed something or more info is required.
 Thank you.

 Regards,
 Muhammad Asif Naeem


 On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera 
 alvhe...@2ndquadrant.comwrote:

 Peter Eisentraut wrote:

  The attached patch should speak for itself.

 Yeah, it's a very nice cleanup.

  I have supplied a few variants:
 
  - asprintf() is the standard function, supplied by libpgport if
  necessary.
 
  - pg_asprintf() is asprintf() with automatic error handling (like
  pg_malloc(), etc.)
 
  - psprintf() is the same idea but with palloc.

 Looks good to me, except that pg_asprintf seems to be checking ret
 instead of rc.

 Is there a reason for the API discrepancy of pg_asprintf vs. psprintf?
 I don't see that we use the integer return value anywhere.  Callers
 interested in the return value can use asprintf directly (and you have
 already inserted callers that do nonstandard things using direct
 asprintf).

 --
 Álvaro Herrerahttp://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


 --
 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] [PATCH] Add use of asprintf()

[Asif Naeem]

Thank you Peter.
When I try to apply the patch on latest PG source code on master branch, it
gives the following error message i.e.

 asif$ git apply /Users/asif/core/Patch\:\ Add\ use\ of\
 asprintf\(\)/v2-0001-Add-use-of-asprintf.patch
 /Users/asif/core/Patch: Add use of
 asprintf()/v2-0001-Add-use-of-asprintf.patch:76: trailing whitespace.
 /Users/asif/core/Patch: Add use of
 asprintf()/v2-0001-Add-use-of-asprintf.patch:77: trailing whitespace.
 for ac_func in asprintf crypt fls getopt getrusage inet_aton random rint
 srandom strerror strlcat strlcpy
 /Users/asif/core/Patch: Add use of
 asprintf()/v2-0001-Add-use-of-asprintf.patch:90: trailing whitespace.
 AC_REPLACE_FUNCS([asprintf crypt fls getopt getrusage inet_aton random
 rint srandom strerror strlcat strlcpy])
 /Users/asif/core/Patch: Add use of
 asprintf()/v2-0001-Add-use-of-asprintf.patch:104: trailing whitespace.
   values[1] = psprintf(%s/%s, fctx-location, de-d_name);
 /Users/asif/core/Patch: Add use of
 asprintf()/v2-0001-Add-use-of-asprintf.patch:118: trailing whitespace.
  pg_asprintf(todo,
 fatal: git apply: bad git-diff - expected /dev/null on line 1557


Neither git nor patch command apply the patch successfully. Can you please
guide ?. Thanks.

Best Regards,
Muhammad Asif Naeem



On Wed, Oct 2, 2013 at 7:29 AM, Peter Eisentraut pete...@gmx.net wrote:

 On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote:
  I did put some time review the patch, please see my findings below
  i.e.

 Updated patch for this.

Re: [HACKERS] [PATCH] Add use of asprintf()

[Asif Naeem]

You are right, wget worked. Latest patch looks good to me. make check run
fine. Thank you Peter.



On Wed, Oct 2, 2013 at 5:02 PM, Peter Eisentraut pete...@gmx.net wrote:

 On 10/2/13 5:12 AM, Asif Naeem wrote:
  Neither git nor patch command apply the patch successfully. Can you
  please guide ?. Thanks.

 Works for me.  Check that your email client isn't mangling line endings.

Re: [HACKERS] [PATCH] Add use of asprintf()

[Asif Naeem]

+1

I think you can safely use va_list without copy on Windows. va_copy is
available in Visual Studio 2013 as part of support for C99, previous
versions don't have it.

Regards,
Muhammad Asif Naeem

On Tue, Oct 15, 2013 at 10:33 AM, Amit Kapila amit.kapil...@gmail.comwrote:

 On Tue, Oct 15, 2013 at 2:18 AM, Peter Eisentraut pete...@gmx.net wrote:
  On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote:
 
 
  Looks like something like:
 
 
  #ifndef WIN32
  #define HAVE_VA_COPY 1
  #endif
 
 
  would need to be added to asprintf.c, but also some work needs to be
  done with mcxt.c as it uses va_copy unconditionally. Perhaps just
  defining a macro for va_copy would be better for windows. I was not
  quite sure the best header file for such a macro so I did not write a
  patch to fix it.
 
  Does Windows not have va_copy?  What do they use instead?

 No, Windows doesn't have va_copy, instead they use something like below:
 #define va_copy(dest, src) (dest = src)

 Please refer below link for details of porting va_copy() on Windows:
 http://stackoverflow.com/questions/558223/va-copy-porting-to-visual-c

 I could see that there is similar handling in code of vasprintf(),
 such that if va_copy is not available then directly assign src to dst.

 #if defined(HAVE_VA_COPY)
 va_copy(ap2, ap);
 #define my_va_end(ap2) va_end(ap2)
 #elif defined(HAVE___BUILTIN_VA_COPY)
 __builtin_va_copy(ap2, ap);
 #define my_va_end(ap2) __builtin_va_end(ap2)
 #else
 ap2 = ap;
 #define my_va_end(ap2) do {} while (0)
 #endif

 I think rather than having writing code like above at places where
 va_copy is used, we can use something like:
 #ifdef WIN32
 #define va_copy(dest, src) (dest = src)
 #endif

 and define HAVE_VA_COPY to 1 for non-windows platform.

 With Regards,
 Amit Kapila.
 EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] [PATCH] Add use of asprintf()

[Asif Naeem]

On Tue, Oct 15, 2013 at 10:55 AM, David Rowley dgrowle...@gmail.com wrote:

 Though this is not yet enough to get the windows build work with me... I'm
 still getting link failures for isolationtester.c


 D:\Postgres\c\pgsql.sln (default target) (1) -
 D:\Postgres\c\isolationtester.vcxproj (default target) (89) -
 (Link target) -
   isolationtester.obj : error LNK2019: unresolved external symbol
 _pg_strdup referenced in function _try_complete_step
 [D:\Postgres\c\isolationtester.vcxproj]
   isolationtester.obj : error LNK2019: unresolved external symbol
 _pg_asprintf referenced in function _try_complete_step
 [D:\Postgres\c\isolationtester.vcxproj
 ]
   .\Release\isolationtester\isolationtester.exe : fatal error LNK1120: 2
 unresolved externals [D:\Postgres\c\isolationtester.vcxproj]

 1 Warning(s)

 I guess this is down to a make file error somewhere.


Can you please try the attached patch ?. I hope it will solve the problem.



 David






Win_isolationtester_fix.patch
Description: Binary data

-- 
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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

[Asif Naeem]

Hi Naoya,

I am not able to reproduce the problem. Do you mean pg windows service
installed by installer is not working or bin\pg_ctl binary is not accepting
spaces in the patch ?. Following worked for me i.e.

C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
 C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting


Can you please share the exact steps ?. Thanks.

Regards,
Muhammad Asif Naeem



On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp
 wrote:

 Hi All,

 I have found a case that PostgreSQL Service does not start.
 When it happens, the following error appears.

  is not a valid Win32 application

 This failure occurs when the following conditions are true.

 1. There is postgres.exe in any directory that contains a space,
such as Program Files.

e.g.)
C:\Program Files\PostgreSQL\bin\postgres.exe

 2. A file using the first white space-delimited
tokens of that directory as the file name exists,
and there is it in the same hierarchy.

e.g.)
C:\Program //file

 pg_ctl.exe as PostgreSQL Service creates a postgres
 process using an absolute path which indicates the
 location of postgres.exe,but the path is not enclosed
 in quotation.

 Therefore,if the above-mentioned conditions are true,
 CreateProcessAsUser(a Windows Function called by pg_ctl.exe)
 tries to create a process using the other file such
 as Program, so the service fails to start.

 Accordingly, I think that the command path should be
 enclosed in quotation.

 I created a patch to fix this failure,
 So could anyone confirm?

 Regards,

 Naoya

 ---
 Naoya Anzai
 Engineering Department
 NEC Soft, Ltd.
 E-Mail: anzai-na...@mxu.nes.nec.co.jp
 ---


 --
 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] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

[Asif Naeem]

It is related to windows unquoted service path vulnerability in the the
installer that creates service path without quotes that make service.exe to
look for undesirable path for executable.

postgresql-9.3 service path : C:/Users/asif/Desktop/Program
files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
C:/Users/asif/Desktop/Program files/9.3/data -w

service.exe

 C:\Users\asif\Desktop\Program NAME NOT FOUND
 C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS DENIED

 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice NAME
 NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N.exe
   NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w.exe
   NAME INVALID


Fix :

postgresql-9.3 service path : C:/Users/asif/Desktop/Program
files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
C:/Users/asif/Desktop/Program files/9.3/data -w

It would be good if this is reported on pg installer forum or security
forum. Thanks.

Regards,
Asif Naeem

On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai anzai-na...@mxu.nes.nec.co.jp
wrote:

 Hi, Asif.

 Thank you for response.


C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting

 This failure does not occur by the command line.
 PostgreSQL needs to start by Windows Service.

 Additionally,In this case,
 A file Program needs to be exist at C:\Users\asif\Desktop\, and
 postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
files\9.3\bin.
 
 C:\Users\asif\Desktop\Program files\9.3\bindir
 ...
 4,435,456   postgres.exe
80,896   pg_ctl.exe
 ...

 C:\Users\asif\Desktoppdir
 ...
 0  Program
 DIR  Program files
 ...
 

 Regards,
 Naoya

  Hi Naoya,
 
  I am not able to reproduce the problem. Do you mean pg windows service
installed by installer is not working or bin\pg_ctl binary is not accepting
spaces in the patch ?. Following worked for me i.e.
 
 
C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
server starting
 
 
  Can you please share the exact steps ?. Thanks.
 
 
  Regards,
  Muhammad Asif Naeem
 
 
 
  On Mon, Oct 28, 2013 at 10:26 AM, Naoya Anzai 
anzai-na...@mxu.nes.nec.co.jp wrote:
 
 
Hi All,
 
I have found a case that PostgreSQL Service does not start.
When it happens, the following error appears.
 
 is not a valid Win32 application
 
This failure occurs when the following conditions are true.
 
1. There is postgres.exe in any directory that contains a space,
   such as Program Files.
 
   e.g.)
   C:\Program Files\PostgreSQL\bin\postgres.exe
 
2. A file using the first white space-delimited
   tokens of that directory as the file name exists,
   and there is it in the same hierarchy.
 
   e.g.)
   C:\Program //file
 
pg_ctl.exe as PostgreSQL Service creates a postgres
process using an absolute path which indicates the
location of postgres.exe,but the path is not enclosed
in quotation.
 
Therefore,if the above-mentioned conditions are true,
CreateProcessAsUser(a Windows Function called by pg_ctl.exe)
tries to create a process using the other file such
as Program, so

Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

[Asif Naeem]

Hi Sandeep,

PFA Naoya's patch (pg_ctl.c.patch).

Hi Naoya,

Good finding. I have attached another version of patch
(pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
code changes, can you please take a look ?. Thanks.

Best Regards,
Asif Naeem


On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar 
sandeep.thak...@enterprisedb.com wrote:

 Hi Dave

 We register the service using pg_ctl. When I manually executed the
 following on the command prompt, I saw that the service path of the
 registered service did not have the pg_ctl.exe path in quotes. May be it
 should be handled in the pg_ctl code.

 *c:\Users\Sandeep Thakkar\Documents*c:\Program
 Files\PostgreSQL\9.3\bin\pg_ctl.e
 xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D c:\Program
 Files\P
 ostgreSQL\9.3\data -w

 Naoya,  I could not find your patch here. Can you please share it again?



 On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org wrote:

 Sandeep, can you look at this please? Thanks.

 On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem anaeem...@gmail.com wrote:
  It is related to windows unquoted service path vulnerability in the the
  installer that creates service path without quotes that make
 service.exe to
  look for undesirable path for executable.
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  service.exe
 
  C:\Users\asif\Desktop\Program NAME NOT FOUND
  C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS
 DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe ACCESS
 DENIED
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice
 NAME
  NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice
 -N.exe
  NAME NOT FOUND
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 NAME
  INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data.exe
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data -w
  NAME INVALID
  C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe runservice -N
  postgresql-9.3 -D C:\Users\asif\Desktop\Program files\9.3\data
 -w.exe
  NAME INVALID
 
 
  Fix :
 
  postgresql-9.3 service path : C:/Users/asif/Desktop/Program
  files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3 -D
  C:/Users/asif/Desktop/Program files/9.3/data -w
 
  It would be good if this is reported on pg installer forum or security
  forum. Thanks.
 
  Regards,
  Asif Naeem
 
  On Mon, Oct 28, 2013 at 12:06 PM, Naoya Anzai
  anzai-na...@mxu.nes.nec.co.jp wrote:
 
  Hi, Asif.
 
  Thank you for response.
 
 
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
 
  This failure does not occur by the command line.
  PostgreSQL needs to start by Windows Service.
 
  Additionally,In this case,
  A file Program needs to be exist at C:\Users\asif\Desktop\, and
  postgres.exe needs to be exist at C:\Users\asif\Desktop\Program
  files\9.3\bin.
  
  C:\Users\asif\Desktop\Program files\9.3\bindir
  ...
  4,435,456   postgres.exe
 80,896   pg_ctl.exe
  ...
 
  C:\Users\asif\Desktoppdir
  ...
  0  Program
  DIR  Program files
  ...
  
 
  Regards,
  Naoya
 
   Hi Naoya,
  
   I am not able to reproduce the problem. Do you mean pg windows
 service
   installed by installer is not working or bin\pg_ctl binary is not
 accepting
   spaces in the patch ?. Following worked for me i.e.
  
  
 C:\Users\asif\Desktop\Program files\9.3bin\pg_ctl -D
   C:\Users\asif\Desktop\Program files\9.3\data1 -l logfile start
 server starting
  
  
   Can you please share the exact steps ?. Thanks

Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

[Asif Naeem]

Yes. It should not be installer issue as installer is using pg_ctl to
register and run the service on Windows. Thanks.

Best Regards,
Muhammad Asif Naeem


On Tue, Oct 29, 2013 at 9:57 AM, Sandeep Thakkar 
sandeep.thak...@enterprisedb.com wrote:

 So, this is not an installer issue. Is this bug raised to the PostgreSQL
 community? If yes, you should submit the patch there.


 On Tue, Oct 29, 2013 at 6:23 AM, Naoya Anzai 
 anzai-na...@mxu.nes.nec.co.jp wrote:

 Hi, Asif

 Thank you for providing my patch (pg_ctl.c.patch) to Sandeep on my behalf.

  Good finding. I have attached another version of patch
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
 code changes, can you please take a look ?. Thanks.

 I think your patch is not sufficient to fix.
 Not only pg_ctl.exe but postgres.exe also have the same problem.
 Even if your patch is attached,
 A Path of postgres.exe passed to CreateRestrictedProcess is not
 enclosed in quotation.(See pgwin32_ServiceMain at pg_ctl.c)

 So, processing enclosed in quotation should do in both conditions.

 Regards,
 Naoya

 ---
 Naoya Anzai
 Engineering Department
 NEC Soft, Ltd.
 E-Mail: anzai-na...@mxu.nes.nec.co.jp
 ---


  Hi Sandeep,
 
  PFA Naoya's patch (pg_ctl.c.patch).
 
  Hi Naoya,
 
  Good finding. I have attached another version of patch
 (pg_ctl.c_windows_vulnerability.patch) attached that has fewer lines of
 code changes, can you please take a look ?. Thanks.
 
  Best Regards,
  Asif Naeem
 
 
  On Mon, Oct 28, 2013 at 4:46 PM, Sandeep Thakkar 
 sandeep.thak...@enterprisedb.com wrote:
 
 
Hi Dave
 
We register the service using pg_ctl. When I manually executed
 the following on the command prompt, I saw that the service path of the
 registered service did not have the pg_ctl.exe path in quotes. May be it
 should be handled in the pg_ctl code.
 
c:\Users\Sandeep Thakkar\Documentsc:\Program
 Files\PostgreSQL\9.3\bin\pg_ctl.e
xe register -N pg-9.3 -U NT AUTHORITY\NetworkService -D
 c:\Program Files\P
ostgreSQL\9.3\data -w
 
Naoya,  I could not find your patch here. Can you please share it
 again?
 
 
 
On Mon, Oct 28, 2013 at 2:53 PM, Dave Page dp...@pgadmin.org
 wrote:
 
 
Sandeep, can you look at this please? Thanks.
 
On Mon, Oct 28, 2013 at 8:18 AM, Asif Naeem 
 anaeem...@gmail.com wrote:
 It is related to windows unquoted service path
 vulnerability in the the
 installer that creates service path without quotes that
 make service.exe to
 look for undesirable path for executable.

 postgresql-9.3 service path :
 C:/Users/asif/Desktop/Program
 files/9.3/bin/pg_ctl.exe runservice -N postgresql-9.3
 -D
 C:/Users/asif/Desktop/Program files/9.3/data -w

 service.exe

 C:\Users\asif\Desktop\Program NAME NOT FOUND
 C:\Users\asif\Desktop\Program.exe NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 ACCESS DENIED
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice NAME
 NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N.exe
 NAME NOT FOUND
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D.exe NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program.exe
 NAME INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 files\9.3\data NAME
 INVALID
 C:\Users\asif\Desktop\Program files\9.3\bin\pg_ctl.exe
 runservice -N
 postgresql-9.3 -D C:\Users\asif\Desktop\Program
 files\9.3\data.exe

Re: [HACKERS] PostgreSQL Service on Windows does not start. ~ is not a valid Win32 application

[Asif Naeem]

On Thu, Oct 31, 2013 at 10:17 AM, Amit Kapila amit.kapil...@gmail.comwrote:

 On Tue, Oct 29, 2013 at 12:46 PM, Naoya Anzai
 anzai-na...@mxu.nes.nec.co.jp wrote:
  Hi Sandeep
 
  I think, you should change the subject line  to Unquoted service path
 containing space is vulnerable and can be exploited on Windows to get the
 attention..  :)
  Thank you for advice!
  I'll try to post to pgsql-bugs again.

 I could also reproduce this issue. The situation is very rare such
 that an exe with name same as first part of directory should exist
 in installation path.


I believe it is a security risk with bigger impact as it is related to
Windows environment and as installers rely on it.


 I suggest you can post your patch in next commit fest.


Yes. Are not vulnerabilities/security risk's taken care of more urgent
bases ?


 With Regards,
 Amit Kapila.
 EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] [bug fix] PostgreSQL fails to start on Windows if it crashes after tablespace creation

[Asif Naeem]

I did worked on testing the patch on Windows, test scenario mentioned above
thread is reproducible and the provided patch resolve the issue. In case of
junction or directory unlink function
(deprecatedhttp://msdn.microsoft.com/en-us/library/ms235350.aspx)
returns -1 with errno “EACCES” (i.e. Permission denied). As you have
followed destroy_tablespace_directories() function, Is there any specific
reason not to use same logic to detect type of the file/link i.e.
“(lstat(linkloc, st) == 0  S_ISDIR(st.st_mode))”, It also seems have
more appropriate error message i.e.

src/backend/commands/tablespace.c

 ….
 745 /*
 746  * Try to remove the symlink.  We must however deal with
 the possibility
 747  * that it's a directory instead of a symlink --- this
 could happen during
 748  * WAL replay (see TablespaceCreateDbspace), and it is
 also the case on
 749  * Windows where junction points lstat() as directories.
 750  *
 751  * Note: in the redo case, we'll return true if this final
 step fails;
 752  * there's no point in retrying it.  Also, ENOENT should
 provoke no more
 753  * than a warning.
 754  */
 755 remove_symlink:
 756 linkloc = pstrdup(linkloc_with_version_dir);
 757 get_parent_directory(linkloc);
 758 if (lstat(linkloc, st) == 0  S_ISDIR(st.st_mode))
 759 {
 760 if (rmdir(linkloc)  0)
 761 ereport(redo ? LOG : ERROR,
 762 (errcode_for_file_access(),
 763  errmsg(could not remove
 directory \%s\: %m,
 764 linkloc)));
 765 }
 766 else
 767 {
 768 if (unlink(linkloc)  0)
 769 ereport(redo ? LOG : (errno == ENOENT ?
 WARNING : ERROR),
 770 (errcode_for_file_access(),
 771  errmsg(could not remove
 symbolic link \%s\: %m,
 772 linkloc)));
 773 }
 ….


Other than this patch looks good to me.

Regards,
Muhammad Asif Naeem



On Thu, Oct 31, 2013 at 8:03 PM, MauMau maumau...@gmail.com wrote:

 From: MauMau maumau...@gmail.com

  I thought the same situation could happen as in
 destroy_tablespace_directories(), but it doesn't seem to apply on second
 thought.  Revised patch attached, which is very simple based on compile
 time
 check.


 Sorry, I was careless to leave an old comment.  Please use this version.

 Regards
 MauMau


 --
 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] [bug fix] PostgreSQL fails to start on Windows if it crashes after tablespace creation

[Asif Naeem]

Hi MauMau,

Ah. Sorry, I missed that part. As NTFS junctions and symbolic links are
different (although they behave similarly), there seems only a minor
inconvenience related to misleading error message i.e.

+ #ifdef WIN32
 +  if (rmdir(linkloc)  0  errno != ENOENT)
 + #else
if (unlink(linkloc)  0  errno != ENOENT)
 + #endif
ereport(ERROR,
(errcode_for_file_access(),
 errmsg(could not remove symbolic link \%s\: %m,


What is your opinion about it, Is it not worth changing ? . Thanks.



On Wed, Jan 15, 2014 at 7:42 PM, MauMau maumau...@gmail.com wrote:

 From: Asif Naeem anaeem...@gmail.com

  As you have

 followed destroy_tablespace_directories() function, Is there any specific
 reason not to use same logic to detect type of the file/link i.e.
 “(lstat(linkloc, st) == 0  S_ISDIR(st.st_mode))”, It also seems have
 more appropriate error message i.e.

 Thanks for reviewing and testing the patch.  Yes, at first I did what you
 mentioned, but modified the patch according to some advice in the mail
 thread.  During redo, create_tablespace_directories() needs to handle the
 case where the $PGDATA/pg_tblspc/xxx is not a symlink but a directory even
 on UNIX/Linux.  Please see TablespaceCreateDbspace is(). 
 destroy_tablespace_directories()
 doesn't have to handle such situation.

 Regards
 MauMau

Re: [HACKERS] [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

[Asif Naeem]

Hi MauMau,

I have't completed tested all the expects of submitted patch yet. I would
like to share my findings so far. By looking at the patch I do feel that
there is room for improvement in the patch, Instead of moving related dll's
from lib directory to bin directory later in the installation process, copy
these files directly from release/debug directory earlier (PFA patch).

It seems unfortunate that Windows don't have RPATH or similar logic
implemented in OS. Alternate methods seems not appropriate, Only feasible
option seems to be placing related dependency dll in same executable
directory. At first one may think an alternate to create symbolic link for
relative path in bin directory e.g. libpq.dll - ..\lib\libpq.dll but it is
unfortunate that normal user do require special permissions to create
symbolic link otherwise it could help. There is SetDllDirectory or
AddDllDirectory function available that effects only subsequent calls to
LoadLibrary and LoadLibraryEx.

I will look into this patch further and let you know about my more
findings. Thanks.

Regards,
Muhammad Asif Naeem



On Wed, Dec 4, 2013 at 5:07 PM, MauMau maumau...@gmail.com wrote:

 From: MauMau maumau...@gmail.com

  In addition, I'll remove libpq.dll from lib folder unless somebody
 objects.
 Currently, libpq.dll is placed in both bin and lib.  I guess libpq.dll was
 left in lib because it was considered necessary for ECPG DLLs.


 The attached patch also removes libpq.dll from lib folder.  I don't mind
 whether this patch or yesterday's one will be adopted.  I'll add this to
 2014-1 commitfest this weekend if the patch is not committed until then.

 Regards
 MauMau


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




Win_lib_bin.patch
Description: Binary data

-- 
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] Extending MSVC scripts to support --with-extra-version

[Asif Naeem]

Hi,

I have spent some time reviewing the code. It applies well and PG master
branch build fine with setting extraver or keep it undefined. I have
observed the following output applying the patch i.e.

*Keeping extraver undefined* :

C:\PG\postgresql\inst_withpatch_no_extra-versionbin\psql.exe -d postgres
 psql (9.5devel)
 WARNING: Console code page (437) differs from Windows code page (1252)
  8-bit characters might not work correctly. See psql reference
  page Notes for Windows users for details.
 Type help for help.
 postgres=# select version();
 version
 
  PostgreSQL 9.5devel, compiled by Visual C++ build 1600, 64-bit
 (1 row)
 C:\PG\postgresql\inst_withpatch_no_extra-versionbin\initdb.exe -V
 initdb (PostgreSQL) 9.5devel


*Setting extraver as ‘June27’* :

C:\PG\postgresql\inst_withpatch_extra-versionbin\psql -d postgres
 psql (9.5devel)
 WARNING: Console code page (437) differs from Windows code page (1252)
  8-bit characters might not work correctly. See psql reference
  page Notes for Windows users for details.
 Type help for help.
 postgres=# select version();
version
 --
  PostgreSQL 9.5develJune27, compiled by Visual C++ build 1600, 64-bit
 (1 row)

 C:\PG\postgresql\inst_withpatch_extra-versionbin\initdb.exe -V
 initdb (PostgreSQL) 9.5devel


It seems that extraver information only appears when version function is
being used. If we use -V (--version)  with pg utilities/binaries, it does
not include additional provided information.

Can you please guide how can I perform similar functionality via configure
script (that can be used on Unix like OS/MinGW) or It is intended for
Window specific requirement ?. Thanks.

Regards,
Muhammad Asif Naeem


On Tue, May 27, 2014 at 5:58 AM, Michael Paquier michael.paqu...@gmail.com
wrote:

 Hi all,

 Please find attached a patch extending support of --with-extra-version
 in the MSVC scripts. This is something I have been using internally,
 and I believe that it is useful for Windows packagers.
 Similarly to the other options, a new field called extraver is added
 in config_default.pl and it can be overwritten in config.pl to have an
 additional version string, similarly to what is currently doable with
 ./configure.

 I'll add this patch to the next commit fest.
 Regards,
 --
 Michael


 --
 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] Extending MSVC scripts to support --with-extra-version

[Asif Naeem]

Thank you for sharing updated patch. I have compared it with MSVC and
configure generated build i.e.

*MacOSX (*--with-extra-version -30JUN*)*

pc1dotnetpk:inst asif$ ./bin/psql -d postgres
 psql (9.5devel-30JUN)
 Type help for help.
 postgres=# select version();
   version

 
  PostgreSQL 9.5devel-30JUN on x86_64-apple-darwin13.2.0, compiled by Apple
 LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn), 64-bit
 (1 row)
 pc1dotnetpk:inst asif$ ./bin/initdb -V
 initdb (PostgreSQL) 9.5devel-30JUN


*Windows7 64bit (*extraver = '-30JUN'*)*

C:\PG\postgresql\inst_withpatch_v2_extra-versionbin\psql -d postgres
 psql (9.5devel-30JUN)
 WARNING: Console code page (437) differs from Windows code page (1252)
  8-bit characters might not work correctly. See psql reference
  page Notes for Windows users for details.
 Type help for help.
 postgres=# select version();
version
 --
  PostgreSQL 9.5devel-30JUN, compiled by Visual C++ build 1600, 64-bit
 (1 row)
 C:\PG\postgresql\inst_withpatch_v2_extra-versionbin\initdb.exe -V
 initdb (PostgreSQL) 9.5devel-30JUN


Patch looks good to me. I think it is ready for committer. Thanks.

Regards,
Muhammad Asif Naeem


On Fri, Jun 27, 2014 at 5:00 AM, Michael Paquier michael.paqu...@gmail.com
wrote:




 On Fri, Jun 27, 2014 at 8:26 AM, Asif Naeem anaeem...@gmail.com wrote:

 I have spent some time reviewing the code. It applies well and PG master
 branch build fine with setting extraver or keep it undefined.

 Thanks for reviewing that.


 I have observed the following output applying the patch i.e.

 It seems that extraver information only appears when version function is
 being used. If we use -V (--version)  with pg utilities/binaries, it does
 not include additional provided information.

 You are right. The first version of this patch updates PG_VERSION_STR but
 not PG_VERSION, which is the string used for all the binaries to report the
 version.


  Can you please guide how can I perform similar functionality via
 configure script (that can be used on Unix like OS/MinGW) or It is intended
 for Window specific requirement ?. Thanks.

 Sure, you can do the equivalent with plain configure like that:
 ./configure --with-extra-version=-foo --prefix=/to/path/
 And here is the output that I get with such options on OSX for example:
 $ psql -c 'select substring(version(), 1, 52)'
   substring
 --
  PostgreSQL 9.5devel-foo on x86_64-apple-darwin13.2.0
 (1 row)
 $ initdb --version
 initdb (PostgreSQL) 9.5devel-foo

 With the new patch attached, the following output is generated for an MSVC
 build:
 $ psql -c 'select version()'
   version
 
  PostgreSQL 9.5devel-foo, compiled by Visual C++ build 1600, 64-bit
 (1 row)
 $ initdb --version
 initdb (PostgreSQL) 9.5devel-foo

 Regards,
 --
 Michael

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

Hi Haribabu,

I am not able to apply latest patch on REL9_4_STABLE or master branch i.e.

pc1dotnetpk:postgresql asif$ git apply
 ~/core/min_max_support_for_inet_datatypes/inet_agg_v4.patch
 fatal: unrecognized input


pc1dotnetpk:postgresql asif$ patch -p0 
 ~/core/min_max_support_for_inet_datatypes/inet_agg_v4.patch
 can't find file to patch at input line 3
 Perhaps you used the wrong -p or --strip option?
 The text leading up to this was:
 --
 |*** a/src/backend/utils/adt/network.c
 |--- b/src/backend/utils/adt/network.c
 --
 File to patch:


Is there any other utility required to apply the patch, Can you please
guide ?. Thanks.

Regards,
Muhammad Asif Naeem


On Thu, Jun 5, 2014 at 6:28 AM, Haribabu Kommi kommi.harib...@gmail.com
wrote:

 On Thu, Jun 5, 2014 at 9:12 AM, Andres Freund and...@2ndquadrant.com
 wrote:
  Hi,
 
  On 2014-06-04 10:37:48 +1000, Haribabu Kommi wrote:
  Thanks for the test. Patch is re-based to the latest head.
 
  Did you look at the source of the conflict? Did you intentionally mark
  the functions as leakproof and reviewed that that truly is the case? Or
  was that caused by copy  paste?

 Yes it is copy  paste mistake. I didn't know much about that parameter.
 Thanks for the information. I changed it.

 Regards,
 Hari Babu
 Fujitsu Australia


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

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

On Mon, Jun 30, 2014 at 4:45 PM, Abhijit Menon-Sen a...@2ndquadrant.com
wrote:

 At 2014-06-30 16:35:45 +0500, anaeem...@gmail.com wrote:
 
  pc1dotnetpk:postgresql asif$ patch -p0 
   ~/core/min_max_support_for_inet_datatypes/inet_agg_v4.patch
   can't find file to patch at input line 3
   Perhaps you used the wrong -p or --strip option?
   The text leading up to this was:
   --
   |*** a/src/backend/utils/adt/network.c
   |--- b/src/backend/utils/adt/network.c
   --

 You need to use patch -p1, to strip off the a/b prefix.


Thank you Abhijit, It worked.



 -- Abhijit

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

Hi Haribabu,

Thank you for sharing the patch. I have spent some time to review the
changes. Overall patch looks good to me, make check and manual testing
seems run fine with it. There seems no related doc/sgml changes ?. Patch
added network_smaller() and network_greater() functions but in PG source
code, general practice seems to be to use “smaller and “larger” as related
function name postfix e.g. timestamp_smaller()/timestamp_larger(),
interval_smaller/interval_larger(), cashsmaller()/cashlarger() etc. Thanks.

Regards,
Muhammad Asif Naeem


On Mon, Jul 7, 2014 at 1:56 PM, Asif Naeem anaeem...@gmail.com wrote:

 On Mon, Jun 30, 2014 at 4:45 PM, Abhijit Menon-Sen a...@2ndquadrant.com
 wrote:

 At 2014-06-30 16:35:45 +0500, anaeem...@gmail.com wrote:
 
  pc1dotnetpk:postgresql asif$ patch -p0 
   ~/core/min_max_support_for_inet_datatypes/inet_agg_v4.patch
   can't find file to patch at input line 3
   Perhaps you used the wrong -p or --strip option?
   The text leading up to this was:
   --
   |*** a/src/backend/utils/adt/network.c
   |--- b/src/backend/utils/adt/network.c
   --

 You need to use patch -p1, to strip off the a/b prefix.


 Thank you Abhijit, It worked.



 -- Abhijit

Re: [HACKERS] [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

[Asif Naeem]

Hi MauMau,

Other than my pervious comments, patch looks good to me. Thanks.

Regards,
Muhammad Asif Naeem


On Wed, Feb 26, 2014 at 2:14 AM, Peter Eisentraut pete...@gmx.net wrote:

 You should be able to do this without specifically referencing the names
 libpq or ecpg.  Look into the Makefile, if it defines
 SO_MAJOR_VERSION, then it's a library you are concerned with, and then
 do the copying or moving.

Re: [HACKERS] [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

[Asif Naeem]

Yes. Can you please take a look at Win_lib_bin.patch I shared earlier ?, I
think it is or similar approach will be appropriate. Thanks.

Regards,
Muhammad Asif Naeem


On Tue, Jul 8, 2014 at 5:53 PM, MauMau maumau...@gmail.com wrote:

 From: Asif Naeem anaeem...@gmail.com

  Other than my pervious comments, patch looks good to me. Thanks.


 Thanks for reviewing.  I understood that your previous comment was to
 suggest copying the desired DLLs directly from Release/Debug folder to bin.
 But I'm afraid it cannot be done cleanly...  CopySolutionOutput() copies
 all DLLS to lib folder with no exception as follows:

   if ($1 == 1)
   {
$dir = bin;
$ext = exe;
   }
   elsif ($1 == 2)
   {
$dir = lib;
$ext = dll;
   }

 It seems like I have to do something like this, listing the relevant DLL
 names anyway.  I don't think this is not cleaner.

   if ($1 == 1)
   {
$dir = bin;
$ext = exe;
   }
   elsif ($1 == 2  /* the project is libpq, libecpg, etc. */)
   {
$dir = bin;
$ext = dll;
   }
   elsif ($1 == 2)
   {
$dir = lib;
$ext = dll;
   }

 What do you think?  Am I misunderstanding your suggestion?

 Regards
 MauMau

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

Hi Haribabu,

Sorry for being late. Thank you for sharing updated patch, sgml changes
seems not working i.e.

postgres=# select max('192.168.1.5', '192.168.1.4');
 ERROR:  function max(unknown, unknown) does not exist
 LINE 1: select max('192.168.1.5', '192.168.1.4');
^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.
 postgres=# select min('192.168.1.5', '192.168.1.4');
 ERROR:  function min(unknown, unknown) does not exist
 LINE 1: select min('192.168.1.5', '192.168.1.4');
^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.


I would suggest the following or similar changes e.g.

doc/src/sgml/func.sgml
 /indexterm
 functionmax(replaceable
 class=parameterexpression/replaceable)/function
/entry
 -  entryany array, numeric, string, or date/time type/entry
 +  entryany inet, array, numeric, string, or date/time type/entry
entrysame as argument type/entry
entry
 maximum value of replaceable
 @@ -12204,7 +12228,7 @@ NULL baz/literallayout(3 rows)/entry
 /indexterm
 functionmin(replaceable
 class=parameterexpression/replaceable)/function
/entry
 -  entryany array, numeric, string, or date/time type/entry
 +  entryany inet, array, numeric, string, or date/time type/entry
entrysame as argument type/entry
entry
 minimum value of replaceable


Other than this patch looks good to me. Thanks.

Regards,
Muhammad Asif Naeem

On Wed, Jul 9, 2014 at 6:21 PM, Haribabu Kommi kommi.harib...@gmail.com
wrote:

 On Mon, Jul 7, 2014 at 6:59 PM, Asif Naeem anaeem...@gmail.com wrote:
  Hi Haribabu,
 
  Thank you for sharing the patch. I have spent some time to review the
  changes. Overall patch looks good to me, make check and manual testing
 seems
  run fine with it. There seems no related doc/sgml changes ?. Patch added
  network_smaller() and network_greater() functions but in PG source code,
  general practice seems to be to use “smaller and “larger” as related
  function name postfix e.g. timestamp_smaller()/timestamp_larger(),
  interval_smaller/interval_larger(), cashsmaller()/cashlarger() etc.
 Thanks.

 Thanks for reviewing the patch.

 I corrected the function names as smaller and larger.
 and also added documentation changes.

 Updated patch attached in the mail.

 Regards,
 Hari Babu
 Fujitsu Australia

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

Thank you Haribabu. Please see my comments inlined below i.e.

On Sun, Jul 27, 2014 at 11:42 AM, Haribabu Kommi kommi.harib...@gmail.com
wrote:

 On Thu, Jul 24, 2014 at 5:59 PM, Asif Naeem anaeem...@gmail.com wrote:
  Sorry for being late. Thank you for sharing updated patch, sgml changes
  seems not working i.e.
 
  postgres=# select max('192.168.1.5', '192.168.1.4');
  ERROR:  function max(unknown, unknown) does not exist
  LINE 1: select max('192.168.1.5', '192.168.1.4');
 
 ^
  HINT:  No function matches the given name and argument types. You might
  need to add explicit type casts.
  postgres=# select min('192.168.1.5', '192.168.1.4');
  ERROR:  function min(unknown, unknown) does not exist
  LINE 1: select min('192.168.1.5', '192.168.1.4');
 
 ^
  HINT:  No function matches the given name and argument types. You might
  need to add explicit type casts.

 I didn't get your point. I tested the patch, the sgml changes are working
 fine.


Sorry for not being clear, above mentioned test is related to following doc
(sgml) changes that seems not working as described i.e.

*Table 9-35. cidr and inet Functions*
FunctionReturn TypeDescriptionExampleResult




max(inet, inet)inetlarger of two inet typesmax('192.168.1.5', '192.168.1.4')
192.168.1.5min(inet, inet)inetsmaller of two inet typesmin('192.168.1.5',
'192.168.1.4')192.168.1.4





Can you please elaborate these sgml change ?


  I would suggest the following or similar changes e.g.
 
  doc/src/sgml/func.sgml
  /indexterm
  functionmax(replaceable
  class=parameterexpression/replaceable)/function
 /entry
  -  entryany array, numeric, string, or date/time type/entry
  +  entryany inet, array, numeric, string, or date/time
 type/entry
 entrysame as argument type/entry
 entry
  maximum value of replaceable
  @@ -12204,7 +12228,7 @@ NULL baz/literallayout(3 rows)/entry
  /indexterm
  functionmin(replaceable
  class=parameterexpression/replaceable)/function
 /entry
  -  entryany array, numeric, string, or date/time type/entry
  +  entryany inet, array, numeric, string, or date/time
 type/entry
 entrysame as argument type/entry
 entry
  minimum value of replaceable

 Thanks for reviewing the patch.
 I missed the above change. Corrected the same in the attached patch.

 Regards,
 Hari Babu
 Fujitsu Australia

Re: [HACKERS] [BUGS] BUG #9652: inet types don't support min/max

[Asif Naeem]

 Thank you for sharing updated patch. With latest 9.5 source code, patch
build is failing with following error message i.e.

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C catalog
 schemapg.h
 cd ../../../src/include/catalog  '/opt/local/bin/perl' ./duplicate_oids
 3255
 make[3]: *** [postgres.bki] Error 1
 make[2]: *** [submake-schemapg] Error 2
 make[1]: *** [all-backend-recurse] Error 2
 make: *** [all-src-recurse] Error 2


New function is being added by following commit i.e.

commit b34e37bfefbed1bf9396dde18f308d8b96fd176c
 Author: Robert Haas rh...@postgresql.org
 Date:   Thu Aug 14 12:09:52 2014 -0400
 Add sortsupport routines for text.
 This provides a small but worthwhile speedup when sorting text, at
 least
 in cases to which the sortsupport machinery applies.
 Robert Haas and Peter Geoghegan


It seems that you need to use another oid. Other than this patch looks good
to me, please share updated patch and feel free to assign it to committer.
Thanks.

Regards,
Muhammad Asif Naeem



On Tue, Aug 12, 2014 at 3:12 PM, Haribabu Kommi kommi.harib...@gmail.com
wrote:

 On Mon, Aug 4, 2014 at 3:22 PM, Asif Naeem anaeem...@gmail.com wrote:
  Sorry for not being clear, above mentioned test is related to following
 doc (sgml) changes that seems not working as described i.e.
 
  Table 9-35. cidr and inet Functions
 
  FunctionReturn TypeDescriptionExampleResult
 
  max(inet, inet) inetlarger of two inet typesmax('192.168.1.5',
 '192.168.1.4')192.168.1.5
  min(inet, inet) inetsmaller of two inet typesmin('192.168.1.5',
 '192.168.1.4')192.168.1.4
 
  Can you please elaborate these sgml change ?

 I thought of adding them for newly added network functions but
 mistakenly I kept the names as max and min.
 Anyway with your suggestion in the earlier mail, these changes are not
 required.

 I removed these changes in the attached patch.
 Thanks for reviewing the patch.

 Regards,
 Hari Babu
 Fujitsu Australia

Re: [HACKERS] Add shutdown_at_recovery_target option to recovery.conf

[Asif Naeem]

Hi Petr,

I have spent sometime to review the patch, overall patch looks good, it
applies fine and make check run without issue. If recovery target is
specified and shutdown_at_recovery_target is set to true, it shutdown the
server at specified recovery point. I do have few points to share i.e.

1. It seems that following log message need to be more descriptive about
reason for shutdown i.e.

+   if (recoveryShutdownAtTarget  reachedStopPoint)
 +   {
 +   ereport(LOG, (errmsg(shutting down)));


2. As Simon suggesting following recovery settings are not clear i.e.

shutdown_at_recovery_target = true
 pause_at_recovery_target = true


It is going to make true both but patch describe as following i.e.

+Setting this to true will set link
 linkend=pause-at-recovery-target
 +varnamepause_at_recovery_target//link to false.


3. As it don’t rename reconvery.conf, subsequent attempt (without any
changes in reconvery.conf) to start of server keep showing the following
i.e.

 ...
 LOG:  redo starts at 0/1803620
 DEBUG:  checkpointer updated shared memory configuration values
 LOG:  consistent recovery state reached at 0/1803658
 LOG:  restored log file 00010002 from archive
 DEBUG:  got WAL segment from archive
 LOG:  restored log file 00010003 from archive
 DEBUG:  got WAL segment from archive
 LOG:  restored log file 00010004 from archive
 DEBUG:  got WAL segment from archive
 LOG:  restored log file 00010005 from archive
 DEBUG:  got WAL segment from archive
 LOG:  restored log file 00010006 from archive
 DEBUG:  got WAL segment from archive
 …


Is that right ?. Thanks.

Regards,
Muhammad Asif Naeem


On Thu, Oct 16, 2014 at 7:24 AM, Simon Riggs si...@2ndquadrant.com wrote:

 On 11 September 2014 16:02, Petr Jelinek p...@2ndquadrant.com wrote:

  What about adding something like
 action_at_recovery_target=pause|shutdown
  instead of increasing the number of parameters?
 
 
  That will also increase number of parameters as we can't remove the
 current
  pause one if we want to be backwards compatible. Also there would have
 to be
  something like action_at_recovery_target=none or off or something since
 the
  default is that pause is on and we need to be able to turn off pause
 without
  having to have shutdown on. What more, I am not sure I see any other
 actions
  that could be added in the future as promote action already works and
 listen
  (for RO queries) also already works independently of this.

 I accept your argument, though I have other thoughts.

 If someone specifies

 shutdown_at_recovery_target = true
 pause_at_recovery_target = true

 it gets a little hard to work out what to do; we shouldn't allow such
 lack of clarity.

 In recovery its easy to do this

 if (recoveryShutdownAtTarget)
recoveryPauseAtTarget = false;

 but it won't be when these become GUCs, so I think Fuji's suggestion
 is a good one.

 No other comments on patch, other than good idea.

 --
  Simon Riggs   http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training  Services


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

[HACKERS] chkpass with RANDOMIZE_ALLOCATED_MEMORY

[Asif Naeem]

Hi,

It is been observed on RANDOMIZE_ALLOCATED_MEMORY enabled PG95 build that
chkpass is failing because of uninitialized memory and seems showing false
alarm. I have tried to add code snippets to explain as following i.e.

postgres=# CREATE EXTENSION chkpass;
 WARNING:  type input function chkpass_in should not be volatile
 CREATE EXTENSION
 postgres=# CREATE TABLE test_tbl ( name text, pass chkpass );
 CREATE TABLE
 postgres=# INSERT INTO test_tbl VALUES('foo','foo1');
 WARNING:  type chkpass has unstable input conversion for foo1
 LINE 1: INSERT INTO test_tbl VALUES('foo','foo1');
   ^
 INSERT 0 1


It is giving warning type chkpass has unstable input conversion because
chkpass structure hold uninitialized memory area’s that are left unused.
When chkpass_in() is called with input “foo1”, it allocate 16 bytes of
randomized memory for chkpass i.e.

contrib/chkpass/chkpass.c

 /*
  * This is the internal storage format for CHKPASSs.
  * 15 is all I need but add a little buffer
  */
 typedef struct chkpass
 {
 charpassword[16];
 } chkpass;


chkpass_in()

 result = (chkpass *) palloc(sizeof(chkpass));


*result memory*

 0x7ffc93047a68: 0xdd 0xde 0xdf 0xe0 0xe1 0xe2 0xe3 0xe4
 0x7ffc93047a70: 0xe5 0xe6 0xe7 0xe8 0xe9 0xea 0xeb 0xec


It copies string (16 bytes max) returned from crypt() function, it copies
until null character reached i.e.

strlcpy(result-password, crypt_output, sizeof(result-password));


*crypt_output memory*

 0x7fff7d1577f0: 0x6a 0x6e 0x6d 0x54 0x44 0x53 0x55 0x33
 0x7fff7d1577f8: 0x4b 0x48 0x52 0x55 0x6f 0x00 0x00 0x00


*result memory*

 0x7ffc93047a68: 0x6a 0x6e 0x6d 0x54 0x44 0x53 0x55 0x33
 0x7ffc93047a70: 0x4b 0x48 0x52 0x55 0x6f 0x00 0xeb 0xec


It left remaining last 2 byte left untouched. It is the cause for unstable
input conversion” warning.

I think these uninitialized memory areas are false alarms. Along with this
there seems another issue that makes chkpass troubling
RANDOMIZE_ALLOCATED_MEMORY comparison logic is that crypt() function is
being passed with random salt value for DES based result. PFA patch, to
generate consistent results, it uses constant salt value.

It seems a minor inconvenience but it will make results better. Please do
let me know if I missed something or more information is required. Thanks.

Regards,
Muhammad Asif Naeem


chkpass_RANDOMIZE_ALLOCATED_MEMORY_v1.patch
Description: Binary data

-- 
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] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)

[Asif Naeem]

Thank you Michael overall v3 patch looks good to me, There is one
observation that it is not installing following lib files that are required
for dev work i.e.

 inst\lib\libpq.lib
 inst\lib\libpgtypes.lib
 inst\lib\libecpg_compat.lib
 inst\lib\libecpg.lib

Please do let me know if I missed something but was not there a need to
avoid installing related .dll files in lib (duplicate) along with bin
directory e.g.

src\tools\msvc\Install.pm


 if ($is_sharedlib)
 {
 @dirs = qw(bin);
 }
 else
 {
 @dirs = qw(lib);
 }


Thanks.


On Mon, Mar 9, 2015 at 1:16 PM, Michael Paquier michael.paqu...@gmail.com
wrote:

 On Wed, Mar 4, 2015 at 4:13 AM, Michael Paquier wrote:
  Those entries are removed as well in the patch.

 Please find attached a new version of the patch, fixing a failure for
 plperl installation that contains GNUmakefile instead of Makefile.
 --
 Michael

Re: [HACKERS] chkpass with RANDOMIZE_ALLOCATED_MEMORY

[Asif Naeem]

Thank you Tom, Thank you Amit.

Regards,
Muhammad Asif Naeem

On Wed, Mar 4, 2015 at 9:30 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Amit Kapila amit.kapil...@gmail.com writes:
  On Sat, Feb 14, 2015 at 10:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  It's not a false alarm, unfortunately, because chkpass_in actually does
  give different results from one call to the next.  We could fix the
 aspect
  of that involving failing to zero out unused bytes (which it appears was
  introduced by sloppy replacement of strncpy with strlcpy).  But we can't
  really do anything about the dependency on random(), because that's part
  of the fundamental specification of the data type.  It was a bad idea,
  no doubt, to design the input function to do this; but we're stuck with
  it now.

  It seems to me that fix for this issue has already been committed
  (commit-id: 80986e85).  So isn't it better to mark as Committed in
  CF app [1] or are you expecting anything more related to this issue?

  [1]: https://commitfest.postgresql.org/4/144/

 Ah, I didn't realize there was a CF entry for it, I think.  Yeah,
 I think we committed as much as we should of this, so I marked the
 CF entry as committed.

 regards, tom lane

Re: [HACKERS] New CF app deployment

[Asif Naeem]

On Fri, Feb 27, 2015 at 11:32 AM, Stefan Kaltenbrunner 
ste...@kaltenbrunner.cc wrote:

 On 02/26/2015 01:59 PM, Michael Paquier wrote:
  On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.com wrote:
  This thread seems relevant, Please guide me to how can access older CF
 pages
  The MSVC portion of this fix got completely lost in the void:
  https://commitfest.postgresql.org/action/patch_view?id=1330
 
  Above link result in the following error i.e.
 
  Not found
  The specified URL was not found.
 
  Please do let me know if I missed something. Thanks.
 
  Try commitfest-old instead, that is where the past CF app stores its
  data, like that:
  https://commitfest-old.postgresql.org/action/patch_view?id=1330

 hmm maybe we should have some sort of handler the redirects/reverse
 proxies to the old commitfest app for this.


1+ for seamless integration, at least error message seems require to be
more helpful. Thanks.


 Stefan

Re: [HACKERS] New CF app deployment

[Asif Naeem]

Thank you Michael, It works.

On Thu, Feb 26, 2015 at 5:59 PM, Michael Paquier michael.paqu...@gmail.com
wrote:

 On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.com wrote:
  This thread seems relevant, Please guide me to how can access older CF
 pages
  The MSVC portion of this fix got completely lost in the void:
  https://commitfest.postgresql.org/action/patch_view?id=1330
 
  Above link result in the following error i.e.
 
  Not found
  The specified URL was not found.
 
  Please do let me know if I missed something. Thanks.

 Try commitfest-old instead, that is where the past CF app stores its
 data, like that:
 https://commitfest-old.postgresql.org/action/patch_view?id=1330
 --
 Michael

Re: [HACKERS] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)

[Asif Naeem]

Thank you Michael. I have looked the patch. Overall logic looks good to me,
I have checked it with MSVC{2013,2008}. It works for MSVC 2013 but fail for
MSVC 2008, I think the condition if ($proj =~
qr{ResourceCompile\s*Include=([^]+)”})” is not going to work for MSVC2008
and MSVC2005 i.e.

MSVC2013

  ResourceCompile Include=src\interfaces\ecpg\ecpglib\win32ver.rc /


MSVC2008

  File RelativePath=src\interfaces\ecpg\ecpglib\win32ver.rc /


For more details please check i.e.

 src/tools/msvc/MSBuildProject.pm (Visual C++ 2010 or greater)
 src/tools/msvc/VCBuildProject.pm (Visual C++ 2005/2008)


It seems that search criteria can be narrowed to skip reading related
Makefile for SO_MAJOR_VERSION string when VS project file is related to
StaticLibrary or Application. Although this patch is going to make MSVC
build consistent with Cygwin and MinGW build, following files seems
redundant now, is there any use for them other than backward compatibility
? i.e.

inst\lib\libpq.dll
 inst\lib\libpgtypes.dll
 inst\lib\libecpg_compat.dll
 inst\lib\libecpg.dll


Thanks.

Regards,
Muhammad Asif Naeem

On Tue, Jan 20, 2015 at 6:06 AM, Michael Paquier michael.paqu...@gmail.com
wrote:

 Hi all,

 Here is a continuation of the thread which covered $subject for MinGW
 and Cygwin:
 http://www.postgresql.org/message-id/51f19059.7050...@pgexperts.com
 But this time it is for MSVC.

 Just a bit of background... Since commit cb4a3b04 we are installing
 shared libraries in bin/ and lib/ for Cygwin and MinGW on Windows, but
 we are still missing MSVC, so as of now build method is inconsistent.
 Attached is a patch aimed at fixing this inconsistency. What it does
 is simple: when kicking Install.pm to install the deliverables of each
 project, we check if the project Makefile contains SO_MAJOR_VERSION.
 If yes, libraries of this project are installed in bin/ and lib/. The
 path to the Makefile is found by scanning ResourceCompile in the
 vcproj file, this method having the advantage to make the patch
 independent of build process.

 This also removes a wart present in Install.pm installing copying
 manually libpq.dll:
 -   lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');

 I am adding an entry in the upcoming CF for this patch, and let's use
 this thread for this discussion.
 Regards,
 --
 Michael


 --
 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] New CF app deployment

[Asif Naeem]

Hi,

This thread seems relevant, Please guide me to how can access older CF
pages e.g.

Thread
http://www.postgresql.org/message-id/flat/51f19059.7050...@pgexperts.com#51f19059.7050...@pgexperts.com
mentions
following link i.e.

The MSVC portion of this fix got completely lost in the void:
 https://commitfest.postgresql.org/action/patch_view?id=1330


Above link result in the following error i.e.

Not found
 The specified URL was not found.


Please do let me know if I missed something. Thanks.

Regards,
Muhammad Asif Naeem

On Tue, Feb 24, 2015 at 11:59 PM, Peter Eisentraut pete...@gmx.net wrote:

 On 2/22/15 3:12 PM, Magnus Hagander wrote:
  Would you suggest removing the automated system completely, or keep it
  around and just make it possible to override it (either by removing the
  note that something is a patch, or by making something that's not listed
  as a patch become marked as such)?

 I would remove it completely.  It has been demonstrated to not work.



 --
 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] Fix broken Install.bat when target directory contains a space

[Asif Naeem]

The v2 patch looks good to me, just a minor concern on usage message i.e.

C:\PG\postgresql\src\tools\msvcinstall
 Invalid command line options.
 Usage: install.bat targetdir [installtype]
 installtype: client


It seems that there are two install options i.e. client, all (any other
string other than client is being considered or treated as all), the
following install command works i.e.

install C:\PG\postgresql\inst option_does_not_exist


As your patch effects this area of code, I thought to share these findings
with you, BTW, it is a minor thing that can be handled in another patch, If
you like please feel free to change status to ready for committer. Thanks.


On Fri, Apr 17, 2015 at 10:36 AM, Michael Paquier michael.paqu...@gmail.com
 wrote:

 On Thu, Apr 16, 2015 at 5:40 PM, Asif Naeem wrote:
  Along with fixing the space in installation path, it is also changing the
  behavior of install script, why not just if NOT [%1]==[] GOTO
 RUN_INSTALL,
  checking for /? seems good for help message but it seem not well
 handled
  in the script, it is still saying Invalid command line options., along
  with this, option /? seems not handled by any other .bat build script.
  Other than this, with the patch applied, is it an acceptable behavior
 that
  (2) shows usage message as 'Usage: install.pl targetdir
 [installtype]' but
  (3) shows usage message as 'Usage: install.bat path'. Thanks.

 Thanks for your review!

 OK, let's remove the use of /? then, consistency with the other
 scripts is a good argument for its removal.Attached is an updated
 patch that does the following regarding missing arguments:
 install
 Invalid command line options.
 Usage: install.bat targetdir [installtype]
 installtype: client
 install
 Installing version 9.5 for release in /?
 Copying build output files...Could not copy release\postgres\postgres.exe
 to /?\
 bin\postgres.exe
  at Install.pm line 40.
 Install::lcopy(release\\postgres\\postgres.exe,
 /?\\bin\\postgres.exe
 ) called at Install.pm line 324
 Install::CopySolutionOutput(release, /?) called at Install.pm
 line 9
 3
 Install::Install(/?, undef) called at install.pl line 13

 This patch fixes of course the issue with spaces included in the
 target path. I updated as well the Usage in install.bat to be
 consistent with install.pl.
 Regards,
 --
 Michael

Re: [HACKERS] Fix broken Install.bat when target directory contains a space

[Asif Naeem]

Thank you Michael, latest patch looks good to me. I have changed its status
to ready for committer.

Regards,
Muhammad Asif Naeem

On Tue, Apr 21, 2015 at 6:02 PM, Michael Paquier michael.paqu...@gmail.com
wrote:



 On Tue, Apr 21, 2015 at 4:33 PM, Asif Naeem anaeem...@gmail.com wrote:

 The v2 patch looks good to me, just a minor concern on usage message i.e.

 C:\PG\postgresql\src\tools\msvcinstall
 Invalid command line options.
 Usage: install.bat targetdir [installtype]
 installtype: client


 It seems that there are two install options i.e. client, all (any other
 string other than client is being considered or treated as all), the
 following install command works i.e.

 install C:\PG\postgresql\inst option_does_not_exist


 As your patch effects this area of code, I thought to share these
 findings with you,o BTW, it is a minor thing that can be handled in another
 patch,


 Well, that's the same behavior that this script has been having for ages.
 Let's just update the usage message to mention both all and client. I
 see no point in breaking a behavior that has been like that for ages, and
 the main point of this patch is to fix the install path issue.


 If you like please feel free to change status to ready for committer.


 Well, I don't think that the patch author should do that. So I won't do it
 by myself.

 Attached is an updated patch.
 Regards,
 --
 Michael

Re: [HACKERS] Fix broken Install.bat when target directory contains a space

[Asif Naeem]

Hi Michael,

I spend spend time look into the patch. Good catch, I am also surprised to
see that current Windows install script don’t support spaces in the path.
Please see my findings as following i.e.

*Without the patch*

1.

 C:\PG\postgresql\src\tools\msvcinstall C:\PG\postgresql\inst with space
 without patch
 with was unexpected at this time.


2.

 C:\PG\postgresql\src\tools\msvcinstall
 Invalid command line options.
 Usage: install.bat path


3.

 C:\PG\postgresql\src\tools\msvcinstall /?
 Installing version 9.5 for release in /?
 Copying build output files...Could not copy release\postgres\postgres.exe
 to /?\bin\postgres.exe
 at Install.pm line 40
 Install::lcopy('release\postgres\postgres.exe', '/?\bin\postgres.exe')
 called at Install.pm line 324
 Install::CopySolutionOutput('release', '/?') called at Install.pm line 93
 Install::Install('/?', undef) called at install.pl line 13


*With the patch*

1.

 C:\PG\postgresql\src\tools\msvcinstall C:\PG\postgresql\inst with space
 without patch
 Works fine.


2.

 C:\PG\postgresql\src\tools\msvcinstall
 Usage: install.pl targetdir [installtype]
 installtype: client


3.

 C:\PG\postgresql\src\tools\msvcinstall /?
 Invalid command line options.
 Usage: install.bat path


Following change looks confusing to me i.e.

 -if NOT %1== GOTO RUN_INSTALL
 +if NOT [%1]==[/?] GOTO RUN_INSTALL


Along with fixing the space in installation path, it is also changing the
behavior of install script, why not just if NOT [%1]==[] GOTO
RUN_INSTALL, checking for /? seems good for help message but it seem not
well handled in the script, it is still saying “Invalid command line
options.”, along with this, option /? seems not handled by any other .bat
build script. Other than this, with the patch applied, is it an acceptable
behavior that (2) shows usage message as 'Usage:* install.pl
http://install.pl* targetdir [installtype]' but (3) shows usage message
as 'Usage: *install.bat* path'. Thanks.

Regards,
Muhammad Asif Naeem

On Mon, Mar 2, 2015 at 9:27 AM, Michael Paquier michael.paqu...@gmail.com
wrote:

 Hi all,

 When using install.bat with a path containing spaces, I got surprised
 by a couple of errors.
 1) First with this path:
 $ install c:\Program Files\pgsql
 I am getting the following error:
 Files\pgsql== was unexpected at this time.
 This is caused by an incorrect evaluation of the first parameter in
 install.bat.
 2) After correcting the first error, the path is truncated to
 c:\Program because first argument value does not seem to be correctly
 parsed when used with install.pl.

 Attached is a patch fixing both problems. I imagine that it would be
 good to get that backpatched.
 Regards,
 --
 Michael


 --
 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] 9.5: Can't connect with PGSSLMODE=require on Windows

[Asif Naeem]

I have spent sometime to investigate the issue, it is reproduciable. In
case of Windows, when pqsecure_raw_read() function error code
WSAEWOULDBLOCK (EWOULDBLOCK) when no data queued to be read from the non
blocking socket there is a need to log retry flag. Related error code can
be retrieved via Windows WSAGetLastError() instead of errno, preprocessor
SOCK_ERRNO handle it gracefully. PFA patch, it resolve the issue i.e.

C:\PG\postgresql\pg_with_openssl_inst_v1_patch>bin\psql.exe -d postgres -h
>  172.16.141.210
> psql (9.5alpha2)
> WARNING: Console code page (437) differs from Windows code page (1252)
>  8-bit characters might not work correctly. See psql reference
>  page "Notes for Windows users" for details.
> SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384,
> bits: 256, compression: off)
> Type "help" for help.
> postgres=# select version();
>  version
> -
>  PostgreSQL 9.5alpha2, compiled by Visual C++ build 1800, 64-bit
> (1 row)


Regards,
Muhammad Asif Naeem


On Thu, Sep 24, 2015 at 5:12 PM, Thom Brown <t...@linux.com> wrote:

> On 23 September 2015 at 13:10, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
> >
> >
> > On Wed, Sep 23, 2015 at 2:15 AM, Robert Haas <robertmh...@gmail.com>
> wrote:
> >>
> >> On Tue, Sep 22, 2015 at 11:23 AM, Andrew Dunstan <and...@dunslane.net>
> >> wrote:
> >> > "git bisect" is your friend.
> >>
> >> Yeah, but finding someone who has a working Windows build environment
> >> and a lot of time to run this down is my enemy.  We're trying, but if
> >> anyone else has a clue, that would be much appreciated.
> >
> >
> > That's not cool. I have added this problem in the list of open items for
> > 9.5.
>
> This appears that it might be related to the version of OpenSSL that's
> been packaged with PostgreSQL 9.5 alpha 2.  When swapping this out for
> the version that's shipped with 9.4, it works.  I don't have the
> specific OpenSSL versions to hand, but I'll report back anything as I
> learn more.
>
> --
> Thom
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


win_ssl_issue_v1.patch
Description: Binary data

-- 
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] 9.5: Can't connect with PGSSLMODE=require on Windows

[Asif Naeem]

Thank you Tom. The issue seems not reproducible anymore with latest PG95
source code (commit 60fcee9e5e77dc748a9787fae34328917683b95e) Windows build
i.e.

C:\PG\postgresql\pg95_with_openssl>bin\psql.exe -d postgres -h
> 172.16.141.232
> psql (9.5alpha2)
> WARNING: Console code page (437) differs from Windows code page (1252)
>  8-bit characters might not work correctly. See psql reference
>  page "Notes for Windows users" for details.
> SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384,
> bits: 256, compression: off)
> Type "help" for help.
> postgres=# select version();
>  version
> -
>  PostgreSQL 9.5alpha2, compiled by Visual C++ build 1800, 64-bit
> (1 row)


Regards,
Muhammad Asif Naeem

On Tue, Sep 29, 2015 at 3:03 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:

> Thom Brown <t...@linux.com> writes:
> > With 9.5 alpha 2 on Windows 8 (64-bit), trying to require SSL results
> > in a blocking error:
>
> I've pushed a patch for this; can you verify it on Windows?
>
> regards, tom lane
>
>
> --
> 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] Truncating/vacuuming relations on full tablespaces

[Asif Naeem]

Thank you for useful suggestions. PFA patch, I have tried to cover all the
points mentioned.

Regards,
Muhammad Asif Naeem

On Wed, Apr 6, 2016 at 9:06 PM, Robert Haas <robertmh...@gmail.com> wrote:

> On Wed, Apr 6, 2016 at 3:32 AM, Asif Naeem <anaeem...@gmail.com> wrote:
> >> Oh, I see.  I think it's probably not a good idea to skip truncating
> >> those maps, but perhaps the option could be defined as making no new
> >> entries, rather than ignoring them altogether, so that they never
> >> grow.  It seems that both of those are defined in such a way that if
> >> page Y follows page X in the heap, the entries for page Y in the
> >> relation fork will follow the one for page X.  So truncating them
> >> should be OK; it's just growing them that we need to avoid.
> >
> > Thank you Robert. PFA basic patch, it introduces EMERGENCY option to
> VACUUM
> > that forces to avoid extend any entries in the VM or FSM. It seems
> working
> > fine in simple test scenarios e.g.
> >
> >> postgres=# create table test1 as (select generate_series(1,10));
> >> SELECT 10
> >> postgres=# vacuum  EMERGENCY test1;
> >> VACUUM
> >> postgres=# select pg_relation_filepath('test1');
> >>  pg_relation_filepath
> >> --
> >>  base/13250/16384
> >> (1 row)
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> postgres=# vacuum test1;
> >> VACUUM
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> ./data/base/13250/16384_fsm
> >> ./data/base/13250/16384_vm
> >
> >
> > Please do let me know if I missed something or more information is
> required.
> > Thanks.
>
> This is too late for 9.6 at this point and certainly requires
> discussion anyway, so please add it to the next CommitFest.  But in
> the meantime, here are a few quick comments:
>
> You should only support EMERGENCY using the new-style, parenthesized
> options syntax.  That way, you won't need to make EMERGENCY a keyword.
> We don't want to do that, and we especially don't want it to be
> partially reserved, as you have done here.
>
> Passing the EMERGENCY flag around in a global variable is probably not
> a good idea; use parameter lists.  That's what they are for.  Also,
> calling the variable that decides whether EMERGENCY was set
> Extend_VM_FSM is confusing.
>
> I see why you changed the calling convention for visibilitymap_pin()
> and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder
> if there's a better way to do that, like maybe having vacuumlazy.c ask
> the VM and FSM for their length in pages and then not trying to use
> those functions for block numbers that are too large.
>
> Don't forget to update the documentation.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


VACUUM_EMERGENCY_Option_v2.patch
Description: Binary data

-- 
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] Truncating/vacuuming relations on full tablespaces

[Asif Naeem]

On Tue, Jan 19, 2016 at 2:04 AM, Robert Haas <robertmh...@gmail.com> wrote:

> On Mon, Jan 18, 2016 at 2:26 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> > Robert Haas <robertmh...@gmail.com> writes:
> >> On Fri, Jan 15, 2016 at 2:16 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> >>> Presumably the hope would be that VACUUM would truncate off some of the
> >>> heap, else there's not much good that's going to happen.  That leaves
> >>> me wondering exactly what the invariant is for the maps, and if it's
> >>> okay to not touch them during a heap truncation.
> >
> >> No, you're missing the point, or at least I think you are.  Suppose
> >> somebody creates a big table and then deletes all the tuples in the
> >> second half, but VACUUM never runs.  When at last VACUUM does run on
> >> that table, it will try to build the VM and FSM forks as it scans the
> >> table, and will only truncate AFTER that has been done.  If building
> >> the VM and FSM forks fails because there is no freespace, we will
> >> never reach the part of the operation that could create some.
> >
> > No, I follow that perfectly.  I think you missed *my* point, which is:
> > suppose that we do have a full-length VM and/or FSM fork for a relation,
> > and VACUUM decides to truncate the relation.  Is it okay to not truncate
> > the VM/FSM?  If it isn't, we're going to have to have very tricky
> > semantics for any "don't touch the map forks" option, because it will
> > have to suppress only some of VACUUM's map updates.
>
> Oh, I see.  I think it's probably not a good idea to skip truncating
> those maps, but perhaps the option could be defined as making no new
> entries, rather than ignoring them altogether, so that they never
> grow.  It seems that both of those are defined in such a way that if
> page Y follows page X in the heap, the entries for page Y in the
> relation fork will follow the one for page X.  So truncating them
> should be OK; it's just growing them that we need to avoid.
>

Thank you Robert. PFA basic patch, it introduces EMERGENCY option to VACUUM
that forces to avoid extend any entries in the VM or FSM. It seems working
fine in simple test scenarios e.g.

postgres=# create table test1 as (select generate_series(1,10));
> SELECT 10
> postgres=# vacuum  EMERGENCY test1;
> VACUUM
> postgres=# select pg_relation_filepath('test1');
>  pg_relation_filepath
> --
>  base/13250/16384
> (1 row)
> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> ./data/base/13250/16384
> postgres=# vacuum test1;
> VACUUM
> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> ./data/base/13250/16384
> ./data/base/13250/16384_fsm
> ./data/base/13250/16384_vm


Please do let me know if I missed something or more information is
required. Thanks.

Regards,
Muhammad Asif Naeem


> > An alternative approach that might avoid such worries is to have a mode
> > wherein VACUUM always truncates the map forks to nothing, rather than
> > attempting to update them.
>
> That could work, too, but might be stronger medicine than needed.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


VACUUM_EMERGENCY_Option_v1.patch
Description: Binary data

-- 
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] Truncating/vacuuming relations on full tablespaces

[Asif Naeem]

On Thu, Apr 7, 2016 at 2:15 AM, Jim Nasby  wrote:

> On 4/6/16 11:06 AM, Robert Haas wrote:
>
>> This is too late for 9.6 at this point and certainly requires
>> discussion anyway, so please add it to the next CommitFest.
>>
>
> If the goal here is to free up space via truncation when there's a real
> emergency, perhaps there's some other steps that should be taken as well.
> What immediately comes to mind is scanning the heap backwards and stopping
> on the first page we can't truncate.
>
> There might be some other non-critical things we could skip in emergency
> mode, with an eye towards making it as fast as possible.
>
> BTW, if someone really wanted to put some effort into this, it would be
> possible to better handle filling up a single filesystem that has both data
> and WAL by slowly shrinking the VM/FSM to make room in the WAL for vacuum
> records. ISTM that's a much more common problem people run into than
> filling up a separate tablespace.


Thank you Jim. I will look into it and share my findings about it.


> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

Re: [HACKERS] Truncating/vacuuming relations on full tablespaces

[Asif Naeem]

Thank you Robert. Sure, I will add the updated patch on the next CommitFest
with all the suggested changes.

On Wed, Apr 6, 2016 at 9:06 PM, Robert Haas <robertmh...@gmail.com> wrote:

> On Wed, Apr 6, 2016 at 3:32 AM, Asif Naeem <anaeem...@gmail.com> wrote:
> >> Oh, I see.  I think it's probably not a good idea to skip truncating
> >> those maps, but perhaps the option could be defined as making no new
> >> entries, rather than ignoring them altogether, so that they never
> >> grow.  It seems that both of those are defined in such a way that if
> >> page Y follows page X in the heap, the entries for page Y in the
> >> relation fork will follow the one for page X.  So truncating them
> >> should be OK; it's just growing them that we need to avoid.
> >
> > Thank you Robert. PFA basic patch, it introduces EMERGENCY option to
> VACUUM
> > that forces to avoid extend any entries in the VM or FSM. It seems
> working
> > fine in simple test scenarios e.g.
> >
> >> postgres=# create table test1 as (select generate_series(1,10));
> >> SELECT 10
> >> postgres=# vacuum  EMERGENCY test1;
> >> VACUUM
> >> postgres=# select pg_relation_filepath('test1');
> >>  pg_relation_filepath
> >> --
> >>  base/13250/16384
> >> (1 row)
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> postgres=# vacuum test1;
> >> VACUUM
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> ./data/base/13250/16384_fsm
> >> ./data/base/13250/16384_vm
> >
> >
> > Please do let me know if I missed something or more information is
> required.
> > Thanks.
>
> This is too late for 9.6 at this point and certainly requires
> discussion anyway, so please add it to the next CommitFest.  But in
> the meantime, here are a few quick comments:
>
> You should only support EMERGENCY using the new-style, parenthesized
> options syntax.  That way, you won't need to make EMERGENCY a keyword.
> We don't want to do that, and we especially don't want it to be
> partially reserved, as you have done here.
>
> Passing the EMERGENCY flag around in a global variable is probably not
> a good idea; use parameter lists.  That's what they are for.  Also,
> calling the variable that decides whether EMERGENCY was set
> Extend_VM_FSM is confusing.
>
> I see why you changed the calling convention for visibilitymap_pin()
> and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder
> if there's a better way to do that, like maybe having vacuumlazy.c ask
> the VM and FSM for their length in pages and then not trying to use
> those functions for block numbers that are too large.
>
> Don't forget to update the documentation.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] Truncating/vacuuming relations on full tablespaces

[Asif Naeem]

On Wed, Apr 6, 2016 at 9:06 PM, Robert Haas <robertmh...@gmail.com> wrote:

> On Wed, Apr 6, 2016 at 3:32 AM, Asif Naeem <anaeem...@gmail.com> wrote:
> >> Oh, I see.  I think it's probably not a good idea to skip truncating
> >> those maps, but perhaps the option could be defined as making no new
> >> entries, rather than ignoring them altogether, so that they never
> >> grow.  It seems that both of those are defined in such a way that if
> >> page Y follows page X in the heap, the entries for page Y in the
> >> relation fork will follow the one for page X.  So truncating them
> >> should be OK; it's just growing them that we need to avoid.
> >
> > Thank you Robert. PFA basic patch, it introduces EMERGENCY option to
> VACUUM
> > that forces to avoid extend any entries in the VM or FSM. It seems
> working
> > fine in simple test scenarios e.g.
> >
> >> postgres=# create table test1 as (select generate_series(1,10));
> >> SELECT 10
> >> postgres=# vacuum  EMERGENCY test1;
> >> VACUUM
> >> postgres=# select pg_relation_filepath('test1');
> >>  pg_relation_filepath
> >> --
> >>  base/13250/16384
> >> (1 row)
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> postgres=# vacuum test1;
> >> VACUUM
> >> [asif@centos66 inst_96]$ find . | grep base/13250/16384
> >> ./data/base/13250/16384
> >> ./data/base/13250/16384_fsm
> >> ./data/base/13250/16384_vm
> >
> >
> > Please do let me know if I missed something or more information is
> required.
> > Thanks.
>
> This is too late for 9.6 at this point and certainly requires
> discussion anyway, so please add it to the next CommitFest.  But in
> the meantime, here are a few quick comments:
>

Sure. Sorry for delay caused.


> You should only support EMERGENCY using the new-style, parenthesized
> options syntax.  That way, you won't need to make EMERGENCY a keyword.
> We don't want to do that, and we especially don't want it to be
> partially reserved, as you have done here.
>

Sure. I have removed EMERGENCY keyword, it made code lot small now i.e.

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
> index b9aeb31..89c4ee0 100644
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
> @@ -9298,6 +9298,20 @@ vacuum_option_elem:
> | VERBOSE   { $$ =
> VACOPT_VERBOSE; }
> | FREEZE{ $$ =
> VACOPT_FREEZE; }
> | FULL  { $$ =
> VACOPT_FULL; }
> +   | IDENT
> +   {
> +   /*
> +* We handle identifiers that
> aren't parser keywords with
> +* the following special-case
> codes.
> +*/
> +   if (strcmp($1, "emergency") == 0)
> +   $$ = VACOPT_EMERGENCY;
> +   else
> +   ereport(ERROR,
> +
> (errcode(ERRCODE_SYNTAX_ERROR),
> +
>  errmsg("unrecognized vacuum option \"%s\"", $1),
> +
>  parser_errposition(@1)));
> +   }
> ;
>
>  AnalyzeStmt:


Passing the EMERGENCY flag around in a global variable is probably not
> a good idea; use parameter lists.  That's what they are for.  Also,
> calling the variable that decides whether EMERGENCY was set
> Extend_VM_FSM is confusing.
>

Sure. I adopted this approach by find other similar cases in the same
source code file i.e.

src/backend/commands/vacuumlazy.c

> /* A few variables that don't seem worth passing around as parameters */
> static int elevel = -1;
> static TransactionId OldestXmin;
> static TransactionId FreezeLimit;
> static MultiXactId MultiXactCutoff;
> static BufferAccessStrategy vac_strategy;


Should I modify code to use parameter lists for these variables too ?

I see why you changed the calling convention for visibilitymap_pin()
> and RecordPageWithFreeSpace(), but that's awfully invasive.  I wonder
> if there's a better way to do that, like maybe having vacuumlazy.c ask
> the VM and FSM for their length in pages and then not trying to use
> those functions for block numbers that are too large.
>
> Don't forget to update the documentation.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

[Asif Naeem]

It make sense. I would like to share more comments as following i.e.

static int
> bf_check_supported_key_len(void)
> {
> ...
>  /* encrypt with 448bits key and verify output */
>  evp_ctx = EVP_CIPHER_CTX_new();
>  if (!evp_ctx)
>   return 1;
>  if (!EVP_EncryptInit_ex(evp_ctx, EVP_bf_ecb(), NULL, NULL, NULL))
>   goto leave;
>  if (!EVP_CIPHER_CTX_set_key_length(evp_ctx, 56))
>   goto leave;
>  if (!EVP_EncryptInit_ex(evp_ctx, NULL, NULL, key, NULL))
>   goto leave;
>  if (!EVP_EncryptUpdate(evp_ctx, out, , data, 8))
>   goto leave;
>  if (memcmp(out, res, 8) != 0)
>   goto leave;/* Output does not match ->
> strong cipher is
>  * not supported */
>  status = 1;
> leave:
>  EVP_CIPHER_CTX_free(evp_ctx);
>  return status;
> }


It seems that it need to return 0 instead of 1 in case of failure i.e.

 /* encrypt with 448bits key and verify output */
>  evp_ctx = EVP_CIPHER_CTX_new();
>  if (!evp_ctx)
>   return 0;


We can avoid multiple if conditions and goto statement something like i.e.

 if (EVP_EncryptInit_ex(evp_ctx, EVP_bf_ecb(), NULL, NULL, NULL) &&
>  EVP_CIPHER_CTX_set_key_length(evp_ctx, 56) &&
>  EVP_EncryptInit_ex(evp_ctx, NULL, NULL, key, NULL) &&
>  EVP_EncryptUpdate(evp_ctx, out, , data, 8) &&
>  memcmp(out, res, 8) == 0 )) /* Output does not match -> strong
> cipher is not supported */
>  status = 1;
>  EVP_CIPHER_CTX_free(evp_ctx);
>  return status;
> }


What is your opinion ?. I am hopeful I will be able to share all my
findings tomorrow. Thanks.


On Wed, Dec 7, 2016 at 2:23 AM, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Tue, Dec 6, 2016 at 11:42 PM, Asif Naeem <anaeem...@gmail.com> wrote:
> > Thanks for updated patch. Although EVP_CIPHER_CTX_cleanup() seems
> deprecated
> > in OpenSSL >= 1.1.0 i.e.
> >
> >> # if OPENSSL_API_COMPAT < 0x1010L
> >> #  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
> >> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
> >> # endif
> >
> >
> > I guess use of deprecated function is fine, until OpenSSL library support
> > it.
>
> We could use some ifdef block with the OpenSSL version number, but I
> am not sure if that's worth complicating the code at this stage.
> --
> Michael
>

Re: [HACKERS] pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

[Asif Naeem]

Thanks for updated patch. Although EVP_CIPHER_CTX_cleanup() seems deprecated in
OpenSSL >= 1.1.0 i.e.

# if OPENSSL_API_COMPAT < 0x1010L
> #  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
> # endif


I guess use of deprecated function is fine, until OpenSSL library support
it.



On Tue, Dec 6, 2016 at 6:15 PM, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Tue, Dec 6, 2016 at 9:31 PM, Asif Naeem <anaeem...@gmail.com> wrote:
> > Thank you for v2 patch, I would like to comment on it. It seems that you
> > have used function EVP_CIPHER_CTX_reset in the patch that was introduced
> in
> > OpenSSL 1.1.0, older library version might not work now, is it
> intentional
> > change ?.
>
> I thought I tested that... But yes, that would not compile when linked
> with 1.0.2 or older. Using EVP_CIPHER_CTX_cleanup() is safe instead as
> that's available down to 0.9.8.
> --
> Michael
>

Re: [HACKERS] pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

[Asif Naeem]

Hi Michael,

Thank you for v2 patch, I would like to comment on it. It seems that you
have used function EVP_CIPHER_CTX_reset in the patch that was introduced in
OpenSSL 1.1.0, older library version might not work now, is it intentional
change ?.

Regards,
Muhammad Asif Naeem

On Tue, Dec 6, 2016 at 12:15 PM, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Mon, Dec 5, 2016 at 6:09 PM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
> > On Mon, Dec 5, 2016 at 5:11 PM, Heikki Linnakangas <hlinn...@iki.fi>
> wrote:
> >> I'm afraid if we just start using EVP_CIPHER_CTX_new(), we'll leak the
> >> context on any error. We had exactly the same problem with
> EVP_MD_CTX_init
> >> being removed, in the patch that added OpenSSL 1.1.0 support. We'll
> have to
> >> use a resource owner to track it, just like we did with EVP_MD_CTX in
> commit
> >> 593d4e47. Want to do that, or should I?
> >
> > I'll send a patch within 24 hours.
>
> And within the delay, attached is the promised patch.
>
> While testing with a linked list, I have found out that the linked
> list could leak with cases like that, where decryption and encryption
> are done in a single transaction;
> select pgp_sym_decrypt(pgp_sym_encrypt(repeat('x',65530),'1'),'1') =
> repeat('x',65530);
>
> What has taken me a bit of time was to figure out that this bit is
> needed to free correctly elements in the open list:
> @@ -219,6 +220,8 @@ encrypt_free(void *priv)
>  {
> struct EncStat *st = priv;
>
> +   if (st->ciph)
> +   pgp_cfb_free(st->ciph);
> px_memset(st, 0, sizeof(*st));
> px_free(st);
>  }
> This does not matter on back-branches as things get cleaned up once
> the transaction memory context gets freed.
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>