Re: [HACKERS] Statistics and selectivity estimation for ranges

2012-08-14 Thread Alexander Korotkov
On Mon, Aug 13, 2012 at 1:11 AM, Alexander Korotkov aekorot...@gmail.comwrote:

 On Thu, Aug 9, 2012 at 12:44 AM, Alexander Korotkov 
 aekorot...@gmail.comwrote:

 My conclusion is so, that current errors are probably ok for selectivity
 estimation. But taking into attention that generated datasets ideally fits
 assumptions of estimation, there could be room for improvement. Especially,
 it's unclear why estimate for @ and @ have much greater error than
 estimate for . Possibly, it's caused by some bugs.


 ITSM, I found reason of inaccuracy. Implementation of linear interpolation
 was wrong. Fixed version is attached. Now, need to rerun tests, possible
 refactoring and comments rework.


After fixing few more bugs, I've a version with much more reasonable
accuracy.

Statistics target = 100.

Relatively large result sets (= 10)

test=# select operator, avg(estimate_count::float8/actual_count::float8) as
avg_ratio, avg(exp(abs(ln(estimate_count::float8/actual_count::float8 -
1.0 as avg_error from datasets d join test_results tr on tr.test_id =
d.idwhere d.stat_target = 100 and actual_count = 10 group by
operator;
 operator |avg_ratio | avg_error
--+--+
 @   | 1.00404179116863 | 0.0504415454560903
 @   | 1.06364108531688 |  0.105646077989812
| 1.00757984721409 | 0.0420984234933233
(3 rows)

Small result sets (1 - 9)

test=# select operator, avg(estimate_count::float8/actual_count::float8) as
avg_ratio, avg(exp(abs(ln(estimate_count::float8/actual_count::float8 -
1.0 as avg_error from datasets d join test_results tr on tr.test_id =
d.idwhere d.stat_target = 100 and actual_count between 1 and 9 group
by
operator;
 operator |avg_ratio | avg_error
--+--+---
 @   | 1.31530838062865 | 0.654886592410495
 @   | 2.78708078320147 |  1.94124123003433
| 1.93268112525538 |  1.09904919063335
(3 rows)

Empty result sets

test=# select operator, avg(estimate_count) as avg_estimate, count(*) as
tests_count from datasets d join test_results tr on tr.test_id = d.id where
d.stat_target = 100 and actual_count = 0 group by operator;
 operator |avg_estimate| tests_count
--++-
 @   | 1.1437670609645132 |1099
 @   | 1.0479430126460701 |   87458
(2 rows)

Statistics target = 1000.

Relatively large result sets (= 10)

test=# select operator, avg(estimate_count::float8/actual_count::float8) as
avg_ratio, avg(exp(abs(ln(estimate_count::float8/actual_count::float8 -
1.0 as avg_error from datasets d join test_results tr on tr.test_id =
d.idwhere d.stat_target = 1000 and actual_count = 10 group by
operator;
 operator |avg_ratio | avg_error
--+--+
 @   | 1.00073999445381 |  0.045099762607524
 @   | 1.05296320350853 | 0.0907489633452971
| 1.00217602359039 | 0.0353421159150165
(3 rows)

Small result sets (1 - 9)

test=# select operator, avg(estimate_count::float8/actual_count::float8) as
avg_ratio, avg(exp(abs(ln(estimate_count::float8/actual_count::float8 -
1.0 as avg_error from datasets d join test_results tr on tr.test_id =
d.idwhere d.stat_target = 1000 and actual_count between 1 and 9 group
by
operator;
 operator |avg_ratio | avg_error
--+--+---
 @   | 1.26946358795998 | 0.577803898836364
 @   | 2.69000633430211 |  1.83165424646645
| 1.48715184186882 | 0.577998652291105
(3 rows)

Empty result sets

test=# select operator, avg(estimate_count) as avg_estimate, count(*) as
tests_count from datasets d join test_results tr on tr.test_id = d.id where
d.stat_target = 1000 and actual_count = 0 group by operator;
 operator |avg_estimate| tests_count
--++-
 @   | 1.0887096774193548 |1364
 @   | 1.0423876983771183 |   89224
| 5. |   1
(3 rows)

--
With best regards,
Alexander Korotkov.


range_stat-0.6.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] patch: shared session variables

2012-08-14 Thread Pavel Stehule
Hello

patch that implements shared client/server session variables

Regards

Pavel Stehule


shared_variables-01.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] betatesting: ERROR: failed to build any 2-way joins on 9.2

2012-08-14 Thread Pavel Stehule
Hello

My colleague found a issue on 9.2 - sorry for query formatting - this
query is generated from ours query engine

testdb=# \i planbug.sql
DROP TABLE
DROP TABLE
DROP TABLE
DROP TABLE
DROP TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE TABLE
psql:planbug.sql:66: ERROR:  failed to build any 2-way joins
LOCATION:  join_search_one_level, joinrels.c:199

it works on 9.1


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

2012-08-14 Thread Heikki Linnakangas

On 14.08.2012 08:23, Kevin Grittner wrote:

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.


A comment in InitPostgres would be nice, explaining why we want a read 
committed snapshot there.



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.


Thanks! This fixes both the assertion failure and the Windows crash, 
which is good.


Now that the error is thrown when the first snapshot is taken, rather 
than at the SET command, I think the error message needs some work:


postgres=# select 123;
ERROR:  cannot use serializable mode in a hot standby
HINT:  You can use REPEATABLE READ instead.

If the isolation level came from default_transaction_isolation, set in 
postgresql.conf file, it might take the user a while to figure that out. 
Perhaps something like  this:


ERROR:  cannot use serializable mode in a hot standby
DETAIL: default_transaction_isolation was set to 'serializable' in the 
config file.
HINT:   You can use SET transaction_isolation = 'repeatable read' 
before the first query in the transaction to override it.



This still leaves the RecoveryInProgress() call in 
check_transaction_read_only(), which isn't a problem at the moment, but 
seems fragile. I think we should still add the IsTransactionState() 
check in there, so that it works without shared memory access. If we're 
not in a transaction yet (TRANS_DEFAULT), setting transaction_read_only 
has no effect, as it's overwritten at the beginning of a transaction. If 
we're in one of the transitory states, TRANS_START or 
TRANS_COMMIT/ABORT/PREPARE, I'm not sure what should happen, but it 
should not be possible for user code to run and change 
transaction_read_only in those states.



Since the existing behavior is so bad, I'm inclined to think this
merits backpatching to 9.1.  Thoughts on that?


Yes, we have to somehow fix the crash and the assertion failure on 9.1.

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

2012-08-14 Thread Nils Goroll

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


I am not sure if I really understood the intention of the question correctly, 
but if the question was if pg should try to work around misuse of signals, then 
my answer would be a definite no.


IMHO, the signal handler should check if the signal was received for good 
reasons (as proposed by Noah) and handle it appropriately, but otherwise ignore it.


Nils


--
Sent 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-14 Thread Greg Stark
It is possible to check if the signal was synchronous or was sent from
an external process. You can check siginfo-si_pid to see who sent you
the signal. I'm not sure checking that and handling it at
check_for_interrupts if it's asynchronous is the best solution or not
though.

I'm a bit confused. Didn't Tom do the laborious process of checking
the whole source tree for situations where there's shared memory
cleanup to be done in and arrange for it to happen? That was the
blocking factor to get pg_cancel_backend() to work. Is the problem
that the sigfpe handler doesn't invoke atexit() handlers?

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

2012-08-14 Thread Kevin Grittner
Heikki Linnakangas  wrote:
 On 14.08.2012 08:23, Kevin Grittner wrote:
 
 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.
 
Oh, further testing this morning shows that while *queries* on the HS
seem OK, streaming replication is now broken.  I probably need to
override transaction isolation on the recovery process.  I'll take a
look at that.
 
 A comment in InitPostgres would be nice, explaining why we want a
 read committed snapshot there.
 
OK.
 
 This fixes both the assertion failure and the Windows crash, which
 is good.
 
I wasn't sure whether it would help with the Windows crash or not. 
I'm not set up to build under Windows, so I'm glad you gave that a
check.
 
 Now that the error is thrown when the first snapshot is taken,
 rather than at the SET command, I think the error message needs
 some work:
 
 postgres=# select 123;
 ERROR: cannot use serializable mode in a hot standby
 HINT: You can use REPEATABLE READ instead.
 
 If the isolation level came from default_transaction_isolation, set
 in postgresql.conf file, it might take the user a while to figure
 that out. Perhaps something like this:
 
 ERROR: cannot use serializable mode in a hot standby
 DETAIL: default_transaction_isolation was set to 'serializable' in
 the config file.
 HINT: You can use SET transaction_isolation = 'repeatable read'
 before the first query in the transaction to override it.
 
Did you mean?:
 
HINT: You can use SET default_transaction_isolation = 'repeatable
read' before the first query in the transaction to override it.
 
Otherwise, I agree and will do.
 
 This still leaves the RecoveryInProgress() call in
 check_transaction_read_only(), which isn't a problem at the moment,
 but seems fragile. I think we should still add the
 IsTransactionState() check in there, so that it works without
 shared memory access. If we're not in a transaction yet
 (TRANS_DEFAULT), setting transaction_read_only has no effect, as
 it's overwritten at the beginning of a transaction. If we're in one
 of the transitory states, TRANS_START or
 TRANS_COMMIT/ABORT/PREPARE, I'm not sure what should happen, but it
 should not be possible for user code to run and change
 transaction_read_only in those states.
 
I'll take a look.
 
 Since the existing behavior is so bad, I'm inclined to think this
 merits backpatching to 9.1. Thoughts on that?
 
 Yes, we have to somehow fix the crash and the assertion failure on
 9.1.
 
Should the check_transaction_read_only() stuff be back-patched to
9.1, too?  So far as we know, that's fragile, not broken, right? 
Could the fix you envision there cause a behavioral change that could
break anything that users might have in place?
 
-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] SIGFPE handler is naive

2012-08-14 Thread Robert Haas
On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 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.

No clue.  According to Wikipedia, it is commonly caused by dividing by
zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
range for the type.  I'd be willing to bet that we have got all the
division-by-zero cases patched up just because we try pretty hard to
emit the right error message for such cases, but I'm a lot less
certain that things like INT_MIN/-1 can't happen anywhere.

-- 
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-14 Thread Robert Haas
On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark st...@mit.edu wrote:
 It is possible to check if the signal was synchronous or was sent from
 an external process. You can check siginfo-si_pid to see who sent you
 the signal. I'm not sure checking that and handling it at
 check_for_interrupts if it's asynchronous is the best solution or not
 though.

If that's portable it might be an option, but I doubt that it is.

 I'm a bit confused. Didn't Tom do the laborious process of checking
 the whole source tree for situations where there's shared memory
 cleanup to be done in and arrange for it to happen? That was the
 blocking factor to get pg_cancel_backend() to work. Is the problem
 that the sigfpe handler doesn't invoke atexit() handlers?

No, the problem is that SIGFPE throws an error *from the signal
handler* rather than waiting for ProcessInterrupts().

-- 
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-14 Thread k...@rice.edu
On Mon, Aug 13, 2012 at 11:52:06PM -0400, Tom Lane wrote:
 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
 

Wouldn't any user level divide-by-zero code cause a SIGFPE? 

Regards,
Ken


-- 
Sent 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-14 Thread Kevin Grittner
Heikki Linnakangas  wrote:
 
 we have to somehow fix the crash and the assertion failure on 9.1.
 
Here's a revision with some changes based on your feedback.  I have
to go to my day job now, and I was unable to find the right place
to fix the streaming replication problem.  I fear I won't be able to
get this sorted out before the wrap of back-branch releases this
evening, so if you feel it is urgent enough to need to get into
that, feel free to finish it; otherwise I'll keep at it tonight.
 
-Kevin




hotstandby-serializable-2.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] SIGFPE handler is naive

2012-08-14 Thread Noah Misch
On Tue, Aug 14, 2012 at 08:38:44AM -0400, Robert Haas wrote:
 On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  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.
 
 No clue.  According to Wikipedia, it is commonly caused by dividing by
 zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
 range for the type.  I'd be willing to bet that we have got all the
 division-by-zero cases patched up just because we try pretty hard to
 emit the right error message for such cases, but I'm a lot less
 certain that things like INT_MIN/-1 can't happen anywhere.

[local] test=# select -9223372036854775808/-1;
ERROR:  floating-point exception


-- 
Sent 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-14 Thread Robert Haas
On Tue, Aug 14, 2012 at 8:55 AM, k...@rice.edu k...@rice.edu wrote:
 On Mon, Aug 13, 2012 at 11:52:06PM -0400, Tom Lane wrote:
 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.

 Wouldn't any user level divide-by-zero code cause a SIGFPE?

If it's written in C, sure.  If it's written in SQL, no, because we
check for that inside int4div et all.

-- 
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-14 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 On Tue, Aug 14, 2012 at 08:38:44AM -0400, Robert Haas wrote:
 On Mon, Aug 13, 2012 at 11:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 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.

 No clue.  According to Wikipedia, it is commonly caused by dividing by
 zero, or by dividing by INT_MIN by -1, resulting in a quotient out of
 range for the type.  I'd be willing to bet that we have got all the
 division-by-zero cases patched up just because we try pretty hard to
 emit the right error message for such cases, but I'm a lot less
 certain that things like INT_MIN/-1 can't happen anywhere.

 [local] test=# select -9223372036854775808/-1;
 ERROR:  floating-point exception

On reflection I think we should just leave this alone.  If we try to
tighten it up, what we will mainly accomplish is to make the system
less robust, not more, because any place we miss then represents an
easily reproducible DOS attack.

If somebody's dumb enough to think that SIGFPE'ing a backend represents
a supported way of canceling a query, that's his problem not ours.
We don't need to expend large amounts of effort on being sure we slap
his hand.

regards, tom lane


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


Re: [HACKERS] WIP patch for consolidating misplaced-aggregate checks

2012-08-14 Thread Alvaro Herrera
Excerpts from Greg Stark's message of vie ago 10 12:57:25 -0400 2012:
 On Thu, Aug 9, 2012 at 5:40 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Fair enough.  I was not sold on doing that either.  I would still like
  to know if it's okay to use one string with %s for the cases where
  there's not a good reason for the context to be more than just a
  SQL keyword.
 
 Given that the SQL keyword is going to be an English word I can't
 imagine how this could be a big deal for translators. It might not
 match gender or case or something but only if the reader is
 automatically mentally translating the keyword into their language and
 then applying that language's rules to it. At least to me it makes
 sense to refer to VALUES as a singular noun or LIMIT as a generic
 male noun even though limitation would be feminine (I had to look
 that one up though so perhaps I'm not the best person to judge).

Speaking of english words, I was wondering at check the other day.
For example, we have

#: catalog/heap.c:2171
#, c-format
msgid check constraint \%s\ already exists

#: catalog/heap.c:2534
#, c-format
msgid only table \%s\ can be referenced in check constraint

And so on (there are several more).  Note that here we use check
constraint without any capitalization.  However this doesn't translate
too well as is; I mean, if I were to translate check into its
equivalent spanish word, I'm sure to cause a great deal of confusion.
So I've opted for putting the check word, verbatim, in quotes; for
example:

msgid check constraint \%s\ already exists
msgstr la restricción «check» «%s» ya existe

However this is also a bit ugly because I now have two sets of quoted
words -- check itself and then the constraint name.

If we were to have CHECK in uppercase, this would be easy:

msgid check constraint \%s\ already exists
msgstr la restricción CHECK «%s» ya existe

Maybe I should just do that -- uppercase the keyword instead of sticking
it in quotes.


(As for the gender of limit, in spanish, you'd probably think of el
límite which is masculine.  But in general, I agree with you: I think
it makes sense to keep the key word in english in the error message.)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


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


Re: [HACKERS] Statistics and selectivity estimation for ranges

2012-08-14 Thread Heikki Linnakangas

On 14.08.2012 09:45, Alexander Korotkov wrote:

After fixing few more bugs, I've a version with much more reasonable
accuracy.


Great! One little thing just occurred to me:

You're relying on the regular scalar selectivity estimators for the , 
,  and  operators. That seems bogus, in particular for  and , 
because ineq_histogram_selectivity then performs a binary search of the 
histogram using those operators.  and  compare the *upper* bound of 
the value in table against the lower bound of constant, but the 
histogram is constructed using regular  operator, which sorts the 
entries by lower bound. I think the estimates you now get for those 
operators are quite bogus if there is a non-trivial amount of overlap 
between ranges. For example:


postgres=# create table range_test as
select int4range(-a, a) as r from generate_series(1,100) a; analyze 
range_test;

SELECT 100
ANALYZE
postgres=# EXPLAIN ANALYZE SELECT * FROM range_test WHERE r 
int4range(20, 21);
QUERY PLAN 




---
 Seq Scan on range_test  (cost=0.00..17906.00 rows=100 width=14) 
(actual time=0.

060..1340.147 rows=20 loops=1)
   Filter: (r  '[20,21)'::int4range)
   Rows Removed by Filter: 80
 Total runtime: 1371.865 ms
(4 rows)

It would be quite easy to provide reasonable estimates for those 
operators, if we had a separate histogram of upper bounds. I also note 
that the estimation of overlap selectivity could be implemented using 
separate histograms of lower bounds and upper bounds, without requiring 
a histogram of range lengths, because a  b == NOT (a  b OR a  b). 
I'm not sure if the estimates we'd get that way would be better or worse 
than your current method, but I think it would be easier to understand.


I don't think the  and  operators could be implemented in terms of a 
lower and upper bound histogram, though, so you'd still need the current 
length histogram method for that.


The code in that traverses the lower bound and length histograms in 
lockstep looks quite scary. Any ideas on how to simplify that? My first 
thought is that there should be helper function that gets a range length 
as argument, and returns the fraction of tuples with length = argument. 
It would do the lookup in the length histogram to find the right 
histogram bin, and do the linear interpolation within the bin. You're 
assuming that length is independent of lower/upper bound, so you 
shouldn't need any other parameters than range length for that estimation.


You could then loop through only the lower bounds, and call the helper 
function for each bin to get the fraction of ranges long enough in that 
bin, instead dealing with both histograms in the same loop. I think a 
helper function like that might simplify those scary loops 
significantly, but I wasn't sure if there's some more intelligence in 
the way you combine values from the length and lower bound histograms 
that you couldn't do with such a helper function.


--
  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] WIP patch for consolidating misplaced-aggregate checks

2012-08-14 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Speaking of english words, I was wondering at check the other day.
 For example, we have

 #: catalog/heap.c:2171
 #, c-format
 msgid check constraint \%s\ already exists

 #: catalog/heap.c:2534
 #, c-format
 msgid only table \%s\ can be referenced in check constraint

 And so on (there are several more).  Note that here we use check
 constraint without any capitalization.

FWIW, I think I changed check to CHECK in a couple of messages
recently, for exactly the reason that it seemed to be used in its
keyword meaning rather than as plain English text.  Perhaps we
should just go around and do that consistently.

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] 9.2 Cascading replication after slave promotion

2012-08-14 Thread Josh Berkus

 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.

Not trivial, no, but not major either.  Really what needs to happen is
for the timeline change record to get transmitted over the WAL stream.

Hmmm.  You know, I bet I could get stream-only remastering working in an
unsafe way just by disabling the timeline checks.  Time to test ...

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

2012-08-14 Thread Heikki Linnakangas

On 14.08.2012 14:25, Kevin Grittner wrote:

Heikki Linnakangas  wrote:

On 14.08.2012 08:23, Kevin Grittner wrote:



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.


Oh, further testing this morning shows that while *queries* on the HS
seem OK, streaming replication is now broken.  I probably need to
override transaction isolation on the recovery process.  I'll take a
look at that.


Hmm, seems to work for me. Do you get an unexpected error or what?


Now that the error is thrown when the first snapshot is taken,
rather than at the SET command, I think the error message needs
some work:

postgres=# select 123;
ERROR: cannot use serializable mode in a hot standby
HINT: You can use REPEATABLE READ instead.

If the isolation level came from default_transaction_isolation, set
in postgresql.conf file, it might take the user a while to figure
that out. Perhaps something like this:

ERROR: cannot use serializable mode in a hot standby
DETAIL: default_transaction_isolation was set to 'serializable' in
the config file.
HINT: You can use SET transaction_isolation = 'repeatable read'
before the first query in the transaction to override it.


Did you mean?:

HINT: You can use SET default_transaction_isolation = 'repeatable
read' before the first query in the transaction to override it.

Otherwise, I agree and will do.


I didn't, I meant to point out that you can set transaction_isolation 
just for the one transaction. But your suggested hint is OK as well - 
you suggest to set it for the whole session, which will also work around 
the problem. The before the first query in the transaction isn't 
necessary in that case though.


Yet another option is to suggest the BEGIN ISOLATION LEVEL REPEATABLE 
READ syntax, instead of SET.



Since the existing behavior is so bad, I'm inclined to think this
merits backpatching to 9.1. Thoughts on that?


Yes, we have to somehow fix the crash and the assertion failure on
9.1.


Should the check_transaction_read_only() stuff be back-patched to
9.1, too?  So far as we know, that's fragile, not broken, right?
Could the fix you envision there cause a behavioral change that could
break anything that users might have in place?


Good question. I don't see how it could cause a behavioral change, but 
I've been wrong before.


--
  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] TRUE/FALSE vs true/false

2012-08-14 Thread Bruce Momjian
On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
 On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
  2011-08-04 14:32 keltezéssel, Robert Haas írta:
   2011/8/4 Boszormenyi Zoltan z...@cybertec.at:
   Shouldn't these get fixed to be consistent?
   I believe I already did.  See commit 
   85b436f7b1f06a6ffa8d2f29b03d6e440de18784.
  
  I meant a mass sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g' run
  so all the ~200 occurrences of both TRUE and FALSE get
  converted so the whole source tree is consistent.
 
 I would be in favor of that.

I have implemented this with the patch at:

http://momjian.us/expire/true_diff.txt

I did not modify the Win32 code which traditionally uses uppercase for
TRUE/FALSE.

It would be applied only to HEAD.

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

2012-08-14 Thread Noah Misch
On Tue, Aug 14, 2012 at 08:40:06AM -0400, Robert Haas wrote:
 On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark st...@mit.edu wrote:
  It is possible to check if the signal was synchronous or was sent from
  an external process. You can check siginfo-si_pid to see who sent you
  the signal. I'm not sure checking that and handling it at
  check_for_interrupts if it's asynchronous is the best solution or not
  though.
 
 If that's portable it might be an option, but I doubt that it is.

I suspect it is portable.  Nonetheless, kill() is not the only SIGFPE source
that ought to produce a PANIC.  Library code might trigger the signal, at
which point we cannot assume that elog(ERROR) will leave an acceptable system
state.  To call this fixed, we need a whitelist of safe sources, not a
blacklist of bogus sources.

That said, I agree that the effort and risk may be out of proportion.


-- 
Sent 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-14 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:
 On 14.08.2012 14:25, Kevin Grittner wrote:
 Heikki Linnakangas  wrote:
 On 14.08.2012 08:23, Kevin Grittner wrote:
 
 Oh, further testing this morning shows that while *queries* on
 the HS seem OK, streaming replication is now broken.  I probably
 need to override transaction isolation on the recovery process. 
 I'll take a look at that.
 
 Hmm, seems to work for me. Do you get an unexpected error or what?
 
No, I wasn't getting errors in the clients or the logs, but I wasn't
seeing data pop up on the replica when I expected.  Perhaps I messed
up my streaming replication configuration somehow.  Do I understand
that you are now seeing correction of both bugs and no new
misbehaviors in your tests?
 
 Now that the error is thrown when the first snapshot is taken,
 rather than at the SET command, I think the error message needs
 some work:

 postgres=# select 123;
 ERROR: cannot use serializable mode in a hot standby
 HINT: You can use REPEATABLE READ instead.

 If the isolation level came from default_transaction_isolation,
 set in postgresql.conf file, it might take the user a while to
 figure that out. Perhaps something like this:

 ERROR: cannot use serializable mode in a hot standby
 DETAIL: default_transaction_isolation was set to 'serializable'
 in the config file.
 HINT: You can use SET transaction_isolation = 'repeatable
 read' before the first query in the transaction to override it.

 Did you mean?:

 HINT: You can use SET default_transaction_isolation =
 'repeatable read' before the first query in the transaction to
 override it.
 
 I didn't, I meant to point out that you can set
 transaction_isolation just for the one transaction. But your
 suggested hint is OK as well - you suggest to set it for the whole
 session, which will also work around the problem. The before the
 first query in the transaction isn't necessary in that case
 though.
 
Yeah, I figured that out before posting version 2 of the patch.  I
should have said something.
 
 Yet another option is to suggest the BEGIN ISOLATION LEVEL
 REPEATABLE READ syntax, instead of SET.
 
I'm inclined toward hinting at a session override of the default. 
If you're typing away in psql, that's a lot less work.  :-)  That's
what I included in version 2 of the patch, but only if the default
was serializable.  I did say to set it before a transaction
because a quick test showed that setting the default in a
transaction didn't keep that transaction from getting the error if
you proceeded to run a SELECT statement.
 
 Since the existing behavior is so bad, I'm inclined to think
 this merits backpatching to 9.1. Thoughts on that?

 Yes, we have to somehow fix the crash and the assertion failure
 on 9.1.

 Should the check_transaction_read_only() stuff be back-patched to
 9.1, too?  So far as we know, that's fragile, not broken, right?
 Could the fix you envision there cause a behavioral change that
 could break anything that users might have in place?
 
 Good question. I don't see how it could cause a behavioral change,
 but I've been wrong before.
 
If we don't know of any actual existing bugs I'm inclined to not
back-patch that part to 9.1, although it makes sense for 9.2 since
we shouldn't be risking breakage of any production systems.  I'm
really cautious about giving anybody any excuse not to apply a minor
update.
 
-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] TRUE/FALSE vs true/false

2012-08-14 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
 On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
 I meant a mass sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g' run
 so all the ~200 occurrences of both TRUE and FALSE get
 converted so the whole source tree is consistent.

 I would be in favor of that.

 I have implemented this with the patch at:
   http://momjian.us/expire/true_diff.txt

Does this really do anything for us that will justify the extra
back-patching pain it will cause?  I don't see that it's improving
code readability any.

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] TRUE/FALSE vs true/false

2012-08-14 Thread Bruce Momjian
On Tue, Aug 14, 2012 at 05:34:02PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
  On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
  I meant a mass sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g' run
  so all the ~200 occurrences of both TRUE and FALSE get
  converted so the whole source tree is consistent.
 
  I would be in favor of that.
 
  I have implemented this with the patch at:
  http://momjian.us/expire/true_diff.txt
 
 Does this really do anything for us that will justify the extra
 back-patching pain it will cause?  I don't see that it's improving
 code readability any.

I think it is more of a consistency issue.  There were multiple people
who wanted this change.  Of course, some of those people don't backport
stuff.

Other comments?

-- 
  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] GetSnapshotData() comments

2012-08-14 Thread Bruce Momjian

Did these comment updates ever get addressed?

---

On Fri, Aug  5, 2011 at 11:02:24AM -0400, Robert Haas wrote:
 I think that the first sentence, in the following comment within
 GetSnapshotData(), is not quite right, because at the time this is
 executed, we already hold ProcArrayLock, which is the only lock that
 gets grabbed here:
 
 /*
  * If we're in recovery then snapshot data comes from a different 
 place,
  * so decide which route we take before grab the lock. It is
 possible for
  * recovery to end before we finish taking snapshot, and for newly
  * assigned transaction ids to be added to the procarray. Xmax cannot
  * change while we hold ProcArrayLock, so those newly added 
 transaction
  * ids would be filtered away, so we need not be concerned about them.
  */
 snapshot-takenDuringRecovery = RecoveryInProgress();
 
 Having thought it over for a bit, I believe that the code is correct
 and it's only the comment that needs to be updated.  If we were to set
 snapshot-takenDuringRecovery before acquiring ProcArrayLock, then the
 following sequence of events might occur:
 
 T1: Enter GetSnapshotData().  Set snapshot-takenDuringRecovery = true.
 Recovery ends
 T2: Begin, get an XID.
 T3: Begin, get an XID.
 T3: Commit, advancing latestCompletedXid.
 T1: Acquire ProcArrayLock and use the in recovery path, missing T2's
 XID (because it's not in the subxip[] array).
 T1: Do some stuff with the snapshot... not seeing T2's XID...
 T2: Commit
 T1: Do some stuff with the snapshot... now seeing T2's XID...
 
 I think if we just delete the first sentence of the comment, the rest
 is all correct.
 
 A little further down, there is this comment:
 
 /*
  * Spin over procArray checking xid, xmin, and
 subxids.  The goal is
  * to gather all active xids, find the lowest xmin,
 and try to record
  * subxids. During recovery no xids will be assigned,
 so all normal
  * backends can be ignored, nor are there any VACUUMs
 running. All
  * prepared transaction xids are held in
 KnownAssignedXids, so these
  * will be seen without needing to loop through procs here.
  */
 
 ...but this code is only executed when recovery is not in progress.
 So I feel like everything after try to record subxids should either
 be removed, or relocated to the following else block, where the
 recovery path is discussed in detail.
 
 -- 
 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

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

2012-08-14 Thread Tom Lane
Noah Misch n...@leadboat.com writes:
 On Tue, Aug 14, 2012 at 08:40:06AM -0400, Robert Haas wrote:
 On Tue, Aug 14, 2012 at 6:50 AM, Greg Stark st...@mit.edu wrote:
 It is possible to check if the signal was synchronous or was sent from
 an external process. You can check siginfo-si_pid to see who sent you
 the signal. I'm not sure checking that and handling it at
 check_for_interrupts if it's asynchronous is the best solution or not
 though.

 If that's portable it might be an option, but I doubt that it is.

 I suspect it is portable.

Single Unix Spec V2 says (on the sigaction man page)

The si_code member contains a code identifying the cause of the
signal. If the value of si_code is less than or equal to 0, then the
signal was generated by a process and si_pid and si_uid respectively
indicate the process ID and the real user ID of the sender.

I'm not sure I would trust checking si_pid alone; it would definitely
fail on my old HPUX box, where I see that field is union'ed with si_addr
and so will read as garbage for a locally-sourced SIGFPE.  But it might
be that checking si_code alone would work reliably.

I think that rejecting an externally sourced SIGFPE might be worth doing
if we can distinguish that reliably.

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] -Wformat-zero-length

2012-08-14 Thread Peter Eisentraut

On 8/10/12 7:48 PM, Dimitri Fontaine wrote:



Another thing worth considering is to have pg_upgrade init, stop and
start clusters as necessary instead of requesting the user to do it.
I think this is two less steps.


Then you'd need to expose the entire pg_ctl shutdown mode logic through 
pg_upgrade, which might not make things simpler.


What about having single user mode talk fe/be protocol, and talk to it via a 
UNIX pipe, with pg_upgrade starting the single user backend as a subprocess?


I think that's essentially equivalent to starting the server on a 
Unix-domain socket in a private directory.  But that has been rejected 
because it doesn't work on Windows.


The question in my mind is, is there some other usable way on Windows 
for two unrelated processes to communicate over file descriptors in a 
private and secure way?




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


Re: [HACKERS] macports and brew postgresql --universal builds

2012-08-14 Thread Peter Eisentraut

On 8/10/12 7:12 PM, Doug Coleman wrote:

What it looks like is the first line of each section is pattern matching.

If __LP64__ is defined, then it's a 64-bit architecture, and we want
to use the top part of the if statement. The #defines they target seem
to be all of the ones that are different on 32bit platforms.

I agree that you would want to do this in the configure script somehow.


That's not going to work.  The configure script can only test one target 
at a time.  You can probably get away with it on many simple programs, 
but when a configure script checks size and alignment of types, the 
whole concept of universal builds is at odds with the approach Autoconf 
takes.  OK, so you get away with it by patching up a few sites, but 
you're effectively overriding the configure checks with hardcoded results.


I think the only sane approach in general is to build the entire project 
twice and merge the resulting binaries together.



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


Re: [HACKERS] -Wformat-zero-length

2012-08-14 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
 What about having single user mode talk fe/be protocol, and talk to it via a 
 UNIX pipe, with pg_upgrade starting the single user backend as a subprocess?

 I think that's essentially equivalent to starting the server on a 
 Unix-domain socket in a private directory.  But that has been rejected 
 because it doesn't work on Windows.

 The question in my mind is, is there some other usable way on Windows 
 for two unrelated processes to communicate over file descriptors in a 
 private and secure way?

You're making this unnecessarily hard, because there is no need for the
two processes to be unrelated.

The implementation I'm visualizing is that a would-be client (think psql
or pg_dump, though the code would actually be in libpq) forks off a
process that becomes a standalone backend, and then they communicate
over a pair of pipes that were created before forking.  This is
implementable on any platform that supports Postgres, because initdb
already relies on equivalent capabilities.

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] -Wformat-zero-length

2012-08-14 Thread Bruce Momjian
On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
  What about having single user mode talk fe/be protocol, and talk to it via 
  a UNIX pipe, with pg_upgrade starting the single user backend as a 
  subprocess?
 
  I think that's essentially equivalent to starting the server on a 
  Unix-domain socket in a private directory.  But that has been rejected 
  because it doesn't work on Windows.
 
  The question in my mind is, is there some other usable way on Windows 
  for two unrelated processes to communicate over file descriptors in a 
  private and secure way?
 
 You're making this unnecessarily hard, because there is no need for the
 two processes to be unrelated.
 
 The implementation I'm visualizing is that a would-be client (think psql
 or pg_dump, though the code would actually be in libpq) forks off a
 process that becomes a standalone backend, and then they communicate
 over a pair of pipes that were created before forking.  This is
 implementable on any platform that supports Postgres, because initdb
 already relies on equivalent capabilities.

I think the big question is whether we need to modify every binary that
pg_upgrade executes to underestand this pipe communication method.

-- 
  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] superusers are members of all roles?

2012-08-14 Thread Michael Braun
Hi,

I've just recently upgraded to postgrsql 9.1 and also hit bug #5763.
Having +group not match all superusers is essential to be able to assign
different authentication backends to different superusers with needing
to edit configuration files on the radius host system. E.g. to be able
to authenticate some against ldap services and some against the password
stored in the database, so the superusers can opt into the central
authentication system if they want to. With the old postgresql version,
all user managers would only need postgresql tcp access, no access to
files or similar.

Could the different behaviour (superusers matching all/not all group
entries in hba.conf) perhaps become a configuration item?

Regards,
 M. Braun


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


Re: [HACKERS] small issue with host names in hba

2012-08-14 Thread Bruce Momjian

I assume we didn't feel any action was necessary on this issue.

---

On Thu, Aug 11, 2011 at 01:50:02PM -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Tue, Aug 9, 2011 at 2:16 PM, Peter Eisentraut pete...@gmx.net wrote:
  But I'm a little confused by what this code is really trying
  to accomplish: ...
 
  I think the intended behavior of NI_NUMERICHOST is to suppress the
  name lookup, and return the text format *even if* the name lookup
  would have worked.  So the intention of this code may be to ensure
  that we convert the the sockaddr to some sort of string even if we
  can't resolve the hostname - i.e. if the first call fails, try again
  with NI_NUMERICHOST added to make sure that we didn't fail solely due
  to some kind of DNS hiccup.  I suspect that at some time this was
  defending against an actual hazard but I don't know whether it's still
  a problem on modern operating systems.
 
 POSIX v7 says
 
   If the node's name cannot be located, the numeric form of the
   address contained in the socket address structure pointed to by
   the sa argument is returned instead of its name.
 
 If you read a bit further, apparently this is just supposed to be the
 default behavior if neither NI_NUMERICHOST nor NI_NAMEREQD is set (in
 the first case, it doesn't try to locate a node name, and in the second,
 it fails if it can't locate a node name).  But certainly the dance in
 postmaster.c is not necessary if you believe the spec.
 
 I believe that the existing coding is meant to cope with the behavior of
 our stub version of getnameinfo(), which simply fails outright if
 NI_NUMERICHOST isn't set.  However, if we just removed that test and
 behaved as per spec (return a numeric address anyway), then we could
 eliminate the second call in postmaster.c.
 
  The fix would appear to be using the NI_NAMEREQD flag to getnameinfo.
 
  If you want to do that, you're going to have to fix the version of
  getnameinfo() in src/port/getaddrinfo.c, which apparently doesn't
  support that flag.
 
 Well, it's not just that it doesn't support that flag.  It's
 fundamentally incapable of returning anything but a numeric address,
 and therefore the only support it could offer would be to fail.  So
 that approach seems like a dead end.
 
 I don't really think that there's anything to fix here with respect to
 Peter's original concern, but it might be worth getting rid of the
 double call in postmaster.c.
 
   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

-- 
  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] -Wformat-zero-length

2012-08-14 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
 The implementation I'm visualizing is that a would-be client (think psql
 or pg_dump, though the code would actually be in libpq) forks off a
 process that becomes a standalone backend, and then they communicate
 over a pair of pipes that were created before forking.  This is
 implementable on any platform that supports Postgres, because initdb
 already relies on equivalent capabilities.

 I think the big question is whether we need to modify every binary that
 pg_upgrade executes to underestand this pipe communication method.

I think we can fix it once in libpq and we're done.  It'd be driven
by some new connection-string option, and the clients as such would
never need to know that they're not talking to a regular postmaster.

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] superusers are members of all roles?

2012-08-14 Thread Andrew Dunstan


On 08/14/2012 05:03 PM, Michael Braun wrote:

Hi,

I've just recently upgraded to postgrsql 9.1 and also hit bug #5763.
Having +group not match all superusers is essential to be able to assign
different authentication backends to different superusers with needing
to edit configuration files on the radius host system. E.g. to be able
to authenticate some against ldap services and some against the password
stored in the database, so the superusers can opt into the central
authentication system if they want to. With the old postgresql version,
all user managers would only need postgresql tcp access, no access to
files or similar.

Could the different behaviour (superusers matching all/not all group
entries in hba.conf) perhaps become a configuration item?




This is a feature in the upcoming 9.2. IIRC the consensus was not to 
backport it. There is no point in making it a configuration item, 
really, since the workaround for the old behaviour would be to add the 
superusers explicitly to the required groups. If you're interested and 
want to apply it to your own build, it's pretty much a one line patch: 
See 
https://github.com/postgres/postgres/commit/94cd0f1ad8af722a48a30a1087377b52ca99d633


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] TRUE/FALSE vs true/false

2012-08-14 Thread Peter Eisentraut
On Tue, 2012-08-14 at 17:36 -0400, Bruce Momjian wrote:
 On Tue, Aug 14, 2012 at 05:34:02PM -0400, Tom Lane wrote:
  Bruce Momjian br...@momjian.us writes:
   On Thu, Aug  4, 2011 at 09:00:11PM +0300, Peter Eisentraut wrote:
   On tor, 2011-08-04 at 14:44 +0200, Boszormenyi Zoltan wrote:
   I meant a mass sed -e 's/TRUE/true/g'  -e 's/FALSE/false/g' run
   so all the ~200 occurrences of both TRUE and FALSE get
   converted so the whole source tree is consistent.
  
   I would be in favor of that.
  
   I have implemented this with the patch at:
 http://momjian.us/expire/true_diff.txt
  
  Does this really do anything for us that will justify the extra
  back-patching pain it will cause?  I don't see that it's improving
  code readability any.
 
 I think it is more of a consistency issue.  There were multiple people
 who wanted this change.  Of course, some of those people don't backport
 stuff.

I guess you could argue this particular case without end, but I think we
should be open to these kinds of changes.  They will make future code
easier to deal with and confuse new developers less (when to use which,
do they mean different things, etc.).

If back-patching really becomes a problem, we might want to look a
little deeper into git options.  I think the Linux kernel people do
these kinds of cleanups more often, so there is probably some better
support for it.



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


[HACKERS] Re: [HACKERS] PL/Perl build problem: error: ‘OP_SETSTATE’ undeclared

2012-08-14 Thread Alex Hunsaker
On Sun, Aug 12, 2012 at 9:57 PM, Peter Eisentraut pete...@gmx.net wrote:

 It appears that a recent Perl version (I have 5.14.2) has eliminated
 OP_SETSTATE, which causes the current PostgreSQL build to fail:

 plperl.c: In function ‘_PG_init’:
 plperl.c:442:5645: error: ‘OP_SETSTATE’ undeclared (first use in this
 function)
 plperl.c:442:5645: note: each undeclared identifier is reported only once
 for each function it appears in


Hrm, Thats strange, PLPERL_SET_OPMASK() is generated by plperl_opmask.pl--
that should use whatever OP's your current perl has defined (They come from
the use Opcode at the top and unless you somehow installed a different
Opcode module than what your perl has...).

Im running a non threaded 5.16.0 and I don't see OP_SETSTATE anywhere:
$ pwd
/home/alex/src/postgresql/src/pl/plperl
$ grep -RI 'OP_SETSTATE' *
$

I know i've used 5.14 in the past successfully.  Wat happens if you
regenerate plperl_opmask.h? (rm plperl_opmask.h  make)


Re: [HACKERS] -Wformat-zero-length

2012-08-14 Thread Bruce Momjian
On Tue, Aug 14, 2012 at 06:53:49PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  On Tue, Aug 14, 2012 at 05:56:39PM -0400, Tom Lane wrote:
  The implementation I'm visualizing is that a would-be client (think psql
  or pg_dump, though the code would actually be in libpq) forks off a
  process that becomes a standalone backend, and then they communicate
  over a pair of pipes that were created before forking.  This is
  implementable on any platform that supports Postgres, because initdb
  already relies on equivalent capabilities.
 
  I think the big question is whether we need to modify every binary that
  pg_upgrade executes to understand this pipe communication method.
 
 I think we can fix it once in libpq and we're done.  It'd be driven
 by some new connection-string option, and the clients as such would
 never need to know that they're not talking to a regular postmaster.

OK, I like the idea if we can make it work.  I am unclear how we would
pass this connection thing.  Peter's idea of putting the socket in the
current directory is great, except for Windows support.  Maybe we
should just implement this for non-Windows.

FYI, we only use new binaries, so we would only need the support in the
new libpq client libraries.  However, we can't backpatch this into the
old servers, so I am concerned it will not be possible to implement
this if it requires server changes.

-- 
  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] sha1, sha2 functions into core?

2012-08-14 Thread Bruce Momjian

Is there a TODO here?

---

On Wed, Aug 10, 2011 at 09:43:18PM +0300, Peter Eisentraut wrote:
 On ons, 2011-08-10 at 19:29 +0100, Dave Page wrote:
  On Wed, Aug 10, 2011 at 7:06 PM, Peter Eisentraut pete...@gmx.net wrote:
   I would like to see whether there is support for adding sha1 and sha2
   functions into the core.  These are obviously well-known and widely used
   functions, but currently the only way to get them is either through
   pgcrypto or one of the PLs.  We could say that's OK, but then we do
   support md5 in core, which then encourages people to use that, when they
   really shouldn't use that for new applications.
  
  Slightly different, but related - I've seen complaints that we only
  use md5 for password storage/transmission, which is apparently not
  acceptable under some government security standards. In the most
  recent case, they wanted to be able to use sha256 for password storage
  (transmission isn't really an issue where SSL can be used of course).
 
 Yeah, that's one of those things.  These days, using md5 for anything
 raises red flags, so it would be better to slowly move some alternatives
 into place.
 
  If we're ready to move more hashing functions into core, then it seems
  reasonable to add more options for password storage to help those who
  need to meet mandated standards.
 
 Yes, that would be good.
 
 
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers

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

2012-08-14 Thread Kevin Grittner
 Kevin Grittner wrote:
 Heikki Linnakangas wrote:
 On 14.08.2012 14:25, Kevin Grittner wrote:

Attached is version 3.

 Oh, further testing this morning shows that while *queries* on
 the HS seem OK, streaming replication is now broken. I probably
 need to override transaction isolation on the recovery process.
 I'll take a look at that.

 Hmm, seems to work for me. Do you get an unexpected error or what?

 No, I wasn't getting errors in the clients or the logs, but I
 wasn't seeing data pop up on the replica when I expected. Perhaps I
 messed up my streaming replication configuration somehow.

Yeah, setup error. I accidentally dropped the primary_conninfo
setting from my recovery.conf file. Now that I've put that back, it
works just fine.

 I didn't, I meant to point out that you can set
 transaction_isolation just for the one transaction. But your
 suggested hint is OK as well - you suggest to set it for the whole
 session, which will also work around the problem. The before the
 first query in the transaction isn't necessary in that case
 though.

 Yet another option is to suggest the BEGIN ISOLATION LEVEL
 REPEATABLE READ syntax, instead of SET.

 I'm inclined toward hinting at a session override of the default.
 If you're typing away in psql, that's a lot less work. :-)

I tinkered with the messages (including correcting a reverse-logic
bug in which was displayed under what circumstances) and tweaked a
comment. I struggled with how to capitalize and quote. Let me know
what you think.

 Since the existing behavior is so bad, I'm inclined to think
 this merits backpatching to 9.1. Thoughts on that?

 Yes, we have to somehow fix the crash and the assertion failure
 on 9.1.

 Should the check_transaction_read_only() stuff be back-patched to
 9.1, too? So far as we know, that's fragile, not broken, right?
 Could the fix you envision there cause a behavioral change that
 could break anything that users might have in place?

 Good question. I don't see how it could cause a behavioral change,
 but I've been wrong before.

 If we don't know of any actual existing bugs I'm inclined to not
 back-patch that part to 9.1, although it makes sense for 9.2 since
 we shouldn't be risking breakage of any production systems. I'm
 really cautious about giving anybody any excuse not to apply a
 minor update.

How about we fix the serializable versus HS  Windows bugs in one
patch, and then look at the other as a separate patch? If that's OK,
I think this is ready, unless my message text can be improved. (And
I will have a shot at my first back-patching)

-Kevin




hotstandby-serializable-3.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] WIP patch for consolidating misplaced-aggregate checks

2012-08-14 Thread Peter Eisentraut
On Fri, 2012-08-10 at 17:57 +0100, Greg Stark wrote:
 On Thu, Aug 9, 2012 at 5:40 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Fair enough.  I was not sold on doing that either.  I would still like
  to know if it's okay to use one string with %s for the cases where
  there's not a good reason for the context to be more than just a
  SQL keyword.
 
 Given that the SQL keyword is going to be an English word I can't
 imagine how this could be a big deal for translators. It might not
 match gender or case or something but only if the reader is
 automatically mentally translating the keyword into their language and
 then applying that language's rules to it. At least to me it makes
 sense to refer to VALUES as a singular noun or LIMIT as a generic
 male noun even though limitation would be feminine (I had to look
 that one up though so perhaps I'm not the best person to judge).

In some languages the grammatical adjustments do not depend on the
gender but on other aspects of the word, such as the last letter.  So
it's possible to construct cases where this is a problem.

That said, you can usually work around this by translating something
like cannot use aggregates with %s along the lines of cannot use
aggregates in context %s.



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


Re: [HACKERS] WIP patch for consolidating misplaced-aggregate checks

2012-08-14 Thread Peter Eisentraut
On Tue, 2012-08-14 at 12:04 -0400, Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  Speaking of english words, I was wondering at check the other day.
  For example, we have
 
  #: catalog/heap.c:2171
  #, c-format
  msgid check constraint \%s\ already exists
 
  #: catalog/heap.c:2534
  #, c-format
  msgid only table \%s\ can be referenced in check constraint
 
  And so on (there are several more).  Note that here we use check
  constraint without any capitalization.
 
 FWIW, I think I changed check to CHECK in a couple of messages
 recently, for exactly the reason that it seemed to be used in its
 keyword meaning rather than as plain English text.  Perhaps we
 should just go around and do that consistently.

I'm not in favor of that.  Check constraint is a database term that
exists outside of SQL, just like primary key, for instance.  You
wouldn't write the latter in all upper case everywhere, I think.



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


Re: [HACKERS] WIP patch for consolidating misplaced-aggregate checks

2012-08-14 Thread Peter Eisentraut
On Tue, 2012-08-14 at 11:30 -0400, Alvaro Herrera wrote:
 And so on (there are several more).  Note that here we use check
 constraint without any capitalization.  However this doesn't
 translate
 too well as is; I mean, if I were to translate check into its
 equivalent spanish word, I'm sure to cause a great deal of confusion.
 So I've opted for putting the check word, verbatim, in quotes; for
 example:
 
 msgid check constraint \%s\ already exists
 msgstr la restricción «check» «%s» ya existe
 
 However this is also a bit ugly because I now have two sets of quoted
 words -- check itself and then the constraint name. 

I can't really advise you with the language issue, but I think just
writing check without quotes would be fine here.  I also find it useful
on occasion to consult existing (non-PostgreSQL, non-MySQL) books about
databases in the respective language for typical translations.



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


Re: [HACKERS] -Wformat-zero-length

2012-08-14 Thread Peter Eisentraut
On Tue, 2012-08-14 at 17:56 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On 8/10/12 7:48 PM, Dimitri Fontaine wrote:
  What about having single user mode talk fe/be protocol, and talk to it via 
  a UNIX pipe, with pg_upgrade starting the single user backend as a 
  subprocess?
 
  I think that's essentially equivalent to starting the server on a 
  Unix-domain socket in a private directory.  But that has been rejected 
  because it doesn't work on Windows.
 
  The question in my mind is, is there some other usable way on Windows 
  for two unrelated processes to communicate over file descriptors in a 
  private and secure way?
 
 You're making this unnecessarily hard, because there is no need for the
 two processes to be unrelated.
 
 The implementation I'm visualizing is that a would-be client (think psql
 or pg_dump, though the code would actually be in libpq) forks off a
 process that becomes a standalone backend, and then they communicate
 over a pair of pipes that were created before forking.  This is
 implementable on any platform that supports Postgres, because initdb
 already relies on equivalent capabilities.

Well, that would be an interesting feature, but it's debatable which
among this or just adding a new socket type (if available) is harder.



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


Re: [HACKERS] Re: [HACKERS] PL/Perl build problem: error: ‘OP_SETSTATE’ undeclared

2012-08-14 Thread Peter Eisentraut
On Tue, 2012-08-14 at 20:58 -0600, Alex Hunsaker wrote:
 I know i've used 5.14 in the past successfully.  Wat happens if you
 regenerate plperl_opmask.h? (rm plperl_opmask.h  make)

Yeah, it seems to have something to do with a perl upgrade happening
between builds.  It was fixed by building from scratch.



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


Re: [HACKERS] betatesting: ERROR: failed to build any 2-way joins on 9.2

2012-08-14 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 My colleague found a issue on 9.2 - sorry for query formatting - this
 query is generated from ours query engine

Fixed, thanks for the report.

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] [COMMITTERS] pgsql: Revert commit_delay change; just add comment that we don't hav

2012-08-14 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes:
 On 14 August 2012 21:26, Bruce Momjian br...@momjian.us wrote:
 Revert commit_delay change; just add comment that we don't have
 a microsecond specification.

 I think that if we eventually decide to change the name of
 commit_delay for 9.3 (you previously suggested that that might be
 revisited), it will be reasonable to have the new GUC in units of
 milliseconds.

Well, the reason why it's like that at all is the thought that values
of less than 1 millisecond might be useful.  Are we prepared to suppose
that that is not and never will be true?

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] betatesting: ERROR: failed to build any 2-way joins on 9.2

2012-08-14 Thread Pavel Stehule
2012/8/15 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 My colleague found a issue on 9.2 - sorry for query formatting - this
 query is generated from ours query engine

 Fixed, thanks for the report.

thank you

Pavel

 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