Re: [HACKERS] New WAL code dumps core trivially on replay of bad data
On 18.08.2012 08:52, Amit kapila wrote: Tom Lane Sent: Saturday, August 18, 2012 7:16 AM so it merrily tries to compute a checksum on a gigabyte worth of data, and soon falls off the end of memory. In reality, inspection of the WAL file suggests that this is the end of valid data and what should have happened is that replay just stopped. The xl_len and so forth shown above are just garbage from off the end of what was actually read from the file (everything beyond offset 0xcebff8 in file 4 is in fact zeroes). I'm not sure whether this is just a matter of having failed to sanity-check that xl_tot_len is at least SizeOfXLogRecord, or whether there is a deeper problem with the new design of continuation records that makes it impossible to validate records safely. Earlier there was a check related to total length in ReadRecord, before it calls RecordIsValid() if (record->xl_tot_len< SizeOfXLogRecord + record->xl_len || record->xl_tot_len> SizeOfXLogRecord + record->xl_len + XLR_MAX_BKP_BLOCKS * (sizeof(BkpBlock) + BLCKSZ)) I think that missing check of total length has caused this problem. However now this check will be different. That check still exists, in ValidXLogRecordHeader(). However, we now allocate the buffer for the whole record before that check, based on xl_tot_len, if the record header is split across pages. The theory in allocating the buffer is that a bogus xl_tot_len field will cause the malloc() to fail, returning NULL, and we treat that the same as a broken header. However, with memory overcommit, what happens is that the malloc() succeeds, but the process is killed when it actually tries to use all that memory. I think we need to delay the allocation of the record buffer. We need to read and validate the whole record header first, like we did before, before we trust xl_tot_len enough to call malloc() with it. I'll take a shot at doing that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Docs: Make notes on sequences and rollback more obvious
On 08/19/2012 03:01 AM, Jeff Janes wrote: >Or would you instead say that >"changes made to a sequence are immediately visible to all other >transactions" ? Yes, that sounds better. OK, how about the attached series, then? The 2nd probably needs improvement - and I expect I've missed some other areas that aren't strictly transactional. Comments? Working branch: https://github.com/ringerc/postgres/tree/sequence_documentation_fixes -- Craig Ringer >From 311ea8a493c939a70a89234246f662514687d8c2 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Sat, 18 Aug 2012 12:51:28 +0800 Subject: [PATCH 1/3] Make sure you can't read through mvcc.sgml without realising that not everything is MVCC. --- doc/src/sgml/mvcc.sgml | 12 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index 8f88582..9dc65f5 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *** *** 260,265 --- 260,277 command . + + +Some PostgreSQL data types and functions have +special rules regarding transactional behaviour. In particular, changes +made to a SEQUENCE (and therefore the counter of a +SERIAL) are immediately visible to all other +transactions and are not rolled back if the transaction that made the +changes aborts. See and +. + + + Read Committed Isolation Level -- 1.7.11.2 >From 69c892ed85070de8df41f2b00088f503453b2e97 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Mon, 20 Aug 2012 11:55:25 +0800 Subject: [PATCH 2/3] Collect a list of features with abberant transactional behavour --- doc/src/sgml/mvcc.sgml | 67 ++ 1 file changed, 67 insertions(+) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index 9dc65f5..e2930c9 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *** SELECT pg_advisory_lock(q.id) FROM *** 1540,1543 --- 1540,1610 indexes should be used instead. + + +Exceptions to normal transactional rules + + + Some PostgreSQL features, functions and data types differ from the + usual transactional behaviour described in this chapter. Differences + are generally mentioned in the documentation sections for the + features they affect. Such exceptions are collected here for + easy reference. + + + + The following actions and features don't follow the typical + transactional rules: + + + + + + Serial pseudo-types + + + + + SEQUENCEs - + + + + + Advisory locks - + + + + + Disk writes to files outside the database, as performed by + COPY ... TO, adminpack functions, and other add-ons. + See , . + + + + + Any network I/O or inter-process communication not explicitly + described as transactional in its documentation. For example, + sending an email from PL/PerlU would not be transactional; + the email would be sent before the transaction commits and + could not be un-sent if the transaction were to roll back. + + + + + + When working with external non-transactional resources like files + on disk or network sockets the two-phase commit feature can be + useful. See: + + + LISTEN/NOTIFY provides a lighter weight but still transaction-friendly method of + triggering changes outside the database in response to changes inside the + database. A LISTENing helper program running outside the database can + perform actions when it gets a NOTIFY after a transaction commits. See: + . + + + + + -- 1.7.11.2 >From 35b2aec7f6816c7ecd7bb1f8e2ecb2439043d982 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Mon, 20 Aug 2012 12:02:53 +0800 Subject: [PATCH 3/3] Change xref of note re SERIAL to point to mvcc-exceptions --- doc/src/sgml/mvcc.sgml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml new file mode 100644 index e2930c9..0de4b75 *** a/doc/src/sgml/mvcc.sgml --- b/doc/src/sgml/mvcc.sgml *** *** 267,274 made to a SEQUENCE (and therefore the counter of a SERIAL) are immediately visible to all other transactions and are not rolled back if the transaction that made the !changes aborts. See and !. --- 267,273 made to a SEQUENCE (and therefore the counter of a SERIAL) are immediately visible to all other transactions and are not rolled back if the transaction that made the !changes aborts. See . -- 1.7.11.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make
Re: [HACKERS] temporal support patch
> > If there is some syntax that offers a convenient shorthand for WHERE, > that's fine with me. Or using two tables, one called foo and one called > foo_history, is also fine. But I don't want the DML syntax to introduce > new mechanisms that aren't available without the fancy syntax (though > new DDL arrangements might be fine). > convention foo_history is used often - and it should be significant break for migration Regards Pavel Stehule > Regards, > Jeff Davis > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] gistchoose vs. bloat
On Mon, 2012-06-18 at 15:12 +0400, Alexander Korotkov wrote:
> Hackers,
>
>
> While experimenting with gistchoose I achieve interesting results
> about relation of gistchoose behaviour and gist index bloat.
...
>
> Current implementation of gistchoose select first index tuple which
> have minimal penalty. It is possible for several tuples to have same
> minimal penalty. I've created simple patch which selects random from
> them. I then I've following results for same testcase.
>
I took a look at this patch. The surrounding code is pretty messy (not
necessarily because of your patch). A few comments would go a long way.
The 'which_grow' array is initialized as it goes, first using pointer
notations ("*which_grows = -1.0") and then using subscript notation. As
far as I can tell, the first r->rd_att->natts of the array (the only
elements that matter) need to be written the first time through anyway.
Why not just replace "which_grow[j] < 0" with "i == FirstOffsetNumber"
and add a comment that we're initializing the penalties with the first
index tuple?
The 'sum_grow' didn't make any sense, thank you for getting rid of that.
Also, we should document that the earlier attributes always take
precedence, which is why we break out of the inner loop as soon as we
encounter an attribute with a higher penalty.
Please add a comment indicating why you are randomly choosing among the
equal penalties.
I think that there might be a problem with the logic, as well. Let's say
you have two attributes and there are two index tuples, it1 and it2;
with penalties [10,10] and [10,100] respectively. The second time
through the outer loop, with i = 2, you might (P=0.5) assign 2 to the
'which' variable in the first iteration of the inner loop, before it
realizes that it2 actually has a higher penalty. I think you need to
finish out the inner loop and have a flag that indicates that all
attributes are equal before you do the probabilistic replacement.
Also, I think you should use random() rather than rand().
Regards,
Jeff Davis
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] temporal support patch
On Mon, 2012-06-25 at 17:46 +0900, Vlad Arkhipov wrote: > It's not sufficient to store only a period of validity for a row. If two > transactions started in the same time change the same record, you have a > problem with TSTZRANGE type because it's normalized to empty interval. That's an interesting point. Let's say you tried setting it to [T1, T2) where T1 is the time of the last transaction to update it and T2 is the time of the current transaction. If T2 <= T1, then TSTZRANGE will throw an error, not store the empty interval. And we don't want to store the empty interval, because it would be a lie. There could have been some transaction T3 that happened during T2 that saw the value from T1, so saying that there were no times where that was visible to the system is false. Throwing an error allows you to retry T2, which should allow a microsecond or so to pass, and the problem should resolve itself (assuming your clock didn't move backwards, which is a different issue). We could also argue about the start versus end times of transactions, and snapshot acquisition times, because that could cause confusion if there are long-running transactions. It might be a good reason to store the modifying transaction ID as well, but then you get into transaction wraparound problems. > The other issue is how to handle multiple changes of the same record > within the transaction. Should they be stored or not? In a typical audit log, I don't see any reason to. The internals of a transaction should be implementation details; invisible to the outside, right? > Also it's necessary to store some kind of operation type that was > applied to the record (insert/update/delete). For example, there is a > table with one record with validity period [0, ) and value 'A'. > > First way > 1. Delete this record in time 1, now there is [0, 1), A in the history > table. > 2. Insert a new record in time 1, now there is [0, 1), A in the history > table and [1, ), B record in the current data table. > > Second way > 1. Update this record in time 1, now there is [0, 1), A in the history > table and [1, ), B record in the current data table. > > So you have the same data in the tables but the actions that led to this > configuration were different and the history has been lost partly. Right. Those are yet more possible options that people might want for an audit log. > > * There is other useful information that could be recorded, such as the > > user who inserted/updated/deleted the record. > I'm not sure that the database user is the proper thing to be stored in > the history table. Many applications usually connect to a database using > some virtual user and have their own users/roles tables to handle with > privileges. There should be some way to substitute the stored user in > the history table with the application's one. It's also helpful to store > transaction id that inserted/updated/deleted the record. If the system is recording it for audit purposes, then it better be sure that it's true. You can't allow the application to pick and choose what gets stored there. While it may be true that many applications just all use the same DB user, if you want an audit log that includes user information you have to let the DB do some authentication. > It's a great proposal but seems to be impossible to implement with > triggers only solution, isn't it? Is there any kind of hooks on ALTER > TABLE ... in PostgreSQL to update changed columns bitmaps when table > structure changes? Column numbers are never reused, so I think it would be stable. But if you do need to be notified of schema changes, the new event triggers mechanism may be able to do that. > In SQL2011 there is only one table with the all data, historical and > current. So it's not very convenient to specifiy WHERE condition on > system time everywhere and for all tables in the query. By default only > the current data is selected with a query like SELECT * FROM table. If there is some syntax that offers a convenient shorthand for WHERE, that's fine with me. Or using two tables, one called foo and one called foo_history, is also fine. But I don't want the DML syntax to introduce new mechanisms that aren't available without the fancy syntax (though new DDL arrangements might be fine). Regards, Jeff Davis -- Sent 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 Thu, Aug 16, 2012 at 4:40 PM, Heikki Linnakangas < heikki.linnakan...@enterprisedb.com> wrote: > On 15.08.2012 11:34, Alexander Korotkov wrote: > >> Ok, we've to decide if we need "standard" histogram. In some cases it can >> be used for more accurate estimation of< and> operators. >> But I think it is not so important. So, we can replace "standard" >> histogram >> with histograms of lower and upper bounds? >> > > Yeah, I think that makes more sense. The lower bound histogram is still > useful for < and > operators, just not as accurate if there are lots of > values with the same lower bound but different upper bound. New version of patch. * Collect new stakind STATISTIC_KIND_BOUNDS_HISTOGRAM, which is lower and upper bounds histograms combined into single ranges array, instead of STATISTIC_KIND_HISTOGRAM. * Selectivity estimations for >, >=, <, <=, <<, >>, &<, &> using this histogram. -- With best regards, Alexander Korotkov. range_stat-0.7.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
Re: [HACKERS] "CLUSTER VERBOSE" tab completion
On Fri, Aug 17, 2012 at 7:18 PM, Jeff Janes wrote: > tab completion will add "USING" after CLUSTER VERBOSE, as if VERBOSE > were the name of a table. > > Instead of just making it not do the wrong thing, I tried to make it > actually do the right thing. > > It doesn't fill in the VERBOSE for you, you have to type that in full, This short coming has now been rectified. Cheers, Jeff cluster_verbose_complete_v2.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] SP-GiST for ranges based on 2d-mapping and quad-tree
On Sat, 2012-07-28 at 17:50 -0400, Tom Lane wrote: > which would come > back to bite us if we ever try to support index-only scans with SPGiST. I'm confused: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=92203624934095163f8b57b5b3d7bbd2645da2c8 And the patch that was just committed for Range Types SP-GiST is already using index-only scans. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Tab completion for DROP CONSTRAINT
Interactively dropping primary key constraints has been annoying me. I believe this patch fixes that, hopefully for other kinds of cataloged constraints as well. I believe this finishes, at least for a while, my tab-completion related gripes. Cheers, Jeff drop_constraint_complete_v1.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
[HACKERS] Unexpected plperl difference between 8.4 and 9.1
After upgrading from 8.4 to 9.1, one of my plperl functions stopped working
properly.
For some reason, when matching a string using a regex, the $1 variable
cannot be returned directly using return_next() but must be
set to a variable first.
If returned directly, it appears to be cached in some strange way,
returning the same value for all 10 rows in the example below.
In 8.4, these two functions returns the same thing, 10 rows of random
numbers.
Is this a feature or a bug?
CREATE OR REPLACE FUNCTION test1() RETURNS SETOF NUMERIC AS $BODY$
use strict;
use warnings;
for(my $i=0 ; $i<10; $i++) {
my $rand = rand();
$rand =~ m/(.*)/;
return_next($1);
}
return;
$BODY$ LANGUAGE plperl;
joel=# select * from test1();
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
0.482287904847535
(10 rows)
CREATE OR REPLACE FUNCTION test2() RETURNS SETOF NUMERIC AS $BODY$
use strict;
use warnings;
for(my $i=0 ; $i<10; $i++) {
my $rand = rand();
$rand =~ m/(.*)/;
my $val = $1;
return_next($val);
}
return;
$BODY$ LANGUAGE plperl;
joel=# select * from test2();
test2
0.504361211998972
0.015757483449562
0.154422531777254
0.383329383899088
0.578318997407354
0.0022126436077059
0.970868502733449
0.465566753133679
0.215372148522395
0.390036490131536
(10 rows)
Re: [HACKERS] SP-GiST for ranges based on 2d-mapping and quad-tree
On Sat, 2012-08-18 at 18:10 +0400, Alexander Korotkov wrote: > On Thu, Aug 16, 2012 at 3:46 PM, Heikki Linnakangas > wrote: > I committed the patch now, but left out the support for > adjacent for now. Not because there was necessarily anything > wrong with that, but because I have limited time for > reviewing, and the rest of the patch looks ready for commit > now. I reworded the comments quite a lot, you might want to > proofread those to double-check that they're still correct. > I'll take a look at the adjacent-support next, as a separate > patch. > > > Thanks! There is a separate patch for adjacent. I've reworked adjacent > check in order to make it more clear. I am taking a look at this patch now. A few quick comments: * It looks like bounds_adjacent modifies it's by-reference arguments, which is a little worrying to me. The lower/upper labels are flipped back, but the inclusivities are not. Maybe just pass by value instead? * Bounds_adjacent is sensitive to the argument order. Can't it just take bound1 and bound2? * I tried some larger tests and they seemed to work. I haven't reviewed the spgist code changes in detail though. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Rules and WITH and LATERAL
Six years ago, we punted on allowing rules to use OLD and NEW in
multi-row VALUES constructs, because we didn't have LATERAL:
http://archives.postgresql.org/pgsql-hackers/2006-08/msg00044.php
I thought maybe that restriction could be fixed now that we do have
LATERAL, and indeed the attached quick-and-dirty POC seems to make it
work. Barring objection I'll clean this up and apply it.
While poking at this, though, I noticed that the workaround proposed
in the aforementioned thread does not actually work, and AFAICT never
has:
regression=# create rule r2 as on update to src do also insert into log select
old.*, 'old' union all select new.*, 'new';
ERROR: 42P10: UNION/INTERSECT/EXCEPT member statement cannot refer to other
relations of same query level
LINE 1: ...s on update to src do also insert into log select old.*, 'ol...
^
LOCATION: transformSetOperationTree, analyze.c:1629
I tried hacking transformSetOperationTree in the same fashion, to set
the subquery RTE's lateral flag instead of throwing an error. That
just moved the problem though:
regression=# create rule r2 as on update to src do also insert into log select
old.*, 'old' union all select new.*, 'new';
ERROR: 0A000: conditional UNION/INTERSECT/EXCEPT statements are not implemented
LOCATION: transformRuleStmt, parse_utilcmd.c:2255
transformRuleStmt's problem seems much more fundamental: it has noplace
to inject the extra jointree entry needed for the relation the rule is
attached to. So fixing that looks like a dead end.
While thinking about this I wondered whether it might be possible to
clean up the implementation of rules, and perhaps also get rid of some
of their semantic issues, by making the rule rewriter rely on WITH
and/or LATERAL, neither of which we had back in the dark ages when the
current rules implementation was built. In particular, WITH might offer
a fix for the multiple-evaluation gotchas that people so often trip
over. For instance, perhaps an UPDATE with rules could be rewritten
into something like
WITH data_src AS (
SELECT ctid, all-old-values, all-new-values FROM target_rel FOR UPDATE
),
rule_1 AS (
... rule body here ...
),
rule_2 AS (
... rule body here ...
)
UPDATE target_rel SET col1 = newval1, col2 = newval2, ...
FROM data_src WHERE ctid = data_src.ctid;
Rewriting the rule rewriter would be a fairly sizable project of course,
and it's not one I have much interest in tackling personally. I'm just
throwing it out there as a possible TODO.
regards, tom lane
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6c3d89a14f6b1f19176864af4a0ea18eebd9f4bd..4db57a704a46a4737f951e5d8543fcc21676472c 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
*** transformInsertStmt(ParseState *pstate,
*** 652,657
--- 652,658
* to ensure that the values would be available while evaluating the
* VALUES RTE. This is a shame. FIXME
*/
+ #if 0
if (list_length(pstate->p_rtable) != 1 &&
contain_vars_of_level((Node *) exprsLists, 0))
ereport(ERROR,
*** transformInsertStmt(ParseState *pstate,
*** 660,665
--- 661,667
errhint("Use SELECT ... UNION ALL ... instead."),
parser_errposition(pstate,
locate_var_of_level((Node *) exprsLists, 0;
+ #endif
/*
* Generate the VALUES RTE
*** transformInsertStmt(ParseState *pstate,
*** 672,677
--- 674,683
Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));
pstate->p_joinlist = lappend(pstate->p_joinlist, rtr);
+ if (list_length(pstate->p_rtable) != 1 &&
+ contain_vars_of_level((Node *) exprsLists, 0))
+ rte->lateral = true;
+
/*
* Generate list of Vars referencing the RTE
*/
*** transformValuesClause(ParseState *pstate
*** 1222,1233
--- 1228,1242
*/
if (list_length(pstate->p_rtable) != 1 &&
contain_vars_of_level((Node *) exprsLists, 0))
+ rte->lateral = true;
+ #if 0
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VALUES must not contain OLD or NEW references"),
errhint("Use SELECT ... UNION ALL ... instead."),
parser_errposition(pstate,
locate_var_of_level((Node *) exprsLists, 0;
+ #endif
qry->rtable = pstate->p_rtable;
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
--
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: psql boolean display
Hello all, I am providing a patch to allow you to change the output of a boolean value in psql much like you can do with NULL. A client requested this feature and we thought it may appeal to someone else in the community. The patch includes updated docs and a regression test. The code changes themselves are pretty simple and straightforward. Example from the regression test: SELECT true, false; bool | bool --+-- t| f (1 row) \pset booltrue 'foo' \pset boolfalse 'bar' SELECT true, false; bool | bool --+-- foo | bar (1 row) \pset booltrue 't' \pset boolfalse 'f' SELECT true, false; bool | bool --+-- t| f (1 row) As always, comments welcome. Thanks. psql-bool-display.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
