Re: [HACKERS] includedir_internal headers are not self-contained

2014-04-29 Thread Heikki Linnakangas

On 04/28/2014 10:32 PM, Tom Lane wrote:

Robert Haas robertmh...@gmail.com writes:

I have to admit it's been a few years since I've had to play with
WAL_DEBUG, so I don't really remember what I was trying to do.  But I
don't see any real argument that three slash-separated numbers will be
more useful to somebody who has to dig through this than a pathname,
even an approximate pathname, and I think people wanting to figure out
approximately where they need to look to find the data affected by the
WAL record will be pretty common among people decoding WAL records.


Meh.  I still think it's a bad idea to have CATALOG_VERSION_NO getting
compiled into libpgcommon.a, where there will be no way to cross-check
that it matches anything.  But I guess I'm losing this argument.


FWIW, I agree it's a bad idea. I just have no better ideas (and haven't 
given it much thought anyway).


- Heikki


--
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] Proposal for Merge Join for Non '=' Operators

2014-04-29 Thread Hadi Moshayedi
Hello Dilip,

Query: select count(*) from t1,t2 where t1.bt2.b and t1.b  12000;

 Test Result:
 Nest Loop Join with Index Scan   : 1653.506 ms
 Sort Merge Join for (seq scan)   : 610.257ms


This looks like a great improvement. Repeating Nicolas's question, do you
have a real-world example of such joins?

In my experience, I see more queries like self-join table A and table B
where A.time BETWEEN B.time - '1 week' and B.time, similar to
what Nicolas and Tom mentioned. As an example, count users who placed an
order in the week following their registration.

Can you send a patch so we can also try it?

Thanks,
  -- Hadi


[HACKERS] using array of char pointers gives wrong results

2014-04-29 Thread Ashutosh Bapat
Hi,
When array of char * is used as target for the FETCH statement returning
more than one row, it tries to store all the result in the first element.
PFA test_char_select.pgc, which fetches first 3 relnames from pg_class
ordered by relname. The program prints following result

steps to compile and build the program
ecpg -c -Iecpg_include_dir test_char_select.pgc
cc -Ipg installation include dir -g   -c -o test_char_select.o
test_char_select.c
cc -g  test_char_select.o  -Lpg installation lib dir -lecpg -lpq
-lpgtypes -o test_char_select

output
./test_char_select
relname=___pg_foreign_table_columns
relname=
relname=

The first three relnames should have been
postgres=# select relname from pg_class order by relname limit 3;
  relname
---
 _pg_foreign_data_wrappers
 _pg_foreign_servers
 _pg_foreign_table_columns

It's obvious that the first element of the array is being overwritten with
an offset of 1.

This happens because, the array of char pointer is dumped as
 /* Fetch multiple columns into one structure. */
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, fetch 3 from cur1,
ECPGt_EOIT,
ECPGt_char,(strings),(long)0,(long)3,*(1)*sizeof(char)*,
ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EORT);

Since the offset is 1, the next result overwrites the previous result
except for the first byte.

PFA patch ecpg_char_ptr_arr.patch to fix this issue. It has changes as
follows
1. Dump array of char pointer with right offset i.e. sizeof(char *)
2. While reading array of char pointer in ecpg_do_prologue(), use the
address instead of the value at that address
3. The pointer arithmetic should treat such variable as char **, instead of
char *

ECPG regression tests do not show any failures with this patch.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c
index 5f9a3d4..3ec774c 100644
--- a/src/interfaces/ecpg/ecpglib/data.c
+++ b/src/interfaces/ecpg/ecpglib/data.c
@@ -448,20 +448,28 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno,
 			   ECPG_SQLSTATE_DATATYPE_MISMATCH, pval);
 	return (false);
 	break;
 
 case ECPGt_char:
 case ECPGt_unsigned_char:
 case ECPGt_string:
 	{
 		char	   *str = (char *) (var + offset * act_tuple);
 
+		/*
+		 * If varcharsize is unknown and the offset is that of
+		 * char *, then this variable represents the array of
+		 * character pointers. So, use extra indirection.
+		 */
+		if (varcharsize == 0  offset == sizeof(char *))
+			str = *(char **)str;
+
 		if (varcharsize == 0 || varcharsize  size)
 		{
 			strncpy(str, pval, size + 1);
 			/* do the rtrim() */
 			if (type == ECPGt_string)
 			{
 char	   *last = str + size;
 
 while (last  str  (*last == ' ' || *last == '\0'))
 {
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index a90fb41..aa917b9 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -1923,25 +1923,33 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
 return false;
 			}
 
 			var-type = type;
 			var-pointer = va_arg(args, char *);
 
 			var-varcharsize = va_arg(args, long);
 			var-arrsize = va_arg(args, long);
 			var-offset = va_arg(args, long);
 
-			if (var-arrsize == 0 || var-varcharsize == 0)
+			if ((var-arrsize == 0 || var-varcharsize == 0))
 var-value = *((char **) (var-pointer));
 			else
 var-value = var-pointer;
 
+			/* 
+			 * If the type is character without known varcharsize but with known array
+			 * size, it is an array of pointers to char, so use the pointer as
+			 * it is.
+			 */
+			if ((var-type == ECPGt_char || type == ECPGt_unsigned_char) 
+	var-arrsize  1  var-varcharsize == 0)
+var-value = var-pointer;
 			/*
 			 * negative values are used to indicate an array without given
 			 * bounds
 			 */
 			/* reset to zero for us */
 			if (var-arrsize  0)
 var-arrsize = 0;
 			if (var-varcharsize  0)
 var-varcharsize = 0;
 
diff --git a/src/interfaces/ecpg/preproc/type.c b/src/interfaces/ecpg/preproc/type.c
index 308660e..e54f05d5 100644
--- a/src/interfaces/ecpg/preproc/type.c
+++ b/src/interfaces/ecpg/preproc/type.c
@@ -441,36 +441,49 @@ ECPGdump_a_simple(FILE *o, const char *name, enum ECPGttype type,
  */
 if (counter)
 	sprintf(offset, sizeof(struct varchar_%d), counter);
 else
 	sprintf(offset, sizeof(struct varchar));
 break;
 			case ECPGt_char:
 			case ECPGt_unsigned_char:
 			case ECPGt_char_variable:
 			case ECPGt_string:
-
+			{
+char	*sizeof_name = char;
 /*
  * we have to use the pointer except for arrays with given
  * bounds, ecpglib will distinguish between * and []
  */
 if ((atoi(varcharsize)  1 ||
 	 (atoi(arrsize)  0) ||
  

[HACKERS] Small doc patch for pg_replication_slots

2014-04-29 Thread Thomas Reiss
Hello,

You can find attached a small patch to fix the pg_replication_slots
documentation. The slot_type and plugin are not in the appropriate
order, slot_name and plugin have a wrong type and xmin appears two times.

Regards
-- 
Thomas Reiss

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 415a3bc..d8c41d9 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5252,23 +5252,23 @@
 tbody
  row
   entrystructfieldslot_name/structfield/entry
-  entrytypetext/type/entry
+  entrytypename/type/entry
   entry/entry
   entryA unique, cluster-wide identifier for the replication slot/entry
  /row
 
  row
-  entrystructfieldslot_type/structfield/entry
-  entrytypetext/type/entry
+  entrystructfieldplugin/structfield/entry
+  entrytypename/type/entry
   entry/entry
-  entryThe slot type - literalphysical/ or literallogical//entry
+  entryThe basename of the shared object containing the output plugin this logical slot is using, or null for physical slots./entry
  /row
 
  row
-  entrystructfieldplugin/structfield/entry
+  entrystructfieldslot_type/structfield/entry
   entrytypetext/type/entry
   entry/entry
-  entryThe basename of the shared object containing the output plugin this logical slot is using, or null for physical slots./entry
+  entryThe slot type - literalphysical/ or literallogical//entry
  /row
 
  row
@@ -5305,7 +5305,7 @@
  /row
 
  row
-  entrystructfieldxmin/structfield/entry
+  entrystructfieldcatalog_xmin/structfield/entry
   entrytypexid/type/entry
   entry/entry
   entryThe oldest transaction that this slot needs the database to
@@ -5315,14 +5315,6 @@
  /row
 
  row
-  entrystructfieldcatalog_xmin/structfield/entry
-  entrytypexid/type/entry
-  entry/entry
-  entryThe literalxmin/literal, or oldest transaction ID, that this
-  slot forces to be retained in the system catalogs. /entry
- /row
-
- row
   entrystructfieldrestart_lsn/structfield/entry
   entrytypepg_lsn/type/entry
   entry/entry

-- 
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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Bernd Helmle



--On 26. April 2014 19:42:47 -0700 Peter Geoghegan p...@heroku.com wrote:


I suggest removing it for 9.5, and instead logging individual
occurrences of backend fsync requests within ForwardFsyncRequest(). It
seems fair to treat that as an anomaly to draw particular attention
to.


But wouldn't that make it more complicated/unlikely to discover cases, 
where it still doesn't work?


--
Thanks

Bernd


--
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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Peter Geoghegan
On Tue, Apr 29, 2014 at 2:51 AM, Bernd Helmle maili...@oopsware.de wrote:
 I suggest removing it for 9.5, and instead logging individual
 occurrences of backend fsync requests within ForwardFsyncRequest(). It
 seems fair to treat that as an anomaly to draw particular attention
 to.

 But wouldn't that make it more complicated/unlikely to discover cases, where
 it still doesn't work?

I don't think so, no.


-- 
Peter Geoghegan


-- 
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] Problem with displaying wide tables in psql

2014-04-29 Thread Greg Stark
On Tue, Apr 29, 2014 at 2:52 AM, Peter Eisentraut pete...@gmx.net wrote:
 Please fix this compiler warning.  I think it came from this patch.


Oops, I fixed it in a previous version but didn't notice it had crept
back in in the back-and-forth. Will do.

-- 
greg


-- 
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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Robert Haas
On Tue, Apr 29, 2014 at 5:54 AM, Peter Geoghegan p...@heroku.com wrote:
 On Tue, Apr 29, 2014 at 2:51 AM, Bernd Helmle maili...@oopsware.de wrote:
 I suggest removing it for 9.5, and instead logging individual
 occurrences of backend fsync requests within ForwardFsyncRequest(). It
 seems fair to treat that as an anomaly to draw particular attention
 to.

 But wouldn't that make it more complicated/unlikely to discover cases, where
 it still doesn't work?

 I don't think so, no.

I think it just depends.  For people who are running a log scraper
anyway, a message would be better than a statistics counter, because
it's one less thing to check.  For people who are running something
that monitors the stats views anyway, but perhaps not a log scraper,
the counter is better.

Overall, I don't see much reason to tinker with this.  If we had no
reporting at all of this condition now, I'd probably be mildly more
supportive of adding a log message than a counter.  But since we've
already got something and there's no real problem with it, I'm
disinclined to make a change.

-- 
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


Re: [HACKERS] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Overall, I don't see much reason to tinker with this.  If we had no
 reporting at all of this condition now, I'd probably be mildly more
 supportive of adding a log message than a counter.  But since we've
 already got something and there's no real problem with it, I'm
 disinclined to make a change.

+1 ... if it ain't broke, why fix it?

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] pg_dump --pretty-print-views

2014-04-29 Thread Keith Fiske
On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Marko Tiikkaja pgm...@joh.to writes:
  Here's the third version of this patch, hopefully this time without any
  problems.  I looked through the patch and it looked OK, but I did that
  last time too so I wouldn't trust myself on that one.

 Applied with corrections.

 The xml expected output was still wrong - to do that part right, you
 need to update xml.out with an xml-enabled build and xml_1.out with a
 non-xml-enabled build.

 Also, it seemed to me that the patch didn't go far enough, in that it
 only touched pg_get_viewdef and not the sister functions.  pg_dump would
 certainly want pg_get_ruledef to have the same behavior, and in general
 the documentation seems to me to be clear that all these functions have
 similar pretty-print-vs-not behavior.  As committed, the pretty_bool
 argument only affects PRETTY_PAREN processing for all of them.

 I also went ahead and set the default wrap column to zero rather than
 the former 79, since it seemed clear that people like that behavior
 better.

 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



Was this ever committed into core? Apologies, I'm not very familiar with
looking through the commit history of the source code and I don't see
anything about this option or pretty-print outputs in the pg_dump/restore
docs for 9.3. Had someone asking me about this feature for pg_extractor

https://github.com/omniti-labs/pg_extractor/issues/28

--
Keith Fiske
Database Administrator
OmniTI Computer Consulting, Inc.
http://www.keithf4.com


Re: [HACKERS] pg_dump --pretty-print-views

2014-04-29 Thread Marko Tiikkaja

On 4/29/14 4:29 PM, Keith Fiske wrote:

Was this ever committed into core? Apologies, I'm not very familiar with
looking through the commit history of the source code and I don't see
anything about this option or pretty-print outputs in the pg_dump/restore
docs for 9.3. Had someone asking me about this feature for pg_extractor


No, the idea was rejected.


Regards,
Marko Tiikkaja


--
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] includedir_internal headers are not self-contained

2014-04-29 Thread Andrew Dunstan


On 04/29/2014 02:56 AM, Heikki Linnakangas wrote:

On 04/28/2014 10:32 PM, Tom Lane wrote:

Robert Haas robertmh...@gmail.com writes:

I have to admit it's been a few years since I've had to play with
WAL_DEBUG, so I don't really remember what I was trying to do.  But I
don't see any real argument that three slash-separated numbers will be
more useful to somebody who has to dig through this than a pathname,
even an approximate pathname, and I think people wanting to figure out
approximately where they need to look to find the data affected by the
WAL record will be pretty common among people decoding WAL records.


Meh.  I still think it's a bad idea to have CATALOG_VERSION_NO getting
compiled into libpgcommon.a, where there will be no way to cross-check
that it matches anything.  But I guess I'm losing this argument.


FWIW, I agree it's a bad idea. I just have no better ideas (and 
haven't given it much thought anyway).





Sure sounds like a bad idea.

cheers

andrew



--
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] includedir_internal headers are not self-contained

2014-04-29 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 04/29/2014 02:56 AM, Heikki Linnakangas wrote:
 On 04/28/2014 10:32 PM, Tom Lane wrote:
 Meh.  I still think it's a bad idea to have CATALOG_VERSION_NO getting
 compiled into libpgcommon.a, where there will be no way to cross-check
 that it matches anything.  But I guess I'm losing this argument.

 FWIW, I agree it's a bad idea. I just have no better ideas (and 
 haven't given it much thought anyway).

 Sure sounds like a bad idea.

One idea would be to have relpath.h/.c expose a function (not a #define)
that returns the value of CATALOG_VERSION_NO compiled into it.  Then,
if pg_rewind were to replace all its direct references to
CATALOG_VERSION_NO (including the value it checks against pg_control)
with calls of that function, consistency would be ensured.

A notational problem is that if pg_rewind or similar program is directly
using the TABLESPACE_VERSION_DIRECTORY macro anywhere, this wouldn't
work.  But we could perhaps expose a function to return that string too.

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] pg_dump --pretty-print-views

2014-04-29 Thread Tom Lane
Keith Fiske ke...@omniti.com writes:
 On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Applied with corrections.

 Was this ever committed into core? Apologies, I'm not very familiar with
 looking through the commit history of the source code and I don't see
 anything about this option or pretty-print outputs in the pg_dump/restore
 docs for 9.3. Had someone asking me about this feature for pg_extractor

Yeah, if I say applied that means I committed it.  Here's the commit
log entry:

Author: Tom Lane t...@sss.pgh.pa.us
Branch: master Release: REL9_3_BR [62e666400] 2013-02-03 15:56:45 -0500

Perform line wrapping and indenting by default in ruleutils.c.

This patch changes pg_get_viewdef() and allied functions so that
PRETTY_INDENT processing is always enabled.  Per discussion, only the
PRETTY_PAREN processing (that is, stripping of unnecessary parentheses)
poses any real forward-compatibility risk, so we may as well make dump
output look as nice as we safely can.

Also, set the default wrap length to zero (i.e, wrap after each SELECT
or FROM list item), since there's no very principled argument for the
former default of 80-column wrapping, and most people seem to agree this
way looks better.

Marko Tiikkaja, reviewed by Jeevan Chalke, further hacking by Tom Lane

As per the branch annotation, this is in 9.3 and up.

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


[HACKERS] What about a castNode() macro?

2014-04-29 Thread Andres Freund
Hi,

There's a repeated pattern of

Assert(IsA(ptr, nodetype));
foo = (nodetype *) ptr;

how about adding a castNode() that combines those? Something like:

#if !defined(USE_ASSERT_CHECKING)

#define castNode(nodeptr,_type_) \
((_type_ *) (nodeptr))

#elif defined(__GNUC__)

#define castNode(nodeptr,_type_) \
((_type_ *) ({ \
Node   *_result; \
_result = nodeptr; \
Assert(IsA(_result, _type_)); \
_result; \
}))

#else

extern PGDLLIMPORT Node *newNodeMacroHolder;
#define castNode(nodePtr,_type_) \
( \
newNodeMacroHolder = nodePtr, \
AssertMacro(IsA(newNodeMacroHolder, _type_)), \
(_type_ *) newNodeMacroHolder \
)

#endif

Greetings,

Andres Freund

-- 
 Andres Freund 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] Planned downtime @ Rackspace - 2014-04-29 2100-2200 UTC

2014-04-29 Thread Stephen Frost
Greetings,

  This is take-2 on this.  Apologies for the short notice.

  As some may be aware, we are currently working with Rackspace to
  upgrade the PostgreSQL infrastructure systems which they graciously
  host for us.  As part of these upgrades there will be downtime for
  systems hosted there.

  To facilitate the migration of systems, Rackspace is upgrading the
  switching infrastructure to Gigabit (from 100Mbps).  This upgrade will
  happen on Tuesday, April 29th (today, for most of the world), between
  2100-2200 UTC (5pm-6pm US/Eastern).  We hope the downtime will be
  minimal but the window allows for up to 1 hour.  To be clear- this is
  ~4 hours from the time of this email.

  This downtime will impact all systems hosted @ Rackspace as they are
  upgrading the switch for us.  End-user services which will be impacted
  include:

yum.postgresql.org
wiki.postgresql.org
git master server (Committers only)
planet.postgresql.org
www.pgadmin.org, ftp.pgadmin.org
media.postgresql.org
commitfest.postgresql.org
developer.postgresql.org (Developer personal homepages)
redmine.postgresql.org
jdbc.postgresql.org
postgresopen.org
developer.pgadmin.org (sandbox, Jenkins)
babel.postgresql.org (Translation services)

  Redundant services (minimal impact expected):
ns2.postgresql.org (other nameservers will still work)
.us inbound mail relay (other MXs will still work)
.us website front-end (should be removed from pool)
FTP mirror (should be removed from pool)

  We anticipate this being the only whole-environment outage during
  this migration.  Future outages will happen for individual systems
  as we migrate them to the new hardware. 

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pg_dump --pretty-print-views

2014-04-29 Thread Keith Fiske
On Tue, Apr 29, 2014 at 11:14 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Keith Fiske ke...@omniti.com writes:
  On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Applied with corrections.

  Was this ever committed into core? Apologies, I'm not very familiar with
  looking through the commit history of the source code and I don't see
  anything about this option or pretty-print outputs in the pg_dump/restore
  docs for 9.3. Had someone asking me about this feature for pg_extractor

 Yeah, if I say applied that means I committed it.  Here's the commit
 log entry:

 Author: Tom Lane t...@sss.pgh.pa.us
 Branch: master Release: REL9_3_BR [62e666400] 2013-02-03 15:56:45 -0500

 Perform line wrapping and indenting by default in ruleutils.c.

 This patch changes pg_get_viewdef() and allied functions so that
 PRETTY_INDENT processing is always enabled.  Per discussion, only the
 PRETTY_PAREN processing (that is, stripping of unnecessary
 parentheses)
 poses any real forward-compatibility risk, so we may as well make dump
 output look as nice as we safely can.

 Also, set the default wrap length to zero (i.e, wrap after each SELECT
 or FROM list item), since there's no very principled argument for the
 former default of 80-column wrapping, and most people seem to agree
 this
 way looks better.

 Marko Tiikkaja, reviewed by Jeevan Chalke, further hacking by Tom Lane

 As per the branch annotation, this is in 9.3 and up.

 regards, tom lane


Great! Thanks, Tom

--
Keith Fiske
Database Administrator
OmniTI Computer Consulting, Inc.
http://www.keithf4.com


Re: [HACKERS] What about a castNode() macro?

2014-04-29 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 There's a repeated pattern of

 Assert(IsA(ptr, nodetype));
 foo = (nodetype *) ptr;

 how about adding a castNode() that combines those?

Doesn't really seem worth it.  The notational advantage is not great,
and to the extent that it were to touch a lot of places, the main outcome
would be pain for back-patching.

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] pg_dump --pretty-print-views

2014-04-29 Thread Greg Stark
Huh, I had assumed this was old behaviour. I didn't realize this was
new with 9.3.

Considering the thread pg_get_viewdefs() indentation considered
harmful I'm beginning to think this was a regression. It results in
some dump files being unnecessarily large and the pg_dump consuming
too much memory and crashing.

Tom liked my suggestion of doing the indentation modulo 40. There are
plenty of other suggestions that can work too, giving up on
indentation after an indent of 40, switching to an indent distance of
1 if it's more than 10 levels deep, and so on. I think it doesn't
really matter which we choose but we have to do something. And given
this is new behaviour in 9.3 perhaps it should be backported too.


-- 
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_dump --pretty-print-views

2014-04-29 Thread Tom Lane
Greg Stark st...@mit.edu writes:
 Huh, I had assumed this was old behaviour. I didn't realize this was
 new with 9.3.

 Considering the thread pg_get_viewdefs() indentation considered
 harmful I'm beginning to think this was a regression. It results in
 some dump files being unnecessarily large and the pg_dump consuming
 too much memory and crashing.

 Tom liked my suggestion of doing the indentation modulo 40. There are
 plenty of other suggestions that can work too, giving up on
 indentation after an indent of 40, switching to an indent distance of
 1 if it's more than 10 levels deep, and so on. I think it doesn't
 really matter which we choose but we have to do something. And given
 this is new behaviour in 9.3 perhaps it should be backported too.

I'm still a bit skeptical about this being a catastrophic problem in
practice ... but anyway, I thought there were two somewhat different
proposals on the table in that thread:

1. Arrange for x UNION y UNION z ... to put all the UNION arms at
the same indentation level.

2. Change the indentation rules globally, in one or another fashion
as Greg mentions above, to prevent ruleutils from ever prepending
silly amounts of whitespace.

These are not mutually exclusive, and I think we should do both.
But it seems we're hung up on exactly which flavor of #2 to do.

I would argue that rules such as reduce the indent step once we're too
far over don't fix the problem, just postpone it.  If we're taking this
complaint seriously at all then there needs to be a guaranteed maximum
indent distance, one way or another.  So I'd go for either a modulo-N
rule or a hard limit, with some preference for the modulo-N approach.
Yeah, it does sound silly at first, but I think it'd preserve
readability better than just capping the indent.

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


[HACKERS] Fix initdb for path with whitespace and at char

2014-04-29 Thread Nikhil Deshpande
Hi,

On win32, initdb fails if it's path includes a space and at ('@')
character. E.g.

C:\C:\Program Files\user@company\Postgres\9.3\bin\initdb.exe -D c:\baz
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

Here's a patch that fixes initdb by enclosing the command string in
extra double quotes being passed to popen() (similar to system() call).

Thanks,
 Nikhil


0001-Fix-initdb-for-path-with-whitespace-and-at-char.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


[HACKERS] libpq: How to get the error code after a failed PGconn connection

2014-04-29 Thread Hello World
Given the following code.

PGconn* const conn=PQconnectdbParams(keywords, values, false);
if(! conn || PQstatus(conn)!=CONNECTION_OK){ /* error code? */ }

- In case of a failed connection is there a way to get the error code to be
able to distinguish between a (e.g.) bad password and the server being down.

(I know I can get the error message, but I want to be able to react to the
cause of the error according to its cause, plus the error message is
localized so I can't even scan that for keywords such as permission
denied).

ps. I've looked at how psql does it, and it seems it just prints the error
message and exists.

ps. I've tried to take a look at the source but it seems it just sets the
status to CONNECTION_BAD no matter the cause of error, then sets a specific
error message.

Any help appreciated.

Thanks.


Re: [HACKERS] pg_get_viewdefs() indentation considered harmful

2014-04-29 Thread Tom Lane
Greg Stark st...@mit.edu writes:
 I propose the attached patch. It wraps at 40 and also divides the
 indent level by half the std indent level. I tried a few different
 combinations and this is the one that produced the output I liked
 best.

I doubt you can do that (the half-size-step bit), at least not without
a much larger patch than this: there are assorted places that just
unconditionally append PRETTYINDENT_STD spaces, and would have to be
taught to do something different.  OTOH those places might need to be
adjusted anyway.

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


[HACKERS] Considerer Harmful Considered Harmful

2014-04-29 Thread Josh Berkus
... so let's stop using that phrase, OK?

http://meyerweb.com/eric/comment/chech.html

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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 initdb for path with whitespace and at char

2014-04-29 Thread Heikki Linnakangas

On 04/29/2014 09:14 PM, Nikhil Deshpande wrote:

On win32, initdb fails if it's path includes a space and at ('@')
character. E.g.

C:\C:\Program Files\user@company\Postgres\9.3\bin\initdb.exe -D c:\baz
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

Here's a patch that fixes initdb by enclosing the command string in
extra double quotes being passed to popen() (similar to system() call).


This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like 
system() does. We already use SYSTEMQUOTEs in some popen() calls, like 
in pg_ctl, but initdb is missing them. get_bin_version function in 
pg_upgrade is also missing it, as is the popen() call in COPY TO/FROM 
PROGRAM command.


Is there any situation where would *not* want to wrap the command in 
SYSTEMQUOTEs? If there isn't, ISTM it would be better to create a 
wrapper function, pgwin32_popen(), which adds the quotes instead of 
sprinkling them into all callers. Even if we go around and fix all of 
the callers now, we're bound to forget it again in the future.


- Heikki


--
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] Buildfarm master-next branch?

2014-04-29 Thread Jim Nasby

On 4/17/14, 9:38 AM, Tom Lane wrote:

But the ability to easily spin up temporary branches for testing would
also be great.  Unfortunately, I suspect that only a minority of the
buildfarm owners would choose to participate, which would make it less
useful, but if we could solve that problem I'd be all in favor of it.

... Of course, all this would be done in my copious spare time*cough*. I'm
not sure this would be the best use of it.

I agree that this would not be worth the effort needed to make it happen.


There's also a sizeable security risk there, of someone putting something 
malicious in a branch and then triggering a run from that branch. I suppose 
that could be overcome if this was purposefully limited to the main git repo 
that only our core committers had access to, but we'd need to be careful.
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


--
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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Jim Nasby

On 4/26/14, 9:42 PM, Peter Geoghegan wrote:

Backend fsyncs are theoretically still possible after the fsync
request queue compaction patch (which was subsequently back-patched to
all supported release branches). However, I'm reasonably confident
that that patch was so effective as to make a backend fsync all but
impossible. As such, it seems like the buffers_backend_fsync column in
the pg_stat_bgwriter view is more or less obsolete.

I suggest removing it for 9.5, and instead logging individual
occurrences of backend fsync requests within ForwardFsyncRequest(). It
seems fair to treat that as an anomaly to draw particular attention
to.


All else equal, I don't like the idea of removing this from pg_stat_bgwriter. 
Being able to look there and see if this is occurring since last stats reset is 
much easier than grepping logfiles.

I don't have an issue with logging it, though I think we need to be careful to 
ensure we don't go crazy if something happens in the system where suddenly all 
backends are fsyncing. If that happens you're going to have major IO problems 
and trying to log thousands (or more) of extra entries is just going to make it 
worse.
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net


--
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] Buildfarm master-next branch?

2014-04-29 Thread Magnus Hagander
On Tue, Apr 29, 2014 at 9:11 PM, Jim Nasby j...@nasby.net wrote:

 On 4/17/14, 9:38 AM, Tom Lane wrote:

 But the ability to easily spin up temporary branches for testing would
 also be great.  Unfortunately, I suspect that only a minority of the
 buildfarm owners would choose to participate, which would make it less
 useful, but if we could solve that problem I'd be all in favor of it.

 ... Of course, all this would be done in my copious spare time*cough*.
 I'm

 not sure this would be the best use of it.

 I agree that this would not be worth the effort needed to make it happen.


 There's also a sizeable security risk there, of someone putting something
 malicious in a branch and then triggering a run from that branch. I suppose
 that could be overcome if this was purposefully limited to the main git
 repo that only our core committers had access to, but we'd need to be
 careful.


I would suggest a separate repo to keep the main one clean, but other
than that, yes, it would have to be limited to the same committers as the
rest I think.

It's reasonably easy to set up build environments in containers/jais on
many Unix boxes where that would actually not be a problem (just blow the
whole jail away once the build is complete), but one of the main platforms
that people would want to use this on I bet is Windows, which has no such
facilities AFAIK.

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


Re: [HACKERS] Should pg_stat_bgwriter.buffers_backend_fsync be removed?

2014-04-29 Thread Peter Geoghegan
On Tue, Apr 29, 2014 at 12:02 PM, Jim Nasby j...@nasby.net wrote:
 All else equal, I don't like the idea of removing this from
 pg_stat_bgwriter. Being able to look there and see if this is occurring
 since last stats reset is much easier than grepping logfiles.

Have you ever actually seen it at a non-zero value after the
compaction patch went in?

-- 
Peter Geoghegan


-- 
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] Planned downtime @ Rackspace - 2014-04-29 2100-2200 UTC

2014-04-29 Thread Stephen Frost
All,

  We have confirmation from Rackspace that the maintenance will begin in
  ~5 minutes.

Thanks!

Stephen

* Stephen Frost (sfr...@snowman.net) wrote:
 Greetings,
 
   This is take-2 on this.  Apologies for the short notice.
 
   As some may be aware, we are currently working with Rackspace to
   upgrade the PostgreSQL infrastructure systems which they graciously
   host for us.  As part of these upgrades there will be downtime for
   systems hosted there.
 
   To facilitate the migration of systems, Rackspace is upgrading the
   switching infrastructure to Gigabit (from 100Mbps).  This upgrade will
   happen on Tuesday, April 29th (today, for most of the world), between
   2100-2200 UTC (5pm-6pm US/Eastern).  We hope the downtime will be
   minimal but the window allows for up to 1 hour.  To be clear- this is
   ~4 hours from the time of this email.
 
   This downtime will impact all systems hosted @ Rackspace as they are
   upgrading the switch for us.  End-user services which will be impacted
   include:
 
 yum.postgresql.org
 wiki.postgresql.org
 git master server (Committers only)
 planet.postgresql.org
 www.pgadmin.org, ftp.pgadmin.org
 media.postgresql.org
 commitfest.postgresql.org
 developer.postgresql.org (Developer personal homepages)
 redmine.postgresql.org
 jdbc.postgresql.org
 postgresopen.org
 developer.pgadmin.org (sandbox, Jenkins)
 babel.postgresql.org (Translation services)
 
   Redundant services (minimal impact expected):
 ns2.postgresql.org (other nameservers will still work)
 .us inbound mail relay (other MXs will still work)
 .us website front-end (should be removed from pool)
 FTP mirror (should be removed from pool)
 
   We anticipate this being the only whole-environment outage during
   this migration.  Future outages will happen for individual systems
   as we migrate them to the new hardware. 
 
 Thanks!
 
 Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pg_dump --pretty-print-views

2014-04-29 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Here's a draft patch tackling point 1.  This gets rid of a whole lot
 of parenthesization, as well as indentation, for simple UNION lists.
 You can see the results in the changed regression test outputs.
[...]
 Comments?

+1.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] libpq: How to get the error code after a failed PGconn connection

2014-04-29 Thread Tom Lane
Hello World worldani...@gmail.com writes:
 Given the following code.
 PGconn* const conn=PQconnectdbParams(keywords, values, false);
 if(! conn || PQstatus(conn)!=CONNECTION_OK){ /* error code? */ }

 - In case of a failed connection is there a way to get the error code to be
 able to distinguish between a (e.g.) bad password and the server being down.

1. This question is not really material for the -hackers list.

2. No, I'm afraid.  libpq does not currently assign SQLSTATE error codes
to errors it detects internally, so even if there were an API for this,
it would fail to return anything in a lot of cases.  Fixing that is on
the TODO list, but it's been there for a long time, so don't hold your
breath ...

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] Considerer Harmful Considered Harmful

2014-04-29 Thread Tom Lane
Josh Berkus j...@agliodbs.com writes:
 ... so let's stop using that phrase, OK?
 http://meyerweb.com/eric/comment/chech.html

Shrug ... what I see there is a rant from a guy with no sense of humor.

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] Considerer Harmful Considered Harmful

2014-04-29 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Josh Berkus j...@agliodbs.com writes:
  ... so let's stop using that phrase, OK?
  http://meyerweb.com/eric/comment/chech.html
 
 Shrug ... what I see there is a rant from a guy with no sense of humor.

+1

'pt', I say.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Fix initdb for path with whitespace and at char

2014-04-29 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes:
 This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like 
 system() does. We already use SYSTEMQUOTEs in some popen() calls, like 
 in pg_ctl, but initdb is missing them. get_bin_version function in 
 pg_upgrade is also missing it, as is the popen() call in COPY TO/FROM 
 PROGRAM command.

Yuck.

 Is there any situation where would *not* want to wrap the command in 
 SYSTEMQUOTEs? If there isn't, ISTM it would be better to create a 
 wrapper function, pgwin32_popen(), which adds the quotes instead of 
 sprinkling them into all callers. Even if we go around and fix all of 
 the callers now, we're bound to forget it again in the future.

We might forget to use the wrapper function too, if it has a nonstandard
name, no?  A better idea would be to redefine popen() and system() on
Windows.  It looks like we're already using a #define to redefine popen().
This approach would let us get rid of nonstandard notation for this
problem, instead of adding more.

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] pg_get_viewdefs() indentation considered harmful

2014-04-29 Thread Greg Stark
On Tue, Apr 29, 2014 at 7:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I doubt you can do that (the half-size-step bit), at least not without
 a much larger patch than this: there are assorted places that just
 unconditionally append PRETTYINDENT_STD spaces, and would have to be
 taught to do something different.  OTOH those places might need to be
 adjusted anyway.

As far as I can see this is the only place that needs to be adjusted.
That function handles pretty much all the indentation. The only other
places that insert spaces just insert a fixed number in strings like
CREATE FUNCTION before the LANGUAGE or CREATE RULE before the ON.

Actually the only thing that might want to be adjusted is the
indentation in the beginning of the setop (ruleutils.c:4720) which is
what causes that long line of parentheses at the beginning of the
example. I suppose in an ideal world it would start following the
reduced spacing and wrap to new lines whenever the indentation goes
back to the left. But I can't get too excited by it in the example and
I'm not sure it's even intended to line up anyways. It just inserts
STD spaces without a newline.




-- 
greg


-- 
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_get_viewdefs() indentation considered harmful

2014-04-29 Thread Tom Lane
Greg Stark st...@mit.edu writes:
 Actually the only thing that might want to be adjusted is the
 indentation in the beginning of the setop (ruleutils.c:4720) which is
 what causes that long line of parentheses at the beginning of the
 example. I suppose in an ideal world it would start following the
 reduced spacing and wrap to new lines whenever the indentation goes
 back to the left. But I can't get too excited by it in the example and
 I'm not sure it's even intended to line up anyways. It just inserts
 STD spaces without a newline.

Actually, the patch I posted a little bit ago gets rid of that.  However,
I'm still dubious about halving the step distance, because there are
assorted places that adjust the indentation of specific keywords by
distances that aren't a multiple of 2 (look for odd last arguments to
appendContextKeyword).  I'm not sure how that will look after we make such
a change.  Possibly it would work to only apply the scale factor to
context-indentLevel, and not the indentPlus number?

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] Fix initdb for path with whitespace and at char

2014-04-29 Thread Amit Kapila
On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas hlinnakan...@vmware.com writes:
 This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like
 system() does.

It seems right now  SYSTEMQUOTE is used before popen both for
Windows and non-Windows, ex. adjust_data_dir() in pg_ctl.c

 We might forget to use the wrapper function too, if it has a nonstandard
 name, no?  A better idea would be to redefine popen() and system() on
 Windows.  It looks like we're already using a #define to redefine popen().

Won't defining variant of popen just for Windows to add SYSTEMQUOTE
effect such (where currently it is used for both win and non-winows)
usage?  Also, I think we might want to remove use of SYSTEMQUOTE
before popen calls where ever it is currently used to avoid usage of the
same two times.

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


-- 
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 initdb for path with whitespace and at char

2014-04-29 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 We might forget to use the wrapper function too, if it has a nonstandard
 name, no?  A better idea would be to redefine popen() and system() on
 Windows.  It looks like we're already using a #define to redefine popen().

 Won't defining variant of popen just for Windows to add SYSTEMQUOTE
 effect such (where currently it is used for both win and non-winows)
 usage?  Also, I think we might want to remove use of SYSTEMQUOTE
 before popen calls where ever it is currently used to avoid usage of the
 same two times.

Well, yeah: the point would be to remove SYSTEMQUOTE altogether,
probably.  Certainly the idea I'm suggesting is that we don't need any
Windows-specific notation at the call sites.

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] pg_dump --pretty-print-views

2014-04-29 Thread Michael Paquier
On Wed, Apr 30, 2014 at 6:33 AM, Stephen Frost sfr...@snowman.net wrote:
 * Tom Lane (t...@sss.pgh.pa.us) wrote:
 Here's a draft patch tackling point 1.  This gets rid of a whole lot
 of parenthesization, as well as indentation, for simple UNION lists.
 You can see the results in the changed regression test outputs.
 [...]
 Comments?

 +1.
+1. Output is far easier to read.
-- 
Michael


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