Re: [HACKERS] Statistics and selectivity estimation for ranges
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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?
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
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
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
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?
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
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
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
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
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
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
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
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
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/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
