Re: [HACKERS] SSI bug?

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Itagaki Takahiro
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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Jan Urbański
- 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

2011-02-15 Thread Greg Stark
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Stephen Frost
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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Gurjeet Singh
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

2011-02-15 Thread Bruce Momjian
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]

2011-02-15 Thread Alexey Klyukin

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

2011-02-15 Thread Roman Prykhodchenko
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

2011-02-15 Thread Bruce Momjian
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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Andrew Dunstan



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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Stephen Frost
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Simon Riggs
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

2011-02-15 Thread Dimitri Fontaine
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Simon Riggs
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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Andrew Dunstan



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

2011-02-15 Thread Simon Riggs
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Andrew Dunstan



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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Chris Browne
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Dimitri Fontaine
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]

2011-02-15 Thread David E. Wheeler
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]

2011-02-15 Thread Alexey Klyukin

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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Stefan Kaltenbrunner

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]

2011-02-15 Thread David E. Wheeler
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

2011-02-15 Thread Simon Riggs
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

2011-02-15 Thread Stephen Frost
* 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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Peter Eisentraut
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

2011-02-15 Thread Kohei Kaigai

 -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

2011-02-15 Thread Peter Eisentraut
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

2011-02-15 Thread Peter Eisentraut
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread marcin mank
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread David E. Wheeler
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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Heikki Linnakangas

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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Robert Haas
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

2011-02-15 Thread Simon Riggs
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

2011-02-15 Thread David E. Wheeler
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

2011-02-15 Thread Alvaro Herrera
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

2011-02-15 Thread Thom Brown
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-02-15 Thread Cédric Villemain
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

2011-02-15 Thread Tom Lane
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

2011-02-15 Thread Dimitri Fontaine
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

2011-02-15 Thread Thom Brown
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

2011-02-15 Thread Andrew Dunstan



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

2011-02-15 Thread Thom Brown
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

2011-02-15 Thread Tom Lane
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


  1   2   >