Re: [HACKERS] Event Triggers: adding information

2012-12-30 Thread Dimitri Fontaine
Hi,

Peter Eisentraut pete...@gmx.net writes:
 There is a new compiler warning coming from this, I believe:

I don't get this warning here, at least not in -O2, and I did forget to
make maintainer-clean then rebuild with -O0 just in case gcc is now
complaining about something else. I wish this situation could be fixed.

Now, this warning needs to be fixed, too, and here's a patch for that.

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

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 31a0288..00f859e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -750,7 +750,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
 	bool		is_from = stmt-is_from;
 	bool		pipe = (stmt-filename == NULL);
 	Relation	rel;
-	Oid relid;
+	Oid relid = InvalidOid;
 
 	/* Disallow file COPY except to superusers. */
 	if (!pipe  !superuser())

-- 
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] enhanced error fields

2012-12-30 Thread Stephen Frost
* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 so - cannot be a solution define CONSTRAINT_TABLE field - constaint
 names in table are unique.

Adding a table column, and a schema column, would be ideal.  Those would
all be part of the PK and not null'able, but then we wouldn't
necessairly always return all that information- that's the situation
that we've been talking about.

 sure there is a problem with long names, but I am thinking so it has
 solution - when constraint has no name, then we can try to generate
 name, and when this name is longer than 63 chars, then CREATE
 STATEMENT fails and users should be define name manually - this
 feature should be disabled by guc due compatibility issues.

CREATE doesn't fail if the name is too long today, it truncates it
instead.  I continue to feel that's also the wrong thing to do.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] enhanced error fields

2012-12-30 Thread Pavel Stehule
2012/12/30 Stephen Frost sfr...@snowman.net:
 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 so - cannot be a solution define CONSTRAINT_TABLE field - constaint
 names in table are unique.

 Adding a table column, and a schema column, would be ideal.  Those would
 all be part of the PK and not null'able, but then we wouldn't
 necessairly always return all that information- that's the situation
 that we've been talking about.

 sure there is a problem with long names, but I am thinking so it has
 solution - when constraint has no name, then we can try to generate
 name, and when this name is longer than 63 chars, then CREATE
 STATEMENT fails and users should be define name manually - this
 feature should be disabled by guc due compatibility issues.

 CREATE doesn't fail if the name is too long today, it truncates it
 instead.  I continue to feel that's also the wrong thing to do.

probably it is far to ideal - but I have not any feedback about
related problems in production.

Regards

Pavel Stehule


 Thanks,

 Stephen


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


[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

2012-12-30 Thread Pavel Stehule
Hello Stephen

2012/12/29 Stephen Frost sfr...@snowman.net:
 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 ok, so what is proposed solution?

 My recommendation would be to match what glibc's printf does.

 I see two possibilities - a) applying my current patch - although it
 is not fully correct, b) new patch, that do necessary check and raise
 more descriptive error message.

 Right, have a new patch that does error-checking and returns a better
 error on that case, update the docs to reflect that restriction, and
 then (ideally as an additional and independent patch..) implement the
 width capability (and, ideally, the ability to pass the width as an
 argument, as glibc supports) which matches the glibc arguments.

 Part of the reason that this restriction is in place, I believe, is
 because glibc expects the width to come before any explicit argument
 being passed and if an explicit argument is used for width then an
 explicit argument has to be used for the value also, otherwise it
 wouldn't be clear from the format which was the argument number and
 which was the explicit width size.

I found one issue - if I disallow mixing positional and ordered style
I break compatibility with previous implementation.

so maybe third way is better - use fix from my patch - a behave is
same like in glibc - and raise warning (instead errors) when mixing
styles is detected - we can replace warnings by errors in future.

What do you think?

Regards

Pavel

 I don't think it's a good idea to come up with our own format
 definition, particularly one which looks so similar to the well-known
 printf() format.

 I have not strong preferences in this topic - both variants are
 acceptable for me and I invite any community opinion. But current
 state is not intuitive and should be fixed.

 Agreed.

 Thanks,

 Stephen


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


[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

2012-12-30 Thread Pavel Stehule
2012/12/30 Pavel Stehule pavel.steh...@gmail.com:
 Hello Stephen

 2012/12/29 Stephen Frost sfr...@snowman.net:
 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 ok, so what is proposed solution?

 My recommendation would be to match what glibc's printf does.

 I see two possibilities - a) applying my current patch - although it
 is not fully correct, b) new patch, that do necessary check and raise
 more descriptive error message.

 Right, have a new patch that does error-checking and returns a better
 error on that case, update the docs to reflect that restriction, and
 then (ideally as an additional and independent patch..) implement the
 width capability (and, ideally, the ability to pass the width as an
 argument, as glibc supports) which matches the glibc arguments.

 Part of the reason that this restriction is in place, I believe, is
 because glibc expects the width to come before any explicit argument
 being passed and if an explicit argument is used for width then an
 explicit argument has to be used for the value also, otherwise it
 wouldn't be clear from the format which was the argument number and
 which was the explicit width size.

 I found one issue - if I disallow mixing positional and ordered style
 I break compatibility with previous implementation.

 so maybe third way is better - use fix from my patch - a behave is
 same like in glibc - and raise warning (instead errors) when mixing
 styles is detected - we can replace warnings by errors in future.

this is exactly what gcc does - and without breaking applications.


 What do you think?

 Regards

 Pavel

 I don't think it's a good idea to come up with our own format
 definition, particularly one which looks so similar to the well-known
 printf() format.

 I have not strong preferences in this topic - both variants are
 acceptable for me and I invite any community opinion. But current
 state is not intuitive and should be fixed.

 Agreed.

 Thanks,

 Stephen


-- 
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] buffer assertion tripping under repeat pgbench load

2012-12-30 Thread Greg Stark
On Sun, Dec 30, 2012 at 3:07 AM, Greg Smith g...@2ndquadrant.com wrote:
 It is a strange power of two to be appearing there.  I can follow your
 reasoning for why this could be a bit flipping error.  There's no sign of
 that elsewhere though, no other crashes under load.  I'm using this server
 here because it's worked fine for a while now.

Hm. Another idea. Can you use taskset to bind postgres to just one
processor at a time and see if it's reproducible on both processors?
The processor affinity is inherited across fork so doing it once on
the postmaster before starting pgbench ought to be sufficient.

If it's not a hardware error then the most plausible explanation seems
to me that one of the one of the bitsets such as t_infomask or
something is being set based on a wild pointer. For example
src/backend/commands/trigger.c:3469 and 3572 set this bit offset on a
bitmask at the end of a pointer. These lines haven't changed in 4
years but you don't have any command triggers on this database do you?
Maybe this pointer isn't being initialized?

-- 
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] Making view dump/restore safe at the column-alias level

2012-12-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Dec 21, 2012 at 9:46 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I'm suggesting that we could fix this by emitting something that forces
 the right alias to be assigned to t2.q:
 
 SELECT t1.x, t1.y, t2.z
 FROM t1
 JOIN t2 AS t2(x,z)
 USING (x);

 Sneaky.  I didn't know that would even work, but it seems like a
 sensible approach.

I've been idly hacking away at this over Christmas break, and attached
is a draft patch.  The problems turned out to be considerably more
extensive than I'd realized --- in addition to the stated issue with
forcing input aliases for JOIN USING colums to match, I found that:

* For joins without aliases, we can't qualify join column references,
since obviously there's no relation name to use.  (And we can't add one
without breaking queries, because SQL specifies that an aliased JOIN hides
relation names within it.)  That's okay in simple cases because we can
just print the name of the referenced input column instead.  However,
that doesn't work for merged columns in FULL JOIN USING, because in a full
join a merged output column doesn't behave the same as either input.  The
only solution I can see for this is to force the column aliases for such
columns to be unique query-wide, not just within the join RTE.  Then they
can be referenced without a relation name and still be unambiguous.

* When printing a join's column alias list, we were just blindly printing
the user's original alias list.  However, addition or removal of columns
from either input table invalidates that: we have to be able to add or
remove aliases to match the new column set.

I believe the attached patch covers all cases arising from column
additions, deletions, or renames.  It's awfully large though --- about
1100 lines added to ruleutils.c.  We could possibly make it a bit smaller
if we changed the parser to save more information about JOIN USING
columns, so that we don't need the grotty flatten_join_using_qual hack.
But that would only save about 100 lines, and it would add more elsewhere,
so I'm not sure it's worth the trouble.  (It would also prevent anyone
from trying to use the patch in the back branches, not that I plan to
take the risk of back-patching.)

On the whole I think this is a must fix bug, so we don't have a lot of
choice, unless someone has a proposal for a different and more compact
way of solving the problem.

regards, tom lane



rule-column-aliasing-1.patch.gz
Description: rule-column-aliasing-1.patch.gz

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


[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

2012-12-30 Thread Stephen Frost
Pavel,

* Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I found one issue - if I disallow mixing positional and ordered style
 I break compatibility with previous implementation.

Can you elaborate?  In the previous example, an error was returned when
mixing (not a terribly good one, but still an error).  Returning a
better error won't be a problem.

 so maybe third way is better - use fix from my patch - a behave is
 same like in glibc - and raise warning (instead errors) when mixing
 styles is detected - we can replace warnings by errors in future.
 
 What do you think?

If there are cases which work today then I agree that we should issue a
warning to avoid breaking existing applications.  We should still use
the glibc format when adding width support, however.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Event Triggers: adding information

2012-12-30 Thread Peter Eisentraut
On Sun, 2012-12-30 at 13:18 +0100, Dimitri Fontaine wrote:
 Peter Eisentraut pete...@gmx.net writes:
  There is a new compiler warning coming from this, I believe:
 
 I don't get this warning here, at least not in -O2, and I did forget to
 make maintainer-clean then rebuild with -O0 just in case gcc is now
 complaining about something else. I wish this situation could be fixed.

I get this warning in Debian squeeze and wheezy with fairly standard
build options.  I don't think -O0 will give you any more warnings.

 Now, this warning needs to be fixed, too, and here's a patch for that.

Thanks, I fixed it slightly differently.




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


[HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

2012-12-30 Thread Pavel Stehule
Hello

2012/12/31 Stephen Frost sfr...@snowman.net:
 Pavel,

 * Pavel Stehule (pavel.steh...@gmail.com) wrote:
 I found one issue - if I disallow mixing positional and ordered style
 I break compatibility with previous implementation.

 Can you elaborate?  In the previous example, an error was returned when
 mixing (not a terribly good one, but still an error).  Returning a
 better error won't be a problem.

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.

select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

-- but this raises error - and it is same situation like previous example
select format('%s %2$s %s', 'Hello', 'World');

-- so bot examples should be executed or should be disabled if this
functionality should be consistent. And I can't to break first
example, then I have to repair second example

Regards



 so maybe third way is better - use fix from my patch - a behave is
 same like in glibc - and raise warning (instead errors) when mixing
 styles is detected - we can replace warnings by errors in future.

 What do you think?

 If there are cases which work today then I agree that we should issue a
 warning to avoid breaking existing applications.  We should still use
 the glibc format when adding width support, however.

 Thanks,

 Stephen


-- 
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] Documentation bug for LDAP authentication

2012-12-30 Thread Peter Eisentraut
On Sat, 2012-12-29 at 23:09 -0500, Peter Eisentraut wrote:
 On Wed, 2012-12-19 at 15:13 +, Albe Laurenz wrote:
  The second one is new in 9.3 with the URL syntax:
  It is not possible to specify user and password
  in the LDAP URL.
 
 That sounds like a bug to me.  I'll investigate.

Hmm, it seems it is not intended to be supported.  So I have installed
your change.



-- 
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 transaction's read-only mode

2012-12-30 Thread Pavan Deolasee
On Sun, Dec 30, 2012 at 12:38 AM, Stephen Frost sfr...@snowman.net wrote:
 * Pavan Deolasee (pavan.deola...@gmail.com) wrote:
 On Fri, Sep 7, 2012 at 6:06 PM, Kevin Grittner
  That makes sense to me.  The reason I didn't make that change when I
  added the serializable special case to pg_dump was that it seemed
  like a separate question; I didn't want to complicate an already big
  patch with unnecessary changes to non-serializable transactions.
 

 If we agree, should we change that now ?

 This is on the next commitfest, so I figure it deserves some comment.
 For my part- I tend to agree that we should have it always use a read
 only transaction.  Perhaps we should update the pg_dump documentation to
 mention this as well though?  Pavan, do you want to put together an
 actual patch?


I'd posted actual patch on this thread, but probably linked wrong
message-id in the commitfest page. Will check and correct. Regarding
pg_dump's documentation, I don't have strong views on that. Whether
pg_dump runs as a read-only transaction or not is entirely internal to
its implementation, but then if we make this change, it might be worth
telling users that they can trust that pg_dump will not make any
changes to their database and hence a safe operation to carry out.

Thanks,
Pavan


Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee


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


[HACKERS] [PATCH] Change Windows build docs to point to flex and bison from msys

2012-12-30 Thread Craig Ringer
Hi all

For some time it's been impossible to build PostgreSQL on 64-bit Windows
by following the documentation's advice, as the version of Flex we
distribute on the PostgreSQL FTP site does not work on 64-bit Windows
hosts. See this 2011 message (
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00100.php).

The attached patch changes the documentation to point users to the
working flex and bison provided by msys - either as part of MinGW, or
from msysgit. It also mentions the error that people will get when
trying to use the old flex we distribute on 64-bit hosts so it's easier
to find out about the issue.

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

From 1430b3df7f571a4bc27e39bf3d64810e0e7c2900 Mon Sep 17 00:00:00 2001
From: Craig Ringer ring...@ringerc.id.au
Date: Mon, 31 Dec 2012 14:46:40 +0800
Subject: [PATCH] Update Windows build docs to point to msys tools for bison
 and flex

---
 doc/src/sgml/install-windows.sgml | 75 +++
 1 file changed, 53 insertions(+), 22 deletions(-)

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index b6ea0ab..afce78b 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -92,21 +92,25 @@
  /para
 
  para
-  The tools for building using productnameVisual C++/productname,
-  are in the filenamesrc/tools/msvc/filename directory. When building,
-  make sure there are no tools from productnameMinGW/productname or
+  The tools for building using productnameVisual C++/productname or the 
+  productnamePlatform SDK/productname are in the
+  filenamesrc/tools/msvc/filename directory. When building, make sure there
+  are no tools from productnameMinGW/productname or
   productnameCygwin/productname present in your system PATH. Also, make
   sure you have all the required Visual C++ tools available in the PATH. In
-  productnameVisual Studio/productname, start the
-  applicationVisual Studio Command Prompt/application.
+  productnameVisual Studio/productname, start the applicationVisual
+  Studio Command Prompt/application.
   If you wish to build a 64-bit version, you must use the 64-bit version of
-  the command, and vice versa.
+  the command, and vice versa, or use commandvcvarsall/command to change the
+  target environment.
   In the productnameMicrosoft Windows SDK/productname, start the
   applicationCMD shell/application listed under the SDK on the Start Menu.
-  In recent SDK versions you can change the targeted CPU architecture by using
-  the commandsetenv/command command.
-  All commands should be run from the filenamesrc\tools\msvc/filename
-  directory.
+  In recent SDK versions you can change the targeted CPU architecture, build
+  type, and target OS by using the commandsetenv/command command, eg
+  commandsetenv /x86 /release /xp/command to target Windows XP or later
+  with a 32-bit release build. See command/?/command for other options to
+  commandsetenv.cmd/command.  All commands should be run from the
+  filenamesrc\tools\msvc/filename directory.
  /para
 
  para
@@ -191,17 +195,32 @@ $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
 varlistentry
  termproductnameBison/productname and
   productnameFlex/productname/term
- listitempara
-  Bison and Flex are required to build from Git, but not required when
-  building from a release file. Note that only Bison 1.875 or versions
-  2.2 and later will work. Also, Flex version 2.5.31 or later is required.
-  Bison can be downloaded from ulink url=http://gnuwin32.sourceforge.net;/.
-  Flex can be downloaded from
-  ulink url=http://www.postgresql.org/ftp/misc/winflex/;/.
-  If you are using productnamemsysGit/productname for accessing the
-  PostgreSQL productnameGit/productname repository you probably already
-  have recent versions of bison and flex in your productnameGit/productname
-  binary directory.
+ listitem
+ para
+  productnameBison/productname and productnameFlex/productname are
+  required to build from Git, but not required when building from a release
+  file. Only productnameBison/productname 1.875 or versions 2.2 and later
+  will work. productnameFlex/productname must also be version 2.5.31 or later.
+ /para
+
+ para
+  Both productnameBison/productname and productnameFlex/productname
+  are included in the productnamemsys/productname tool suite, available
+  from ulink url=http://www.mingw.org/wiki/MSYS;/ as part of the
+  productnameMinGW/productname compiler suite. You can also get
+  productnamemsys/productname as part of
+  productnamemsysGit/productname from ulink url=http://git-scm.com/;/.
+ /para
+
+ para
+  You will need to add the directory containing
+  filenameflex.exe/filename and filenamebison.exe/filename to the PATH
+  environment variable in