[HACKERS] patch-3 (3-allow-wal-record-header-to-be-split.patch)WAL Format Changes
While reading patch-3 (3-allow-wal-record-header-to-be-split.patch) of
WAL Format
Changes(http://archives.postgresql.org/message-id/4fda5136.6080...@enterprisedb.com),
I had few observations which are summarized below:
1.
ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
+ /*
+ * If we got the whole header already, validate it immediately. Otherwise
+ * we validate it after reading the rest of the header from the next page.
+ */
+ if (targetRecOff = XLOG_BLCKSZ - SizeOfXLogRecord)
+ {
+ if (!ValidXLogRecordHeader(RecPtr, record, emode, randAccess))
+ goto next_record_is_invalid;
+ gotheader = true;
+ }
+ else
+ gotheader = false;
+
Shouldn't the record header validation be done before the check for allocating
a bigger record buffer based
on total length. Otherwise it may lead to allocation of bigger buffer which may
not be required if record header is invalid.
In cases where record header is not split, validation can be done before
otherwise it can be done later.
3. General observation, not related to your changes
XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata)
.
.
if (freespace == 0)
{
updrqst = AdvanceXLInsertBuffer(false);
In the code, AdvanceXLInsertBuffer(false); is called after starting critical
section and acquiring
WALInsertLock, now if any error occurs inside AdvanceXLInsertBuffer()
(in one of the paths it calls XLogWrite(), from which it can call
XLogFileInit() where error can occur);
how will it release WALInsertLock or end critical section.
With Regards,
Amit Kapila.
Re: [HACKERS] Pruning the TODO list
Hi, On 06/22/2012 05:38 PM, Andrew Dunstan wrote: I think the real problem with the TODO list is that some people see it as some sort of official roadmap, and it really isn't. Neither is there anything else that is. To me, it looks like TODO is just a misnomer. The file should be named TODISCUSS, IDEAS, or something. But the current file name implies consensus. Wouldn't renaming solve that kind of misunderstanding? (..in the vain of address(ing) real problems in the simplest way..) Regards Markus Wanner -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] XX000: enum value 117721 not found in cache for enum enumcrash
Hi,
Currently its possible to cause transactions to fail with ALTER ENUM ADD
AFTER/BEFORE:
psql 1:
CREATE TYPE enumcrash AS ENUM('a', 'b');
CREATE FUNCTION randenum() RETURNS enumcrash LANGUAGE sql AS $$SELECT * FROM
unnest(enum_range('a'::enumcrash)) ORDER BY random() LIMIT 1$$;
CREATE TABLE enumcrash_table(id serial primary key, val enumcrash);
CREATE INDEX enumcrash_table__val__id ON enumcrash_table (val, id);
INSERT INTO enumcrash_table (val) SELECT randenum() FROM generate_series(1,
1);INSERT 0 1
psql 2:
BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
INSERT INTO enumcrash_table (val) SELECT randenum() FROM generate_series(1,
1);
psql 1:
ALTER TYPE enumcrash ADD VALUE 'a_1' AFTER 'a';
INSERT INTO enumcrash_table (val) SELECT randenum() FROM generate_series(1,
1);
psql 2:
INSERT INTO enumcrash_table (val) SELECT randenum() FROM generate_series(1,
1);
ERROR: XX000: enum value 117745 not found in cache for enum enumcrash
LOCATION: compare_values_of_enum, typcache.c:957
This is not surprising. psql 2's backend finds rows in the index with enum
values that are not visible in its mvcc snapshot. That mvcc snapshot is needed
because a_1 is an enum value with an uneven numbered oid because its inserted
after the initial creation.
Do we consider that something that needs to be fixed or just something to
document? I can't think of a non-intrusive fix right now.
Greetings,
Andres
--
Andres Freund http://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] Covering Indexes
On 28 June 2012 14:02, Rob Wultsch wult...@gmail.com wrote: On Thu, Jun 28, 2012 at 8:16 AM, David E. Wheeler da...@justatheory.com wrote: I'm particularly intrigued by covering indexes. For example: CREATE INDEX cover1 ON table1(a,b) COVERING(c,d); IRC MS SQL also allow unindexed columns in the index. For what it's worth, DB2 also has this feature, written roughly the same way as MS SQL Server: CREATE INDEX cover1 ON table1(a, b) INCLUDE (c, d). http://pic.dhe.ibm.com/infocenter/db2luw/v9r7/index.jsp?topic=/com.ibm.db2.luw.sql.ref.doc/doc/r919.html Oracle doesn't seem to have this feature (and the SQL standard doesn't mention indexes at all). -- 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] elog/ereport noreturn decoration
On fre, 2012-06-29 at 17:35 -0400, Tom Lane wrote: Yes. The problem with trying to change that is that it's damned if you do and damned if you don't: compilers that are aware that abort() doesn't return will complain about unreachable code if we keep those extra variable initializations, while those that are not so aware will complain about uninitialized variables if we don't. But my point was, there aren't any unused code warnings. None of the commonly used compilers issue any. I'd be interested to know if there is any recent C compiler supported by PostgreSQL that issues some kind of unused code warning under any circumstances, and see an example of that. Then we can determine whether there is an issue here. -- 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] Pruning the TODO list
Le samedi 30 juin 2012 11:39:09, Markus Wanner a écrit : Hi, On 06/22/2012 05:38 PM, Andrew Dunstan wrote: I think the real problem with the TODO list is that some people see it as some sort of official roadmap, and it really isn't. Neither is there anything else that is. To me, it looks like TODO is just a misnomer. The file should be named TODISCUSS, IDEAS, or something. But the current file name implies consensus. Wouldn't renaming solve that kind of misunderstanding? (..in the vain of address(ing) real problems in the simplest way..) +1 -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
[HACKERS] Rewriting existing table tuples on alter type
How hard would it be to rewrite table content on composite attribute type change? For simple use cases: create type complex as (i int, j int); create table numbers (c complex); insert into numbers values(row(1,2)); I can work around alter complex from int to bigint fairly easy with alter type complex add attribute _tmp_i bigint; --loop for each table update numbers set c._tmp_i = (c).i; --end loop alter type complex drop attribute i; alter type complex rename attribute _tmp_i to i; I can easily work with deep nested types, but things starts to get PITA when there is array involved because I need to unroll it and pack it again. Regards, Rikard -- 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] Can someone help me to get ODBC fdw running on windows?
Em 29/06/2012 20:36, Edson Richter escreveu: I've tried to compile ODBC fdw on Win64 with all sort of compilers without success (MingGW, gcc-win32, MS C++2005 32 and 64). I think I'm getting too old for this so many switches, too many dependencies. Could a gently soul help me get back on track, possible providing precompiled binaries that I can use? Regards, Edson Ok, found some advice here http://www.postgresonline.com/journal/archives/246-ODBC-Foreign-Data-wrapper-odbc_fdw-on-windows.html and here http://brunosimioni.wordpress.com/2011/09/29/sqlmed-com-odbc_fdw-no-postgresql-9-1-1/ Regards, Edson Richter -- 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] elog/ereport noreturn decoration
Peter Eisentraut pete...@gmx.net writes: But my point was, there aren't any unused code warnings. None of the commonly used compilers issue any. I'd be interested to know if there is any recent C compiler supported by PostgreSQL that issues some kind of unused code warning under any circumstances, and see an example of that. Then we can determine whether there is an issue here. Well, the Solaris Studio compiler produces warning: statement not reached messages, as seen for example on buildfarm member castoroides. I don't have one available to experiment with, so I'm not sure whether it knows that abort() doesn't return; but I think it rather foolish to assume that such a combination doesn't exist in the wild. 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] Pruning the TODO list
Markus Wanner mar...@bluegap.ch writes: To me, it looks like TODO is just a misnomer. The file should be named TODISCUSS, IDEAS, or something. But the current file name implies consensus. Wouldn't renaming solve that kind of misunderstanding? I think there are enough references to the TODO list in our archives and elsewhere that we can't just go and rename it. Also, it's not the case that *all* the stuff there lacks consensus. It'd be better to put a disclaimer at the front pointing out that some of these items are unfinished because of lack of consensus, not just lack of code. 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] Rewriting existing table tuples on alter type
On Sat, Jun 30, 2012 at 02:59:07PM +0200, Rikard Pavelic wrote: How hard would it be to rewrite table content on composite attribute type change? I wouldn't anticipate especially-thorny challenges. -- 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: patch-3 (3-allow-wal-record-header-to-be-split.patch)WAL Format Changes
On 30.06.2012 10:11, Amit kapila wrote:
ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
+ /*
+ * If we got the whole header already, validate it immediately. Otherwise
+ * we validate it after reading the rest of the header from the next page.
+ */
+ if (targetRecOff= XLOG_BLCKSZ - SizeOfXLogRecord)
+ {
+ if (!ValidXLogRecordHeader(RecPtr, record, emode, randAccess))
+ goto next_record_is_invalid;
+ gotheader = true;
+ }
+ else
+ gotheader = false;
+
Shouldn't the record header validation be done before the check for allocating
a bigger record buffer based
on total length. Otherwise it may lead to allocation of bigger buffer which may
not be required if record header is invalid.
Hmm, doing an unnecessary memory allocation just before giving up isn't
really a problem. And we treat out-of-memory the same as a corrupt
record, so this isn't a correctness issue. But I agree it would still be
better to change the order, if only because you're more likely to get a
better error message than out of memory.
In cases where record header is not split, validation can be done before
otherwise it can be done later.
Committed that way. We could also delay enlarging the buffer until after
we read the next page and get the whole header, but it's probably fine
as it is.
3. General observation, not related to your changes
XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata)
.
.
if (freespace == 0)
{
updrqst = AdvanceXLInsertBuffer(false);
In the code, AdvanceXLInsertBuffer(false); is called after starting critical
section and acquiring
WALInsertLock, now if any error occurs inside AdvanceXLInsertBuffer()
(in one of the paths it calls XLogWrite(), from which it can call
XLogFileInit() where error can occur);
how will it release WALInsertLock or end critical section.
Yep, if an I/O error happens while writing a WAL record - running out of
disk space is the typical example - we PANIC. Nope, it's not ideal.
Even if we somehow managed to avoid doing I/O in the critical section in
XLogInsert(), most callers of XLogInsert() surround the call in a
critical section anyway. For example, when a new tuple is inserted to
heap, once you've modified the page to add the new tuple, it's too late
to back out. The WAL record is written while holding the lock on the
page, and if something goes wrong with writing the WAL record, we have
no choice but PANIC.
It would be nice to avoid that, at least for the common
out-of-disk-space case. Perhaps we could somehow pre-reserve some WAL
space before starting to modify the page. But that's a pretty big project..
--
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] Pruning the TODO list
On lör, 2012-06-30 at 11:08 -0400, Tom Lane wrote: It'd be better to put a disclaimer at the front pointing out that some of these items are unfinished because of lack of consensus, not just lack of code. There is a fairly extensive disclaimer at the top of the wiki page. Maybe it was added recently, though. -- 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] Support for array_remove and array_replace functions
On 30/06/2012 04:16, Alex Hunsaker wrote: Hi, I've been reviewing this patch. Good documentation, and regression tests. The code looked fine but I didn't care for the code duplication between array_replace and array_remove so I merged those into a helper function, array_replace_internal(). Thoughts? It looks reasonable. There was a typo in array_replace which was caught by regression tests. I've fixed the typo and changed a comment in array_replace_internal. Patch v3 attached. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it array-functions-v3.patch.bz2 Description: application/bzip -- 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: relation metapages
Hello, I tried to perform a submission review of your relation metapages patch, but it does not apply cleanly to the current master (fa188b5). I've attached the rejects file for your review. Regards, Albert gist.c.rej Description: application/reject -- 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: relation metapages
Hello, I tried to perform a submission review of your relation metapages patch, but it does not apply cleanly to the current master (fa188b5). I've attached the rejects file for your review. Regards, Albert gist.c.rej Description: application/reject -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
