Re: [HACKERS] Considerer Harmful Considered Harmful
On 04/30/2014 01:27 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Josh Berkus 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. I wasn't sure if the whole article was a parody. - 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] pg_dump --pretty-print-views
On Wed, Apr 30, 2014 at 6:33 AM, Stephen Frost 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
Re: [HACKERS] Fix initdb for path with whitespace and at char
Amit Kapila writes: > On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane 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] Fix initdb for path with whitespace and at char
On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane wrote: > Heikki Linnakangas 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] pg_get_viewdefs() indentation considered harmful
Greg Stark 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] pg_get_viewdefs() indentation considered harmful
On Tue, Apr 29, 2014 at 7:46 PM, Tom Lane 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] Fix initdb for path with whitespace and at char
Heikki Linnakangas 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] Considerer Harmful Considered Harmful
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Josh Berkus 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] Considerer Harmful Considered Harmful
Josh Berkus 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] libpq: How to get the error code after a failed PGconn connection
Hello World 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] pg_dump --pretty-print-views
* 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] Planned downtime @ Rackspace - 2014-04-29 2100-2200 UTC
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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?
On Tue, Apr 29, 2014 at 12:02 PM, Jim Nasby 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] Buildfarm "master-next" branch?
On Tue, Apr 29, 2014 at 9:11 PM, Jim Nasby 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?
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?
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] Fix initdb for path with whitespace and at char
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
[HACKERS] Considerer Harmful Considered Harmful
... 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] pg_get_viewdefs() indentation considered harmful
Greg Stark 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] libpq: How to get the error code after a failed PGconn connection
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.
[HACKERS] Fix initdb for path with whitespace and at char
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
Re: [HACKERS] pg_dump --pretty-print-views
Greg Stark 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
Re: [HACKERS] pg_dump --pretty-print-views
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] What about a castNode() macro?
Andres Freund 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
On Tue, Apr 29, 2014 at 11:14 AM, Tom Lane wrote: > Keith Fiske writes: > > On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane 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 > 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
[HACKERS] Planned downtime @ Rackspace - 2014-04-29 2100-2200 UTC
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
[HACKERS] What about a castNode() macro?
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
Re: [HACKERS] pg_dump --pretty-print-views
Keith Fiske writes: > On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane 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 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
Re: [HACKERS] includedir_internal headers are not self-contained
Andrew Dunstan 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] includedir_internal headers are not self-contained
On 04/29/2014 02:56 AM, Heikki Linnakangas wrote: On 04/28/2014 10:32 PM, Tom Lane wrote: Robert Haas 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] pg_dump --pretty-print-views
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] pg_dump --pretty-print-views
On Sun, Feb 3, 2013 at 4:06 PM, Tom Lane wrote: > "Marko Tiikkaja" 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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?
Robert Haas 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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?
On Tue, Apr 29, 2014 at 5:54 AM, Peter Geoghegan wrote: > On Tue, Apr 29, 2014 at 2:51 AM, Bernd Helmle 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] Problem with displaying "wide" tables in psql
On Tue, Apr 29, 2014 at 2:52 AM, Peter Eisentraut 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?
On Tue, Apr 29, 2014 at 2:51 AM, Bernd Helmle 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] Should pg_stat_bgwriter.buffers_backend_fsync be removed?
--On 26. April 2014 19:42:47 -0700 Peter Geoghegan 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
[HACKERS] Small doc patch for pg_replication_slots
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 @@ slot_name - text + name A unique, cluster-wide identifier for the replication slot - slot_type - text + plugin + name - The slot type - physical or logical + The basename of the shared object containing the output plugin this logical slot is using, or null for physical slots. - plugin + slot_type text - The basename of the shared object containing the output plugin this logical slot is using, or null for physical slots. + The slot type - physical or logical @@ -5305,7 +5305,7 @@ - xmin + catalog_xmin xid The oldest transaction that this slot needs the database to @@ -5315,14 +5315,6 @@ - catalog_xmin - xid - - The xmin, or oldest transaction ID, that this - slot forces to be retained in the system catalogs. - - - restart_lsn pg_lsn -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] using array of char pointers gives wrong results
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 -I test_char_select.pgc
cc -I -g -c -o test_char_select.o
test_char_select.c
cc -g test_char_select.o -L -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) ||
(atoi(varcharsize) == 0 && strcm
Re: [HACKERS] Proposal for Merge Join for Non '=' Operators
Hello Dilip, Query: select count(*) from t1,t2 where 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
