Re: [HACKERS] Event Triggers: adding information
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
* 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 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
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 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
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
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
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
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
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
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
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
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