[HACKERS] Re: [BUGS] BUG #7809: Running pg_dump on slave w/ streaming replication fails if there are unlogged tables
On Tue, Jan 15, 2013 at 10:35 PM, Magnus Hagander mag...@hagander.net wrote: On Tue, Jan 15, 2013 at 12:13 AM, j...@tanga.com wrote: The following bug has been logged on the website: Bug reference: 7809 Logged by: Joe Van Dyk Email address: j...@tanga.com PostgreSQL version: 9.2.2 Operating system: Ubuntu Description: Running pg_dump on a streaming replication slave with a database that has unlogged_tables will fail unless you provide the --no-unlogged-table-data option with the following (scary) error: pg_dump: Dumping the contents of table tracking_import_data failed: PQgetResult() failed. pg_dump: Error message from server: ERROR: could not open file base/16388/190326: No such file or directory pg_dump: The command was: COPY public.tracking_import_data (uuid, tracking_number) TO stdout; (this guy encountered the error as well: http://www.postgresql.org/message-id/de2de764-307d-4a23-a9a9-6608ac097...@ticketevolution.com ) Could running pg_dump against a slave always use the --no-unlogged-table-data option? That sounds like a pretty reasonable idea, I think. Should be easy enough to figure out at an early stage, too. That said, it wouldn't hurt if we could make that error a little less scary. Instead of saying could not open file, could we find a way to say this is an unlogged table on a slave, it's not going to work? We can fix pg_dump the easy way, but what about custom tools... Here's a patch to fix this in pg_dump. I intentionally made the check for pg_is_in_recovery() on everything since 9.0, since we might well end up with other things we want to do different in hot standby (in theory. but not likely). And since we're not going to end up with any unlogged tables on 9.0 anyway, it doesn't hurt to turn them off. I'm thinking we can consider this a bug and it should be backpatched (to 9.1 where we added unlogged tables). Comments? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ pg_dump_unlogged.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] Review: Patch to compute Max LSN of Data Pages
2013/1/20 Amit kapila amit.kap...@huawei.com: On Sunday, January 20, 2013 4:04 AM Dickson S. Guedes wrote: 2013/1/18 Amit kapila amit.kap...@huawei.com: Please find the rebased Patch for Compute MAX LSN. The function 'remove_parent_refernces' couldn't be called 'remove_parent_references' ? Shall fix this. Why not an extension in PGXN instead of a contrib? This functionality has similarity to pg_resetxlog, so we thought of putting it either in bin or in contrib. Finally based on suggestions from other community members, we have added to contrib. Indeed. -- Dickson S. Guedes mail/xmpp: gue...@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New statistics for WAL buffer dirty writes
(2012/11/27 7:42), Alvaro Herrera wrote: Satoshi Nagayasu escribió: I attached the latest one, which splits the reset_time for bgwriter and walwriter, and provides new system view, called pg_stat_walwriter, to show the dirty write counter and the reset time. Thanks. I gave this a look and I have a couple of comments: 1. The counter is called dirty_write. I imagine that this comes directly from the name of the nearby DTrace probe, TRACE_POSTGRESQL_WAL_BUFFER_WRITE_DIRTY_START. That probe comes from email 494c1565.3060...@sun.com committed in 4ee79fd20d9a. But there wasn't much discussion about the name back then. Maybe that was fine at the time because it was pretty much an internal matter, being so deep in the code. But we're now going to expose it to regular users, so we'd better choose a very good name because we're going to be stuck with it for a very long time. And the name dirty doesn't ring with me too well; what matters here is not that we're writing a buffer that is dirty, but the fact that we're writing while holding the WalInsertLock, so the name should convey the fact that this is a locked write or something like that. Or at least that's how I see the issue. Note the documentation wording: + entrystructfielddirty_writes//entry + entrytypebigint/type/entry + entryNumber of dirty writes, which means flushing wal buffers + because of its full./entry Yes, dirty_writes came from the probe name, and if it needs to be changed, buffers_flush would make sense for me in this situation, because this counter is intended to show WAL writes due to wal buffer full. 2. Should we put bgwriter and walwriter data in separate structs? I don't think this is necessary, but maybe someone opines differently? I tried to minimize an impact of this patch, but if I can change this struct, yes, I'd like to split into two structs. 3. +/* + * WalWriter global statistics counter. + * Despite its name, this counter is actually used not only in walwriter, + * but also in each backend process to sum up xlog dirty writes. + * Those processes would increment this counter in each XLogWrite call, + * then send it to the stat collector process. + */ +PgStat_MsgWalWriter WalWriterStats; Maybe we should use a different name for the struct, to avoid having to excuse ourselves for the name being wrong ... Ok. How about WalBufferStats? I think this name could be accepted in both the wal writer and each backend process. -- Satoshi Nagayasu sn...@uptime.jp Uptime Technologies, LLC. http://www.uptime.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New statistics for WAL buffer dirty writes
(2012/12/10 3:06), Tomas Vondra wrote:
On 29.10.2012 04:58, Satoshi Nagayasu wrote:
2012/10/24 1:12, Alvaro Herrera wrote:
Satoshi Nagayasu escribi�:
With this patch, walwriter process and each backend process
would sum up dirty writes, and send it to the stat collector.
So, the value could be saved in the stat file, and could be
kept on restarting.
The statistics could be retreive with using
pg_stat_get_xlog_dirty_writes() function, and could be reset
with calling pg_stat_reset_shared('walwriter').
Now, I have one concern.
The reset time could be captured in globalStats.stat_reset_timestamp,
but this value is the same with the bgwriter one.
So, once pg_stat_reset_shared('walwriter') is called,
stats_reset column in pg_stat_bgwriter does represent
the reset time for walwriter, not for bgwriter.
How should we handle this? Should we split this value?
And should we have new system view for walwriter?
I think the answer to the two last questions is yes. It doesn't seem to
make sense, to me, to have a single reset timings for what are
effectively two separate things.
Please submit an updated patch to next CF. I'm marking this one
returned with feedback. Thanks.
I attached the latest one, which splits the reset_time
for bgwriter and walwriter, and provides new system view,
called pg_stat_walwriter, to show the dirty write counter
and the reset time.
I've done a quick review of the v4 patch:
Thanks for the review, and sorry for my delayed response.
1) applies fine on HEAD, compiles fine
2) make installcheck fails because of a difference in the 'rules'
test suite (there's a new view pg_stat_walwriter - see the
attached patch for a fixed version or expected/rules.out)
Ah, I forgot about the regression test. I will fix it. Thanks.
3) I do agree with Alvaro that using the same struct for two separate
components (bgwriter and walwriter) seems a bit awkward. For example
you need to have two separate stat_reset fields, the reset code
becomes much more verbose (because you need to list individual
fields) etc.
So I'd vote to either split this into two structures or keeping it
as a single structure (although with two views on top of it).
Ok, I will split it into two structs, PgStat_BgWriterGlobalStats and
PgStat_WalWriterGlobalStats, and will modify PgStat_GlobalStats to hold
those two structs in the stat collector.
4) Are there any other fields that might be interesting? Right now
there's just dirty_writes but I guess there are other values. E.g.
how much data was actually written etc.?
AFAIK, I think those numbers can be obtained by calling
pg_current_xlog_insert_location() or pg_current_xlog_location(),
but if we need it, I will add it.
Regards,
Tomas
--
Satoshi Nagayasu sn...@uptime.jp
Uptime Technologies, LLC. http://www.uptime.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dividing privileges for replication role.
On Sat, Jan 19, 2013 at 4:47 AM, Tomonari Katsumata t.katsumata1...@gmail.com wrote: Hi, I made a patch to divide privileges for replication role. Currently(9.2), the privilege for replication role is true / false which means that standby server is able to connect to another server or not with the replication role. Why is it better to do this with a privilege, rather than just using pg_hba.conf? It doesn't represent an actual permission level after all - it's just an administrative flag to say you can't connect. Which AFAICS can just as easily be handled in pg_hba.conf? I guess I'm missing something? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] [WIP] pg_ping utility
On Fri, Jan 18, 2013 at 4:17 PM, Phil Sorber p...@omniti.com wrote:
Updated patch is rebased against current master and copyright year is updated.
I took a look at this. According to the documentation for
PQpingParams: It accepts connection parameters identical to those of
PQconnectdbParams, described above. It is not, however, necessary to
supply correct user name, password, or database name values to obtain
the server status. The -U option therefore seems quite useless
except as a source of user confusion, and -d is only useful in that
you could instead pass a connections string. That is definitely a
useful thing to be able to do, but calling the option -d for database
might be viewed as confusing. Or, it might be viewed as consistent
with other commands, if you tilt your head just the right way.
I would be tempted to change the syntax synopsis of the command to
pg_isready [OPTIONS]... [CONNSTR] and delete -d and -U from the list
of options, along the way that psql already works, but making
allowance for the fact that database and username are apparently not
relevant.
I am also a little bit baffled by the loop that begins here:
+ while (conn_opt_ptr-keyword)
It appears to me that what this loop does is iterate through all of
the possible connection options and then, when we get to the host,
port, user, or dbname options, add them to the keywords and values
arrays. But... what do we get out of iterating through all of the
other options, then? It seems to me that you could dispense with the
loop and just keep the stuff that actually adds the non-default values
to the arrays:
+ if (pghost != NULL)
+ {
+ keywords[opt_index] = host;
+ values[opt_index] = pghost;
+ opt_index++;
+ }
+ if (pgport != NULL)
+ {
+ keywords[opt_index] = port;
+ values[opt_index] = pgport;
+ opt_index++;
+ }
+ if (pgconnstr != NULL)
+ {
+ keywords[opt_index] = dbname;
+ values[opt_index] = pgconnstr;
+ opt_index++;
+ }
Maybe there's some purpose to this I'm not seeing, but from here the
loop looks like an unnecessary frammish.
Finally, I think there should be a check that the user hasn't supplied
more command-line arguments than we know what to do with, similar to
this:
[rhaas pgsql]$ vacuumdb foo bar baz
vacuumdb: too many command-line arguments (first is bar)
Try vacuumdb --help for more information.
That error message text seems like it might not have been given
sufficient thought, but for purposes of this patch it's probably
better to copy it than to invent something new.
--
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] allowing privileges on untrusted languages
On Sat, Jan 19, 2013 at 8:54 AM, Simon Riggs si...@2ndquadrant.com wrote: On 19 January 2013 13:45, Kohei KaiGai kai...@kaigai.gr.jp wrote: I think, it is a time to investigate separation of database superuser privileges into several fine-grained capabilities, like as operating system doing. https://github.com/torvalds/linux/blob/master/include/uapi/linux/capability.h In case of Linux, the latest kernel has 36 kinds of capabilities that reflects a part of root privileges, such as privilege to open listen port less than 1024, privilege to override DAC permission and so on. Traditional root performs as a user who has all the capability in default. Sounds like the best way to go. The reasoning that led to that change works for us as well. Yeah. We'd need to think a little bit about how to make this work, since I think that adding a gajillion booleans to pg_authid will not make anyone very happy. But I like the idea. GRANT kill_sessions_of_other_users TO bob? GRANT install_untrusted_pls TO any_database_owner? GRANT install_an_extension_called(hstore) TO any_database_owner? I know there are other ways of doing all of these things, so don't take the specific proposals too seriously, but we clearly have a need to parcel out controlled bits of the superuser mojo to individual users in a nice, clean, convenient way. Getting agreement on the details is likely to be difficult, but it seems like a good concept from 10,000 feet. -- 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] [PATCH] Fix NULL checking in check_TSCurrentConfig()
2013/1/20 Xi Wang xi.w...@gmail.com: The correct NULL check should use `*newval'; `newval' must be non-null. [... cutting code ...] Please see [1] to know how is our submit patch process. [1] http://wiki.postgresql.org/wiki/Submitting_a_Patch regards, -- Dickson S. Guedes mail/xmpp: gue...@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br -- 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] Query to help in debugging
Bruce Momjian wrote: Why are you insisting on cramming version() into this? It could just as easily be a different query. I am fine with that: Done. -Kevin -- 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] [WIP] pg_ping utility
On Sun, Jan 20, 2013 at 8:40 AM, Robert Haas robertmh...@gmail.com wrote:
On Fri, Jan 18, 2013 at 4:17 PM, Phil Sorber p...@omniti.com wrote:
Updated patch is rebased against current master and copyright year is
updated.
I took a look at this. According to the documentation for
PQpingParams: It accepts connection parameters identical to those of
PQconnectdbParams, described above. It is not, however, necessary to
supply correct user name, password, or database name values to obtain
the server status. The -U option therefore seems quite useless
except as a source of user confusion, and -d is only useful in that
you could instead pass a connections string. That is definitely a
useful thing to be able to do, but calling the option -d for database
might be viewed as confusing. Or, it might be viewed as consistent
with other commands, if you tilt your head just the right way.
I would be tempted to change the syntax synopsis of the command to
pg_isready [OPTIONS]... [CONNSTR] and delete -d and -U from the list
of options, along the way that psql already works, but making
allowance for the fact that database and username are apparently not
relevant.
This was done to silence useless error messages in the logs. If you
attempt to connect as some user that does not exist, or to some
database that does not exist, it throws an error in the logs, even
with PQping. You could fix it with env vars, but since the point is to
change the user/database that we were connecting as, I figured it
should be consistent with all the other methods to do that.
I am also a little bit baffled by the loop that begins here:
+ while (conn_opt_ptr-keyword)
It appears to me that what this loop does is iterate through all of
the possible connection options and then, when we get to the host,
port, user, or dbname options, add them to the keywords and values
arrays. But... what do we get out of iterating through all of the
other options, then? It seems to me that you could dispense with the
loop and just keep the stuff that actually adds the non-default values
to the arrays:
+ if (pghost != NULL)
+ {
+ keywords[opt_index] = host;
+ values[opt_index] = pghost;
+ opt_index++;
+ }
+ if (pgport != NULL)
+ {
+ keywords[opt_index] = port;
+ values[opt_index] = pgport;
+ opt_index++;
+ }
+ if (pgconnstr != NULL)
+ {
+ keywords[opt_index] = dbname;
+ values[opt_index] = pgconnstr;
+ opt_index++;
+ }
Maybe there's some purpose to this I'm not seeing, but from here the
loop looks like an unnecessary frammish.
I use this to find the defaults if they don't pass anything in, so I
know what to put in the status message at the end. I could devise my
own way to come up with those values as I have seen in some other
code, but I thought it was better to ask libpq directly what defaults
it was going to use.
Finally, I think there should be a check that the user hasn't supplied
more command-line arguments than we know what to do with, similar to
this:
[rhaas pgsql]$ vacuumdb foo bar baz
vacuumdb: too many command-line arguments (first is bar)
Try vacuumdb --help for more information.
That error message text seems like it might not have been given
sufficient thought, but for purposes of this patch it's probably
better to copy it than to invent something new.
I had not considered this. I will take a look and provide an updated patch.
--
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] [PATCH] Fix NULL checking in check_TSCurrentConfig()
* Xi Wang (xi.w...@gmail.com) wrote: The correct NULL check should use `*newval'; `newval' must be non-null. Why isn't this using pstrdup()..? Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [PATCH] Fix off-by-one in PQprintTuples()
* Xi Wang (xi.w...@gmail.com) wrote: Don't write past the end of tborder; the size is width + 1. This whole block of code is woefully without any comments. :( Strictly speaking, it's this: tborder[i] = '\0'; Which ends up writing past the end of the buffer (which is allocated as 'width + 1'). Perhaps we should also change that to be: tborder[width] = '\0'; Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] allowing privileges on untrusted languages
Robert Haas robertmh...@gmail.com writes: Yeah. We'd need to think a little bit about how to make this work, since I think that adding a gajillion booleans to pg_authid will not make anyone very happy. But I like the idea. GRANT kill_sessions_of_other_users TO bob? GRANT install_untrusted_pls TO any_database_owner? GRANT install_an_extension_called(hstore) TO any_database_owner? I know there are other ways of doing all of these things, so don't take the specific proposals too seriously, but we clearly have a need to parcel out controlled bits of the superuser mojo to individual users in a nice, clean, convenient way. Getting agreement on the details is likely to be difficult, but it seems like a good concept from 10,000 feet. The traditional answer to that, which not only can be done already in all existing releases but is infinitely more flexible than any hard-wired scheme we could implement, is that you create superuser-owned security-definer functions that can execute any specific operation you want to allow, and then GRANT EXECUTE on those functions to just the people who should have it. I'm really entirely un-thrilled with a proposal to clutter the privilege system like this. Admittedly, it might be a hair more secure than user-written plpgsql functions, which could perhaps be subverted if the author is careless. But there are a hundred other places where we could more usefully spend our implementation and future-maintenance efforts than here. 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] [BUGS] BUG #7809: Running pg_dump on slave w/ streaming replication fails if there are unlogged tables
Magnus Hagander mag...@hagander.net writes: + PGresult *res = ExecuteSqlQueryForSingleRow(fout, SELECT pg_is_in_recovery()); That function call needs to be schema-qualified for security. 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] Re: [BUGS] BUG #7809: Running pg_dump on slave w/ streaming replication fails if there are unlogged tables
On Sun, Jan 20, 2013 at 4:59 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: + PGresult *res = ExecuteSqlQueryForSingleRow(fout, SELECT pg_is_in_recovery()); That function call needs to be schema-qualified for security. Ha! I wonder if I can set up an autoresponder to *myself* with that review whenever I commit to pgdump :) Thanks! -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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_upgrade and system() return value
Bruce Momjian br...@momjian.us writes: Can someone comment on the attached patch? pg_upgrade was testing if system() returned a non-zero value, while I am thinking I should be adjusting system()'s return value with WEXITSTATUS(). AFAIK it's not very good style to test the result as an integer, and testing for -1 is not an improvement on that. Actually it's a disimprovement, because the only case where the standard guarantees anything about the integer representation is that zero corresponds to exited with status 0. See the Single Unix Spec, wherein system's result code is defined in terms of wait's, and the wait man page saith If and only if the status returned is from a terminated child process that returned 0 from main() or passed 0 as the status argument to _exit() or exit(), the value stored at the location pointed to by stat_loc will be 0. Regardless of its value, this information may be interpreted using the following macros ... If you want to do something different, then you could test for WIFEXITED WEXITSTATUS == 0. (Testing the latter alone is flat wrong.) But I'm not particularly convinced that that's an improvement on what's there now. I note that your proposed patch would prevent any possibility of printing debug information about failure cases, since it loses the original result value. In short: it's not broken now, but this patch would break 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] ALTER command reworks
Kohei KaiGai kai...@kaigai.gr.jp writes: About ALTER FUNCTION towards aggregate function, why we should raise an error strictly? I agree we probably shouldn't --- traditionally we have allowed that, AFAIR, so changing it would break existing applications for little benefit. Similarly, you should not be throwing error when ALTER TABLE is applied to a view, sequence, etc, and the command would otherwise be sensible. 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] allowing privileges on untrusted languages
On Sun, Jan 20, 2013 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Yeah. We'd need to think a little bit about how to make this work, since I think that adding a gajillion booleans to pg_authid will not make anyone very happy. But I like the idea. GRANT kill_sessions_of_other_users TO bob? GRANT install_untrusted_pls TO any_database_owner? GRANT install_an_extension_called(hstore) TO any_database_owner? I know there are other ways of doing all of these things, so don't take the specific proposals too seriously, but we clearly have a need to parcel out controlled bits of the superuser mojo to individual users in a nice, clean, convenient way. Getting agreement on the details is likely to be difficult, but it seems like a good concept from 10,000 feet. The traditional answer to that, which not only can be done already in all existing releases but is infinitely more flexible than any hard-wired scheme we could implement, is that you create superuser-owned security-definer functions that can execute any specific operation you want to allow, and then GRANT EXECUTE on those functions to just the people who should have it. I'm really entirely un-thrilled with a proposal to clutter the privilege system like this. Admittedly, it might be a hair more secure than user-written plpgsql functions, which could perhaps be subverted if the author is careless. But there are a hundred other places where we could more usefully spend our implementation and future-maintenance efforts than here. It's not terribly personally important to me, either ... but it's important enough to other people here that I'm pretty sure we will see future patches aiming at this target. Extensions to event triggers, inter alia. Even had I the power, I'm not prepared to reject all of those things out of hand, so I think it would behoove us to think about by what means we want to enable these sorts of things rather than whether we want to enable them. -- 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] Thinking about WITH CHECK OPTION for views
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: I've been thinking about WITH CHECK OPTION for auto-updatable views. Given the timing I doubt if this will be ready for 9.3, since I only get occasional evenings and weekends to hack on postgres, but I think it's probably worth kicking off a discussion, starting with a description of what the feature actually is. If this isn't intended for 9.3, I think it'd be good to move it to the post-9.3 commitfest. That said, it actually looks pretty decent to me on first blush. I did have a couple of comments though: Why have validateWithCheckOption instead of handling that in gram.y..? If the SQL spec says we should be disallowing WITH CHECK when there are INSTEAD rules or triggers, could we actually do that..? This kind of an error: ERROR: attribute number 2 exceeds number of columns 1 is really pretty bad, any chance we could improve that or disallow the possibility of getting there by erroring earlier on? Lastly, documentation for this appears to be missing. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [WIP] pg_ping utility
On Sun, Jan 20, 2013 at 9:58 AM, Phil Sorber p...@omniti.com wrote: This was done to silence useless error messages in the logs. If you attempt to connect as some user that does not exist, or to some database that does not exist, it throws an error in the logs, even with PQping. You could fix it with env vars, but since the point is to change the user/database that we were connecting as, I figured it should be consistent with all the other methods to do that. Uh, OK. Well, in that case, I'm inclined to think that a documentation mention is in order, and perhaps an update to the PQpingParams documentation as well. Because that's hardly obvious. :-( I use this to find the defaults if they don't pass anything in, so I know what to put in the status message at the end. I could devise my own way to come up with those values as I have seen in some other code, but I thought it was better to ask libpq directly what defaults it was going to use. Oh, I see. Is it really important to have the host and port in the output, or should we trim that down to just e.g. accepting connections? It seems useful to have that if a human is looking at the output, but maybe not if a machine is looking at the output. And if somebody doesn't want it, getting rid of it with sed or awk is nontrivial - imagine: pg_isready -d /tmp:5432 - accepting connections I had not considered this. I will take a look and provide an updated patch. Sounds good. -- 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] Passing connection string to pg_basebackup
On Sat, Jan 19, 2013 at 12:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: Dimitri Fontaine dimi...@2ndquadrant.fr writes: On the other hand, discrepancies in between command line arguments processing in our tools are already not helping our users (even if pg_dump -d seems to have been fixed along the years); so much so that I'm having a hard time finding any upside into having a different set of command line argument capabilities for the same tool depending on the major version. We are not talking about a new feature per se, but exposing a feature that about every other command line tool we ship have. So I think I'm standing on my position that it should get backpatched as a fix. I don't think that argument holds any water at all. There would still be differences in command line argument capabilities out there --- they'd just be between minor versions not major ones. That's not any easier for people to deal with. And what will you say to someone whose application got broken by a minor-version update? I heartily agree. I can say from firsthand experience that when minor releases break things for customers (and they do), the customers get *really* cranky. Based on recent experience, I think we should be tightening our standards for what gets back-patched, not loosening them. (No, I don't have a specific example off-hand, sorry.) -- 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] Reporting hba lines
On 5 January 2013 16:58, Magnus Hagander mag...@hagander.net wrote: Attached is an updated version of the patch, per the comments from Tom and rebased on top of the current master. Since it's been a long time ago, and some code churn in the area, another round of review is probably a good thing... I took a look at this patch, and it seems to be in pretty good shape. It applies cleanly to head, and seems to work as advertised/discussed. I have a couple of comments on the code... In next_token(), in the case of an overlong token, this change looks wrong: /* Discard remainder of line */ ! while ((c = getc(fp)) != EOF c != '\n') ! ; break; } --- 188,195 errmsg(authentication file token too long, skipping: \%s\, start_buf))); /* Discard remainder of line */ ! while ((c = (*(*lineptr)++)) != '\0' c != '\n') ! (*lineptr)++; break; It appears to be incrementing lineptr twice per loop iteration, so it risks missing the EOL/EOF and running off the end of the buffer. Nitpicking, at the end of the loop you have: ! c = (**lineptr); ! (*lineptr)++; perhaps for consistency with the preceding code, that should be c = (*(*lineptr)++). Personally, I'd also get rid of the outer sets of brackets in each of these expressions and just write c = *(*lineptr)++, since I don't think they add anything. Finally, the comment block for tokenize_file() needs updating, now that it returns three lists. Regards, Dean -- 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] Passing connection string to pg_basebackup
Robert Haas wrote: I heartily agree. I can say from firsthand experience that when minor releases break things for customers (and they do), the customers get *really* cranky. Based on recent experience, I think we should be tightening our standards for what gets back-patched, not loosening them. +1 Any change in a minor release which causes working production code to break very quickly and seriously erodes confidence in the ability to apply a minor release without extensive (and expensive) testing. When that confidence erordes, users stay on old minor releases for extended periods -- often until they hit one of the bugs which was fixed in a minor release. We need to be very conservative about back-patching any changes in user-visible behavior. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch to add \watch to psql
2013/1/17 Daniel Farina dan...@heroku.com: I realized while making my adjustments that I pointlessly grew some input checking in the inner loop. I just hoisted it out in this version. Since psql uses libreadline, what do you think about to call rl_clear_screen() inside that while (true) loop? Obviously we should test #if we have readline enabled to use it, but when we have it a nice output will bring to us. BTW, I don't know how this will behaves on OSX or Windows. []s -- Dickson S. Guedes mail/xmpp: gue...@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br -- 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: fix corner use case of variadic fuctions usage
On Sat, Jan 19, 2013 at 11:58 AM, Pavel Stehule pavel.steh...@gmail.com wrote: We introduced VARIADIC any function. Motivation for this kind of function was bypassing postgresql's coerce rules - and own rules implementation for requested functionality. Some builtins function does it internally - but it is not possible for custom functions or it is not possible without some change in parser. Same motivation is reason why format function is VARIADIC any function. I'd just like to draw the attention of all assembled to the fact that this is another example of the cottage industry we've created in avoiding our own burdensome typecasting rules. I not long ago proposed a patch that went nowhere which would have obviated the need for this sort of nonsense in a much more principled way, which of course went nowhere, despite the design being one which Tom himself proposed. Is there any amount of this which will sway popular opinion to the point of view that the problem is not with the individual cases, but the rules themselves? -- 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] proposal: fix corner use case of variadic fuctions usage
On Sat, Jan 19, 2013 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: disagree - non variadic manner call should not be used for walk around FUNC_MAX_ARGS limit. So there should not be passed big array. That's utter nonsense. Why wouldn't people expect concat(), for example, to work for large (or even just moderate-sized) arrays? /me blinks. What does that have to do with anything? IIUC, the question isn't whether CONCAT() would work for large arrays, but rather for very large numbers of arrays written out as CONCAT(a1, ..., a1000). I don't understand why an appropriately-placed check against FUNC_MAX_ARGS does anything other than enforce a limit we already have. Or are we currently not consistently enforcing that limit? -- 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] Further pg_upgrade analysis for many tables
* Jeff Janes (jeff.ja...@gmail.com) wrote: By making the list over-flowable, we fix a demonstrated pathological workload (restore of huge schemas); we impose no detectable penalty to normal workloads; and we fail to improve, but also fail to make worse, a hypothetical pathological workload. All at the expense of a few bytes per backend. [...] Why does the list not grow as needed? It would increase the code complexity for no concretely-known benefit. I'm curious if this is going to help with rollback's of transactions which created lots of tables..? We've certainly seen that take much longer than we'd like, although I've generally attributed it to doing all of the unlink'ing and truncating of files. I also wonder about making this a linked-list or something which can trivially grow as we go and then walk later. That would also keep the size of it small instead of a static/fixed amount. 1) It would have to have some transactions that cause 10 or 100 of relations to need clean up. That doesn't seem hard. 2) It would have to have even more hundreds of relations in RelationIdCache but which don't need cleanup (otherwise, if most of RelationIdCache needs cleanup then iterating over that hash would be just as efficient as iterating over a list which contains most of the said hash) Good point. 3) The above described transaction would have to happen over and over again, because if it only happens once there is no point in worrying about a little inefficiency. We regularly do builds where we have lots of created tables which are later either committed or dropped (much of that is due to our hand-crafted partitioning system..). Looking through the pach itself, it looks pretty clean to me. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes jeff.ja...@gmail.com wrote: Sometime this type of high-level summary review does happen, at the senior person's whim, but is not a formal part of the commit fest process. What I don't know is how much work it takes for one of those senior people to make one of those summary judgments, compared to how much it takes for them to just do an entire review from scratch. IME, making such summary judgements can often be done in a few minutes, but convincing that the patch submitter that you haven't created the objection purely as an obstacle to progress is the work of a lifetime. We could perhaps do better at avoiding perverse incentives, there. -- 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] Thinking about WITH CHECK OPTION for views
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: Thanks for looking at it. I'll move it 9.4 CF-1. Awesome, thanks. Stephen signature.asc Description: Digital signature
Re: [HACKERS] Further pg_upgrade analysis for many tables
Jeff Janes jeff.ja...@gmail.com writes: [ patch for AtEOXact_RelationCache ] I've reviewed and committed this with some mostly-cosmetic adjustments, notably: * Applied it to AtEOSubXact cleanup too. AFAICS that's just as idempotent, and it seemed weird to not use the same technique both places. * Dropped the hack to force a full-table scan in Assert mode. Although that's a behavioral change that I suspect Jeff felt was above his pay grade, it seemed to me that not exercising the now-normal hash_search code path in assert-enabled testing was a bad idea. Also, the value of exhaustive checking for relcache reference leaks is vastly lower than it once was, because those refcounts are managed mostly automatically now. * Redid the representation of the overflowed state a bit --- the way that n_eoxact_list worked seemed a bit too cute/complicated for my taste. On Wednesday, January 9, 2013, Simon Riggs wrote: Why does the list not grow as needed? It would increase the code complexity for no concretely-known benefit. Actually there's a better argument for that: at some point a long list is actively counterproductive, because N hash_search lookups will cost more than the full-table scan would. I did some simple measurements that told me that with 100-odd entries in the hashtable (which seems to be about the minimum for an active backend), the hash_seq_search() traversal is about 40x more expensive than one hash_search() lookup. (I find this number slightly astonishing, but that's the answer I got.) So the crossover point is at least 40 and probably quite a bit more, since (1) my measurement did not count the cost of uselessly doing the actual relcache-entry cleanup logic on non-targeted entries, and (2) if the list is that long there are probably more than 100-odd entries in the hash table, and hash table growth hurts the seqscan approach much more than the search approach. Now on the other side, simple single-command transactions are very unlikely to have created more than a few list entries anyway. So it's probably not worth getting very tense about the exact limit as long as it's at least a couple dozen. I set the limit to 32 as committed, because that seemed like a nice round number in the right general area. BTW, this measurement also convinced me that the patch is a win even when the hashtable is near minimum size, even though there's no practical way to isolate the cost of AtEOXact_RelationCache in vivo in such cases. It's good to know that we're not penalizing simple cases to speed up the huge-number-of-relations case, even if the penalty would be small. 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] Further pg_upgrade analysis for many tables
Bruce Momjian br...@momjian.us writes: ! * Using pg_restore --single-transaction is faster than other ! * methods, like --jobs. Is this still the case now that Jeff's AtEOXact patch is in? The risk of locktable overflow with --single-transaction makes me think that pg_upgrade should avoid it unless there is a *really* strong performance case for it, and I fear your old measurements are now invalidated. 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] proposal: fix corner use case of variadic fuctions usage
Robert Haas robertmh...@gmail.com writes: On Sat, Jan 19, 2013 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: That's utter nonsense. Why wouldn't people expect concat(), for example, to work for large (or even just moderate-sized) arrays? /me blinks. What does that have to do with anything? IIUC, the question isn't whether CONCAT() would work for large arrays, but rather for very large numbers of arrays written out as CONCAT(a1, ..., a1000). No, the question is what happens with CONCAT(VARIADIC some-array-here), which currently just returns the array as-is, but which really ought to concat all the array elements as if they'd been separate arguments. Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. 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] Further pg_upgrade analysis for many tables
Stephen Frost sfr...@snowman.net writes: I'm curious if this is going to help with rollback's of transactions which created lots of tables..? We've certainly seen that take much longer than we'd like, although I've generally attributed it to doing all of the unlink'ing and truncating of files. If a single transaction creates lots of tables and then rolls back, this patch won't change anything because we'll long since have overflowed the eoxact list. But you weren't seeing an O(N^2) penalty in such cases anyway: that penalty came from doing O(N) work in each of N transactions. I'm sure you're right that you're mostly looking at the filesystem cleanup work, which we can't do much about. 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] missing rename support
On 3 January 2013 13:49, Ali Dar ali.munir@gmail.com wrote: Find attached an initial patch for ALTER RENAME RULE feature. Please note that it does not have any documentation yet. Hi, I just got round to looking at this. All-in-all it looks OK. I just have a few more review comments, in addition to Stephen's comment about renaming SELECT rules... This compiler warning should be fixed with another #include: alter.c:107:4: warning: implicit declaration of function ‘RenameRewriteRule’ In gram.y, I think you can just use qualified_name instead of makeRangeVarFromAnyName(). In RenameRewriteRule(), I think it's worth doing a check to ensure that the relation actually is a table or a view (you might have some other relation kind at that point in the code). If the user accidentally types the name of an index, say, instead of a table, then it is better to throw an error saying xxx is not a table or a view rather than reporting that the rule doesn't exist. I think this could probably use some simple regression tests to test both the success and failure cases. It would be nice to extend psql tab completion to support this too, although perhaps that could be done as a separate patch. Don't forget the docs! Regards, Dean -- 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: fix corner use case of variadic fuctions usage
On 01/20/2013 01:37 PM, Robert Haas wrote: On Sat, Jan 19, 2013 at 11:58 AM, Pavel Stehule pavel.steh...@gmail.com wrote: We introduced VARIADIC any function. Motivation for this kind of function was bypassing postgresql's coerce rules - and own rules implementation for requested functionality. Some builtins function does it internally - but it is not possible for custom functions or it is not possible without some change in parser. Same motivation is reason why format function is VARIADIC any function. I'd just like to draw the attention of all assembled to the fact that this is another example of the cottage industry we've created in avoiding our own burdensome typecasting rules. I not long ago proposed a patch that went nowhere which would have obviated the need for this sort of nonsense in a much more principled way, which of course went nowhere, despite the design being one which Tom himself proposed. Is there any amount of this which will sway popular opinion to the point of view that the problem is not with the individual cases, but the rules themselves? Uh, reference please? This doesn't jog my memory despite it being something that's obviously interesting in light of my recent work. (That could be a a case of dying synapses too.) 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] CF3+4 (was Re: Parallel query execution)
On 20 January 2013 18:42, Robert Haas robertmh...@gmail.com wrote: On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes jeff.ja...@gmail.com wrote: Sometime this type of high-level summary review does happen, at the senior person's whim, but is not a formal part of the commit fest process. What I don't know is how much work it takes for one of those senior people to make one of those summary judgments, compared to how much it takes for them to just do an entire review from scratch. IME, making such summary judgements can often be done in a few minutes, but convincing that the patch submitter that you haven't created the objection purely as an obstacle to progress is the work of a lifetime. We could perhaps do better at avoiding perverse incentives, there. (Without meaning to paraphrase you in any negative way...) Judgements made in a few minutes are very frequently wrong, and it takes a lot of time to convince the person making snap decisions that they should revise their thinking in light of new or correct information. I feel we are very prone to making judgements on little information. This is especially true with regard to voting, with people zooming in from the side without having even read a patch to vote one way or the other, voting for the person not the idea. It can be a big problem telling the difference between a patch submitter that really is in possession of information that should be heeded and someone so blinded by their patch/idea that they mistakenly push forwards. At times, I have been both and I've also witnessed both as committer. There is a clear and understandable conservatism in being a reviewer/committer that people that haven't done it don't understand. I definitely didn't until I was a committer and I remember clearly me pushing for HS to go into 8.4 when it was a long way from being ready. I think it would be useful to expand the pool of committers and perhaps that can be done via some intermediate stage, though I do worry that the sense of responsibility might not be as strong in the intermediate rank ($NAME). I don't think we should be encouraging people to make fast decisions. Senior or not. (Though we must make decisions and have some coming soon). This is high in my mind right now since I've been looking at skip checkpoint patch for months, seeing how I feel about it. Nervous, basically. From that I think that some areas of the code are more critical than others and harder to fix in production if they go wrong. We need to be taking more care in critical areas and it would be useful to be able to mark a patch for its level of risk, rather than just is it ready. That way we can gauge the risk/benefit. Examples of high risk would be checksums, examples of low risk would be logical replication patches. Anyway, some thoughts to discuss in May. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [WIP] pg_ping utility
On Sun, Jan 20, 2013 at 11:38 AM, Robert Haas robertmh...@gmail.com wrote: On Sun, Jan 20, 2013 at 9:58 AM, Phil Sorber p...@omniti.com wrote: This was done to silence useless error messages in the logs. If you attempt to connect as some user that does not exist, or to some database that does not exist, it throws an error in the logs, even with PQping. You could fix it with env vars, but since the point is to change the user/database that we were connecting as, I figured it should be consistent with all the other methods to do that. Uh, OK. Well, in that case, I'm inclined to think that a documentation mention is in order, and perhaps an update to the PQpingParams documentation as well. Because that's hardly obvious. :-( Ok. I can add something to the notes section of the docs. I can also add some code comments for this and for grabbing the default params. I use this to find the defaults if they don't pass anything in, so I know what to put in the status message at the end. I could devise my own way to come up with those values as I have seen in some other code, but I thought it was better to ask libpq directly what defaults it was going to use. Oh, I see. Is it really important to have the host and port in the output, or should we trim that down to just e.g. accepting connections? It seems useful to have that if a human is looking at the output, but maybe not if a machine is looking at the output. And if somebody doesn't want it, getting rid of it with sed or awk is nontrivial - imagine: pg_isready -d /tmp:5432 - accepting connections If you are scripting I'd assume you would use the return code value. It might be reasonable to make adding the host and port the verbose method and have just accepting connections as the default output, but my concern there is a misdiagnosis because someone doesn't realize what server they are connecting to. This way they can't miss it and they don't have to add another command line option to get that output. The other thing I thought about when you mentioned this is not doing the default lookups if it's in quiet mode. I could move things around to accomplish this, but not sure it is worth the effort and complexity. Thoughts? I had not considered this. I will take a look and provide an updated patch. Sounds good. -- 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] proposal: fix corner use case of variadic fuctions usage
Hello 2013/1/20 Tom Lane t...@sss.pgh.pa.us: Robert Haas robertmh...@gmail.com writes: On Sat, Jan 19, 2013 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: That's utter nonsense. Why wouldn't people expect concat(), for example, to work for large (or even just moderate-sized) arrays? /me blinks. What does that have to do with anything? IIUC, the question isn't whether CONCAT() would work for large arrays, but rather for very large numbers of arrays written out as CONCAT(a1, ..., a1000). No, the question is what happens with CONCAT(VARIADIC some-array-here), which currently just returns the array as-is, but which really ought to concat all the array elements as if they'd been separate arguments. Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. I am sending patch that is based on last Tom's proposal it missing some small fixes for other variadic any functions concat, concat_ws - I'll send it tomorrow Regards Pavel regards, tom lane variadic_any_fix.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
2013/1/20 Simon Riggs si...@2ndquadrant.com: On 20 January 2013 18:42, Robert Haas robertmh...@gmail.com wrote: On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes jeff.ja...@gmail.com wrote: Sometime this type of high-level summary review does happen, at the senior person's whim, but is not a formal part of the commit fest process. What I don't know is how much work it takes for one of those senior people to make one of those summary judgments, compared to how much it takes for them to just do an entire review from scratch. IME, making such summary judgements can often be done in a few minutes, but convincing that the patch submitter that you haven't created the objection purely as an obstacle to progress is the work of a lifetime. We could perhaps do better at avoiding perverse incentives, there. (Without meaning to paraphrase you in any negative way...) Judgements made in a few minutes are very frequently wrong, and it takes a lot of time to convince the person making snap decisions that they should revise their thinking in light of new or correct information. I feel we are very prone to making judgements on little information. This is especially true with regard to voting, with people zooming in from the side without having even read a patch to vote one way or the other, voting for the person not the idea. It can be a big problem telling the difference between a patch submitter that really is in possession of information that should be heeded and someone so blinded by their patch/idea that they mistakenly push forwards. At times, I have been both and I've also witnessed both as committer. There is a clear and understandable conservatism in being a reviewer/committer that people that haven't done it don't understand. I definitely didn't until I was a committer and I remember clearly me pushing for HS to go into 8.4 when it was a long way from being ready. I think it would be useful to expand the pool of committers and perhaps that can be done via some intermediate stage, though I do worry that the sense of responsibility might not be as strong in the intermediate rank ($NAME). I don't think we should be encouraging people to make fast decisions. Senior or not. (Though we must make decisions and have some coming soon). This is high in my mind right now since I've been looking at skip checkpoint patch for months, seeing how I feel about it. Nervous, basically. From that I think that some areas of the code are more critical than others and harder to fix in production if they go wrong. We need to be taking more care in critical areas and it would be useful to be able to mark a patch for its level of risk, rather than just is it ready. That way we can gauge the risk/benefit. Examples of high risk would be checksums, examples of low risk would be logical replication patches. Anyway, some thoughts to discuss in May. +1 Pavel -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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: fix corner use case of variadic fuctions usage
Robert Haas robertmh...@gmail.com writes: On Sat, Jan 19, 2013 at 11:58 AM, Pavel Stehule pavel.steh...@gmail.com wrote: We introduced VARIADIC any function. Motivation for this kind of function was bypassing postgresql's coerce rules - and own rules implementation for requested functionality. Some builtins function does it internally - but it is not possible for custom functions or it is not possible without some change in parser. Same motivation is reason why format function is VARIADIC any function. I'd just like to draw the attention of all assembled to the fact that this is another example of the cottage industry we've created in avoiding our own burdensome typecasting rules. I suppose this complaint is based on the idea that we could have declared format() as format(fmt text, VARIADIC values text[]) if only the argument matching rules were sufficiently permissive. I disagree with that though. For that to be anywhere near equivalent, we would have to allow argument matching to cast any data type to text, even if the corresponding cast were explicit-only. Surely that is too dangerous to consider. Even then it wouldn't be quite equivalent, because format() will work on any type whether or not there is a cast to text at all (since it relies on calling the type I/O functions instead). While VARIADIC ANY functions are surely a bit of a hack, I think they are a better solution than destroying the type system's ability to check function calls at all. At least the risks are confined to those specific functions, and not any function anywhere. 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] CF3+4 (was Re: Parallel query execution)
On Sun, Jan 20, 2013 at 2:39 PM, Simon Riggs si...@2ndquadrant.com wrote: (Without meaning to paraphrase you in any negative way...) Judgements made in a few minutes are very frequently wrong, and it takes a lot of time to convince the person making snap decisions that they should revise their thinking in light of new or correct information. I feel we are very prone to making judgements on little information. This is especially true with regard to voting, with people zooming in from the side without having even read a patch to vote one way or the other, voting for the person not the idea. It can be a big problem telling the difference between a patch submitter that really is in possession of information that should be heeded and someone so blinded by their patch/idea that they mistakenly push forwards. At times, I have been both and I've also witnessed both as committer. There is a clear and understandable conservatism in being a reviewer/committer that people that haven't done it don't understand. I definitely didn't until I was a committer and I remember clearly me pushing for HS to go into 8.4 when it was a long way from being ready. I think it would be useful to expand the pool of committers and perhaps that can be done via some intermediate stage, though I do worry that the sense of responsibility might not be as strong in the intermediate rank ($NAME). I don't think we should be encouraging people to make fast decisions. Senior or not. (Though we must make decisions and have some coming soon). This is high in my mind right now since I've been looking at skip checkpoint patch for months, seeing how I feel about it. Nervous, basically. From that I think that some areas of the code are more critical than others and harder to fix in production if they go wrong. We need to be taking more care in critical areas and it would be useful to be able to mark a patch for its level of risk, rather than just is it ready. That way we can gauge the risk/benefit. Examples of high risk would be checksums, examples of low risk would be logical replication patches. Anyway, some thoughts to discuss in May. I agree with some but not all of your observations here, but they're along a different line than the point I was trying to make. I would distinguish between value judgements (i.e. this patch is bad because we don't want that) and architectural judgments (i.e. this patch is bad because the logic you've added needs to go in the planner, not the parser). I often disagree with the value judgements that others make, regardless of how much or little time they've spent on them, because, well, at the end of the day, it's an opinion. Our experiences inform our opinions, and since we all have different experiences, we won't always have the same opinions about everything. Architectural judgements are something else again. If Tom says that a particular piece of logic needs to live in the planner rather than the parser, the chances that he is right are upwards of 90%, in my experience. There are more complicated or controversial cases, of course, but I find it's often not difficult to answer the question assuming we were going to do this, how ought we to do it?. People don't always like hearing those answers, and they're not *always* easy, but there are many cases where I think they are. -- 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] proposal: fix corner use case of variadic fuctions usage
On Sun, Jan 20, 2013 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sat, Jan 19, 2013 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: That's utter nonsense. Why wouldn't people expect concat(), for example, to work for large (or even just moderate-sized) arrays? /me blinks. What does that have to do with anything? IIUC, the question isn't whether CONCAT() would work for large arrays, but rather for very large numbers of arrays written out as CONCAT(a1, ..., a1000). No, the question is what happens with CONCAT(VARIADIC some-array-here), which currently just returns the array as-is, but which really ought to concat all the array elements as if they'd been separate arguments. Wow, that's pretty surprising behavior. Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. I don't know - how many of those will there really ever be? I mean, people only write functions as VARIADIC as a notational convenience, don't they? If you actually need to pass more than 100 separate pieces of data to a function, sending over 100+ parameters is almost certainly the Wrong Way To Do It. -- 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] proposal: fix corner use case of variadic fuctions usage
On Sun, Jan 20, 2013 at 3:21 PM, Tom Lane t...@sss.pgh.pa.us wrote: I suppose this complaint is based on the idea that we could have declared format() as format(fmt text, VARIADIC values text[]) if only the argument matching rules were sufficiently permissive. I disagree with that though. For that to be anywhere near equivalent, we would have to allow argument matching to cast any data type to text, even if the corresponding cast were explicit-only. Surely that is too dangerous to consider. Even then it wouldn't be quite equivalent, because format() will work on any type whether or not there is a cast to text at all (since it relies on calling the type I/O functions instead). Well, as previously discussed a number of times, all types are considered to have assignment casts to text via IO whether or not there is an entry in pg_cast. So the only case in which my proposal would have failed to make this work is if someone added an entry in pg_cast and tagged it as an explicit cast. I can't quite imagine what sort of situation might justify such a perplexing step, but if someone does it and it breaks this then I think they're getting what they paid for. So I think it's pretty close to equivalent. While VARIADIC ANY functions are surely a bit of a hack, I think they are a better solution than destroying the type system's ability to check function calls at all. At least the risks are confined to those specific functions, and not any function anywhere. I think this is hyperbole which ignores the facts on the ground. Over and over again, we've seen examples of users who are perplexed because there's only one function called wumpus() and we won't call it due to some perceived failure of the types to match sufficiently closely. Destroying the type system's ability to needlessly reject *unambiguous* calls is, IMHO, a feature, not a bug. -- 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] proposal: fix corner use case of variadic fuctions usage
2013/1/20 Robert Haas robertmh...@gmail.com: On Sun, Jan 20, 2013 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sat, Jan 19, 2013 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: That's utter nonsense. Why wouldn't people expect concat(), for example, to work for large (or even just moderate-sized) arrays? /me blinks. What does that have to do with anything? IIUC, the question isn't whether CONCAT() would work for large arrays, but rather for very large numbers of arrays written out as CONCAT(a1, ..., a1000). No, the question is what happens with CONCAT(VARIADIC some-array-here), which currently just returns the array as-is, but which really ought to concat all the array elements as if they'd been separate arguments. Wow, that's pretty surprising behavior. Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. one correction - I would to raise error, if array is larger than FUNC_MAX_ARGS. But is true, so this limit is for VARIADIC function synthetic, because parameters are passed in array. On second hand this is inconsistency. I don't know - how many of those will there really ever be? I mean, people only write functions as VARIADIC as a notational convenience, don't they? If you actually need to pass more than 100 separate pieces of data to a function, sending over 100+ parameters is almost certainly the Wrong Way To Do It. -- 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] proposal: fix corner use case of variadic fuctions usage
2013/1/20 Robert Haas robertmh...@gmail.com: On Sun, Jan 20, 2013 at 3:21 PM, Tom Lane t...@sss.pgh.pa.us wrote: I suppose this complaint is based on the idea that we could have declared format() as format(fmt text, VARIADIC values text[]) if only the argument matching rules were sufficiently permissive. I disagree with that though. For that to be anywhere near equivalent, we would have to allow argument matching to cast any data type to text, even if the corresponding cast were explicit-only. Surely that is too dangerous to consider. Even then it wouldn't be quite equivalent, because format() will work on any type whether or not there is a cast to text at all (since it relies on calling the type I/O functions instead). Well, as previously discussed a number of times, all types are considered to have assignment casts to text via IO whether or not there is an entry in pg_cast. So the only case in which my proposal would have failed to make this work is if someone added an entry in pg_cast and tagged it as an explicit cast. I can't quite imagine what sort of situation might justify such a perplexing step, but if someone does it and it breaks this then I think they're getting what they paid for. So I think it's pretty close to equivalent. While VARIADIC ANY functions are surely a bit of a hack, I think they are a better solution than destroying the type system's ability to check function calls at all. At least the risks are confined to those specific functions, and not any function anywhere. I think this is hyperbole which ignores the facts on the ground. Over and over again, we've seen examples of users who are perplexed because there's only one function called wumpus() and we won't call it due to some perceived failure of the types to match sufficiently closely. Destroying the type system's ability to needlessly reject *unambiguous* calls is, IMHO, a feature, not a bug. I am thinking so VARIADIC ANY functions is only one possible solution for functions with more complex coercion rules like oracle DECODE function. Just modification coercion rules is not enough - or we need to thinking about extensible parser and analyser. Regards Pavel -- 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] Removing PD_ALL_VISIBLE
On Thu, Jan 17, 2013 at 5:50 PM, Jeff Davis pg...@j-davis.com wrote: If the without interruption part becomes a practical problem, it seems fairly easy to fix: drop the pin and pick it up again once every K pages. Unless I'm missing something, this is a minor concern. I think probably so. Test plan: 1. Take current patch (without skip VM check for small tables optimization mentioned above). 2. Create 500 tables each about 1MB. 3. VACUUM them all. 4. Start 500 connections (one for each table) 5. Time the running of a loop that executes a COUNT(*) on that connection's table 100 times. I think shared_buffers=64MB is probably appropriate. We want some memory pressure so that it has to find and evict pages to satisfy the queries. But we don't want it to be totally exhausted and unable to even pin a new page; that really doesn't tell us much except that max_connections is too high. Sound reasonable? Well, it's certainly a data point, but each table in that case has 128 pages, so the effect is still pretty small. The place where you're going to run into trouble is when many of those tables have 4 pages each, or 2 pages each, or 1 page each. All of which is to say that I think this patch is premature. If we adopt something like this, we're likely never going to revert back to the way we do it now, and whatever cache-pressure or other costs this approach carries will be hear to stay - so we had better think awfully carefully before we do that. What about this patch makes it hard to undo/rework in the future? Well, if you have a bunch of code, and it preserves property X at all times, it is pretty easy to decide that new code need no longer preserve property X. It is essentially a no-op. The reverse, going through a bunch of existing code that does not consistently preserve property X and making it do so, is always much harder, because you've got to audit all the code you've already got. I don't want to undo the work that's been done on this over the last four years without a really good reason, and I'm not seeing one. My mistake. I believe that is already fixed, and certainly not a fundamental issue. It at least calls for a repetition of any performance testing that has already been done. -- 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] CF3+4 (was Re: Parallel query execution)
On Sunday, January 20, 2013, Simon Riggs wrote: On 20 January 2013 18:42, Robert Haas robertmh...@gmail.comjavascript:; wrote: On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes jeff.ja...@gmail.comjavascript:; wrote: Sometime this type of high-level summary review does happen, at the senior person's whim, but is not a formal part of the commit fest process. What I don't know is how much work it takes for one of those senior people to make one of those summary judgments, compared to how much it takes for them to just do an entire review from scratch. IME, making such summary judgements can often be done in a few minutes, but convincing that the patch submitter that you haven't created the objection purely as an obstacle to progress is the work of a lifetime. We could perhaps do better at avoiding perverse incentives, there. As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. That would provide me with some direction, rather than just having me pondering whether a certain variable name ought to have one more or one fewer underscores in it. On the other hand if a summary judgement is that the patch is fundamentally sound but needs some line-by-line code-lawyering, or some performance testing, or documentation/usability testing, or needs to ponder the implications to part XYZ of the code base (which neither I nor the other have even heard of before), that would also be good to know. My desire is not for you to tell me what the outcome of the review should be, but rather what the focus of it should be. As it is now, when I see a patch that needs review but has no comments, I don't know if that is because no senior developer has read it, or because they have read it but didn't think it needed comment. The wiki page does list points to consider when reviewing a submission, but not all points are equally relevant to all patches--and it is not always obvious to me which are more relevant. (Without meaning to paraphrase you in any negative way...) Judgements made in a few minutes are very frequently wrong, and it takes a lot of time to convince the person making snap decisions that they should revise their thinking in light of new or correct information. This is true, but I'd like to know up front that convincing them to revise their thinking is the main thing I need to do during the review process. Restating and clarifying the submitter's arguments, perhaps with the addition of some experimental evidence, is one part where I think I can contribute, provided I know that that is what I need to be doing. Having them reserve their opinion until after it is marked ready for commiter doesn't make it easier to change their mind, I don't think. As a reviewer I can't help address their specific concerns, if I don't know what those were. Cheers, Jeff
Re: [HACKERS] Doc patch making firm recommendation for setting the value of commit_delay
On 19 January 2013 20:38, Noah Misch n...@leadboat.com wrote: staticloud.com seems to be gone. Would you repost these? I've pushed these to a git repo, hosted on github. https://github.com/petergeoghegan/commit_delay_benchmarks I'm sorry that I didn't take the time to make the html benchmarks easily viewable within a browser on this occasion. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] WIP patch for hint bit i/o mitigation
On 01/18/2013 11:50 PM, Robert Haas wrote: On Fri, Jan 18, 2013 at 10:36 AM, Merlin Moncure mmonc...@gmail.com wrote: Any scenario that involves non-trivial amount of investigation or development should result in us pulling the patch for rework and resubmission in later 'festit's closing time as they say :-). Amen. OK, bumped to the next CF. -- Craig Ringer 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] dividing privileges for replication role.
Katsumata-san, In this patch, I made below. a) adding new privileges for replication:MASTER REPLICATION and CASCADE REPLICATION MASTER REPLICATION: Replication-connection to master server is only allowed CASCADE REPLICATION: Replication-connection to cascade server is only allowed (REPLICATION already implemented means replication-connection to both servers is allowed) This seems to me a case of making things more complicated for everyone in order to satisfy a very narrow use-case. It certainly doesn't seem to me to do anything about the accidental cycle issue. Am I missing something? -- 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] Making testing on Windows easier
On 01/17/2013 09:36 PM, Magnus Hagander wrote: Yeah. I used to have an AMI with the VS environment preinstalled on Amazon, but I managed to fat finger things and delete it at some point and haven't really had time to rebuild it. Having a script that would download and install all the pre-requisites on such a box would be *great*. I'm working on it: https://github.com/2ndQuadrant/pg_build_win http://blog.2ndquadrant.com/easier-postgresql-builds-for-windows/ I've identified the silent install procedures for most of the required tools (see the docs) and got build recipes written for some of the library dependencies. The next planned step is to have the scripts automatically download and silent-install Visual Studio, etc, rather than have the user run the command lines given in the README manually. It's usable as-is, I just need the time to finish it off. The goal is to have a script that turns building PostgreSQL on a clean fresh Windows install into: - Download ActivePerl - Install ActivePerl - Run buildgit.pl check install Right now it takes a fair bit more than that, but it's already better than a fully manual build. Then you could get up and going pretty quickly, and getting a Windows box up running for a few hours there is almost free, and you don't have to deal with licensing hassles. (Of course, the AMI method doesn't work all the way since you'd be distributing Visual Studio, but if we can have a script that auto-downloads-and-installs it as necessary we can get around that) I've found EC2 to be unusably slow for Windows builds, with a medium instance taking an hour and a half to do a simple build and vcregress check. They're also restrictive in disk space terms, so you land up needing to add a second EBS volume. A local kvm instance works well if a physical host isn't available; so do some of the alternative cloud providers like LunaCloud, which seems to perform significantly better in the quick testing I did. I haven't tried GoGrid yet. Many of us have Windows license stickers on laptops/desktops, even if we don't use the thing, so for a signficiant proportion of people it's as simple as downloading Windows install media ( http://blog.ringerc.id.au/2012/05/you-can-download-legal-windows-7-iso.html) and installing a KVM instance then shapshotting it. I've also put together a Jenkins server that runs builds on Windows whenever they're pushed to watched git repos. I'd love to make this public, but unfortunately allowing a wide group to run arbitrary code on the current build box isn't something I can afford. I'd need a system that launched a snapshot Windows instance for each build and then destroyed it at the end. This is entirely practical with something like KVM, so if I ever get the chance to work on a Jenkins plugin to do that (or to launch/destroy Windows cloud instances automatically), it's possible a semi-public Windows build testing service may be possible. While we're in fantasy land, the next step would be adding git URLs and branch names to the commitfest app, so it could ping a continuous integration server to build-test any new patch added to the CF. Right now I'm doing that manually when I have time, but it's slow going. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On 01/19/2013 04:08 AM, Boszormenyi Zoltan wrote: However, I am not sure whether Cygwin provides the mkstemp() call or not. Searching... Found bugzilla reports against mkstemp on Cygwin. Is Cygwin a platform that should be targeted for the server backend these days? I can understand making sure that libpq works on Cygwin, but is there any reason at all to run a Pg server backend on Cygwin rather than as native Windows binaries? -- Craig Ringer 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] Removing PD_ALL_VISIBLE
On Fri, Jan 18, 2013 at 3:31 AM, Jeff Davis pg...@j-davis.com wrote: So, I attached a new version of the patch that doesn't look at the VM for tables with fewer than 32 pages. That's the only change. That certainly seems worthwhile, but I still don't want to get rid of this code. I'm just not seeing a reason why that's something that desperately needs to be done. I don't think this is a barrier to anything else we want to do, and it might well be that the situations where this patch would hurt us are currently masked by other bottlenecks, but won't be always. Right now, the vast majority of heap updates don't need to pin the visibility map page; with this change, all of them do. Now, I accept that your test results show that that doesn't matter, but how can that not be an artifact of some kind? Can we really credit that accessing two pages costs no more than accessing one? To what countervailing factor could we plausibly attribute that? Now, even if it costs more in some narrow sense, the difference might not be enough to matter. But without some big gain on the other side, why tinker with it? -- 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On 01/21/2013 10:03 AM, Craig Ringer wrote: On 01/19/2013 04:08 AM, Boszormenyi Zoltan wrote: However, I am not sure whether Cygwin provides the mkstemp() call or not. Searching... Found bugzilla reports against mkstemp on Cygwin. Is Cygwin a platform that should be targeted for the server backend these days? I can understand making sure that libpq works on Cygwin, but is there any reason at all to run a Pg server backend on Cygwin rather than as native Windows binaries? I'm not suggesting immediately dropping working support, since this is so trivially worked around. I'm just wondering why anybody cares about the platform. -- Craig Ringer 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] How to build contrib module separately in PostgreSQL on windows
On 01/19/2013 11:09 PM, 朱冯贶天 wrote: After downloading the source code, I enter the postgresql-9.2.2\contrib\cube to type 'nmake' with VS2010 command environment. However, the Makefile is not compatible with vs2010. Correct - PostgreSQL's makefiles are written for GNU make and a unix-like shell (sh/bash/ash/dash etc). They will not work with NMake. The Windows build infrastructure uses src/tools/msvc/build.pl as pointed out by the link Andrew posted, http://www.postgresql.org/docs/current/static/install-windows.html . build.pl reads the GNU makefiles and produces MSBuild (Microsoft's nmake replacement) files that it then runs to compile the sources with the Visual Studio compilers. I've never worked out how to build contrib modules externally with it, though I haven't looked particularly hard. I find it effective to just drop the contrib module I want to build into the contrib/ directory of the source tree then use build.pl to compile the tree. I'd really like to improve this situation - finding and documenting the external module build method if one exists, and failing that implementing it. As always, it's down to time and priorities. I know that there is a way to build the whole PostgreSQL from source code. But it is not convenient. As far as I know it's your best option right now. I've tried to make it a bit more convenient with this code and documentation: https://github.com/2ndQuadrant/pg_build_win -- Craig Ringer 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] Contrib PROGRAM problem
On 01/19/2013 05:42 AM, Boszormenyi Zoltan wrote: Hi, I want to test my lock_timeout code under Windows and I compiled the whole PG universe with the MinGW cross-compiler for 64-bit under Fedora 18. You're significantly better off compiling for native Windows if at all possible. Windows cloud hosted instances with bundled licenses are available for peanuts or you can download a Windows DVD and install it in a KVM instance if you have a license sticker sticking to a now-running-Linux box somewhere. -- Craig Ringer 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] dividing privileges for replication role.
On Sat, Jan 19, 2013 at 12:47 PM, Tomonari Katsumata t.katsumata1...@gmail.com wrote: a) adding new privileges for replication:MASTER REPLICATION and CASCADE REPLICATION MASTER REPLICATION: Replication-connection to master server is only allowed CASCADE REPLICATION: Replication-connection to cascade server is only allowed (REPLICATION already implemented means replication-connection to both servers is allowed) This does not really solve the case you reported because, as reported in your bug, you could still have each slave connecting to each other using the privilege CASCADE REPLICATION. It makes even the privilege level more complicated. What would be necessary to solve your problem would be to have each standby being aware that it is connected to a unique master. This is not really an issue with privileges but more of something like having a standby scanning its upper cluster node tree and check if there is a master connected. While checking the cluster node tree, you will also need to be aware if a node has already been found when you scanned it to be sure that the same node has not been scanned, what would mean that you are in a cycle. -- Michael Paquier http://michael.otacoo.com
Re: [HACKERS] CF3+4 (was Re: Parallel query execution)
On Sun, Jan 20, 2013 at 7:07 PM, Jeff Janes jeff.ja...@gmail.com wrote: As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. That's a hard question. I'm not sure there's a categorical answer. I take positions both in support of and in opposition to the positions of other reviewers, and I suppose I don't see why you shouldn't do the same. It is of course worth bearing in mind that unless at least one committer is willing to support a given approach, it's not going anywhere ... but on the other hand, committers are just people, and do sometimes change their minds, too. So, really, I think you should try to persuade the person that you think is wrong, whoever that is. What I like least is when someone writes a review and says some people think this is a good idea and some people think it's a bad idea. When I go back to look at the discussion and make a decision about whether to commit something, I want to have a clear idea whether most people liked it or most people didn't like it, and why. That helps to inform my thinking. When it's just me and the submitter, it's hard to tell whether anyone cares at all, one way or the other. As it is now, when I see a patch that needs review but has no comments, I don't know if that is because no senior developer has read it, or because they have read it but didn't think it needed comment. The wiki page does list points to consider when reviewing a submission, but not all points are equally relevant to all patches--and it is not always obvious to me which are more relevant. The main things I personally like to see reviewers do, in descending order of importance, are: 1. express about whether we want it or not, with supporting reasoning 2. review the architecture and draw attention to any possibly worrisome points 3. checklist items (does it have docs? does it have regression tests? coding style OK? etc.) This is true, but I'd like to know up front that convincing them to revise their thinking is the main thing I need to do during the review process. Restating and clarifying the submitter's arguments, perhaps with the addition of some experimental evidence, is one part where I think I can contribute, provided I know that that is what I need to be doing. Agreed. Having them reserve their opinion until after it is marked ready for commiter doesn't make it easier to change their mind, I don't think. As a reviewer I can't help address their specific concerns, if I don't know what those were. Also agreed. I think one important duty of a reviewer (alluded to above) is to try to draw focus to whatever the central issues around the patch are. For some patches, the big question is is it OK to break backward compatibility like this?, whereas for others it may be is this actually useful? and for still others it may be does the SQL standard have anything to say about this?. Even if you as a reviewer don't know the answer to those questions, clearly delineating the key issues may enable others to comment without having to read and understand the whole patch, which can expedite things considerably. -- 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] logical changeset generation v4
On Fri, Jan 18, 2013 at 12:32 PM, Andres Freund and...@2ndquadrant.com wrote: Makes sense? Yes. The catalog timetravel stuff still gives me heartburn. The idea of treating system catalogs in a special way has never sat well with me and still doesn't - not that I am sure what I'd like better. The complexity of the whole system is also somewhat daunting. But my question with relation to this specific patch was mostly whether setting the table OID everywhere was worth worrying about from a performance standpoint, or whether any of the other adjustments this patch makes could have negative consequences there, since the Satisfies functions can get very hot on some workloads. It seems like the consensus is no, that's not worth worrying about, at least as far as the table OIDs are concerned. -- 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] patch to add \watch to psql
On 01/21/2013 02:11 AM, Dickson S. Guedes wrote: 2013/1/17 Daniel Farina dan...@heroku.com: I realized while making my adjustments that I pointlessly grew some input checking in the inner loop. I just hoisted it out in this version. Since psql uses libreadline, what do you think about to call rl_clear_screen() inside that while (true) loop? Obviously we should test #if we have readline enabled to use it, but when we have it a nice output will bring to us. BTW, I don't know how this will behaves on OSX or Windows. OSX uses libedit, which is readline-compatible modulo some bugs. Windows doesn't have line-editing (sadly) so it won't do anything at all. -- Craig Ringer 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] proposal: fix corner use case of variadic fuctions usage
Robert Haas robertmh...@gmail.com writes: On Sun, Jan 20, 2013 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. I don't know - how many of those will there really ever be? I mean, people only write functions as VARIADIC as a notational convenience, don't they? If you actually need to pass more than 100 separate pieces of data to a function, sending over 100+ parameters is almost certainly the Wrong Way To Do It. Well, not necessarily, if they're reasonably expressed as an array. I would also point out that there is no corresponding limitation on variadic functions that take any type other than ANY. Indeed, despite Pavel's claim to the contrary, I'm pretty sure it's seen as a feature that there's no specific upper limit to how many parameters you can pass to a variadic function when using the VARIADIC array-value syntax. It's certainly a feature that you can pass a varying number of parameters that way, thereby evading the syntactic fact that you can't pass a varying number of parameters any other way. I don't see how come it isn't a feature that you can evade the FUNC_MAX_ARGS limit that way, or why we'd consider it acceptable for variably-sized parameter arrays to have such a small arbitrary limit. 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] CF3+4 (was Re: Parallel query execution)
On 01/20/2013 09:37 PM, Robert Haas wrote: On Sun, Jan 20, 2013 at 7:07 PM, Jeff Janes jeff.ja...@gmail.com wrote: As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. That's a hard question. I don't think so. IMNSHO your job is neither - it is to give us your independent judgment. 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] CF3+4 (was Re: Parallel query execution)
On Sun, Jan 20, 2013 at 10:04 PM, Andrew Dunstan and...@dunslane.net wrote: On 01/20/2013 09:37 PM, Robert Haas wrote: On Sun, Jan 20, 2013 at 7:07 PM, Jeff Janes jeff.ja...@gmail.com wrote: As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. That's a hard question. I don't think so. IMNSHO your job is neither - it is to give us your independent judgment. That's pretty much what I went on to say. -- 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] CF3+4 (was Re: Parallel query execution)
Robert Haas robertmh...@gmail.com writes: On Sun, Jan 20, 2013 at 7:07 PM, Jeff Janes jeff.ja...@gmail.com wrote: As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. ... I think you should try to persuade the person that you think is wrong, whoever that is. Absolutely. Form your own opinion and marshal an argument for it. At the end of the day, most of us are reasonable people and can be convinced by appropriate evidence. 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] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Monday, January 21, 2013 7:36 AM Craig Ringer wrote: On 01/21/2013 10:03 AM, Craig Ringer wrote: On 01/19/2013 04:08 AM, Boszormenyi Zoltan wrote: However, I am not sure whether Cygwin provides the mkstemp() call or not. Searching... Found bugzilla reports against mkstemp on Cygwin. Is Cygwin a platform that should be targeted for the server backend these days? I can understand making sure that libpq works on Cygwin, but is there any reason at all to run a Pg server backend on Cygwin rather than as native Windows binaries? I'm not suggesting immediately dropping working support, since this is so trivially worked around. I'm just wondering why anybody cares about the platform. We have avoided the use of mkstemp with small native implementation so now it won't be problem for any platform. With Regards, Amit Kapila. -- 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] Removing PD_ALL_VISIBLE
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 18, 2013 at 3:31 AM, Jeff Davis pg...@j-davis.com wrote: So, I attached a new version of the patch that doesn't look at the VM for tables with fewer than 32 pages. That's the only change. That certainly seems worthwhile, but I still don't want to get rid of this code. I'm just not seeing a reason why that's something that desperately needs to be done. Yeah, I'm having the same problem. Despite Jeff's test results, I can't help thinking that lack of PD_ALL_VISIBLE *will* hurt us under some workloads, and it's not obvious to me what benefit we get from dropping 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] proposal: fix corner use case of variadic fuctions usage
On Sun, Jan 20, 2013 at 10:01 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jan 20, 2013 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. I don't know - how many of those will there really ever be? I mean, people only write functions as VARIADIC as a notational convenience, don't they? If you actually need to pass more than 100 separate pieces of data to a function, sending over 100+ parameters is almost certainly the Wrong Way To Do It. Well, not necessarily, if they're reasonably expressed as an array. I would also point out that there is no corresponding limitation on variadic functions that take any type other than ANY. Indeed, despite Pavel's claim to the contrary, I'm pretty sure it's seen as a feature that there's no specific upper limit to how many parameters you can pass to a variadic function when using the VARIADIC array-value syntax. It's certainly a feature that you can pass a varying number of parameters that way, thereby evading the syntactic fact that you can't pass a varying number of parameters any other way. I don't see how come it isn't a feature that you can evade the FUNC_MAX_ARGS limit that way, or why we'd consider it acceptable for variably-sized parameter arrays to have such a small arbitrary limit. OK, I see. If people are already counting on there being no fixed limit for variadic functions with a type other than any, then it would indeed seem weird to make any an exception. I'm not sure how much practical use case there is for such a thing, but still. -- 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] [WIP] pg_ping utility
On Sun, Jan 20, 2013 at 2:59 PM, Phil Sorber p...@omniti.com wrote: Ok. I can add something to the notes section of the docs. I can also add some code comments for this and for grabbing the default params. Sounds good. Oh, I see. Is it really important to have the host and port in the output, or should we trim that down to just e.g. accepting connections? It seems useful to have that if a human is looking at the output, but maybe not if a machine is looking at the output. And if somebody doesn't want it, getting rid of it with sed or awk is nontrivial - imagine: pg_isready -d /tmp:5432 - accepting connections If you are scripting I'd assume you would use the return code value. It might be reasonable to make adding the host and port the verbose method and have just accepting connections as the default output, but my concern there is a misdiagnosis because someone doesn't realize what server they are connecting to. This way they can't miss it and they don't have to add another command line option to get that output. It's a fair concern. Does anyone else have an opinion on this? The other thing I thought about when you mentioned this is not doing the default lookups if it's in quiet mode. I could move things around to accomplish this, but not sure it is worth the effort and complexity. Thoughts? That doesn't seem to buy us anything. I'm fine with the code, now that I see what it's intended to do. It doesn't cost anything noticeable in terms of efficiency; I think, I just didn't want to make things complicated without a reason. -- 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] dividing privileges for replication role.
On 01/19/2013 11:47 AM, Tomonari Katsumata wrote: Hi, I made a patch to divide privileges for replication role. I've added your patch to the commitfest tracking app for the post-9.3 release; see https://commitfest.postgresql.org/action/patch_view?id=1072 . If it's convenient for you to keep that entry up to date with any revised patches you get and any reviews people write that will be rather helpful. I'll keep an eye on it and update it when I see something change, but you're paying attention to this one patch so you'll notice first. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] [PATCH] Fix NULL checking in check_TSCurrentConfig()
Stephen Frost sfr...@snowman.net writes: * Xi Wang (xi.w...@gmail.com) wrote: The correct NULL check should use `*newval'; `newval' must be non-null. Why isn't this using pstrdup()..? The GUC API uses malloc, mainly because guc.c can't afford to lose control on out-of-memory situations. 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] [PATCH] Fix NULL checking in check_TSCurrentConfig()
Xi Wang xi.w...@gmail.com writes: The correct NULL check should use `*newval'; `newval' must be non-null. Great catch, will commit. (But first I'm looking through commit 2594cf0e to see if I made the same mistake anywhere else :-(.) How did you find that, coverity or some such tool? 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] [WIP] pg_ping utility
On 01/21/2013 11:26 AM, Robert Haas wrote: On Sun, Jan 20, 2013 at 2:59 PM, Phil Sorber p...@omniti.com wrote: Ok. I can add something to the notes section of the docs. I can also add some code comments for this and for grabbing the default params. Sounds good. Oh, I see. Is it really important to have the host and port in the output, or should we trim that down to just e.g. accepting connections? It seems useful to have that if a human is looking at the output, but maybe not if a machine is looking at the output. And if somebody doesn't want it, getting rid of it with sed or awk is nontrivial - imagine: pg_isready -d /tmp:5432 - accepting connections If you are scripting I'd assume you would use the return code value. It might be reasonable to make adding the host and port the verbose method and have just accepting connections as the default output, but my concern there is a misdiagnosis because someone doesn't realize what server they are connecting to. This way they can't miss it and they don't have to add another command line option to get that output. It's a fair concern. Does anyone else have an opinion on this? I have a strong view that the host and port *should* be printed in output. One of the most common issues on Stack Overflow questions from new users is with I can't connect problems that turn out to be them connecting to the wrong host, port, or socket path. It is absolutely vital that the unix socket path being used be printed if unix socket connections are tested, as Mac OS X's insane setup means that PostgreSQL tool builds on that platform regularly use the system libpq not the user-installed PostgreSQL's libpq, and it defaults to a different socket path. If users use pg_ping to verify that their PostgreSQL instance is running it'll use the user-installed libpq - fine, that's what we want. But the user will then wonder why the heck Ruby on Rails's `pg' gem reports it can't connect to the unix socket. They can't compare the unix socket paths printed in the error message if pg_ping doesn't show it. I would like to see pg_ping produce a similar error to psql on connection failure. $ psql -p psql: could not connect to server: No such file or directory Is the server running locally and accepting connections on Unix domain socket /tmp/.s.PGSQL.? $ psql -h localhost -p psql: could not connect to server: Connection refused Is the server running on host localhost (::1) and accepting TCP/IP connections on port ? could not connect to server: Connection refused Is the server running on host localhost (127.0.0.1) and accepting TCP/IP connections on port ? -- Craig Ringer 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] [PATCH] Fix off-by-one in PQprintTuples()
Stephen Frost sfr...@snowman.net writes: Strictly speaking, it's this: tborder[i] = '\0'; Which ends up writing past the end of the buffer (which is allocated as 'width + 1'). Perhaps we should also change that to be: tborder[width] = '\0'; Yeah, I like that better too. Will commit. 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] proposal: fix corner use case of variadic fuctions usage
2013/1/21 Robert Haas robertmh...@gmail.com: On Sun, Jan 20, 2013 at 10:01 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Sun, Jan 20, 2013 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Pavel is claiming it's okay for that to fall over if the array has more than 100 elements. I disagree, not only for the specific case of CONCAT(), but with the more general implication that such a limitation is going to be okay for any VARIADIC ANY function that anyone will ever write. I don't know - how many of those will there really ever be? I mean, people only write functions as VARIADIC as a notational convenience, don't they? If you actually need to pass more than 100 separate pieces of data to a function, sending over 100+ parameters is almost certainly the Wrong Way To Do It. Well, not necessarily, if they're reasonably expressed as an array. I would also point out that there is no corresponding limitation on variadic functions that take any type other than ANY. Indeed, despite Pavel's claim to the contrary, I'm pretty sure it's seen as a feature that there's no specific upper limit to how many parameters you can pass to a variadic function when using the VARIADIC array-value syntax. It's certainly a feature that you can pass a varying number of parameters that way, thereby evading the syntactic fact that you can't pass a varying number of parameters any other way. I don't see how come it isn't a feature that you can evade the FUNC_MAX_ARGS limit that way, or why we'd consider it acceptable for variably-sized parameter arrays to have such a small arbitrary limit. OK, I see. If people are already counting on there being no fixed limit for variadic functions with a type other than any, then it would indeed seem weird to make any an exception. I'm not sure how much practical use case there is for such a thing, but still. after sleeping and some thinking about topic - yes - Tom opinion is correct too - theoretically we can count all variadic argument as one. Exception is just VARIADIC any when is called usually - it can be only better documented - I don't see a problem now Regards Pavel -- 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
[HACKERS] Error Building rpm
Hi All, I am trying to build rpm of PostgreSQL, and the approach is building rpm for 1. CentOS 6+ 2. Fedora 15+ 3. RedHat 6+ 4. OpenSUSE 5. SuSE via single spec file and not using any external rpm or repo while building the problem i am facing right now is there is some dependencies which is now not provided by RedHat and SUSE 1. SLE_11_SP2 perl-ExtUtils-Embed, uuid-devel, openldap-devel (For this i am using openldap2-devel its working fine so no problem) 2.openSUSE_12.2 perl-ExtUtils-Embed, openldap-devel (For this i am using openldap2-devel its working fine so no problem) 3.RedHat_RHEL-6 uuid-devel (Now RedHat is not providing this rpm) If i am installing uuid-devel from external source i am able to build rpm but how to build without installing it from external repo. now redhat is giving libuuid. and Suse is not having perl-ExtUtils-Embed rpm. Please advised. -- ViVek Raghuwanshi Mobile -+91-09595950504 Skype - vivek_raghuwanshi IRC vivekraghuwanshi http://vivekraghuwanshi.wordpress.com/ http://in.linkedin.com/in/vivekraghuwanshi
Re: [HACKERS] Error Building rpm
On 1/20/2013 9:23 PM, Vivek Singh Raghuwanshi wrote: 3.RedHat_RHEL-6 uuid-devel (Now RedHat is not providing this rpm) you sure about that? now, I'm running CentOS 6 not RHEL6, but the packages are 1:1 and built from the same SRPMs. uuid-devel.i686 1.6.1-10.el6 base uuid-devel.x86_64 1.6.1-10.el6 base -- 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] [pgsql-www] Error Building rpm
Vivek Singh Raghuwanshi vivekraghuwan...@gmail.com writes: 3.RedHat_RHEL-6 uuid-devel (Now RedHat is not providing this rpm) works for me in RHEL-6 ... 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] Error Building rpm
Hi, On Mon, 2013-01-21 at 10:53 +0530, Vivek Singh Raghuwanshi wrote: I am trying to build rpm of PostgreSQL, and the approach is building rpm for 1. CentOS 6+ 2. Fedora 15+ 3. RedHat 6+ 4. OpenSUSE 5. SuSE via single spec file and not using any external rpm or repo while building Building RPMs using a single spec file is almost impossible, as I wrote you in my previous emails. SuSE has different package names, Fedora 15+ has separate init system (systemd), etc. That's why I am keeping separate copies of each spec file for Fedora and its derivatives (RHEL, SL, CentOS) separately. -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] gistchoose vs. bloat
Jeff Davis pg...@j-davis.com writes: On Fri, 2012-12-14 at 18:36 +0200, Heikki Linnakangas wrote: BTW, I don't much like the option name randomization. It's not clear what's been randomized. I'd prefer something like distribute_on_equal_penalty, although that's really long. Better ideas? I agree that randomization is vague, but I can't think of anything better. I looked at this patch. ISTM we should not have the option at all but just do it always. I cannot believe that always-go-left is ever a preferable strategy in the long run; the resulting imbalance in the index will surely kill any possible benefit. Even if there are some cases where it miraculously fails to lose, how many users are going to realize that applies to their case and make use of the option? 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] Further pg_upgrade analysis for many tables
On Sunday, January 20, 2013, Stephen Frost wrote: * Jeff Janes (jeff.ja...@gmail.com javascript:;) wrote: By making the list over-flowable, we fix a demonstrated pathological workload (restore of huge schemas); we impose no detectable penalty to normal workloads; and we fail to improve, but also fail to make worse, a hypothetical pathological workload. All at the expense of a few bytes per backend. [...] Why does the list not grow as needed? It would increase the code complexity for no concretely-known benefit. I'm curious if this is going to help with rollback's of transactions which created lots of tables..? We've certainly seen that take much longer than we'd like, although I've generally attributed it to doing all of the unlink'ing and truncating of files. If you are using large shared_buffers, then you will probably get more benefit from a different recent commit: 279628a Accelerate end-of-transaction dropping of relations. Cheers, Jeff
Re: [HACKERS] Removing PD_ALL_VISIBLE
On Mon, Jan 21, 2013 at 8:49 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jan 18, 2013 at 3:31 AM, Jeff Davis pg...@j-davis.com wrote: So, I attached a new version of the patch that doesn't look at the VM for tables with fewer than 32 pages. That's the only change. That certainly seems worthwhile, but I still don't want to get rid of this code. I'm just not seeing a reason why that's something that desperately needs to be done. Yeah, I'm having the same problem. Despite Jeff's test results, I can't help thinking that lack of PD_ALL_VISIBLE *will* hurt us under some workloads, and it's not obvious to me what benefit we get from dropping it. I tend to agree. When I looked at the patch, I thought since its removing a WAL record (and associated redo logic), it has some additional value. But that was kind of broken (sorry, I haven't looked at the latest patch if Jeff fixed it without requiring to reinstate the WAL logging). I also thought that bug invalidates the performance numbers reported. Of course, there is an argument that this patch will simplify the code, but I'm not sure if its enough to justify the additional contention which may or may not show up in the benchmarks we are running, but we know its there. 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
Re: [HACKERS] pg_dump transaction's read-only mode
On Sun, Jan 20, 2013 at 4:29 AM, Tom Lane t...@sss.pgh.pa.us wrote: Pavan Deolasee pavan.deola...@gmail.com writes: Sorry for posting on such an old thread. But here is a patch that fixes this. I'm also adding to the next commitfest so that we don't lose track of it again. As submitted, this broke pg_dump for dumping from pre-8.0 servers. (7.4 didn't accept commas in SET TRANSACTION syntax, and versions before that didn't have the READ ONLY option at all.) My bad. I did not realize that pg_dump is still supported for pre-8.0 releases. I fixed that and committed it. 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
Re: [HACKERS] Error Building rpm
Thanks Devrim, But i am trying to achieve this via multiple if conditions , can you send me your redhat and suse spec files. On Mon, Jan 21, 2013 at 11:09 AM, Devrim GÜNDÜZ dev...@gunduz.org wrote: Hi, On Mon, 2013-01-21 at 10:53 +0530, Vivek Singh Raghuwanshi wrote: I am trying to build rpm of PostgreSQL, and the approach is building rpm for 1. CentOS 6+ 2. Fedora 15+ 3. RedHat 6+ 4. OpenSUSE 5. SuSE via single spec file and not using any external rpm or repo while building Building RPMs using a single spec file is almost impossible, as I wrote you in my previous emails. SuSE has different package names, Fedora 15+ has separate init system (systemd), etc. That's why I am keeping separate copies of each spec file for Fedora and its derivatives (RHEL, SL, CentOS) separately. -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz -- ViVek Raghuwanshi Mobile -+91-09595950504 Skype - vivek_raghuwanshi IRC vivekraghuwanshi http://vivekraghuwanshi.wordpress.com/ http://in.linkedin.com/in/vivekraghuwanshi
Re: [HACKERS] Removing PD_ALL_VISIBLE
On Sun, 2013-01-20 at 22:19 -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jan 18, 2013 at 3:31 AM, Jeff Davis pg...@j-davis.com wrote: So, I attached a new version of the patch that doesn't look at the VM for tables with fewer than 32 pages. That's the only change. That certainly seems worthwhile, but I still don't want to get rid of this code. I'm just not seeing a reason why that's something that desperately needs to be done. Yeah, I'm having the same problem. Despite Jeff's test results, I can't help thinking that lack of PD_ALL_VISIBLE *will* hurt us under some workloads, and it's not obvious to me what benefit we get from dropping it. OK. I respectfully disagree with the arguments I've seen so far, but we can all move on. I already had some more test results (again, thanks to Nathan Boley), so I finished them up and attached them to the bottom of this email for the archives (there's always hope, right?). Regards, Jeff Davis Test goal: is 32 is an appropriate threshold for using the VM after we remove PD_ALL_VISIBLE? Test setup: 500 31-page tables and 500 33-page tables. Test recent build against patched version, with varying shared_buffers. The first test is 500 connections each doing 20 iterations of COUNT(*) against the 500 31-page tables. The next test is the same, except against the 33-page tables. Test results: (There were a few outliers I discarded as being too fast. It always happened in the first run, and I strongly suspect the connections began unevenly, leading to lower concurrency. They didn't seem to favor one build over another.) master, 31-page (first column is shared_buffers, second is range of seconds): 32MB: 5.8 - 6.1 64MB: 3.1 - 3.7 96MB: 1.7 - 2.2 112MB: 0.6 - 1.1 128MB: 0.4 - 0.4 256MB: 0.4 - 0.4 patch, 31-page (doesn't use VM because 31 is below threshold): 32MB: 5.1 - 5.9 64MB: 3.4 - 3.8 96MB: 1.7 - 2.0 112MB: 0.7 - 1.1 128MB: 0.4 - 0.5 256MB: 0.4 - 0.5 master, 33-page: 32MB: 5.9 - 7.0 64MB: 4.2 - 4.7 96MB: 2.4 - 2.8 112MB: 1.2 - 1.6 128MB: 0.5 - 0.5 256MB: 0.4 - 0.5 patch, 33-page (does use VM because 33 is above threshold): 32MB: 6.2 - 7.2 64MB: 4.4 - 4.7 96MB: 2.8 - 3.0 112MB: 1.7 - 1.8 128MB: 0.5 - 1.0 256MB: 0.4 - 0.5 Conclusion: 32 pages is a low enough threshold that skipping the VM is insignificant. When the 500 tables are 33 pages, and it does use the VM, we do see a measurable cost under cache pressure. The penalty is fairly small (10% ballpark), and this is a worst-case, so I don't think it's a problem. From the last test results, we know it gets back to even by the time the tables are 128 pages (1MB). So it could be that there is a slightly higher threshold (between 32 and 128) that would be slightly better. But given how specific this case is and how small the penalty is, I think 32 is a fine threshold. Also, to reiterate, I focused my tests almost entirely on scans, though some of the concern was around inserts/updates/deletes. I tried, but was unable to show any difference on those tests. I suspect that the bottleneck is elsewhere. -- 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] Error Building rpm
Hi, On Mon, 2013-01-21 at 11:33 +0530, Vivek Singh Raghuwanshi wrote: But i am trying to achieve this via multiple if conditions , can you send me your redhat and suse spec files. As I have emailed you before, spec files,etc. are at http://svn.pgrpms.org/repo Regards, -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] Error Building rpm
On Mon, Jan 21, 2013 at 12:11 PM, Devrim GÜNDÜZ dev...@gunduz.org wrote: Hi, On Mon, 2013-01-21 at 11:33 +0530, Vivek Singh Raghuwanshi wrote: But i am trying to achieve this via multiple if conditions , can you send me your redhat and suse spec files. As I have emailed you before, spec files,etc. are at http://svn.pgrpms.org/repo Spec file for SuSE is only for 8.4 not for 9.2+ Regards, -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz -- ViVek Raghuwanshi Mobile -+91-09595950504 Skype - vivek_raghuwanshi IRC vivekraghuwanshi http://vivekraghuwanshi.wordpress.com/ http://in.linkedin.com/in/vivekraghuwanshi
Re: [HACKERS] Removing PD_ALL_VISIBLE
On Mon, 2013-01-21 at 11:27 +0530, Pavan Deolasee wrote: I tend to agree. When I looked at the patch, I thought since its removing a WAL record (and associated redo logic), it has some additional value. But that was kind of broken (sorry, I haven't looked at the latest patch if Jeff fixed it without requiring to reinstate the WAL logging). I also thought that bug invalidates the performance numbers reported. I revalidated the performance numbers after reinstating the WAL logging. No difference (which is unsurprising, since my tests were read-only). Of course, there is an argument that this patch will simplify the code, but I'm not sure if its enough to justify the additional contention which may or may not show up in the benchmarks we are running, but we know its there. What additional contention? How do you know it's there? The design is to keep the VM page pinned, and to read it without requiring locks (like index-only scans already do). So I don't see any obvious additional contention there, unless you're talking about the work the CPU does for cache coherency (which did not show up in any of my tests). I understand that my patch is probably not going in, but I would like to be clear about what is a known practical problem, what is a theoretical problem that has eluded my testing capabilities, and what is no problem at all. Regards, Jeff Davis -- 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] gistchoose vs. bloat
On Mon, 2013-01-21 at 00:48 -0500, Tom Lane wrote: I looked at this patch. ISTM we should not have the option at all but just do it always. I cannot believe that always-go-left is ever a preferable strategy in the long run; the resulting imbalance in the index will surely kill any possible benefit. Even if there are some cases where it miraculously fails to lose, how many users are going to realize that applies to their case and make use of the option? Sounds good to me. If I remember correctly, there was also an argument that it may be useful for repeatable test results. That's a little questionable for performance (except in those cases where few penalties are identical anyway), but could plausibly be useful for a crash report or something. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] standby, pg_basebackup and last xlog file
Hello! I wrote to general ( [GENERAL] standby, pg_basebackup and last xlog file ) some times ago. but still hasn't got any feedback. Hello! Is there any reason why pg_basebackup has limitation in an online backup from the standby: The backup history file is not created in the database cluster backed up. ? So i can't get last xlog file needed to restore :( Think i can use -x option for getting last xlog file, but i would like to minimize size of resulting backup. // i also get all WALs by archive_command, so there is no reason to get two copies of each wal during basebackup. Also maybe i can use something like ( pg_last_xlog_replay_location() + 1 ) after pg_basebackup finished. Does anybody know true way to getting last xlog file in case of applying pg_basebackup to standby? How pg_basebackup gets last xlog file in case of standby and -x option?
Re: [HACKERS] Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
On Jan 21, 2013 3:06 AM, Craig Ringer cr...@2ndquadrant.com wrote: On 01/21/2013 10:03 AM, Craig Ringer wrote: On 01/19/2013 04:08 AM, Boszormenyi Zoltan wrote: However, I am not sure whether Cygwin provides the mkstemp() call or not. Searching... Found bugzilla reports against mkstemp on Cygwin. Is Cygwin a platform that should be targeted for the server backend these days? I can understand making sure that libpq works on Cygwin, but is there any reason at all to run a Pg server backend on Cygwin rather than as native Windows binaries? I'm not suggesting immediately dropping working support, since this is so trivially worked around. I'm just wondering why anybody cares about the platform. I have suggested similar before, and been voted down :) iirc Andrew uses it, no? Either way, the consensus earlier had been that as long as it doesn't require major surgery or blocks something else, we should try to keep it working. And as you say this sounds like something that can be handled trivially, I think now is not the time. /Magnus
Re: [HACKERS] Removing PD_ALL_VISIBLE
On Mon, Jan 21, 2013 at 12:22 PM, Jeff Davis pg...@j-davis.com wrote: On Mon, 2013-01-21 at 11:27 +0530, Pavan Deolasee wrote: Of course, there is an argument that this patch will simplify the code, but I'm not sure if its enough to justify the additional contention which may or may not show up in the benchmarks we are running, but we know its there. What additional contention? How do you know it's there? At the minimum your patch will need to have one additional buffer pinned for every K 8192 * 8 heap pages. For most cases, the value of K will be large enough to ignore the overhead, but for small values of K, I'm not sure if we can say that with confidence. Of course, for very small tables the real contention might be somewhere else and so this change may not show up anything on the benchmarks. Doesn't your own tests for 33 page tables confirm that there is indeed contention for this case ? I agree its not huge, but I don't know if there are workloads where it will matter. I understand that my patch is probably not going in, Sorry about that. I know how that feels. But we need some more reasons to justify this change, especially because the performance numbers themselves are not showing any signs either way. One could have argued that this saves us a valuable page level bit, but with pg_upgrade etc it has become hard to re-utilize page level bits for other purposes and we don't yet have a pressing need for more bits. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee
Re: [HACKERS] logical changeset generation v4
On 2013-01-20 21:45:11 -0500, Robert Haas wrote: On Fri, Jan 18, 2013 at 12:32 PM, Andres Freund and...@2ndquadrant.com wrote: Makes sense? Yes. The catalog timetravel stuff still gives me heartburn. The idea of treating system catalogs in a special way has never sat well with me and still doesn't - not that I am sure what I'd like better. The complexity of the whole system is also somewhat daunting. Understandable :( Althoutg it seems to me most parts of it have already been someplace else in the pg code, and the actual timetravel code is relatively small. But my question with relation to this specific patch was mostly whether setting the table OID everywhere was worth worrying about from a performance standpoint, or whether any of the other adjustments this patch makes could have negative consequences there, since the Satisfies functions can get very hot on some workloads. It seems like the consensus is no, that's not worth worrying about, at least as far as the table OIDs are concerned. I agree, I don't really see any such potential of that here, the effectively changed amount of code is very minor since the interface mostly stayed the same due to the HeapTupleSatisfiesVisibility wrapper. 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] Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
On 2013-01-19 17:33:05 -0500, Steve Singer wrote: On 13-01-09 03:07 PM, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Well, I *did* benchmark it as noted elsewhere in the thread, but thats obviously just machine (E5520 x 2) with one rather restricted workload (pgbench -S -jc 40 -T60). At least its rather palloc heavy. Here are the numbers: before: #101646.763208 101350.361595 101421.425668 101571.211688 101862.172051 101449.857665 after: #101553.596257 102132.277795 101528.816229 101733.541792 101438.531618 101673.400992 So on my system if there is a difference, its positive (0.12%). pgbench-based testing doesn't fill me with a lot of confidence for this --- its numbers contain a lot of communication overhead, not to mention that pgbench itself can be a bottleneck. It struck me that we have a recent test case that's known to be really palloc-intensive, namely Pavel's example here: http://www.postgresql.org/message-id/CAFj8pRCKfoz6L82PovLXNK-1JL=jzjwat8e2bd2pwnkm7i7...@mail.gmail.com I set up a non-cassert build of commit 78a5e738e97b4dda89e1bfea60675bcf15f25994 (ie, just before the patch that reduced the data-copying overhead for that). On my Fedora 16 machine (dual 2.0GHz Xeon E5503, gcc version 4.6.3 20120306 (Red Hat 4.6.3-2)) I get a runtime for Pavel's example of 17023 msec (average over five runs). I then applied oprofile and got a breakdown like this: ... The number of calls of AllocSetAlloc certainly hasn't changed at all, so how did that get faster? I notice that the postgres executable is about 0.2% smaller, presumably because a whole lot of inlined fetches of CurrentMemoryContext are gone. This makes me wonder if my result is due to chance improvements of cache line alignment for inner loops. I would like to know if other people get comparable results on other hardware (non-Intel hardware would be especially interesting). If this result holds up across a range of platforms, I'll withdraw my objection to making palloc a plain function. regards, tom lane Sorry for the delay I only read this thread today. I just tried Pawel's test on a POWER5 machine with an older version of gcc (see the grebe buildfarm animal for details) 78a5e738e: 37874.855 (average of 6 runs) 78a5e738 + palloc.h + mcxt.c: 38076.8035 The functions do seem to slightly slow things down on POWER. I haven't bothered to run oprofile or tprof to get a breakdown of the functions since Andres has already removed this from his patch. I haven't removed it from the patch afaik, so it would be great to get a profile here! Its only for xlogdump, but that tool helped me immensely and I don't want to maintain it independently... 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] CF3+4 (was Re: Parallel query execution)
On 21.01.2013 02:07, Jeff Janes wrote: As a junior reviewer, I'd like to know if my main task should be to decide between 1) writing a review convincing you or Tom that your judgement is hasty, or 2) to convince the author that your judgement is correct. That would provide me with some direction, rather than just having me pondering whether a certain variable name ought to have one more or one fewer underscores in it. On the other hand if a summary judgement is that the patch is fundamentally sound but needs some line-by-line code-lawyering, or some performance testing, or documentation/usability testing, or needs to ponder the implications to part XYZ of the code base (which neither I nor the other have even heard of before), that would also be good to know. My desire is not for you to tell me what the outcome of the review should be, but rather what the focus of it should be. Often a patch contains a contentious change buried deep in the patch. The patch submitter might not realize there's anything out of the ordinary in changing parser behavior based on a GUC, or doing things in the executor that should be done in the planner, so he doesn't mention it in the submission email or highlight it with any comments. The longer the patch, the easier it is for things like that to hide in the crowd. If a reviewer can bring those potentially contentious things that don't smell right to light, that's extremely helpful. It's not so important what judgment you make on it; just bringing it up, in a short, separate reply to the email thread, will allow the submitter to reconsider, and a committer to jump in early to say yeah, you can't do that, because X.. IMHO that's the single most important task of a review. - 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] psql: small patch to correct filename formatting error in '\s FILE' output
I've noticed a filename error in feedback messages from psql's '\s' command when saving the command line history to a file specified by an absolute filepath: psql (9.2.2) Type help for help. pgdevel=# \s history.txt Wrote history to file ./history.txt. pgdevel=# \s /tmp/history.txt Wrote history to file .//tmp/history.txt. pgdevel=# \cd /tmp pgdevel=# \s /tmp/history.txt Wrote history to file /tmp//tmp/history.txt. The second and third '\s' commands display incorrect filepaths in the feedback message, despite writing correctly to the specified file. Also, if the specified file could not be written to, the error message displayed formats the filepath differently (i.e. it does not prepend the current working directory), which is potentially confusing, and certainly visually inconsistent: pgdevel=# \cd /tmp pgdevel=# \s foo/history.txt could not save history to file foo/history.txt: No such file or directory pgdevel=# \! mkdir foo pgdevel=# \s foo/history.txt Wrote history to file /tmp/foo/history.txt. The attached patch rectifies these issues by adding a small function 'format_fname()' to psql/stringutils.c which formats the filepath appropriately, depending on whether an absolute filepath was supplied or psql's cwd is set. pgdevel_head=# \s history.txt Wrote history to file ./history.txt. pgdevel_head=# \s /tmp/history.txt Wrote history to file /tmp/history.txt. pgdevel_head=# \cd /tmp pgdevel_head=# \s /tmp/history.txt Wrote history to file /tmp/history.txt. pgdevel_head=# \cd /tmp pgdevel_head=# \s bar/history.txt could not save history to file /tmp/bar/history.txt: No such file or directory pgdevel_head=# \! mkdir bar pgdevel_head=# \s bar/history.txt Wrote history to file /tmp/bar/history.txt. Notes/caveats - The function 'format_fname()' deterimines whether the supplied filepath is absolute by checking for the presence of a '/' as the first character. This strikes me as a bit hacky but I can't think of an alternative. - As far as I can tell, Windows does not support the '\s' command, so there is presumably no need to worry about supporting Windows-style file paths - As far as I can tell, this is the only psql slash command which, after saving data to a file, provides a feedback message containing the filename/path. Regards Ian Lawrence Barwick psql-save-history-2013-01-21.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
