Re: [HACKERS] PATCH: Implement value_to_json for single-datum conversion

2012-08-13 Thread Tom Lane
Craig Ringer ring...@ringerc.id.au writes:
 As for the value_to_json crashing, works for me:

 postgres=# SELECT value_to_json(42);
   value_to_json
 ---
   42
 (1 row)

Oh, right, because there actually is support for anyelement in the
underlying C function.  There is not in the quote_literal case.

 Purely so I understand what the correct handling of the anyelement+text 
 overload would've been: In light of your comments on opr_sanity would 
 the right approach be to add a second C function like text_to_json that 
 only accepts 'text' to avoid confusing the sanity check?

Actually, given the above, what did you need value_to_json(text) for at
all?  Wouldn't value_to_json(anyelement) have covered it?

But yeah, the general approach to suppressing complaints from that
opr_sanity test is to make more C entry points.  The point of it,
in some sense, is that if you want to make an assumption that two
types are binary-equivalent then it's better to have that assumption
in C code than embedded in the pg_proc entries.  The cases that we
let pass via the expected outputs are only ones where binary
equivalence seems pretty well assured, like text vs varchar.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] PATCH: Implement value_to_json for single-datum conversion

2012-08-13 Thread Craig Ringer

On 08/13/2012 01:55 PM, Tom Lane wrote:

Actually, given the above, what did you need value_to_json(text) for at
all?  Wouldn't value_to_json(anyelement) have covered it?


Usability. Without the version accepting text an explicit cast to text 
is required to disambiguate a literal argument like 
value_to_json('something') .



But yeah, the general approach to suppressing complaints from that
opr_sanity test is to make more C entry points.  The point of it,
in some sense, is that if you want to make an assumption that two
types are binary-equivalent then it's better to have that assumption
in C code than embedded in the pg_proc entries.  The cases that we
let pass via the expected outputs are only ones where binary
equivalence seems pretty well assured, like text vs varchar.


Thanks. I appreciate the explanation, and sorry for the newbie error.


On the JSON stuff, I can see it's not as simple as adding a simple 
escape function. For my needs during the 9.2 timeframe I'll bundle up an 
extension with the functionality I need and deal with the need to port 
to whatever 9.3 includes. Hopefully a json_value or javascript_value 
or similar can be introduced for 9.3, given comments like:


  http://archives.postgresql.org/pgsql-hackers/2012-05/msg00030.php
  http://archives.postgresql.org/pgsql-hackers/2012-05/msg00065.php

.. expressing not only the need for json scalars, but the fact that 
they're already commonplace in pretty much everything else.


Given this:

  http://archives.postgresql.org/pgsql-hackers/2012-05/msg00040.php

it does seem that `json' should be a whole document not a fragment, but 
IMO a way to work with individual JSON values is going to be *necessary* 
to get the most out of the json support - and to stop people who use 
JSON in the real world complaining that Pg's JSON support is broken 
because it follows the standard not real-world practice.


Personally the lack of json scalars has prevented me from using JSON 
support in two different places already, though it's proving very useful 
in many others.


--
Craig Ringer


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Heikki Linnakangas

On 12.08.2012 17:39, Tom Lane wrote:

Heikki Linnakangasheikki.linnakan...@enterprisedb.com  writes:

The problem is that when a postmaster subprocess is launched, it calls
read_nondefault_variables() very early, before shmem initialization, to
read the non-default config options from the file that postmaster wrote.
When check_XactIsoLevel() calls RecoveryInProgress(), it crashes,
because XLogCtl is NULL.


Hm, how did the same code fail to crash in the postmaster itself, when
the postmaster read the setting from postgresql.conf?


It's not the check function for default_transaction_isolation that 
crashes, but the one for transaction_isolation.


I'm not exactly sure how transaction_isolation gets set to a non-default 
value, though. The default for transaction_isolation is 'default', so 
it's understandable that the underlying XactIsoLevel variable gets set 
to XACT_SERIALIZABLE, but AFAICS the code to read/write the GUCs from/to 
file only cares about the string value of the guc, not the integer value 
of the underlying global variable.



A larger point is that I think it's broken for any GUC assignment
function to be calling something as transient as RecoveryInProgress to
start with.  We probably ought to re-think the logic, not just band-aid
this by having it skip the check when shmem isn't initialized yet.
I'm thinking that the check has to occur somewhere outside GUC.


Hmm, it seems like the logical place to complain if you do a manual SET 
transaction_isolation='serializable'. But I think we should only do the 
check if we're not in a transaction. Setting the guc won't have any 
effect outside a transaction anyway, because StartTransaction will 
overwrite it from default_transaction_isolation as soon as you begin a 
transaction.


While playing around, I bumped into another related bug, and after 
googling around I found out that it was already reported by Robert Haas 
earlier, but still not fixed: 
http://archives.postgresql.org/message-id/CA%2BTgmoa0UM2W1YkjjneEgJctzxopC3G53ocYPaCyoEOWT3aKiA%40mail.gmail.com. 
Kevin, the last message on that thread 
(http://archives.postgresql.org/pgsql-hackers/2012-04/msg01394.php) says 
you'll write a patch for that. Ping? Or would you like me to try 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] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 On 12.08.2012 17:39, Tom Lane wrote:
 A larger point is that I think it's broken for any GUC assignment
 function to be calling something as transient as RecoveryInProgress to
 start with.

 Hmm, it seems like the logical place to complain if you do a manual SET 
 transaction_isolation='serializable'. But I think we should only do the 
 check if we're not in a transaction.

You mean if we *are* in a transaction?  Possibly that would work.
The concerns I've got about running this type of test in a GUC hook
are (1) what if you're not in a transaction or (2) what if you aren't
in a process that has access to shared memory; the early-startup-in-
EXEC_BACKEND case loses on both counts.  But I think we can test our
local in-transaction state without touching shared memory, and then
go from there.

 While playing around, I bumped into another related bug, and after 
 googling around I found out that it was already reported by Robert Haas 
 earlier, but still not fixed: 
 http://archives.postgresql.org/message-id/CA%2BTgmoa0UM2W1YkjjneEgJctzxopC3G53ocYPaCyoEOWT3aKiA%40mail.gmail.com

That sounds like a different bug, although perhaps with the same theme
of overreliance on what a GUC hook can do.

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] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Amit Kapila
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Heikki Linnakangas
Sent: Monday, August 13, 2012 12:14 PM
On 12.08.2012 17:39, Tom Lane wrote:
 Heikki Linnakangasheikki.linnakan...@enterprisedb.com  writes:
 The problem is that when a postmaster subprocess is launched, it calls
 read_nondefault_variables() very early, before shmem initialization, to
 read the non-default config options from the file that postmaster wrote.
 When check_XactIsoLevel() calls RecoveryInProgress(), it crashes,
 because XLogCtl is NULL.

 Hm, how did the same code fail to crash in the postmaster itself, when
 the postmaster read the setting from postgresql.conf?

It's not the check function for default_transaction_isolation that 
crashes, but the one for transaction_isolation.

 I 'm not exactly sure how transaction_isolation gets set to a non-default 
 value, though. The default for transaction_isolation is 'default', so 
 it's understandable that the underlying XactIsoLevel variable gets set 
 to XACT_SERIALIZABLE, but AFAICS the code to read/write the GUCs from/to 
 file only cares about the string value of the guc, not the integer value 
 of the underlying global variable.

Here What I am able to trace is that function read_nondefault_variables(),
reads all variables
from config_exec_params which contains both default_transaction_isolation
and transaction_isolation.

1. it first reads default_transaction_isolation and sets value of
DefaultXactIsoLevel to 'serializable'.
2. As for parameter default_transaction_isolation, there is no check
function it passes.
3. After that when variable transaction_isolation is processed, function
check_XactIsoLevel() sets 
   XactIsoLevel to XACT_SERIALIZABLE which causes crash.

Actually function read_nondefault_variables(), should only process non
default values (default_transaction_isolation)
not transaction_isolation, but currently it processes both?


With Regards,
Amit Kapila.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Amit Kapila
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila
Sent: Monday, August 13, 2012 12:47 PM
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Heikki Linnakangas
Sent: Monday, August 13, 2012 12:14 PM
On 12.08.2012 17:39, Tom Lane wrote:
 Heikki Linnakangasheikki.linnakan...@enterprisedb.com  writes:


 Hm, how did the same code fail to crash in the postmaster itself, when
 the postmaster read the setting from postgresql.conf?

It's not the check function for default_transaction_isolation that 
crashes, but the one for transaction_isolation.

 I 'm not exactly sure how transaction_isolation gets set to a non-default

 value, though. The default for transaction_isolation is 'default', so 
 it's understandable that the underlying XactIsoLevel variable gets set 
 to XACT_SERIALIZABLE, but AFAICS the code to read/write the GUCs from/to 
 file only cares about the string value of the guc, not the integer value 
 of the underlying global variable.

 Here What I am able to trace is that function read_nondefault_variables(),
 reads all variables
 from config_exec_params which contains both default_transaction_isolation
 and transaction_isolation.

 1. it first reads default_transaction_isolation and sets value of
 DefaultXactIsoLevel to 'serializable'.
 2. As for parameter default_transaction_isolation, there is no check
 function it passes.
 3. After that when variable transaction_isolation is processed, function
 check_XactIsoLevel() sets 
   XactIsoLevel to XACT_SERIALIZABLE which causes crash.

 Actually function read_nondefault_variables(), should only process non
 default values (default_transaction_isolation)
 not transaction_isolation, but currently it processes both?

transaction_isolation is getting written to config_exec_params file as
function 
write_one_nondefault_variable() checks if conf source is not PGC_S_DEFAULT,
then it writes to file.
For transaction_isolation, the conf source is set to PGC_S_OVERRIDE in
function InitializeGUCOptions()
so this also gets written to config_exec_params file.
Should the parameter transaction_isolation be written to config_exec_params?


With Regards,
Amit Kapila.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Proof of concept: auto updatable views

2012-08-13 Thread Dean Rasheed
I've been looking at this further and I have made some improvements,
but also found a problem.

On 1 July 2012 23:35, Dean Rasheed dean.a.rash...@gmail.com wrote:
 I'm also aware that my new function ChangeVarAttnos() is almost
 identical to the function map_variable_attnos() that Tom recently
 added, but I couldn't immediately see a neat way to merge the two. My
 function handles whole-row references to the view by mapping them to a
 generic RowExpr based on the view definition. I don't think a
 ConvertRowtypeExpr can be used in this case, because the column names
 don't necessarily match.

I improved on this by reusing the existing function ResolveNew() which
reduced the size of the patch.


The problem, however, is that the original patch is not safe for
UPDATE/DELETE on security barrier views, because it mixes the user's
quals with those on the view. For example a query like

UPDATE some_view SET col=... WHERE user_quals

will get rewritten as

UPDATE base_table SET base_col=... WHERE user_quals AND view_quals

which potentially leaks data hidden by the view's quals.

So I think that it needs to use a subquery to isolate user_quals from
view_quals. The least invasive way I could see to do that was to
record the view quals in a new security barrier quals field on the
RTE, and then turn that into a subquery at the end of the rewriting
process. This approach avoids the extensive changes to the rewriter
that I think would otherwise be needed if the RTE were changed into a
subquery near the start of the rewriting process. It is also possible
that this code might be reusable in the row-level security patch by
Kohei KaiGai to handle modifications to tables with RLS quals,
although I haven't looked too closely at that patch yet.


Another complication is that the executor appears to have no
rechecking code for subqueries in nodeSubqueryscan.c, so it looks like
I need to lock rows coming from the base relation in the subquery. So
for example, given the following setup

CREATE TABLE private_t(a int, b text);
INSERT INTO private_t VALUES (1, 'Private');
INSERT INTO private_t
  SELECT i, 'Public '||i FROM generate_series(2,1) g(i);
CREATE INDEX private_t_a_idx ON private_t(a);
ANALYSE private_t;

CREATE VIEW public_v WITH (security_barrier=true)
  AS SELECT b AS bb, a AS aa FROM private_t WHERE a != 1;

CREATE OR REPLACE FUNCTION snoop(b text)
RETURNS boolean AS
$$
BEGIN
  RAISE NOTICE 'b=%', b;
  RETURN false;
END;
$$
LANGUAGE plpgsql STRICT IMMUTABLE COST 0.01;

an update on the view will get rewritten as an update on the base
table from a subquery scan as follows:

EXPLAIN (costs off) UPDATE public_v SET aa=aa WHERE snoop(bb) AND aa=2;

 Update on private_t
   -  Subquery Scan on private_t
 Filter: snoop(private_t.b)
 -  LockRows
   -  Index Scan using private_t_a_idx on private_t
 Index Cond: (a = 2)
 Filter: (a  1)

The LockRows node appears to give the expected behaviour for
concurrently modified rows, but I'm not really sure if that's the
right approach.


None of this new code kicks in for non-security barrier views, so the
kinds of plans I posted upthread remain unchanged in that case. But
now a significant fraction of the patch is code added to handle
security barrier views. Of course we could simply say that such views
aren't updatable, but that seems like an annoying limitation if there
is a feasible way round it.

Thoughts?

Regards,
Dean


auto-update-views.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] Yet another failure mode in pg_upgrade

2012-08-13 Thread Magnus Hagander
On Mon, Aug 13, 2012 at 4:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I've been experimenting with moving the Unix socket directory to
 /var/run/postgresql for the Fedora distribution (don't ask :-().
 It's mostly working, but I found out yet another way that pg_upgrade
 can crash and burn: it doesn't consider the possibility that the
 old or new postmaster is compiled with a different default
 unix_socket_directory than what is compiled into the libpq it's using
 or that pg_dump is using.

 This is another hazard that we could forget about if we had some way for
 pg_upgrade to run standalone backends instead of starting a postmaster.

Yeah, that would be nice.


 But in the meantime, I suggest it'd be a good idea for pg_upgrade to
 explicitly set unix_socket_directory (or unix_socket_directories in
 HEAD) when starting the postmasters, and also explicitly set PGHOST
 to ensure that the client-side code plays along.

That sounds like a good idea for other reasons as well - manual
connections attempting to get in during an upgrade will just fail with
a no connection error, which makes sense...

So, +1.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Bug in libpq implentation and omission in documentation?

2012-08-13 Thread Heikki Linnakangas

On 08.08.2012 17:35, Tom Lane wrote:

A runtime check for too many parameters seems appropriate.  Assuming
that the error message it throws is well written, I don't think we need
to adjust the documentation.  There are many limitations that are not
spelled out in the docs, and adding every one of them would not amount
to a net improvement.  Considering that Jim is the first to try this in
however many years it's been since 7.4, I don't think that everybody
needs to read about this restriction while they're trying to absorb what
PQexecParams does.


Runtime check added to the functions that take an nParams argument.

--
  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


[HACKERS] sharing variables between client and server again

2012-08-13 Thread Pavel Stehule
Hello

this is VIP patch that implements shared - status variables to
PostgreSQL. A motivation for this features is possibility to take
client parameters inside inlined PL blocks.

This design is based on Magnus's ideas. Content is synchronized
between server and client. This doesn't need a protocol enhancing.

pavel ~ $ psql postgres -v status.myparam=Hello
psql (9.3devel)
Type help for help.

postgres=# \echo :status.myparam
Hello
postgres=# show status.myparam;
 status.myparam

 Hello
(1 row)

postgres=# do $$ begin raise notice '%',
current_setting('status.myparam'); end $$ language plpgsql;
NOTICE:  Hello
DO

postgres=# set status.myparam = 'Bobo';
SET
postgres=# \echo :status.myparam
Bobo

postgres=# \set status.myparam Otis
postgres=# show status.myparam;
 status.myparam

 Otis
(1 row)

Regards

Pavel Stehule


shared_variables.diff
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


[HACKERS] SIGFPE handler is naive

2012-08-13 Thread Robert Haas
A member of our technical team brought it to my attention that if you
have a recalcitrant backend that doesn't die when you send it a
SIGTERM, and you don't want to cause a crash-and-restart cycle by
sending it SIGQUIT, you can have your cake and eat it too by sending
it SIGFPE, which will cheerfully throw an error from wherever inside
the backend you happen to be, whether it is safe or not.  I suppose we
had it in mind when this was coded that SIGFPE would only be generated
by the system rather than by user activity, and it is a pretty neat
trick for working around the lack of CHECK_FOR_INTERRUPTS() in some
place where you really wish there were one, but the possibility of
setting yourself on fire also seems rather high.  Since the person who
made the discovery realized the utility of the trick but not the fact
that it might destabilize the backend, it seems that the surface area
for self-destruction is non-zero.

Should we do something to plug this, and if so, what?  If not, should
we document the danger?

-- 
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] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:
 
 While playing around, I bumped into another related bug, and
 after googling around I found out that it was already reported by
 Robert Haas earlier, but still not fixed: 

http://archives.postgresql.org/message-id/CA%2BTgmoa0UM2W1YkjjneEgJctzxopC3G53ocYPaCyoEOWT3aKiA%40mail.gmail.com
 Kevin, the last message on that thread (

http://archives.postgresql.org/pgsql-hackers/2012-04/msg01394.php
 ) says you'll write a patch for that. Ping? Or would you like me
 to try that?
 
Let me try to redeem myself by taking a shot at it tonight.
 
-Kevin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Upcoming back-branch releases

2012-08-13 Thread Tom Lane
We are going to prepare back-branch update releases to take care of
a couple of minor security issues that have been waiting much too
long already.  The schedule will be a bit unusual though: wrap tarballs
this Tuesday evening (that's tomorrow) for public announcement Friday
the 17th.

This is not because the security issues are unusually urgent; it's
because, after much arguing on the core and packagers lists about how
we could fit a release schedule around everybody's vacation and public
holiday dates, we were forced to the realization that it was either do
it right now or wait until September.  So we're doing it now.  Sorry
about the short notice, but there just wasn't any other way.

Memo to self: never again believe that we can get releases out during
August.

regards, tom lane

PS: we are still planning to wrap 9.2RC1 next week, but we are not
expecting that the Windows installers will be available immediately
on the 27th.  They'll probably be a couple days later than that.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] 9.2 Cascading replication after slave promotion

2012-08-13 Thread Mark Kirkwood
Looking at the docs (section 25.2.6 paragraph 6) leads one to believe 
that downstream standbys can continue to receive and process wal merely 
by reconnecting after the cascading standby is promoted - but this does 
not seem to be the case (indeed the same paragraph notes that timelines 
now no longer match). It looks to me like the downstream guys all need 
to be rebuilt - or am I missing something?


Regards

mark


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.2 Cascading replication after slave promotion

2012-08-13 Thread Josh Berkus
Mark,

 Looking at the docs (section 25.2.6 paragraph 6) leads one to believe
 that downstream standbys can continue to receive and process wal merely
 by reconnecting after the cascading standby is promoted - but this does
 not seem to be the case (indeed the same paragraph notes that timelines
 now no longer match). It looks to me like the downstream guys all need
 to be rebuilt - or am I missing something?

Per discussion on this list (search for remastering), 9.2 still won't
allow standbies to point to a new master unless you are doing file-based
replication.

To date, I seem to be the only one convinced that this is an actual
deficiency ...


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.2 Cascading replication after slave promotion

2012-08-13 Thread Mark Kirkwood

On 14/08/12 12:32, Josh Berkus wrote:

Mark,


Looking at the docs (section 25.2.6 paragraph 6) leads one to believe
that downstream standbys can continue to receive and process wal merely
by reconnecting after the cascading standby is promoted - but this does
not seem to be the case (indeed the same paragraph notes that timelines
now no longer match). It looks to me like the downstream guys all need
to be rebuilt - or am I missing something?

Per discussion on this list (search for remastering), 9.2 still won't
allow standbies to point to a new master unless you are doing file-based
replication.

To date, I seem to be the only one convinced that this is an actual
deficiency ...




Make that 2 of us...


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.2 Cascading replication after slave promotion

2012-08-13 Thread Daniel Farina
On Mon, Aug 13, 2012 at 5:32 PM, Josh Berkus j...@agliodbs.com wrote:
 To date, I seem to be the only one convinced that this is an actual
 deficiency ...

It is an actual deficiency, and I am convinced.

-- 
fdr


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] SIGFPE handler is naive

2012-08-13 Thread Noah Misch
On Mon, Aug 13, 2012 at 01:04:58PM -0400, Robert Haas wrote:
 A member of our technical team brought it to my attention that if you
 have a recalcitrant backend that doesn't die when you send it a
 SIGTERM, and you don't want to cause a crash-and-restart cycle by
 sending it SIGQUIT, you can have your cake and eat it too by sending
 it SIGFPE, which will cheerfully throw an error from wherever inside
 the backend you happen to be, whether it is safe or not.  I suppose we
 had it in mind when this was coded that SIGFPE would only be generated
 by the system rather than by user activity, and it is a pretty neat
 trick for working around the lack of CHECK_FOR_INTERRUPTS() in some
 place where you really wish there were one, but the possibility of
 setting yourself on fire also seems rather high.  Since the person who
 made the discovery realized the utility of the trick but not the fact
 that it might destabilize the backend, it seems that the surface area
 for self-destruction is non-zero.
 
 Should we do something to plug this, and if so, what?  If not, should
 we document the danger?

If this use of SIGFPE is handy, we should expose it under a better name.  What
hazards make it unsafe?  In a critical section, it escalates to a PANIC
anyway.  In third-party library code, the library state may become corrupt.
In backend code skipping a PG_TRY/PG_CATCH for sections that cannot throw
errors, cleanup will never happen.  That's plenty of danger, but I can
sympathize with folks wanting to take the risk and use SIGFPE this way.

Overall, though, I think it best to plug this.  We could set a flag before
each operation, like evaluation of SQL arithmetic, for which SIGFPE is normal.
If the signal handler sees the flag set, raise ERROR.  Otherwise, PANIC.  Code
running with the flag set would, of course, need to be ready for a spontaneous
elog(ERROR) at any instruction.

Thanks,
nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] 9.2 Cascading replication after slave promotion

2012-08-13 Thread Stephen Frost
* Daniel Farina (dan...@heroku.com) wrote:
 On Mon, Aug 13, 2012 at 5:32 PM, Josh Berkus j...@agliodbs.com wrote:
  To date, I seem to be the only one convinced that this is an actual
  deficiency ...
 
 It is an actual deficiency, and I am convinced.

Yeah, I think there's more people that agree with this use-case than you
seem to think..  That said, I appreciate that it's not a trivial thing
to support cleanly.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] SIGFPE handler is naive

2012-08-13 Thread Robert Haas
On Mon, Aug 13, 2012 at 10:14 PM, Noah Misch n...@leadboat.com wrote:
 If this use of SIGFPE is handy, we should expose it under a better name.  What
 hazards make it unsafe?

Well, the most obvious problem is that a backend might receive it
while holding a spinlock.

 Overall, though, I think it best to plug this.  We could set a flag before
 each operation, like evaluation of SQL arithmetic, for which SIGFPE is normal.
 If the signal handler sees the flag set, raise ERROR.  Otherwise, PANIC.  Code
 running with the flag set would, of course, need to be ready for a spontaneous
 elog(ERROR) at any instruction.

Yeah, that's what I thought of, too.  It seems like it'd be a lot of
work to get there, though.

-- 
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] SIGFPE handler is naive

2012-08-13 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Aug 13, 2012 at 10:14 PM, Noah Misch n...@leadboat.com wrote:
 Overall, though, I think it best to plug this.  We could set a flag before
 each operation, like evaluation of SQL arithmetic, for which SIGFPE is 
 normal.

 Yeah, that's what I thought of, too.  It seems like it'd be a lot of
 work to get there, though.

That would depend on how many places there are where SIGFPE is expected.
Are we sure there are any?  Maybe we should just remove the handler and
let SIGFPE be treated as a core dump.

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] canceling autovacuum task woes

2012-08-13 Thread Peter Eisentraut
On Tue, 2012-07-24 at 16:14 -0400, Robert Haas wrote:
 On Tue, Jul 24, 2012 at 4:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Robert Haas robertmh...@gmail.com writes:
  Yeah, you're right.  So you do get the table name.  But you don't get
  the cause, which is what you really need to understand why it's
  happening.  Attached is a patch that adds some more detail.
 
  Uh, what's the added dependency on pgstat.h for?  Looks sane to the
  eyeball otherwise.
 
 Woops, that was leftovers from some earlier silliness that I (mostly)
 removed before posting.
 
 New version attached.
 

The detail message should have a period at the end.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Kevin Grittner
Heikki Linnakangas  wrote:
 
 While playing around, I bumped into another related bug, and after
 googling around I found out that it was already reported by Robert
 Haas earlier, but still not fixed:
 
 Kevin, the last message on that thread says you'll write a patch
 for that. Ping?
 
OK, attached is a first cut to confirm that the approach looks sane
to everyone; I *think* it is along the lines that we reached
consensus.  After moving the check to the point where we get a
serializable snapshot, it was still behaving badly.  It took a bit,
but forcing the snapshot acquisition in InitPostgres to use 'read
committed' instead of the default isolation level got reasonable
behavior in my initial tests.  It sure looks a lot better to *me*
than what was happening before.
 
Besides confirming that this is the solution people want, this has
not been tested well enough yet to be ready for commit.  It's getting
late, though, and I'm fading.  If the overall approach looks good
I'll beat up on it some more tomorrow.
 
Since the existing behavior is so bad, I'm inclined to think this
merits backpatching to 9.1.  Thoughts on that?
 
-Kevin


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] default_isolation_level='serializable' crashes on Windows

2012-08-13 Thread Kevin Grittner
Kevin Grittner  wrote:
 
 OK, attached
 
[sigh]
 
This time for sure!
 
-Kevin




hotstandby-serializable.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