Re: [HACKERS] SSI bug?
On 14.02.2011 20:10, Kevin Grittner wrote: Promotion of the lock granularity on the prior tuple is where we have problems. If the two tuple versions are in separate pages then the second UPDATE could miss the conflict. My first thought was to fix that by requiring promotion of a predicate lock on a tuple to jump straight to the relation level if nextVersionOfRow is set for the lock target and it points to a tuple in a different page. But that doesn't cover a situation where we have a heap tuple predicate lock which gets promoted to page granularity before the tuple is updated. To handle that we would need to say that an UPDATE to a tuple on a page which is predicate locked by the transaction would need to be promoted to relation granularity if the new version of the tuple wasn't on the same page as the old version. Yeah, promoting the original lock on the UPDATE was my first thought too. Another idea is to duplicate the original predicate lock on the first update, so that the original reader holds a lock on both row versions. I think that would ultimately be simpler as we wouldn't need the next-prior chains anymore. For example, suppose that transaction X is holding a predicate lock on tuple A. Transaction Y updates tuple A, creating a new tuple B. Transaction Y sees that X holds a lock on tuple A (or the page containing A), so it acquires a new predicate lock on tuple B on behalf of X. If the updater aborts, the lock on the new tuple needs to be cleaned up, so that it doesn't get confused with later tuple that's stored in the same physical location. We could store the xmin of the tuple in the predicate lock to check for that. Whenever you check for conflict, if the xmin of the lock doesn't match the xmin on the tuple, you know that the lock belonged to an old dead tuple stored in the same location, and can be simply removed as the tuple doesn't exist anymore. That said, the above is about eliminating false negatives from some corner cases which escaped notice until now. I don't think the changes described above will do anything to prevent the problems reported by YAMAMOTO Takashi. Agreed, it's a separate issue. Although if we change the way we handle the read-update-update problem, the other issue might go away too. Unless I'm missing something, it sounds like tuple IDs are being changed or reused while predicate locks are held on the tuples. That's probably not going to be overwhelmingly hard to fix if we can identify how that can happen. I tried to cover HOT issues, but it seems likely I missed something. Storing the xmin of the original tuple would probably help with that too. But it would be nice to understand and be able to reproduce the issue first. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04
On Tue, Feb 15, 2011 at 01:27, Robert Haas robertmh...@gmail.com wrote: However, file_fdw is in pretty serious trouble because (1) the copy API patch that it depends on still isn't committed and (2) it's going to be utterly broken if we don't do something about the client_encoding vs. file_encoding problem; there was a patch to do that in this CF, but we gave up on it. Will we include the copy API patch in 9.1 even if we won't have file_fdw? Personally, I want to include the APIs because someone can develop file_fdw as a third party extension for 9.1 using the infrastructure. The extension will lack of file encoding support, but still useful for many cases. -- Itagaki Takahiro -- 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] pageinspect's infomask and infomask2 as smallint
On 14.02.2011 21:49, Alvaro Herrera wrote: Thanks to Noah Misch's review of the keylock patch I noticed that pageinspect's heap_page_items(bytea) function returns infomask and infomask2 as smallint (signed). But the fields in the tuple header are 16 bits unsigned, so if the high (16th) bit is set, it returns negative values which seem hard to handle. Not a problem for infomask, because the high bit is used for a VACUUM FULL-era flag; but in infomask2 it is used. This seems hard to fix for existing installations with the unpackaged module already loaded -- IIRC it's not acceptable to drop a function, which is what would need to be done here. pageinspect is just a debugging aid, so I think we should change it from smallint to int4 in 9.1, and not bother backporting. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pl/python do not delete function arguments
- Original message - On mån, 2011-02-14 at 22:22 +0100, Jan Urbański wrote: The problem is that every *second* call to the function fails, regardless of the number. The first execution succeeds, but then PLy_delete_args deletes the argument from the globals, and when the next execution tries to fetch n from it, it raises a KeyError. This isn't quite right either, because it obviously depends on the recursion somehow. So in SELECT recursion_test(5); SELECT recursion_test(4); it is the first recursive invocation of the (4) call that fails. If you just do SELECT recursion_test(1); SELECT recursion_test(1); SELECT recursion_test(1); nothing fails. (We'd have noticed that sooner, obviously. ;-) ) Isn't that because with 1 there is no recursion, i.e. plpy.execute never gets called from Python? But in SELECT recursion_test(1); SELECT recursion_test(4); SELECT recursion_test(1); it's the last (1) call, which is not recursive, that fails. Because the invocation that actually recurses sets up the scene for failure. Jan -- 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] Debian readline/libedit breakage
On Tue, Feb 15, 2011 at 6:12 AM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: from what I can see upstream libedit actually has utf8 support for a while now (as well as some other fixes) but the debian libedit version (and also the one of other distributions) is way too old for that so maybe most of the issues would be mood if debian updated to a newer libedit version... There's utf8 support and then there's utf8 support. last I saw libedit didn't actually stop you from using utf8 and things kind of worked, but none of the editing commands understand what the multibyte characters were or understood what column position you were in so you could easily end up deleting half a character or with the insertion point in the middle of a character. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04
On Tue, Feb 15, 2011 at 3:31 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Tue, Feb 15, 2011 at 01:27, Robert Haas robertmh...@gmail.com wrote: However, file_fdw is in pretty serious trouble because (1) the copy API patch that it depends on still isn't committed and (2) it's going to be utterly broken if we don't do something about the client_encoding vs. file_encoding problem; there was a patch to do that in this CF, but we gave up on it. Will we include the copy API patch in 9.1 even if we won't have file_fdw? Personally, I want to include the APIs because someone can develop file_fdw as a third party extension for 9.1 using the infrastructure. The extension will lack of file encoding support, but still useful for many cases. I've been kind of wondering why you haven't already committed it. If you're confident that the code is in good shape, I don't particularly see any benefit to holding off. -- 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] multiset patch review
On Tue, Feb 15, 2011 at 4:31 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: array_flatten() no longer exists. I added array_trim() as an alias to trim_array() because it would be a FAQ. I don't like the alias thing - let's add one name or the other, not both. Similarly, let's NOT add array_union_all as an alias for array_concat. 'cannot use multi-dimensional arrays' reads awkwardly to me. I think it should say something like sorting of multi-dimensional arrays is not supported. multi-demensional - multi-dimensional slaces - slices The formula in the trim_array comment is apparently misparenthesized. -- 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] Change pg_last_xlog_receive_location not to move backwards
On Tue, Feb 15, 2011 at 12:34 AM, Fujii Masao masao.fu...@gmail.com wrote: You suggest that the shared variable Stream tracks the WAL write location, after it's set to the replication starting position? I don't think that the write location needs to be tracked in the shmem because other processes than walreceiver don't use it. Well, my proposal was to expose it, on the theory that it's useful. As we stream the WAL, we write it, so I think for all intents and purposes write == stream. But using it to convey the starting position makes more sense if you call it stream than it does if you call it write. You propose to rename LogstreamResult.Write to .Stream, and merge it and receiveStart? Yeah, or probably change recieveStart to be called Stream. It's representing the same thing, just in shmem instead of backend-local, so why name it differently? -- 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] Add support for logging the current role
On Tue, Feb 15, 2011 at 12:46 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Mon, Feb 14, 2011 at 23:30, Stephen Frost sfr...@snowman.net wrote: * In assign_csvlog_fields(), we need to cleanup memory and memory context before return on error. Fixed this and a couple of similar issues. Not yet fixed. Switched memory context is not restored on error. Updated patch attached, git log below. Now I mark the patch to Ready for Committer, because I don't have suggestions any more. For reference, I note my previous questions. Some of them might be TODO items, or might not. We can add the basic feature in 9.1, and improve it 9.2 or later versions. * csvlog_fields and csvlog_header won't work with non-default log_filename when it doesn't contain seconds in the format. They expect they can always open empty log files. * The long default value for csvlog_fields leads long text line in postgresql.conf, SHOW ALL, pg_settings view, but there were no better alternative solutions in the past discussion. * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats in a csv file on default log_filename, but other similar GUC variables are usually marked AS PGC_SIGHUP. I think we should push this whole patch out to 9.2. It seems to me that there are significant unresolved design issues here which need more time and thought than we can realistically give them now. In addition to the above, there is the problem of making the data self-identifying, which I think is really, really important for third-party tools. I am not keen to push a half-baked solution into the tree now that we will have to live with for years. The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. -- 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] Add support for logging the current role
On Fri, Feb 11, 2011 at 6:20 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: I wrote: Patch attached. This time with src/backend/utils/misc/postgresql.conf.sample fixed. Committed. -- 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] Add support for logging the current role
* Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. I'd be quite frustrated to not have *any* way to log the current role in 9.1. I don't think anyone is going to be too bent out of shape that we can't do it with CSV initially, so long as we agree that we'll try and add that for 9.2. Pushing the CSV log changes to 9.2 would be fine with me, and I'd be happy to continue working on it and the GUC changes I was suggesting to address the long config line, and perhaps figure out a way to handle changes on SIGHUP. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Fix for Index Advisor related hooks
On 11.02.2011 22:44, Gurjeet Singh wrote: Looks like the function get_actual_variable_range() was written with the knowledge that virtual/hypothetical indexes may exist, but the assumption seems wrong. One one hand get_actual_variable_range() expects that virtual indexes do not have an OID assigned, on the other hand explain_get_index_name_hook() is handed just an index's OID to get its name back; IMHO these are based on two conflicting assumptions about whether a virtual index will have an OID assigned. Attached patch fix_get_actual_variable_range.patch tries to fix this by introducing a new hook that can help Postgres decide if an index is fictitious or not. The new hook takes an index oid as argument, so I gather that you resolved the contradiction by deciding that fictitious indexes have OIDs. How do you assign those OIDs? Do fictitious indexes have entries in pg_index? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TYPE 2: skip already-provable no-work rewrites
Noah, * Noah Misch (n...@leadboat.com) wrote: On Mon, Feb 14, 2011 at 04:06:59PM -0500, Stephen Frost wrote: I've attached a new version of the patch that attempts to flesh out the comments based on your feedback. Does it improve things? Yes, much better, thanks! Offhand, I can't think of any concrete implementation along those lines that would simplify the overall task. Did you have something more specific in mind? Sadly, no. :) For now it seemed like premature abstraction. Fair enough. All in all, this patch looks good to me. Looks like that might be moot, however, based on Robert's comments. Still, thanks for it, I certainly look forward to having it eventually. :) Thanks again, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_ctl failover Re: Latches, signals, and waiting
* Fujii Masao (masao.fu...@gmail.com) wrote: On Tue, Feb 15, 2011 at 2:10 AM, Stephen Frost sfr...@snowman.net wrote: * You removed trigger_file from the list in doc/src/sgml/high-availability.sgml and I'm not sure I agree with that. It's still perfectly valid and could be used by someone instead of pg_ctl promote. I'd recommend two things: - Adding comments into this recovery.conf snippet Adding the following is enough? +# NOTE that if you plan to use pg_ctl promote command to promote +# the standby, no trigger file needs to be specified. No.. I was thinking of having comments throughout the whole file, similar to what we have for postgresql.conf.sample. I realize it's an example in the docs, but people are likely to copy paste it into their own files. - Adding a comment indicationg that trigger_file is only needed if you're not using pg_ctl promote. Where should I add such a comment? This was still in the example recovery.conf in high-availability.sgml. * I'm not happy that pg_ctl.c doesn't #include something which defines all the file names which are used, couldn't we use a header which makes sense and is pulled in by pg_ctl.c and xlog.c to #define all of these? Still, that's not really the fault of this patch. That would make sense. But I'm not sure that's possible. As a trial, I added '#include access/xlog.h' into pg_ctl.c and compiled the source, but I got many compilation errors. So probably hacking Makefiles is required to do that. Do you know where should be changed? No, I wouldn't expect that to work.. I would have thought something that was already included in the necessary places, eg, miscadmin.h. * I'm a bit worried that there's just only so many USR signals that we can send and it looks like we're burning another one here. Should we be considering a better way to do this? You're worried about the case where users wrongly send the SIGUSR2 to the startup process, and then the standby is brought up to the master unexpectedly? No.. My concern was that it looked like we were adding a meaning to SIGUSR2 which might make it unavailable for other uses later, and at least according to man 7 signal on my system, there's only 2 User-defined signals available (SIGUSR1 and SIGUSR2).. I just wouldn't want to use up something which is a finite resource without good cause, in case we have a need for it later. Now, perhaps that's not happening and my concern is unfounded. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
* Itagaki Takahiro (itagaki.takah...@gmail.com) wrote: On Mon, Feb 14, 2011 at 23:30, Stephen Frost sfr...@snowman.net wrote: * In assign_csvlog_fields(), we need to cleanup memory and memory context before return on error. Fixed this and a couple of similar issues. Not yet fixed. Switched memory context is not restored on error. Ugh, sorry about that, I should have realized that needed to be done. Updated patch attached. Updated patch attached, git log below. Now I mark the patch to Ready for Committer, because I don't have suggestions any more. Thanks. For reference, I note my previous questions. Some of them might be TODO items, or might not. We can add the basic feature in 9.1, and improve it 9.2 or later versions. That's what I would have thought, ah well. :) * csvlog_fields and csvlog_header won't work with non-default log_filename when it doesn't contain seconds in the format. They expect they can always open empty log files. Or that the user will deal with changes and header lines mid-file if they decide to use a log_filename which causes it to happen. * The long default value for csvlog_fields leads long text line in postgresql.conf, SHOW ALL, pg_settings view, but there were no better alternative solutions in the past discussion. Not without making GUCs able to be multi-line. If people are interested in this, I'll try to make it happen for 9.2. * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats in a csv file on default log_filename, but other similar GUC variables are usually marked AS PGC_SIGHUP. The problem here is primairly that each backend does write_csvlog() and there's no easy way to make sure that none of them write the new format to the old file (or the old format to the new file) before a switch is done. I can try looking into this but I'm concerned the only solution would be to introduce some amount of locking which could slow down the overall logging process which might be unacceptable performance-wise. Preventing CSV logs from appending to existing files could be pretty easily done, provided we can agree on what to call the new files, but that wouldn't change PGC_POSTMASTER. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
* Stephen Frost (sfr...@snowman.net) wrote: Ugh, sorry about that, I should have realized that needed to be done. Updated patch attached. Errr, for real this time. Thanks, Stephen commit 25e94dcb390f56502bc46e683b438c20d2dc74e0 Author: Stephen Frost sfr...@snowman.net Date: Tue Feb 15 08:50:17 2011 -0500 assign_csvlog_fields() - reset context on error On error, we need to make sure to reset the memory context back to what it was when we entered. *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 3542,3548 local0.*/var/log/postgresql /row row entryliteral%u/literal/entry ! entryUser name/entry entryyes/entry /row row --- 3542,3561 /row row entryliteral%u/literal/entry ! entrySession user name, typically the user name which was used ! to authenticate to productnamePostgreSQL/productname with, ! but can be changed by a superuser, see commandSET SESSION ! AUTHORIZATION//entry ! entryyes/entry ! /row ! row ! entryliteral%U/literal/entry ! entryCurrent role name, when set with commandSET ROLE/; ! the current role identifier is relevant for permission checking; ! Returns 'none' if the current role matches the session user. ! Note: Log messages from inside literalSECURITY DEFINER/ ! functions will show the calling role, not the effective role ! inside the literalSECURITY DEFINER/ function/entry entryyes/entry /row row *** *** 3659,3664 FROM pg_stat_activity; --- 3672,3717 /listitem /varlistentry + varlistentry id=guc-csvlog-fields xreflabel=csvlog_fields + termvarnamecsvlog_fields/varname (typestring/type)/term + indexterm +primaryvarnamecsvlog_fields/ configuration parameter/primary + /indexterm + listitem +para + Controls the set and order of the fields which are written out in + the CSV-format log file. + + The default is: + literallog_time, user_name, database_name, process_id, + connection_from, session_id, session_line_num, command_tag, + session_start_time, virtual_transaction_id, transaction_id, + error_severity, sql_state_code, message, detail, hint, + internal_query, internal_query_pos, context, query, query_pos, + location, application_name/literal + + For details on what these fields are, refer to the + varnamelog_line_prefix/varname and + xref linkend=runtime-config-logging-csvlog documentation. +/para + /listitem + /varlistentry + + varlistentry id=guc-csvlog-header xreflabel=csvlog_header + termvarnamecsvlog_header/varname (typeboolean/type)/term + indexterm +primaryvarnamecsvlog_header/ configuration parameter/primary + /indexterm + listitem +para + Controls if a header should be output for each file logged through + the CSV-format logging. + + The default is: literalfalse/literal, for backwards compatibility. +/para + /listitem + /varlistentry + varlistentry id=guc-log-lock-waits xreflabel=log_lock_waits termvarnamelog_lock_waits/varname (typeboolean/type)/term indexterm *** *** 3766,3799 FROM pg_stat_activity; Including literalcsvlog/ in the varnamelog_destination/ list provides a convenient way to import log files into a database table. This option emits log lines in comma-separated-values ! (acronymCSV/) format, ! with these columns: ! timestamp with milliseconds, ! user name, ! database name, ! process ID, ! client host:port number, ! session ID, ! per-session line number, ! command tag, ! session start time, ! virtual transaction ID, ! regular transaction ID, ! error severity, ! SQLSTATE code, ! error message, ! error message detail, ! hint, ! internal query that led to the error (if any), ! character count of the error position therein, ! error context, ! user query that led to the error (if any and enabled by ! varnamelog_min_error_statement/), ! character count of the error position therein, ! location of the error in the PostgreSQL source code ! (if varnamelog_error_verbosity/ is set to literalverbose/), ! and application name. ! Here is a sample table definition for storing CSV-format log output: programlisting CREATE TABLE postgres_log --- 3819,3971
Re: [HACKERS] Fix for Index Advisor related hooks
On Tue, Feb 15, 2011 at 8:24 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
On 11.02.2011 22:44, Gurjeet Singh wrote:
Looks like the function get_actual_variable_range() was written with the
knowledge that virtual/hypothetical indexes may exist, but the assumption
seems wrong.
One one hand get_actual_variable_range() expects that virtual indexes do
not
have an OID assigned, on the other hand explain_get_index_name_hook() is
handed just an index's OID to get its name back; IMHO these are based on
two
conflicting assumptions about whether a virtual index will have an OID
assigned.
Attached patch fix_get_actual_variable_range.patch tries to fix this by
introducing a new hook that can help Postgres decide if an index is
fictitious or not.
The new hook takes an index oid as argument, so I gather that you resolved
the contradiction by deciding that fictitious indexes have OIDs. How do you
assign those OIDs? Do fictitious indexes have entries in
pg_index?http://www.enterprisedb.com
No, a fictitious index does not touch pg_index. The Index Advisor uses
GetNewOid(pg_class) to generate a new OID for the fictitious index.
An OID is the only way we can identify one fictitious index from the list of
all the others fictitious ones when explain_get_index_name_hook() is called.
I don't see any other way the Postgres server can ask the for the details of
a fictitious index.
Regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com
singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device
Re: [HACKERS] pg_upgrade seems a tad broken
Tom Lane wrote: I wrote: I tried to do a pg_upgrade from 9.0.x to HEAD today. The pg_upgrade run went through without complaint, and I could start the postmaster, but every connection attempt fails with psql: FATAL: could not read block 0 in file base/11964/11683: read only 0 of 8192 bytes The database OID varies depending on which database I try to connect to, but the filenode doesn't. In the source 9.0 database, this relfilenode belongs to pg_largeobject_metadata. I'm not sure whether pg_upgrade would've preserved relfilenode numbering, so that may or may not be a useful hint as to where the problem is. But in any case it's busted. Closer investigation shows that in the new database, relfilenode 11683 belongs to pg_class_oid_index, which explains why it's being touched during backend startup. It is indeed of zero length, and surely should not be. I can't resist the guess that something about the recently added hacks for pg_largeobject_metadata is not right. OK, I will look at this today. Thanks. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 12, 2011, at 9:53 AM, Alex Hunsaker wrote: On Fri, Feb 11, 2011 at 17:17, Alexey Klyukin al...@commandprompt.com wrote: Anyway in playing with this patch a bit more I found another bug return [[]]; would segfault. So find attached a v9 that: - fixes above segfault - made plperl_sv_to_literal vastly simpler (no longer uses SPI and calls regtypin directly) - removed extraneous /para added in v8 in plperl.sgml (my docbook stuff is broken, so I can't build them, hopefully there are no other problems) - we now on the fly (when its requested) make the backwards compatible string for arrays (it was a few lines now that we have plperl_sv_to_literal) - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Anyway barring any more bugs, I think this patch is ready. pg_to_perl_arrays_v9.patch.gz Thank you for finding and fixing the bugs there. I think removing the para was a mistake. I specifically added it to fix the problem I had when building the docs: openjade:plperl.sgml:353:8:E: end tag for PARA omitted, but OMITTAG NO was specified openjade:plperl.sgml:201:2: start tag was here After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. pg_to_perl_arrays_v10.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Students enrollment
Hello guys, I am a lecturer at Kharkiv National University of Radio Electronics and I have a proposal for you. My students have to make their science projects during this half-year. I would like to involve some of them in PostgreSQL to give them an opportunity to work on a real project instead of writing their own equation solvers or any other useless apps. Here is my vision of such collaboration: - I ask students who would like to work on PostgreSQL project and we choose one - You select the supervisor for the student. The supervisor will be the student's boss and will give him tasks and help in solving complex problems. - The project should take student two-three months of time and you can decide can be developed by student during this term. - The student discusses tasks the supervisor and agrees them with me. - When the tasks are agreed the student begins working on them and writing his Project report. - During the term I coordinate the student and solve organization problems. - When the term is finished the student should complete all tasks, agreed on the beginning of the term and also the student should finish his Project report. - The student can continue working with you after the science project is finished, if he likes it. There some big advantages in such collaboration for all participants: - You get a contributor that is interested in working on your project. - The student gets the experience in working on a real project. - If PostgreSQL is interested to student he will continue working on it after the science project is finished. I will not require any payments for the student. Please consider my proposal and tell me what do you think about such collaboration. You can contact me by e-mail, Google talk: rprikhodche...@gmail.com or Skype: rprykhodchenko Sincerely, Roman Prykhodchenko -- 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 seems a tad broken
Tom Lane wrote: I wrote: I tried to do a pg_upgrade from 9.0.x to HEAD today. The pg_upgrade run went through without complaint, and I could start the postmaster, but every connection attempt fails with psql: FATAL: could not read block 0 in file base/11964/11683: read only 0 of 8192 bytes The database OID varies depending on which database I try to connect to, but the filenode doesn't. In the source 9.0 database, this relfilenode belongs to pg_largeobject_metadata. I'm not sure whether pg_upgrade would've preserved relfilenode numbering, so that may or may not be a useful hint as to where the problem is. But in any case it's busted. Closer investigation shows that in the new database, relfilenode 11683 belongs to pg_class_oid_index, which explains why it's being touched during backend startup. It is indeed of zero length, and surely should not be. I can't resist the guess that something about the recently added hacks for pg_largeobject_metadata is not right. FYI, I have reproduced the bug here --- researching the cause now. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Add support for logging the current role
* Stephen Frost (sfr...@snowman.net) wrote:
I'd be happy to go back to the original patch/idea of just the simple
addition of %U as an option for log_line_prefix.
Updated patch attached which just adds %U support to log_line_prefix.
Will work on adding CSV support for this in 9.2, along with associated
other issues regarding supporting variable CSV format output.
Thanks,
Stephen
commit c1b06c04af0c886c6ec27917368f3c674227ed2d
Author: Stephen Frost sfr...@snowman.net
Date: Tue Feb 15 10:21:38 2011 -0500
Add %U option to log_line_prefix
This patch adds a %U option to log_line_prefix, to allow logging
of the current role (previously not possible). Also reworks %u
a bit and adds documentation to clarify what each means.
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 3542,3548 local0.*/var/log/postgresql
/row
row
entryliteral%u/literal/entry
! entryUser name/entry
entryyes/entry
/row
row
--- 3542,3561
/row
row
entryliteral%u/literal/entry
! entrySession user name, typically the user name which was used
! to authenticate to productnamePostgreSQL/productname with,
! but can be changed by a superuser, see commandSET SESSION
! AUTHORIZATION//entry
! entryyes/entry
! /row
! row
! entryliteral%U/literal/entry
! entryCurrent role name, when set with commandSET ROLE/;
! the current role identifier is relevant for permission checking;
! Returns 'none' if the current role matches the session user.
! Note: Log messages from inside literalSECURITY DEFINER/
! functions will show the calling role, not the effective role
! inside the literalSECURITY DEFINER/ function/entry
entryyes/entry
/row
row
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 847,852 assign_session_authorization(const char *value, bool doit, GucSource source)
--- 847,857
return result;
}
+ /*
+ * function to return the stored session username, needed because we
+ * can't do catalog lookups when possibly being called after an error,
+ * eg: from elog.c or part of GUC handling.
+ */
const char *
show_session_authorization(void)
{
***
*** 972,977 assign_role(const char *value, bool doit, GucSource source)
--- 977,987
return result;
}
+ /*
+ * function to return the stored role username, needed because we
+ * can't do catalog lookups when possibly being called after an error,
+ * eg: from elog.c or part of GUC handling.
+ */
const char *
show_role(void)
{
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***
*** 3,8
--- 3,17
* elog.c
* error logging and reporting
*
+ * A few comments about situations where error processing is called:
+ *
+ * We need to be cautious of both a performance hit when logging, since
+ * log messages can be generated at a huge rate if every command is being
+ * logged and we also need to watch out for what can happen when we are
+ * trying to log from an aborted transaction. Specifically, attempting to
+ * do SysCache lookups and possibly use other usually available backend
+ * systems will fail badly when logging from an aborted transaction.
+ *
* Some notes about recursion and errors during error processing:
*
* We need to be robust about recursive-error scenarios --- for example,
***
*** 1817,1831 log_line_prefix(StringInfo buf, ErrorData *edata)
}
break;
case 'u':
- if (MyProcPort)
{
! const char *username = MyProcPort-user_name;
!
! if (username == NULL || *username == '\0')
! username = _([unknown]);
! appendStringInfoString(buf, username);
}
break;
case 'd':
if (MyProcPort)
{
--- 1826,1849
}
break;
case 'u':
{
! const char *session_auth = show_session_authorization();
!
! if (*session_auth != '\0')
! appendStringInfoString(buf, session_auth);
! else if (MyProcPort)
! {
! const char *username = MyProcPort-user_name;
!
! if (username == NULL || *username == '\0')
! username = _([unknown]);
! appendStringInfoString(buf, username);
! }
}
break;
+ case 'U':
+ appendStringInfoString(buf, show_role());
+ break;
case 'd':
if (MyProcPort)
{
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***
*** 360,366
#log_hostname = off
#log_line_prefix = '' # special values:
# %a = application
Re: [HACKERS] Add support for logging the current role
On Tue, Feb 15, 2011 at 10:26 AM, Stephen Frost sfr...@snowman.net wrote: * Stephen Frost (sfr...@snowman.net) wrote: I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. Updated patch attached which just adds %U support to log_line_prefix. Will work on adding CSV support for this in 9.2, along with associated other issues regarding supporting variable CSV format output. Something along these lines would be OK with me (I haven't yet validated every detail), but there were previous objections to adding any new fields to log_line_prefix until we had a flexible CSV format. I think that's raising the bar a bit too high, personally, but I don't have the only vote around here... -- 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] sepgsql contrib module
On Mon, Feb 14, 2011 at 9:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: On the whole, I don't think that sepgsql-regtest.pp should be built or installed at all during the build phase. It ought to be generated during regression test startup, instead. You have to manually install and enable it before you can run the regression tests. This is documented. -- 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] Fwd: [JDBC] Weird issues when reading UDT from stored function
On Sat, Feb 12, 2011 at 6:16 AM, Lukas Eder lukas.e...@gmail.com wrote: I had tried that before. That doesn't seem to change anything. JDBC still expects 6 OUT parameters, instead of just 1... Oh, hrm. I thought you were trying to fix the return value, rather than the signature. I am not sure how to fix the signature. Can you just make it return RECORD? -- 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] Add support for logging the current role
Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. I'd be quite frustrated to not have *any* way to log the current role in 9.1. I don't think anyone is going to be too bent out of shape that we can't do it with CSV initially, so long as we agree that we'll try and add that for 9.2. Given that this has been like this right along, I don't see why it's all that urgent to force a half-baked solution into 9.1. I'm also concerned that if we do do that, you'll lose motivation to work on cleaning it up for 9.2 ;-) 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] pageinspect's infomask and infomask2 as smallint
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 14.02.2011 21:49, Alvaro Herrera wrote: Thanks to Noah Misch's review of the keylock patch I noticed that pageinspect's heap_page_items(bytea) function returns infomask and infomask2 as smallint (signed). But the fields in the tuple header are 16 bits unsigned, so if the high (16th) bit is set, it returns negative values which seem hard to handle. Not a problem for infomask, because the high bit is used for a VACUUM FULL-era flag; but in infomask2 it is used. This seems hard to fix for existing installations with the unpackaged module already loaded -- IIRC it's not acceptable to drop a function, which is what would need to be done here. pageinspect is just a debugging aid, so I think we should change it from smallint to int4 in 9.1, and not bother backporting. I don't see any reason that the old version of the function couldn't be dropped in the upgrade script. It's not likely anything would be depending on it, is 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] Add support for logging the current role
On Tue, Feb 15, 2011 at 10:37 AM, Tom Lane t...@sss.pgh.pa.us wrote: Stephen Frost sfr...@snowman.net writes: * Robert Haas (robertmh...@gmail.com) wrote: The payoff (getting %U) seems quite out of proportion to the potential downsides of making a change of this type at this late date. I'd be happy to go back to the original patch/idea of just the simple addition of %U as an option for log_line_prefix. I'd be quite frustrated to not have *any* way to log the current role in 9.1. I don't think anyone is going to be too bent out of shape that we can't do it with CSV initially, so long as we agree that we'll try and add that for 9.2. Given that this has been like this right along, I don't see why it's all that urgent to force a half-baked solution into 9.1. I'm also concerned that if we do do that, you'll lose motivation to work on cleaning it up for 9.2 ;-) Trying to arm-twist people into working on A before we're willing to give them B doesn't necessary serve us very well. I'd rather leave the problem of making the CSV format more flexible to someone who is really motivated to work on *that problem*, whether that person ends up being Stephen or not. Just my $0.02. -- 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] pageinspect's infomask and infomask2 as smallint
On Tue, Feb 15, 2011 at 10:42 AM, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 14.02.2011 21:49, Alvaro Herrera wrote: Thanks to Noah Misch's review of the keylock patch I noticed that pageinspect's heap_page_items(bytea) function returns infomask and infomask2 as smallint (signed). But the fields in the tuple header are 16 bits unsigned, so if the high (16th) bit is set, it returns negative values which seem hard to handle. Not a problem for infomask, because the high bit is used for a VACUUM FULL-era flag; but in infomask2 it is used. This seems hard to fix for existing installations with the unpackaged module already loaded -- IIRC it's not acceptable to drop a function, which is what would need to be done here. pageinspect is just a debugging aid, so I think we should change it from smallint to int4 in 9.1, and not bother backporting. I don't see any reason that the old version of the function couldn't be dropped in the upgrade script. It's not likely anything would be depending on it, is it? I don't see much point in taking the risk. -- 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] sepgsql contrib module
On 02/15/2011 10:34 AM, Robert Haas wrote: On Mon, Feb 14, 2011 at 9:55 PM, Tom Lanet...@sss.pgh.pa.us wrote: On the whole, I don't think that sepgsql-regtest.pp should be built or installed at all during the build phase. It ought to be generated during regression test startup, instead. You have to manually install and enable it before you can run the regression tests. This is documented. That's not the point. Right now you can't even run just a postgresql build on a machine that doesn't have selinux enabled, let alone run the regression tests. And if we construct the regression gadget at postgresql build time, who is to say it has the right settings for the run time environment, given that the build is apparently dependent on a runtime kernel setting? 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] pageinspect's infomask and infomask2 as smallint
Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 10:42 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't see any reason that the old version of the function couldn't be dropped in the upgrade script. It's not likely anything would be depending on it, is it? I don't see much point in taking the risk. What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is break it for everyone, and damn the torpedoes. 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] sepgsql contrib module
On Tue, Feb 15, 2011 at 10:50 AM, Andrew Dunstan and...@dunslane.net wrote: On 02/15/2011 10:34 AM, Robert Haas wrote: On Mon, Feb 14, 2011 at 9:55 PM, Tom Lanet...@sss.pgh.pa.us wrote: On the whole, I don't think that sepgsql-regtest.pp should be built or installed at all during the build phase. It ought to be generated during regression test startup, instead. You have to manually install and enable it before you can run the regression tests. This is documented. That's not the point. Right now you can't even run just a postgresql build on a machine that doesn't have selinux enabled, let alone run the regression tests. And if we construct the regression gadget at postgresql build time, who is to say it has the right settings for the run time environment, given that the build is apparently dependent on a runtime kernel setting? Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. -- 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] pageinspect's infomask and infomask2 as smallint
On Tue, Feb 15, 2011 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 10:42 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't see any reason that the old version of the function couldn't be dropped in the upgrade script. It's not likely anything would be depending on it, is it? I don't see much point in taking the risk. What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is break it for everyone, and damn the torpedoes. I must be confused. I thought Heikki's proposal was fix it in 9.1, because incompatibilities are an expected part of major release upgrades, but don't break it in 9.0 and prior, because it's not particularly important and we don't want to change behavior or risk breaking things in minor releases. -- 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] Add support for logging the current role
Robert Haas robertmh...@gmail.com writes: Something along these lines would be OK with me (I haven't yet validated every detail), but there were previous objections to adding any new fields to log_line_prefix until we had a flexible CSV format. I think that's raising the bar a bit too high, personally, but I don't have the only vote around here... I think I was the one objecting. I don't necessarily say that we have to have a flexible CSV format, but I do say that facilities that are available in log_line_prefix and not in CSV logs are a bad thing. 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] sepgsql contrib module
Robert Haas robertmh...@gmail.com writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. So? Once you admit that you can do that, it's a matter of a couple more lines to make the installcheck target depend on the policy target iff selinux was enabled. 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] pageinspect's infomask and infomask2 as smallint
Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote: What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is break it for everyone, and damn the torpedoes. I must be confused. I thought Heikki's proposal was fix it in 9.1, because incompatibilities are an expected part of major release upgrades, but don't break it in 9.0 and prior, because it's not particularly important and we don't want to change behavior or risk breaking things in minor releases. No, nobody was proposing changing it before 9.1 (or at least I didn't think anybody was). What's under discussion is how much effort to put into making a 9.0-to-9.1 upgrade go smoothly for people who have the function installed. 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] Add support for logging the current role
On Tue, Feb 15, 2011 at 10:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Something along these lines would be OK with me (I haven't yet validated every detail), but there were previous objections to adding any new fields to log_line_prefix until we had a flexible CSV format. I think that's raising the bar a bit too high, personally, but I don't have the only vote around here... I think I was the one objecting. I don't necessarily say that we have to have a flexible CSV format, but I do say that facilities that are available in log_line_prefix and not in CSV logs are a bad thing. Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. -- 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] Add support for logging the current role
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: Given that this has been like this right along, I don't see why it's all that urgent to force a half-baked solution into 9.1. I'm also concerned that if we do do that, you'll lose motivation to work on cleaning it up for 9.2 ;-) The addition to log_line_prefix is hardly 'half-baked' as a solution, to that problem (I just pulled the hunks from the rest of the patch as they were completely independent, and tested them). I've also gone and added the csvlog_fields/csvlog_header patch to the 2011-Next commitfest. :P I've also already started looking at changing syslogger to have it figure out if it should be writing a header out or not. If we can decide what semantics we should have when the log file exists and we're not planning to rotate it on startup, it won't be hard for me to implement them (well, I hope). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pageinspect's infomask and infomask2 as smallint
On Tue, Feb 15, 2011 at 11:03 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote: What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is break it for everyone, and damn the torpedoes. I must be confused. I thought Heikki's proposal was fix it in 9.1, because incompatibilities are an expected part of major release upgrades, but don't break it in 9.0 and prior, because it's not particularly important and we don't want to change behavior or risk breaking things in minor releases. No, nobody was proposing changing it before 9.1 (or at least I didn't think anybody was). What's under discussion is how much effort to put into making a 9.0-to-9.1 upgrade go smoothly for people who have the function installed. Oh, I see, never mind me then... feel free to make that go smoothly. :-) -- 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] sepgsql contrib module
On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. So? Once you admit that you can do that, it's a matter of a couple more lines to make the installcheck target depend on the policy target iff selinux was enabled. Sure, you could do that, but I don't see what problem it would fix. You'd still have to build and manually install the policy before you could run make installcheck. And once you've done that, you don't need to rebuild it every future time you run make installcheck. -- 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] Add support for logging the current role
* Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. I might be working against myself, but I'll complain right now about the lack of any way to have a header on the CSV logs and that you don't get to control what fields are logged. That said, I'm not currently using them either, so my vote doesn't count for much. Of course, I'll also complain about the lack of any way to get PG to respect the header, forcing me to do fun things like: for file in *results*; do HEADER=`head -1 $file` sed -e 's:::g' $file | \ psql -d beac -h sauron -c \ \copy my_table ($HEADER) from STDIN with csv header done on a regular basis. How forcing me to do that rather than asking someone else to use 'tail -n +2' makes sense is beyond me.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pageinspect's infomask and infomask2 as smallint
On 15.02.2011 18:03, Tom Lane wrote: Robert Haasrobertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 10:53 AM, Tom Lanet...@sss.pgh.pa.us wrote: What risk? And at least we'd be trying to do it cleanly, in a manner that should work for at least 99% of users. AFAICT, Heikki's proposal is break it for everyone, and damn the torpedoes. I must be confused. I thought Heikki's proposal was fix it in 9.1, because incompatibilities are an expected part of major release upgrades, but don't break it in 9.0 and prior, because it's not particularly important and we don't want to change behavior or risk breaking things in minor releases. Right, that's what I meant. No, nobody was proposing changing it before 9.1 (or at least I didn't think anybody was). What's under discussion is how much effort to put into making a 9.0-to-9.1 upgrade go smoothly for people who have the function installed. Oh, never mind then. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
On Tue, Feb 15, 2011 at 11:13 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. I might be working against myself, but I'll complain right now about the lack of any way to have a header on the CSV logs and that you don't get to control what fields are logged. That said, I'm not currently using them either, so my vote doesn't count for much. Of course, I'll also complain about the lack of any way to get PG to respect the header, forcing me to do fun things like: for file in *results*; do HEADER=`head -1 $file` sed -e 's:::g' $file | \ psql -d beac -h sauron -c \ \copy my_table ($HEADER) from STDIN with csv header done on a regular basis. How forcing me to do that rather than asking someone else to use 'tail -n +2' makes sense is beyond me.. It's not an either/or proposition. We could certainly support header on/off/ignore, with the new extensible COPY syntax. -- 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] Sync Rep for 2011CF1
On Tue, 2011-02-15 at 01:45 -0500, Jaime Casanova wrote:
On Sat, Jan 15, 2011 at 4:40 PM, Simon Riggs si...@2ndquadrant.com wrote:
Here's the latest patch for sync rep.
I was looking at this code and found something in SyncRepWaitOnQueue
we declare a timeout variable that is a long and another that is a
boolean (this last one in the else part of the if
(!IsOnSyncRepQueue())), and then use the boolean one as if it were
the long one
OK, thanks.
+ else
+ {
+ bool release = false;
+ bool timeout = false;
+
+ SpinLockAcquire(queue-qlock);
+
+ /*
+* Check the LSN on our queue and if its moved far enough then
+* remove us from the queue. First time through this is
+* unlikely to be far enough, yet is possible. Next time we are
+* woken we should be more lucky.
+*/
+ if (XLByteLE(XactCommitLSN, queue-lsn))
+ release = true;
+ else if (timeout 0
+
TimestampDifferenceExceeds(GetCurrentTransactionStopTimestamp(),
+ now,
+ timeout))
+ {
+ release = true;
+ timeout = true;
+ }
the other two things are on postgresql.conf.sample:
- we have two replication_timeout_client, obviously one of them should
be replication_timeout_server
- synchronous_replication_feedback is off by default, but docs says otherwise
i also have been testing this, until now the only issue i have found
is that if i set allow_standalone_primary to off and there isn't a
standby connected i need to stop the server with -m immediate which is
at least surprising
I think that code is being ripped out, so will check again later.
--
Simon Riggs http://www.2ndQuadrant.com/books/
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
[HACKERS] extensions and psql
Hi, I realize that you didn't keep the \dx behavior I had, that when given an extension name it would list all the objects contained in the extension. Now that's a pretty simple query: select pg_describe_object(classid, objid, 0) from pg_depend d join pg_extension e on d.refclassid = e.tableoid and d.refobjid = e.oid and d.deptype = 'e' where e.extname = 'hstore' order by objid; Do we want to get that back in, and in which psql command? It could well be that having \dx list extension and \dx name list extension's objects wasn't the best design around, and it could be that it's not useful enough, but I know I liked to have a psql shortcut to do that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] sepgsql contrib module
Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. So? Once you admit that you can do that, it's a matter of a couple more lines to make the installcheck target depend on the policy target iff selinux was enabled. Sure, you could do that, but I don't see what problem it would fix. You'd still have to build and manually install the policy before you could run make installcheck. And once you've done that, you don't need to rebuild it every future time you run make installcheck. Oh, I see: you're pointing out the root-only semodule step that has to be done in between there. Good point. But the current arrangement is still a mistake: the required contents of sepgsql-regtest.pp depend on the configuration of the test system, which can't be known at build time. So what we should do is offer a make policy target and alter the test instructions to say you should do that and then run semodule. Or maybe just put the whole make -f /usr/share/selinux/devel/Makefile dance into the instructions --- it doesn't look to me like our makefile infrastructure really has anything useful to add to that. 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] XMin Hot Standby Feedback patch
On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote:
This is another bit of the syncrep patch split out.
I will revisit the replication timeout one Real Soon, I promise -- but
I have a couple things to do today that may delay that until the
evening.
https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1
Context diff supplied here.
Greg just tipped me off to this thread a few hours ago. I saw your other
work on timeouts which looks good.
I've reworked this feature myself, and its roughly the same thing you
have posted, so I will just add on to this thread. The major change from
my earlier patch is that the logic around setting xmin on the master is
considerably tighter, and correctly uses locking.
Patch attached, no docs yet, but the patch is clear.
I'm looking to commit this in next 24 hours barring objections and/or
test failures.
Note that this information is not exposed via catalog in the original
syncrep patch, and is not here. Do we want that kind of reporting?
Probably, but its a small change and will conflict with other work for
now.
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 3277da8..c4ebf97 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -45,6 +45,7 @@
#include replication/walreceiver.h
#include storage/ipc.h
#include storage/pmsignal.h
+#include storage/procarray.h
#include utils/builtins.h
#include utils/guc.h
#include utils/memutils.h
@@ -56,6 +57,7 @@ bool am_walreceiver;
/* GUC variable */
int wal_receiver_status_interval;
+bool hot_standby_feedback;
/* libpqreceiver hooks to these when loaded */
walrcv_connect_type walrcv_connect = NULL;
@@ -610,10 +612,14 @@ XLogWalRcvSendReply(void)
reply_message.apply = GetXLogReplayRecPtr();
reply_message.sendTime = now;
- elog(DEBUG2, sending write %X/%X flush %X/%X apply %X/%X,
+ if (hot_standby_feedback)
+ reply_message.xmin = GetOldestXmin(true, false);
+
+ elog(DEBUG2, sending write %X/%X flush %X/%X apply %X/%X xmin %u,
reply_message.write.xlogid, reply_message.write.xrecoff,
reply_message.flush.xlogid, reply_message.flush.xrecoff,
- reply_message.apply.xlogid, reply_message.apply.xrecoff);
+ reply_message.apply.xlogid, reply_message.apply.xrecoff,
+ reply_message.xmin);
/* Prepend with the message type and send it. */
buf[0] = 'r';
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3ad95b4..36eb3a9 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -53,6 +53,7 @@
#include storage/ipc.h
#include storage/pmsignal.h
#include storage/proc.h
+#include storage/procarray.h
#include tcop/tcopprot.h
#include utils/builtins.h
#include utils/guc.h
@@ -498,6 +499,7 @@ ProcessStandbyReplyMessage(void)
static StringInfoData input_message;
StandbyReplyMessage reply;
char msgtype;
+ TransactionId newxmin = InvalidTransactionId;
initStringInfo(input_message);
@@ -524,10 +526,11 @@ ProcessStandbyReplyMessage(void)
pq_copymsgbytes(input_message, (char *) reply, sizeof(StandbyReplyMessage));
- elog(DEBUG2, write %X/%X flush %X/%X apply %X/%X ,
+ elog(DEBUG2, write %X/%X flush %X/%X apply %X/%X xmin %u,
reply.write.xlogid, reply.write.xrecoff,
reply.flush.xlogid, reply.flush.xrecoff,
- reply.apply.xlogid, reply.apply.xrecoff);
+ reply.apply.xlogid, reply.apply.xrecoff,
+ reply.xmin);
/*
* Update shared state for this WalSender process
@@ -543,6 +546,74 @@ ProcessStandbyReplyMessage(void)
walsnd-apply = reply.apply;
SpinLockRelease(walsnd-mutex);
}
+
+ /*
+ * Update the WalSender's proc xmin to allow it to be visible
+ * to snapshots. This will hold back the removal of dead rows
+ * and thereby prevent the generation of cleanup conflicts
+ * on the standby server.
+ */
+ if (TransactionIdIsValid(reply.xmin))
+ {
+ TransactionId safeLimit;
+#define HOT_STANDBY_FEEDBACK_HORIZON 10
+
+ if (!TransactionIdIsValid(MyProc-xmin))
+ {
+ /*
+ * Initialise MyProc-xmin from standby feedback.
+ * Don't allow use oldestXmin if it is too far back.
+ */
+ safeLimit = ReadNewTransactionId() - HOT_STANDBY_FEEDBACK_HORIZON;
+ if (!TransactionIdIsNormal(safeLimit))
+safeLimit = FirstNormalTransactionId;
+
+ if (TransactionIdPrecedes(safeLimit, reply.xmin))
+ {
+TransactionId oldestXmin = GetOldestXmin(true, true);
+
+if (TransactionIdPrecedes(oldestXmin, reply.xmin))
+ newxmin = reply.xmin;
+else
+ newxmin = oldestXmin;
+ }
+ else
+ereport(WARNING,
+ (errmsg(standby xmin is far in the past),
+ errhint(standby must catch up before hot standby feedback is effective.)));
+ }
+ else
+ {
+ /*
+ * Feedback from standby should move us forwards,
Re: [HACKERS] XMin Hot Standby Feedback patch
On 15.02.2011 18:42, Simon Riggs wrote: On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote: This is another bit of the syncrep patch split out. I will revisit the replication timeout one Real Soon, I promise -- but I have a couple things to do today that may delay that until the evening. https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1 Context diff supplied here. Greg just tipped me off to this thread a few hours ago. I saw your other work on timeouts which looks good. I've reworked this feature myself, and its roughly the same thing you have posted, so I will just add on to this thread. The major change from my earlier patch is that the logic around setting xmin on the master is considerably tighter, and correctly uses locking. It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is 2 billion transactions behind. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql contrib module
On Tue, Feb 15, 2011 at 11:41 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. So? Once you admit that you can do that, it's a matter of a couple more lines to make the installcheck target depend on the policy target iff selinux was enabled. Sure, you could do that, but I don't see what problem it would fix. You'd still have to build and manually install the policy before you could run make installcheck. And once you've done that, you don't need to rebuild it every future time you run make installcheck. Oh, I see: you're pointing out the root-only semodule step that has to be done in between there. Good point. But the current arrangement is still a mistake: the required contents of sepgsql-regtest.pp depend on the configuration of the test system, which can't be known at build time. So what we should do is offer a make policy target and alter the test instructions to say you should do that and then run semodule. Or maybe just put the whole make -f /usr/share/selinux/devel/Makefile dance into the instructions --- it doesn't look to me like our makefile infrastructure really has anything useful to add to that. Yeah, agreed. -- 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] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 11:49 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 15.02.2011 18:42, Simon Riggs wrote: On Sat, 2011-02-12 at 14:11 -0800, Daniel Farina wrote: This is another bit of the syncrep patch split out. I will revisit the replication timeout one Real Soon, I promise -- but I have a couple things to do today that may delay that until the evening. https://github.com/fdr/postgres/commit/ad3ce9ac62f0e128d7d1fd20d47184f867056af1 Context diff supplied here. Greg just tipped me off to this thread a few hours ago. I saw your other work on timeouts which looks good. I've reworked this feature myself, and its roughly the same thing you have posted, so I will just add on to this thread. The major change from my earlier patch is that the logic around setting xmin on the master is considerably tighter, and correctly uses locking. It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is 2 billion transactions behind. That case is probably hopelessly broken anyway. -- 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] XMin Hot Standby Feedback patch
On 15.02.2011 18:52, Robert Haas wrote: On Tue, Feb 15, 2011 at 11:49 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is 2 billion transactions behind. That case is probably hopelessly broken anyway. I don't expect the feedback to do anything too useful in case of xid wraparound, but at least the master would recognize the situation and know to ignore the bogus xmin from that standby. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
On 02/15/2011 11:13 AM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: Well, I guess the other option is to just add it to the format, full stop. But as someone pointed out previously, that's not a terribly scalable solution, but perhaps it could be judged adequate for this particular case. Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). I could live with it for a release if I thought we had a clear path ahead, but I think there are some design issues that we need to think about before we start providing for header lines and variable formats in CSV logs, particularly w.r.t. log rotation etc. So I'm slightly nervous about going ahead with this right now. While I generally agree with the principal, I also wonder if it might be better to just add this field in log_line_prefix and wait for someone to complain about that as other than a theoretical matter. I might be working against myself, but I'll complain right now about the lack of any way to have a header on the CSV logs and that you don't get to control what fields are logged. That said, I'm not currently using them either, so my vote doesn't count for much. Of course, I'll also complain about the lack of any way to get PG to respect the header, forcing me to do fun things like: for file in *results*; do HEADER=`head -1 $file` sed -e 's:::g' $file | \ psql -d beac -h sauron -c \ \copy my_table ($HEADER) from STDIN with csv header done on a regular basis. How forcing me to do that rather than asking someone else to use 'tail -n +2' makes sense is beyond me.. You don't really make your case any better by continuing this argument from years ago. I can tell you from experience that the CSV HEADER feature is distinctly useful as it is. If you want to add a mode that uses the header line as a column list on import, then make that case, and I'll support it in fact, but it's not an alternative to having the header ignored, which is a feature I would vigorously resist removing. (Incidentally, I think it won't be trivial - the COPY code expects to know the columns by the time it opens the file). 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] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 18:49 +0200, Heikki Linnakangas wrote: It would be wise to also transmit the epoch in addition to xmin, to avoid confusion if the standby is 2 billion transactions behind. Yes, good idea, thanks. That has to be the record for the fastest patch review. ;-) -- Simon Riggs http://www.2ndQuadrant.com/books/ 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] Fix for Index Advisor related hooks
Gurjeet Singh singh.gurj...@gmail.com writes: Also attached is the patch expose_IndexSupportInitialize.patch, that makes the static function IndexSupportInitialize() global so that the Index Advisor doesn't have to reinvent the wheel to prepare an index structure with opfamilies and opclasses. We are *not* doing that. That's a private function and will remain so. 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] Sync Rep for 2011CF1
On Mon, Feb 14, 2011 at 12:08 AM, Fujii Masao masao.fu...@gmail.com wrote:
On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
I committed the patch with those changes, and some minor comment tweaks and
other kibitzing.
+ * 'd' means a standby reply wrapped in a COPY BOTH packet.
+ */
Typo: s/COPY BOTH/CopyData
Fixed.
+ msgtype = pq_getmsgbyte(input_message);
+ if (msgtype != 'r')
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg(unexpected message type %c, msgtype)));
I think that proc_exit(0) needs to be called in error case.
Fixed.
+ static StringInfoData input_message;
+ StandbyReplyMessage reply;
+ char msgtype;
+
+ initStringInfo(input_message);
Doesn't the repeat of initStringInfo() cause the memory leak?
Yeah. Fixed, I hope.
@@ -518,6 +584,7 @@ WalSndLoop(void)
{
if (!XLogSend(output_message, caughtup))
break;
+ ProcessRepliesIfAny();
Why is ProcessRepliesIfAny() required there?
I'm not sure if that's 100% necessary, but it seems harmless enough.
We added new columns write_location, flush_location and
apply_location. So, for the sake of consistency, the column
name sent_location should be changed to send_location?
I was thinking about stream_location or streaming_location, per
discussion on the other thread.
--
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] Sync Rep for 2011CF1
On Tue, Feb 15, 2011 at 1:11 AM, Fujii Masao masao.fu...@gmail.com wrote: On Mon, Feb 14, 2011 at 2:08 PM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I committed the patch with those changes, and some minor comment tweaks and other kibitzing. I have another comment: The description of wal_receiver_status_interval is in 18.5.4. Streaming Replication. But I think that it should be moved to 18.5.5. Standby Servers since it's a parameter to control the behavior of the standby server rather than that of the master. Fixed. -- 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] Sync Rep for 2011CF1
On Mon, Feb 14, 2011 at 12:25 AM, Fujii Masao masao.fu...@gmail.com wrote: On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I added a XLogWalRcvSendReply() call into XLogWalRcvFlush() so that it also sends a status update every time the WAL is flushed. If the walreceiver is busy receiving and flushing, that would happen once per WAL segment, which seems sensible. This change can make the callback function WalRcvDie() call ereport(ERROR) via XLogWalRcvFlush(). This looks unsafe. Good catch. Is the cleanest solution to pass a boolean parameter to XLogWalRcvFlush() indicating whether we're in the midst of dying? -- 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] CommitFest 2011-01 as of 2011-02-04
On 02/15/2011 06:55 AM, Robert Haas wrote: On Tue, Feb 15, 2011 at 3:31 AM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: On Tue, Feb 15, 2011 at 01:27, Robert Haasrobertmh...@gmail.com wrote: However, file_fdw is in pretty serious trouble because (1) the copy API patch that it depends on still isn't committed and (2) it's going to be utterly broken if we don't do something about the client_encoding vs. file_encoding problem; there was a patch to do that in this CF, but we gave up on it. Will we include the copy API patch in 9.1 even if we won't have file_fdw? Personally, I want to include the APIs because someone can develop file_fdw as a third party extension for 9.1 using the infrastructure. The extension will lack of file encoding support, but still useful for many cases. I've been kind of wondering why you haven't already committed it. If you're confident that the code is in good shape, I don't particularly see any benefit to holding off. +10. The sooner the better. 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] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 11:42 AM, Simon Riggs si...@2ndquadrant.com wrote: Patch attached, no docs yet, but the patch is clear. I'm looking to commit this in next 24 hours barring objections and/or test failures. Looks pretty good to me, though I haven't tested it. I like some of the safety valves you put in there, but I don't understand this part: + /* +* Feedback from standby should move us forwards, but not too far. +* Avoid grabbing XidGenLock here in case of waits, so use +* a heuristic instead. +*/ What's XIDGenLock got to do with it? On another disk, I think that those warning messages are a bad idea. That could fill up someone's disk really quickly. -- 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] extensions and psql
On Tue, Feb 15, 2011 at 11:37 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Do we want to get that back in, and in which psql command? It could well be that having \dx list extension and \dx name list extension's objects wasn't the best design around, and it could be that it's not useful enough, but I know I liked to have a psql shortcut to do that. +1 for having a psql command to do that. -- 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] XMin Hot Standby Feedback patch
On Tue, Feb 15, 2011 at 12:15 PM, Robert Haas robertmh...@gmail.com wrote: On another disk, I think that those warning messages are a bad idea. That could fill up someone's disk really quickly. On another disk? What the heck am I talking about? On another point? On another note? Anyway, you get the idea... hopefully. -- 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] extensions and psql
Dimitri Fontaine dimi...@2ndquadrant.fr writes: I realize that you didn't keep the \dx behavior I had, that when given an extension name it would list all the objects contained in the extension. Sure I did: \dx+ 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] CommitFest 2011-01 as of 2011-02-04
robertmh...@gmail.com (Robert Haas) writes:
It does, but frankly I don't see much reason to change it, since it's
been working pretty well on the whole. Andrew was on point when he
mentioned that it's not obvious what committers get out of working on
other people's patches. Obviously, the answer is, well, they get a
better PostgreSQL, and that's ultimately good for all of us. But the
trickiest part of this whole process is that, on the one hand, it's
not fair for committers to ignore other people's patches, but on the
other hand, it's not fair to expect committers to sacrifice getting
their own projects done to get other people's projects done.
I had two interesting germane comments in my RSS feed this morning, both
entitled Please send a patch
http://www.lucas-nussbaum.net/blog/?p=630
Where Lucas suggests that, when someone requests an enhancement, the
retort Please send a patch mayn't be the best idea, because the
one receiving the requests may be many times better at contributing
such changes than the one making the request.
http://hezmatt.org/~mpalmer/blog/general/please_send_a_patch.html
On the other hand, Lucas, remember that each time you ask someone
to take some time to implement your pet feature request, you take
some time away from her that could be used to contribute something
in an area where she gives a damn.
These are *both* true statements, and, in order to grow the community
that is capable of enhancing the system, there is merit to the careful
application of both positions.
There's stuff that Tom should do :-). And absent the general
availability of cloning machines, we need to have people improving their
skills so that there are more that are capable of submitting (and
evaluating and committing) usable patches.
--
wm(X,Y):-write(X),write('@'),write(Y). wm('cbbrowne','gmail.com').
http://linuxdatabases.info/info/languages.html
Signs of a Klingon Programmer - 14. Our competitors are without
honor!
--
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] Replication server timeout patch
On Mon, Feb 14, 2011 at 5:13 PM, Daniel Farina dan...@heroku.com wrote: On Mon, Feb 14, 2011 at 12:48 AM, Fujii Masao masao.fu...@gmail.com wrote: On Sat, Feb 12, 2011 at 8:58 AM, Daniel Farina dan...@heroku.com wrote: Context diff equivalent attached. Thanks for the patch! As I said before, the timeout which this patch provides doesn't work well when the walsender gets blocked in sending WAL. At first, we would need to implement a non-blocking write function as an infrastructure of the replication timeout, I think. http://archives.postgresql.org/message-id/AANLkTi%3DPu2ne%3DVO-%2BCLMXLQh9y85qumLCbBP15CjnyUS%40mail.gmail.com Interesting point...if that's accepted as required-for-commit, what are the perceptions of the odds that, presuming I can write the code quickly enough, that there's enough infrastructure/ports already in postgres to allow for a non-blocking write on all our supported platforms? Are you working on this? -- 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] extensions and psql
Tom Lane t...@sss.pgh.pa.us writes: Sure I did: \dx+ And I believe I did test that. Sorry for the noise, really. (shame) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Best, David -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 7:45 PM, David E. Wheeler wrote: On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] psql -l doesn't process psqlrc
On Sun, Feb 13, 2011 at 7:52 AM, Peter Eisentraut pete...@gmx.net wrote: psql -l doesn't process psqlrc. Historically, this was probably not useful, hence no one cared. But with the linestyle option it's useful. So I propose the attached tweak. As a violent hater of the new linestyle, +1 from me. -- 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] Fix for Index Advisor related hooks
Gurjeet Singh singh.gurj...@gmail.com writes: On Tue, Feb 15, 2011 at 8:24 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 11.02.2011 22:44, Gurjeet Singh wrote: One one hand get_actual_variable_range() expects that virtual indexes do not have an OID assigned, on the other hand explain_get_index_name_hook() is handed just an index's OID to get its name back; IMHO these are based on two conflicting assumptions about whether a virtual index will have an OID assigned. The new hook takes an index oid as argument, so I gather that you resolved the contradiction by deciding that fictitious indexes have OIDs. How do you assign those OIDs? Do fictitious indexes have entries in pg_index? No, a fictitious index does not touch pg_index. The Index Advisor uses GetNewOid(pg_class) to generate a new OID for the fictitious index. That seems like a very expensive, and lock-inducing, way of assigning a fictitious OID. They don't need to be globally unique. I suggest you consider the idea I suggested back in 2007: * In this toy example we just assign all hypothetical indexes * OID 0, and the explain_get_index_name hook just prints * hypothetical index. In a realistic situation we'd probably * assume that OIDs smaller than, say, 100 are never the OID of * any real index, allowing us to identify one of up to 100 * hypothetical indexes per plan. Then we'd need to save aside * some state data that would let the explain hooks print info * about the selected index. As far as the immediate problem goes, I agree that get_actual_variable_range is mistaken, but I think a cleaner and cheaper solution would be to add a bool hypothetical to IndexOptInfo. 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] Debian readline/libedit breakage
On 02/15/2011 12:37 PM, Greg Stark wrote: On Tue, Feb 15, 2011 at 6:12 AM, Stefan Kaltenbrunner ste...@kaltenbrunner.cc wrote: from what I can see upstream libedit actually has utf8 support for a while now (as well as some other fixes) but the debian libedit version (and also the one of other distributions) is way too old for that so maybe most of the issues would be mood if debian updated to a newer libedit version... There's utf8 support and then there's utf8 support. last I saw libedit didn't actually stop you from using utf8 and things kind of worked, but none of the editing commands understand what the multibyte characters were or understood what column position you were in so you could easily end up deleting half a character or with the insertion point in the middle of a character. well I have not actually tested - I was just reading the changelog on http://www.thrysoee.dk/editline/ which claims UTF8 support (whatever that means) in the current code drop. Stefan -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 9:49 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. Awesom, thanks Alexey Alex! David -- 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] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 12:20 -0500, Robert Haas wrote: On Tue, Feb 15, 2011 at 12:15 PM, Robert Haas robertmh...@gmail.com wrote: On another disk, I think that those warning messages are a bad idea. That could fill up someone's disk really quickly. On another disk? What the heck am I talking about? On another point? On another note? Anyway, you get the idea... hopefully. Yeh. I quite liked it as a metaphorical turn of phrase. -- Simon Riggs http://www.2ndQuadrant.com/books/ 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] Add support for logging the current role
* Andrew Dunstan (and...@dunslane.net) wrote: On 02/15/2011 11:13 AM, Stephen Frost wrote: Think I suggested that at one point. I'm all for doing that on a major version change like this one, but I think we already had some concerns about that on this thread (Andrew maybe?). I could live with it for a release if I thought we had a clear path ahead, but I think there are some design issues that we need to think about before we start providing for header lines and variable formats in CSV logs, particularly w.r.t. log rotation etc. So I'm slightly nervous about going ahead with this right now. I believe the suggestion that Robert and I were talking about above was to just unilatterally change the CSV log file output format to include current_role. No header lines, no variable output format, etc. I do think we can make header lines and variable output work, if we can get agreement on what the semantics should be. You don't really make your case any better by continuing this argument from years ago. I can tell you from experience that the CSV HEADER feature is distinctly useful as it is. If you want to add a mode that uses the header line as a column list on import, then make that case, and I'll support it in fact, but it's not an alternative to having the header ignored, which is a feature I would vigorously resist removing. I'm not really interested in removing it. I guess I have a vain hope that with arguing I'll convince someone to take up the mantle of implementing the 'use header' option. :) Not getting much traction though, so I expect I'll work on it this summer. (Incidentally, I think it won't be trivial - the COPY code expects to know the columns by the time it opens the file). Thanks for that insight, I'll take a look at how things work and see if I can come up with a sensible proposal. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
I wrote: Dimitri Fontaine dimi...@2ndquadrant.fr writes: I think you'd be interested into this reworked SQL query. It should be providing exactly the script file you need as an upgrade from unpackaged. This seems overly complicated. I have a version of it that I'll publish as soon as I've tested it on all the contrib modules ... Just for the archives' sake: the '@extschema@' business did turn out to be important, at least for tsearch2 where it's necessary to distinguish the objects it's dealing with from similarly-named objects in pg_catalog. So this is what I used to generate the unpackaged scripts. Some of them needed manual adjustment later to cover cases where 9.1 had diverged from 9.0, but the script could hardly be expected to know about that. #! /bin/sh MOD=$1 psql -d testdb -c create extension $MOD ( echo /* contrib/$MOD/$MOD--unpackaged--1.0.sql */ echo psql -A -t -d testdb -c SELECT 'ALTER EXTENSION ' || E.extname || ' ADD ' || replace(pg_describe_object(classid, objid, 0), N.nspname, '@extschema@') || ';' FROM pg_depend D JOIN pg_extension E ON D.refobjid = E.oid AND D.refclassid = E.tableoid JOIN pg_namespace N ON E.extnamespace = N.oid WHERE deptype = 'e' AND E.extname = '$MOD' ORDER BY D.objid | sed -e 's/ADD cast from \(.*\) to \(.*\);/ADD cast (\1 as \2);/' \ -e 's/ for access method / using /' ) contrib/$MOD/$MOD--unpackaged--1.0.sql 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] review: FDW API
On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I'm actually surprised we don't need to distinguish them in more places, but nevertheless it feels like we should have that info available more conveniently, and without requiring a catalog lookup like get_rel_relkind() does. At first I thought we should add a field to RelOptInfo, but that doesn't help transformLockingClause. Adding the field to RangeTblEntry seems quite invasive, and it doesn't feel like the right place to cache that kind of information anyway. Thoughts on that? Maybe it should be a new RTEKind. -- 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] review: FDW API
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: As the patch stands, we have to do get_rel_relkind() in a couple of places in parse analysis and the planner to distinguish a foreign table from a regular one. As the patch stands, there's nothing in RangeTblEntry (which is what we have in transformLockingClause) or RelOptInfo (for set_plain_rel_pathlist) to directly distinguish them. Hmm. I don't have a problem with adding relkind to the planner's RelOptInfo, but it seems to me that if parse analysis needs to know this, you have put functionality into parse analysis that does not belong there. (No, I haven't read the patch...) 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] pl/python do not delete function arguments
On tis, 2011-02-15 at 09:58 +0100, Jan Urbański wrote: Because the invocation that actually recurses sets up the scene for failure. That's what we're observing, but I can't figure out why it is. If you can, could you explain it? It actually makes sense to me that the arguments should be deleted at the end of the call. The data belongs to that call only, and PLy_procedure_delete() that would otherwise clean it up is only called rarely. Apparently, the recursive call ends up deleting the wrong arguments, but it's not clear to me why that would affect the next top-level call, because that would set up its own arguments again anyway. In any case, perhaps the right fix is to fix PLy_function_delete_args() to delete the args correctly. -- 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] sepgsql contrib module
-Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: 15 February 2011 16:52 To: Tom Lane Cc: Andrew Dunstan; Kohei Kaigai; Stephen Frost; KaiGai Kohei; PgHacker Subject: Re: [HACKERS] sepgsql contrib module On Tue, Feb 15, 2011 at 11:41 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Feb 15, 2011 at 11:01 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Those are good points. My point was just that you can't actually build that file at the time you RUN the regression tests, because you have to build it first, then install it, then run the regression tests. It could be a separate target, like 'make policy', but I don't think it works to make it part of 'make installcheck'. So? Once you admit that you can do that, it's a matter of a couple more lines to make the installcheck target depend on the policy target iff selinux was enabled. Sure, you could do that, but I don't see what problem it would fix. You'd still have to build and manually install the policy before you could run make installcheck. And once you've done that, you don't need to rebuild it every future time you run make installcheck. Oh, I see: you're pointing out the root-only semodule step that has to be done in between there. Good point. But the current arrangement is still a mistake: the required contents of sepgsql-regtest.pp depend on the configuration of the test system, which can't be known at build time. So what we should do is offer a make policy target and alter the test instructions to say you should do that and then run semodule. Or maybe just put the whole make -f /usr/share/selinux/devel/Makefile dance into the instructions --- it doesn't look to me like our makefile infrastructure really has anything useful to add to that. Yeah, agreed. I also agree with this direction. The policy type depends on individual installations, it is not easy to assume on build time. Please wait for a small patch to remove this rule from Makefile and update documentation. As a side note, we can have a build option that does not require selinux enabled. The reason why Makefile of selinux tries to /selinux/mls is that we don't specify MLS=1 or MLS=0 explicitly. IIRC, the specfile of RHEL/Fedora gives all the Makefile parameters explicitly, thus, selinux does not need to be enabled on the build server. However, it is not a solution in this case. It is not easy to estimate the required policy type and existence of MLS support on build time. Thanks, -- NEC Europe Ltd, Global Competence Center KaiGai Kohei kohei.kai...@eu.nec.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] why two dashes in extension load files
On mån, 2011-02-14 at 15:08 -0500, Tom Lane wrote: Umm ... we are not requiring version names to be numbers. That's certainly interesting. Why? -- 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] why two dashes in extension load files
On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: I guess the real question is what's Peter's concrete objection to the double-dash method? It just looks a bit silly and error prone. And other packaging systems have been doing without it for decades. -- 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] why two dashes in extension load files
Peter Eisentraut pete...@gmx.net writes: On mån, 2011-02-14 at 15:08 -0500, Tom Lane wrote: Umm ... we are not requiring version names to be numbers. That's certainly interesting. Why? There isn't any packaging system anywhere on the planet that requires them to be purely numeric. By the time you get done allowing for multiple dots and alpha or beta and other such stuff, you might as well try to be agnostic about what they contain. 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] why two dashes in extension load files
On Tue, Feb 15, 2011 at 9:16 PM, Peter Eisentraut pete...@gmx.net wrote: On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: I guess the real question is what's Peter's concrete objection to the double-dash method? It just looks a bit silly and error prone. And other packaging systems have been doing without it for decades. how about : we use a single dash as the separator, and if the extension author insists on having a dash in the name, as a punishment he must duplicate the dash, i.e.: uuid--ossp-1.0--5.5.sql Greetings Marcin Mańk -- 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] why two dashes in extension load files
Peter Eisentraut pete...@gmx.net writes: On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: I guess the real question is what's Peter's concrete objection to the double-dash method? It just looks a bit silly and error prone. And other packaging systems have been doing without it for decades. I can't claim close familiarity with Debian's conventions in this matter, but I do know about RPM's, and I'm uneager to duplicate that silliness. Magic conversion of dots to underscores (sometimes), complete inability to determine which part of the package filename is which without external knowledge, etc. Aside from the double-dash method, we kicked around using colons and pluses as separators (and then forbidding just those characters in extension and version names). Any of those would be workable, but it's not clear to me that any of them have any particular cosmetic advantage over any others. In any case, the time to be voting on them is past so far as I'm concerned. The work is already done and I'm uneager to do it over on one person's say-so. 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] why two dashes in extension load files
On Feb 15, 2011, at 12:32 PM, Tom Lane wrote: Aside from the double-dash method, we kicked around using colons and pluses as separators (and then forbidding just those characters in extension and version names). Any of those would be workable, but it's not clear to me that any of them have any particular cosmetic advantage over any others. In any case, the time to be voting on them is past so far as I'm concerned. The work is already done and I'm uneager to do it over on one person's say-so. I'd prefer a single character, but can live with -- just fine. Best, David -- 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: FDW API
On 15.02.2011 21:13, Tom Lane wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com writes: As the patch stands, we have to do get_rel_relkind() in a couple of places in parse analysis and the planner to distinguish a foreign table from a regular one. As the patch stands, there's nothing in RangeTblEntry (which is what we have in transformLockingClause) or RelOptInfo (for set_plain_rel_pathlist) to directly distinguish them. Hmm. I don't have a problem with adding relkind to the planner's RelOptInfo, but it seems to me that if parse analysis needs to know this, you have put functionality into parse analysis that does not belong there. Possibly. We throw the existing errors, for example if you try to do FOR UPDATE OF foo where foo is a set-returning function, in transformLockingClause(), so it seemed like the logical place to check for foreign tables too. Hmm, one approach would be to go ahead and create the RowMarkClauses for all relations in the parse analysis phase, foreign or not, and throw the error later, in preprocess_rowmarks(). preprocess_rowmarks() doesn't currently know if each RowMarkClause was created by ... FOR UPDATE or FOR UPDATE OF foo, but to be consistent with the logic in transformLockingClause() it would need to. For the former case, it would need to just ignore foreign tables but for the latter it would need to throw an error. I guess we could add an explicit flag to RowMarkClause for that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: FDW API
On 15.02.2011 21:00, Robert Haas wrote: On Tue, Feb 15, 2011 at 1:40 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I'm actually surprised we don't need to distinguish them in more places, but nevertheless it feels like we should have that info available more conveniently, and without requiring a catalog lookup like get_rel_relkind() does. At first I thought we should add a field to RelOptInfo, but that doesn't help transformLockingClause. Adding the field to RangeTblEntry seems quite invasive, and it doesn't feel like the right place to cache that kind of information anyway. Thoughts on that? Maybe it should be a new RTEKind. That would be even more invasive. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] review: FDW API
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 15.02.2011 21:13, Tom Lane wrote: Hmm. I don't have a problem with adding relkind to the planner's RelOptInfo, but it seems to me that if parse analysis needs to know this, you have put functionality into parse analysis that does not belong there. Possibly. We throw the existing errors, for example if you try to do FOR UPDATE OF foo where foo is a set-returning function, in transformLockingClause(), so it seemed like the logical place to check for foreign tables too. Hmm, one approach would be to go ahead and create the RowMarkClauses for all relations in the parse analysis phase, foreign or not, and throw the error later, in preprocess_rowmarks(). I think moving the error check downstream would be a good thing. Consider for example the case of applying FOR UPDATE to a view. You can't know what that entails until after the rewriter expands the view. IIRC, at the moment we're basically duplicating the tests between parse analysis and the planner, but it's not clear what the value of that is. 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] why two dashes in extension load files
On Tue, Feb 15, 2011 at 3:26 PM, marcin mank marcin.m...@gmail.com wrote: On Tue, Feb 15, 2011 at 9:16 PM, Peter Eisentraut pete...@gmx.net wrote: On mån, 2011-02-14 at 12:14 -0500, Tom Lane wrote: I guess the real question is what's Peter's concrete objection to the double-dash method? It just looks a bit silly and error prone. And other packaging systems have been doing without it for decades. how about : we use a single dash as the separator, and if the extension author insists on having a dash in the name, as a punishment he must duplicate the dash, i.e.: uuid--ossp-1.0--5.5.sql That has a certain poetic justice to 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] FOR KEY LOCK foreign keys
On Mon, Feb 14, 2011 at 6:49 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Marti Raudsepp's message of lun feb 14 19:39:25 -0300 2011: On Fri, Feb 11, 2011 at 09:13, Noah Misch n...@leadboat.com wrote: The patch had a trivial conflict in planner.c, plus plenty of offsets. I've attached the rebased patch that I used for review. For anyone following along, all the interesting hunks touch heapam.c; the rest is largely mechanical. A diff -w patch is also considerably easier to follow. Here's a simple patch for the RelationGetIndexAttrBitmap() function, as explained in my last post. I don't know if it's any help to you, but since I wrote it I might as well send it up. This applies on top of Noah's rebased patch. Got it, thanks. I did some tests and it seems to work, although I also hit the same visibility bug as Noah. Yeah, that bug is fixed with the attached, though I am rethinking this bit. I am thinking that the statute of limitations has expired on this patch, and that we should mark it Returned with Feedback and continue working on it for 9.2. I know it's a valuable feature, but I think we're out of time. -- 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] XMin Hot Standby Feedback patch
On Tue, 2011-02-15 at 12:15 -0500, Robert Haas wrote: Looks pretty good to me, though I haven't tested it. I like some of the safety valves you put in there, but I don't understand this part Reworked logic covering all feedback, plus tests, plus docs. Last comments before commit please. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 63c6283..30c33fb 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2029,6 +2029,10 @@ SET ENABLE_SEQSCAN TO OFF; This parameter can only be set in the filenamepostgresql.conf/ file or on the server command line. /para + para +You should also consider setting varnamehot_standby_feedback/ +as an alternative to using this parameter. + /para /listitem /varlistentry /variablelist @@ -2121,6 +2125,22 @@ SET ENABLE_SEQSCAN TO OFF; /listitem /varlistentry + varlistentry id=guc-hot-standby-feedback xreflabel=hot_standby + termvarnamehot_standby_feedback/varname (typeboolean/type)/term + indexterm + primaryvarnamehot_standby_feedback/ configuration parameter/primary + /indexterm + listitem + para +Specifies whether or not a hot standby will send feedback to the primary +about queries currently executing on the standby. This parameter can +be used to eliminate query cancels caused by cleanup records, though +it can cause database bloat on the primary for some workloads. +The default value is literaloff/literal. + /para + /listitem + /varlistentry + /variablelist /sect2 /sect1 diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index a892969..6941e67 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -1486,23 +1486,6 @@ if (!triggered) /para para -The most common reason for conflict between standby queries and WAL replay -is quoteearly cleanup/. Normally, productnamePostgreSQL/ allows -cleanup of old row versions when there are no transactions that need to -see them to ensure correct visibility of data according to MVCC rules. -However, this rule can only be applied for transactions executing on the -master. So it is possible that cleanup on the master will remove row -versions that are still visible to a transaction on the standby. - /para - - para -Experienced users should note that both row version cleanup and row version -freezing will potentially conflict with standby queries. Running a manual -commandVACUUM FREEZE/ is likely to cause conflicts even on tables with -no updated or deleted rows. - /para - - para Once the delay specified by varnamemax_standby_archive_delay/ or varnamemax_standby_streaming_delay/ has been exceeded, conflicting queries will be cancelled. This usually results just in a cancellation @@ -1529,6 +1512,23 @@ if (!triggered) /para para +The most common reason for conflict between standby queries and WAL replay +is quoteearly cleanup/. Normally, productnamePostgreSQL/ allows +cleanup of old row versions when there are no transactions that need to +see them to ensure correct visibility of data according to MVCC rules. +However, this rule can only be applied for transactions executing on the +master. So it is possible that cleanup on the master will remove row +versions that are still visible to a transaction on the standby. + /para + + para +Experienced users should note that both row version cleanup and row version +freezing will potentially conflict with standby queries. Running a manual +commandVACUUM FREEZE/ is likely to cause conflicts even on tables with +no updated or deleted rows. + /para + + para Users should be clear that tables that are regularly and heavily updated on the primary server will quickly cause cancellation of longer running queries on the standby. In such cases the setting of a finite value for @@ -1539,12 +1539,10 @@ if (!triggered) para Remedial possibilities exist if the number of standby-query cancellations -is found to be unacceptable. The first option is to connect to the -primary server and keep a query active for as long as needed to -run queries on the standby. This prevents commandVACUUM/ from removing -recently-dead rows and so cleanup conflicts do not occur. -This could be done using xref linkend=dblink and -functionpg_sleep()/, or via other mechanisms. If you do this, you +is found to be unacceptable. The first option is to set the parameter +varnamehot_standby_feedback/, which prevents commandVACUUM/ from +removing recently-dead rows and so cleanup conflicts do not occur. +If
Re: [HACKERS] FOR KEY LOCK foreign keys
On Feb 15, 2011, at 1:15 PM, Robert Haas wrote: Yeah, that bug is fixed with the attached, though I am rethinking this bit. I am thinking that the statute of limitations has expired on this patch, and that we should mark it Returned with Feedback and continue working on it for 9.2. I know it's a valuable feature, but I think we're out of time. How is such a determination made, exactly? Best, David -- 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] FOR KEY LOCK foreign keys
Excerpts from Robert Haas's message of mar feb 15 18:15:38 -0300 2011: I am thinking that the statute of limitations has expired on this patch, and that we should mark it Returned with Feedback and continue working on it for 9.2. I know it's a valuable feature, but I think we're out of time. Okay, I've marked it as such in the commitfest app. It'll be in 9.2's first commitfest. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] NULLs in array_cat vs array || array
Hi all, I assumed array_cat would behave similarly to array || array, but it appears not when it comes to NULLs. Shouldn't these have identical functionality? The attached patch makes it so, although it would break existing code. Would such a change have any knock-on effect, or cause inconsistency with other functions? Thanks Thom -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 array_cat_nulls.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] NULLs in array_cat vs array || array
2011/2/15 Thom Brown t...@linux.com: Hi all, I assumed array_cat would behave similarly to array || array, but it appears not when it comes to NULLs. Shouldn't these have identical functionality? The attached patch makes it so, although it would break existing code. There is bugreport and todo entry for that if it helps: http://archives.postgresql.org/pgsql-bugs/2008-11/msg00032.php Would such a change have any knock-on effect, or cause inconsistency with other functions? Thanks Thom -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Cédric Villemain 2ndQuadrant http://2ndQuadrant.fr/ PostgreSQL : Expertise, Formation et Support -- 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] NULLs in array_cat vs array || array
Thom Brown t...@linux.com writes: I assumed array_cat would behave similarly to array || array, but it appears not when it comes to NULLs. Shouldn't these have identical functionality? The attached patch makes it so, although it would break existing code. That patch is the hard way: the right change would be to remove the code altogether and mark the function strict in pg_proc. However, the fact that it's not like that already shows that we went out of our way to make it so. I don't think we should undo that decision just because somebody submits a patch to do so. Also, so far as I can see array_cat *is* ||, so I'm not sure what discrepancy in behavior you're on 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes:
Just for the archives' sake: the '@extschema@' business did turn out to
be important, at least for tsearch2 where it's necessary to distinguish
the objects it's dealing with from similarly-named objects in
pg_catalog. So this is what I used to generate the unpackaged
scripts. Some of them needed manual adjustment later to cover cases
where 9.1 had diverged from 9.0, but the script could hardly be expected
to know about that.
Good to know that even contrib needs that!
#! /bin/sh
MOD=$1
psql -d testdb -c create extension $MOD
(
echo /* contrib/$MOD/$MOD--unpackaged--1.0.sql */
echo
psql -A -t -d testdb -c
SELECT 'ALTER EXTENSION ' || E.extname || ' ADD '
|| replace(pg_describe_object(classid, objid, 0),
N.nspname, '@extschema@')
|| ';'
FROM pg_depend D
JOIN pg_extension E ON D.refobjid = E.oid
AND D.refclassid = E.tableoid
JOIN pg_namespace N ON E.extnamespace = N.oid
WHERE deptype = 'e' AND E.extname = '$MOD'
ORDER BY D.objid
| sed -e 's/ADD cast from \(.*\) to \(.*\);/ADD cast (\1 as \2);/' \
-e 's/ for access method / using /'
) contrib/$MOD/$MOD--unpackaged--1.0.sql
Ah well sed makes it simpler to read, but it won't be usable in windows.
I now realize also that the second version of this query did some
useless array type filtering. Adding a replace() step in the query
would not be that ugly I guess, if we wanted to make it so.
Do we want to add such a query in the docs to help pgfoundry authors to
write their own 'from unpackaged' scripts?
CREATE OR REPLACE FUNCTION extension_unpackaged_upgrade_script(text)
RETURNS SETOF text
LANGUAGE SQL
AS $$
WITH objs AS (
SELECT 'ALTER EXTENSION ' || E.extname || ' ADD '
|| replace(pg_describe_object(classid, objid, 0),
N.nspname, '@extschema@')
|| ';' AS d
FROM pg_depend D
JOIN pg_extension E ON D.refobjid = E.oid
AND D.refclassid = E.tableoid
JOIN pg_namespace N ON E.extnamespace = N.oid
WHERE deptype = 'e' AND E.extname = $1
ORDER BY D.objid
)
SELECT regexp_replace(replace(d, ' for access method ', ' using '),
'ADD cast from (.*) to (.*);',
E'ADD cast (\\1 as \\2);')
FROM objs
$$;
dim=# select * from extension_unpackaged_upgrade_script('lo');
extension_unpackaged_upgrade_script
-
ALTER EXTENSION lo ADD type @extschema@.lo;
ALTER EXTENSION lo ADD function @extschema@.lo_oid(@extschema@.lo);
ALTER EXTENSION lo ADD function @extschema@.lo_manage();
(3 rows)
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
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] NULLs in array_cat vs array || array
On 15 February 2011 21:46, Cédric Villemain cedric.villemain.deb...@gmail.com wrote: 2011/2/15 Thom Brown t...@linux.com: Hi all, I assumed array_cat would behave similarly to array || array, but it appears not when it comes to NULLs. Shouldn't these have identical functionality? The attached patch makes it so, although it would break existing code. There is bugreport and todo entry for that if it helps: http://archives.postgresql.org/pgsql-bugs/2008-11/msg00032.php Ah, I see. More to it than meets the eye. My bad. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
On 02/15/2011 04:49 PM, Dimitri Fontaine wrote: Ah well sed makes it simpler to read, but it won't be usable in windows. You can make perl do the same stuff (and perl has psed anyway), and perl is required for MSVC builds. 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] NULLs in array_cat vs array || array
On 15 February 2011 21:47, Tom Lane t...@sss.pgh.pa.us wrote: Also, so far as I can see array_cat *is* ||, so I'm not sure what discrepancy in behavior you're on about. You've confused me now. I had a case where I replaced || with , and surrounded it with array_cat, and the result differed, and now I can't recreate it. I think I should get an early night. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Do we want to add such a query in the docs to help pgfoundry authors to write their own 'from unpackaged' scripts? [ scratches head ... ] Why is your version generating so many unnecessary @extschema@ uses? 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
