Re: [HACKERS] Replication server timeout patch
On 31.03.2011 05:46, Fujii Masao wrote: On Wed, Mar 30, 2011 at 10:54 PM, Robert Haasrobertmh...@gmail.com wrote: On Wed, Mar 30, 2011 at 4:08 AM, Fujii Masaomasao.fu...@gmail.com wrote: On Wed, Mar 30, 2011 at 5:03 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 30.03.2011 10:58, Fujii Masao wrote: On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.comwrote: +A value of zero means wait forever. This parameter can only be set in The first sentence sounds misleading. Even if you set the parameter to zero, replication connections can be terminated because of keepalive or socket error. Hmm, should I change it back to A value of zero disables the timeout ? Any better suggestions? I like that. But I appreciate if anyone suggests the better. Maybe sticking the word mechanism in there would be a bit better. A value of zero disables the timeout mechanism? I'm OK with that. Or, what about A value of zero turns this off which is used in statement_timeout for the sake of consistency? Committed Robert's suggestion. -- 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] crash-safe visibility map, take four
On Wed, Mar 30, 2011 at 8:52 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 30.03.2011 06:24, 高增琦 wrote: Should we do full-page write for visibilitymap all the time? Now, when clear visiblitymap, there is no full-page write for vm since we don't save buffer info in insert/update/delete's log. The full-page write is used to protect pages from disk failure. Without it, 1) set vm: the vm bits that should be set to 1 may still be 0 2) clear vm: the vm bits that should be set to 0 may still be 1 Are these true? Or the page is totally unpredictable? Not quite. The WAL replay will set or clear vm bits, regardless of full page writes. Full page writes protect from torn pages, ie. the problem where some operations on a page have made it to disk while others have not. That's not a problem for VM pages, as each bit on the page can be set or cleared individually. But for something like a heap page where you have an offset in the beginning of the page that points to the tuple elsewhere on the page, you have to ensure that they stay in sync, even if you don't otherwise care if the update makes it to disk or not. Consider a example: 1. delete on two pages, emits two log (1, page1, vm_clear_1), (2, page2, vm_clear_2) 2. vm_clear_1 and vm_clear_2 on same vm page 3. checkpoint, and vm page get torned, vm_clear_2 was lost 4. delete another page, emits one log (3, page1, vm_clear_3), vm_clear_3 still on that vm page 5. power down 6. startup, redo will replay all change after checkpoint, but vm_clear_2 will never be cleared Am I right? Another question: To address the problem in http://archives.postgresql.org/pgsql-hackers/2010-02/msg02097.php , should we just clear the vm before the log of insert/update/delete? This may reduce the performance, is there another solution? Yeah, that's a straightforward way to fix it. I don't think the performance hit will be too bad. But we need to be careful not to hold locks while doing I/O, which might require some rearrangement of the code. We might want to do a similar dance that we do in vacuum, and call visibilitymap_pin first, then lock and update the heap page, and then set the VM bit while holding the lock on the heap page. Do you mean we should lock the heap page first, then get the blocknumber, then release heap page, then pin the vm's page, then lock both heap page and vm page? As Robert Haas said, when lock the heap page again, may there isnot enough free space on it. Is there a way just stop the checkpoint for a while? Thanks. GaoZengqi
Re: [HACKERS] crash-safe visibility map, take four
2011/3/30 Robert Haas robertmh...@gmail.com Maybe we could check PD_ALL_VISIBLE before taking the buffer lock - if it appears to be set, then we pin the visibility map page before taking the buffer lock. Otherwise, we take the buffer lock at once. Either way, once we have the lock, we recheck the bit. Only if it's set and we haven't got a pin do we need to do the drop-lock-pin-reacquire-lock dance. Is that at all sensible? But only lock can make sure the page has enough free space. If we try the drop-lock-...-lock dance, we may fall into a dead loop. -- GaoZengqi pgf...@gmail.com zengqi...@gmail.com
Re: [HACKERS] Re: [COMMITTERS] pgsql: Fix plpgsql to release SPI plans when a function or DO block is
On 31/03/11 07:35, Heikki Linnakangas wrote: On 30.03.2011 21:21, Jan Urbański wrote: Valgrind showed me the way. PFA a trivial patch to avoid leaking a PLyProcedure struct in inline blocks. Hmm, any reason the PLyProcedure struct needs to be allocated in TopMemoryContext in the first place? Could you palloc0 it in a shorter-lived context, or even better, just allocate it in stack? Yeah, you're right, you can keep it on the stack. PS. I don't think the volatile qualifier in 'proc' is in necessary. The variable is not changed in PG_TRY-block. That always confuses me, but I guess you're right, the variable does not change, only the memory it points to. Cheers, Jan -- 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: [COMMITTERS] pgsql: Fix plpgsql to release SPI plans when a function or DO block is
On 31.03.2011 12:30, Jan Urbański wrote: On 31/03/11 07:35, Heikki Linnakangas wrote: On 30.03.2011 21:21, Jan Urbański wrote: Valgrind showed me the way. PFA a trivial patch to avoid leaking a PLyProcedure struct in inline blocks. Hmm, any reason the PLyProcedure struct needs to be allocated in TopMemoryContext in the first place? Could you palloc0 it in a shorter-lived context, or even better, just allocate it in stack? Yeah, you're right, you can keep it on the stack. PS. I don't think the volatile qualifier in 'proc' is in necessary. The variable is not changed in PG_TRY-block. That always confuses me, but I guess you're right, the variable does not change, only the memory it points to. Ok then, committed. -- 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] BUG #5856: pg_attribute.attinhcount is not correct.
[moving to pgsql-hackers]
On Thu, Feb 03, 2011 at 11:24:42AM -0500, Robert Haas wrote:
On Mon, Jan 31, 2011 at 6:42 AM, Naoya Anzai
anzai-na...@mxu.nes.nec.co.jp wrote:
In PostgreSQL8.4.5, I found that the catalog pg_attribute.attinhcount is not
correct.
I executed the following queries.
--
create table 3_grandchild();
create table 2_child();
create table 1_parent();
alter table 3_grandchild inherit 2_child;
alter table 2_child inherit 1_parent;
alter table 3_grandchild add column c1 text;
alter table 2_child add column c1 text;
alter table 1_parent add column c1 text;
select c.relname,a.attname,a.attinhcount from pg_attribute a,pg_class c
where a.attrelid=c.oid
and relname in ('1_parent','2_child','3_grandchild')
and attname not in('xmax','xmin','cmin','cmax','tableoid','ctid')
order by relname;
? ?relname ? ?| attname | attinhcount
?--+-+-
?1_parent ? ? | c1 ? ? ?| ? ? ? ? ? 0
?2_child ? ? ?| c1 ? ? ?| ? ? ? ? ? 1
?3_grandchild | c1 ? ? ?| ? ? ? ? ? 2
?(3 rows)
--
3_grandchild's attinhcount should be 1.
I think this is a manifestation the same problem mentioned here:
http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=31b6fc06d83c6de3644c8f2921eb7de0eb92fac3
I believe this requires some refactoring to fix. It would be good to do that.
The best way I can see is to make ATExecAddColumn more like ATExecDropColumn,
ATAddCheckConstraint, and ATExecDropConstraint. Namely, recurse at Exec-time
rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a
merge. Did you have something else in mind?
Incidentally, when we satisfy an ADD COLUMN with a merge, we do not check or
update attnotnull:
create table parent();
create table child(c1 text) inherits (parent);
alter table parent add column c1 text not null;
\d child
We could either update attnotnull (and schedule a phase-3 scan of the table) or
throw an error. For ALTER TABLE ... INHERIT, we throw the error. For CREATE
TABLE ... INHERITS, we add the NOT NULL (and no scan is needed). I'd weakly
lean toward throwing the error. Opinions?
nm
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] crash-safe visibility map, take four
On 31.03.2011 11:33, 高增琦 wrote: Consider a example: 1. delete on two pages, emits two log (1, page1, vm_clear_1), (2, page2, vm_clear_2) 2. vm_clear_1 and vm_clear_2 on same vm page 3. checkpoint, and vm page get torned, vm_clear_2 was lost 4. delete another page, emits one log (3, page1, vm_clear_3), vm_clear_3 still on that vm page 5. power down 6. startup, redo will replay all change after checkpoint, but vm_clear_2 will never be cleared Am I right? No. A page can only be torn at a hard crash, ie. at step 5. A checkpoint flushes all changes to disk, once the checkpoint finishes all the changes before it are safe on disk. If you crashed between step 2 and 3, the VM page might be torn so that only one of the vm_clears has made it to disk but the other has not. But the WAL records for both are on disk anyway, so that will be corrected at replay. Another question: To address the problem in http://archives.postgresql.org/pgsql-hackers/2010-02/msg02097.php , should we just clear the vm before the log of insert/update/delete? This may reduce the performance, is there another solution? Yeah, that's a straightforward way to fix it. I don't think the performance hit will be too bad. But we need to be careful not to hold locks while doing I/O, which might require some rearrangement of the code. We might want to do a similar dance that we do in vacuum, and call visibilitymap_pin first, then lock and update the heap page, and then set the VM bit while holding the lock on the heap page. Do you mean we should lock the heap page first, then get the blocknumber, then release heap page, then pin the vm's page, then lock both heap page and vm page? As Robert Haas said, when lock the heap page again, may there isnot enough free space on it. I think the sequence would have to be: 1. Pin the heap page. 2. Check if the all-visible flag is set on the heap page (without lock). If it is, pin the vm page 3. Lock heap page, check that it has enough free space 4. Check again if the all-visible flag is set. If it is but we didn't pin the vm page yet, release lock and loop back to step 2 5. Update heap page 6. Update vm page Is there a way just stop the checkpoint for a while? Not at the moment. It wouldn't be hard to add, though. I was about to add a mechnism for that last autumn to fix a similar issue with b-tree parent pointer updates (http://archives.postgresql.org/message-id/4ccfee61.2090...@enterprisedb.com), but in the end it was solved differently. -- 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] SHMEM_INDEX_SIZE exceeded on startup
On 29.03.2011 20:20, Kevin Grittner wrote: I doubt that this is going to matter much, and should only have a trivial impact on shared space calculations and postmaster and connection startup time, but just as a matter of principle we might want to set SHMEM_INDEX_SIZE at least as large as the number of entries in ShmemIndex. At startup that seems to be 40 as of current HEAD. Trivial patch attached. Ok, committed. -- 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] BUG #5856: pg_attribute.attinhcount is not correct.
--On 31. März 2011 06:06:49 -0400 Noah Misch n...@leadboat.com wrote: The best way I can see is to make ATExecAddColumn more like ATExecDropColumn, ATAddCheckConstraint, and ATExecDropConstraint. Namely, recurse at Exec-time rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a merge. Did you have something else in mind? Incidentally, when we satisfy an ADD COLUMN with a merge, we do not check or update attnotnull: create table parent(); create table child(c1 text) inherits (parent); alter table parent add column c1 text not null; \d child We could either update attnotnull (and schedule a phase-3 scan of the table) or throw an error. For ALTER TABLE ... INHERIT, we throw the error. For CREATE TABLE ... INHERITS, we add the NOT NULL (and no scan is needed). I'd weakly lean toward throwing the error. Opinions? Hmm this looks like the same kind of problem i'm currently faced with when working on tracking inheritance counters for NOT NULL constraint at the moment (see http://git.postgresql.org/gitweb?p=users/bernd/postgres.git;a=shortlog;h=refs/heads/notnull_constraint for a heavy WIP patch). It currently recurses and seems to do the right thing (tm) for your example above, but i'm far from being certain that the way i'm undertaking here is correct. It indeed discovered a bug i had in my recursion code... -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] wal_buffers = -1 and SIGHUP
This might be nitpicking (or i'm currently missing something), but i recognized that setting wal_buffers = -1 always triggers the following on reload, even if nothing to wal_buffers had changed: $ pg_ctl reload LOG: received SIGHUP, reloading configuration files LOG: parameter wal_buffers cannot be changed without restarting the server This only happens when you have wal_buffers set to -1. -- Thanks Bernd -- 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] SSI bug?
YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote:
hoge=# select locktype,count(*) from pg_locks group by locktype;
-[ RECORD 1 ]
locktype | virtualxid
count| 1
-[ RECORD 2 ]
locktype | relation
count| 1
-[ RECORD 3 ]
locktype | tuple
count| 7061
I've stared at the code for hours and have only come up with one
race condition which can cause this, although the window is so small
it's hard to believe that you would get this volume of orphaned
locks. I'll keep looking, but if you could try this to see if it
has a material impact, that would be great.
I am very sure this patch is needed and that it is safe. It moves a
LWLockAcquire statement up to cover the setup for the loop that it
already covers. It also includes a fix to a comment that got missed
when we switched from the pointer between lock targets to
duplicating the locks.
-Kevin
*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***
*** 1755,1763 CoarserLockCovers(const PREDICATELOCKTARGETTAG *newtargettag)
}
/*
! * Check whether both the list of related predicate locks and the pointer to
! * a prior version of the row (if this is a tuple lock target) are empty for
! * a predicate lock target, and remove the target if they are.
*/
static void
RemoveTargetIfNoLongerUsed(PREDICATELOCKTARGET *target, uint32 targettaghash)
--- 1755,1762
}
/*
! * Check whether the list of related predicate locks is empty for a
! * predicate lock target, and remove the target if it is.
*/
static void
RemoveTargetIfNoLongerUsed(PREDICATELOCKTARGET *target, uint32 targettaghash)
***
*** 3120,3130 ClearOldPredicateLocks(void)
/*
* Loop through predicate locks on dummy transaction for summarized
data.
*/
predlock = (PREDICATELOCK *)
SHMQueueNext(OldCommittedSxact-predicateLocks,
OldCommittedSxact-predicateLocks,
offsetof(PREDICATELOCK, xactLink));
- LWLockAcquire(SerializablePredicateLockListLock, LW_SHARED);
while (predlock)
{
PREDICATELOCK *nextpredlock;
--- 3119,3129
/*
* Loop through predicate locks on dummy transaction for summarized
data.
*/
+ LWLockAcquire(SerializablePredicateLockListLock, LW_SHARED);
predlock = (PREDICATELOCK *)
SHMQueueNext(OldCommittedSxact-predicateLocks,
OldCommittedSxact-predicateLocks,
offsetof(PREDICATELOCK, xactLink));
while (predlock)
{
PREDICATELOCK *nextpredlock;
--
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] Problem with pg_upgrade?
Bruce Momjian wrote: Bruce Momjian wrote: It does seem possible that that could happen, but I'm not sure exactly what would be causing autovacuum to fire in the first place. It wouldn't have to be triggered by the anti-wraparound machinery - if the table appeared to be in need of vacuuming, then we'd vacuum it, discover that is was empty, and update relfrozenxid. Hmm... could it fire just because the table has no stats? But if that were the case you'd think we'd be seeing this more often. Well, autovacuum=off, so it should only run in freeze mode, and I can't see how that could happen. I am thinking I have to study autovacuum.c. I wonder if datfrozenxid could be incremented because the database is originally empty. It would just need to scan pg_class, not actually vacuum anything. I wonder if we do that. The bottom line is I am hanging too much on autovacuum_freeze_max_age causing autovacuum to do nothing. What if we allow autovacuum_max_workers to be set to zero; the current minimum is one. I can think of one case where autovacuum_freeze_max_age would be insufficient. If you set autovacuum_freeze_max_age in the old cluster to 2B, and you had a database that was near that limit, the tables created by pg_upgrade's --schema-only restore might create enough new transactions to cause autovacuum to run in freeze mode. While I think it is unlikely that is the cause of the problem report, it is enough for me to discount using autovacuum_freeze_max_age to disable autovacuum freeze. I will work on code to allow autovacuum_max_workers to be set to zero in HEAD and 9.0, and have pg_upgrade us that. I think the maintenance overhead of an invisible variable is too much. -- 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] [GENERAL] Date conversion using day of week
On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote:
On 31 March 2011 03:15, Steve Crawford scrawf...@pinpointresearch.com wrote:
On 03/29/2011 04:24 PM, Adrian Klaver wrote:
...
Well the strange part is only fails for SUN:...
test(5432)aklaver=select to_date('2011-13-SUN', 'IYYY-IW-DY');
to_date
2011-03-28
...
You specified Sunday as the day but the date returned is a Monday. I
would categorize that as a bug. (Hackers cc'd). Since Sunday is the last
day of an ISO week, it should have returned 2011-04-03.
My first inclination without consulting source or morning coffee is that
PostgreSQL is seeing Sunday as day zero. Note that while:
The relevant paragraphs in the docs are:
--
An ISO week date (as distinct from a Gregorian date) can be specified
to to_timestamp and to_date in one of two ways:
* Year, week, and weekday: for example to_date('2006-42-4',
'IYYY-IW-ID') returns the date 2006-10-19. If you omit the weekday it
is assumed to be 1 (Monday).
* Year and day of year: for example to_date('2006-291',
'IYYY-IDDD') also returns 2006-10-19.
Attempting to construct a date using a mixture of ISO week and
Gregorian date fields is nonsensical, and will cause an error. In the
context of an ISO year, the concept of a month or day of month has
no meaning. In the context of a Gregorian year, the ISO week has no
meaning. Users should avoid mixing Gregorian and ISO date
specifications.
--
We *could* make the OP's query return the Sunday of ISO week 2011-13,
which would be properly written 2011-13-7, but I think the right move
here would be to throw the error for illegal mixture of format tokens.
This is a trivial change -- just a matter of changing the from_date
type on the DAY, Day, day, DY, Dy, dy keys.
With the attached patch applied, this is what happens instead:
# select to_date('2011-13-SUN', 'IYYY-IW-DY');
ERROR: invalid combination of date conventions
HINT: Do not mix Gregorian and ISO week date conventions in a
formatting template.
If we wanted to make it work, then I think the thing to do would be
to add a new set of formatting tokens IDY, IDAY etc. I don't like the
idea of interpreting DY and co. differently depending on whether the
other tokens happen to be ISO week or Gregorian.
Just to play Devils advocate here, but why not? The day name is the same either
way, it is the index that changes. I am not sure why that could not be context
specific?
Cheers,
BJ
--
Adrian Klaver
adrian.kla...@gmail.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] BUG #5856: pg_attribute.attinhcount is not correct.
On Thu, Mar 31, 2011 at 6:06 AM, Noah Misch n...@leadboat.com wrote: I think this is a manifestation the same problem mentioned here: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=31b6fc06d83c6de3644c8f2921eb7de0eb92fac3 I believe this requires some refactoring to fix. It would be good to do that. The best way I can see is to make ATExecAddColumn more like ATExecDropColumn, ATAddCheckConstraint, and ATExecDropConstraint. Namely, recurse at Exec-time rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a merge. Did you have something else in mind? I had exactly what you just said in mind. Incidentally, when we satisfy an ADD COLUMN with a merge, we do not check or update attnotnull: create table parent(); create table child(c1 text) inherits (parent); alter table parent add column c1 text not null; \d child We could either update attnotnull (and schedule a phase-3 scan of the table) or throw an error. For ALTER TABLE ... INHERIT, we throw the error. For CREATE TABLE ... INHERITS, we add the NOT NULL (and no scan is needed). I'd weakly lean toward throwing the error. Opinions? Not sure. I think that anything we do here is bound to have some corner cases that are not quite right for so long as NOT NULL constraints aren't represented in pg_constraint, and it's way too late to dredge up that issue again for 9.1. I'm somewhat inclined to just defer fixing it until we get that work committed. -- 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] Comments on SQL/Med objects
On Mon, Mar 28, 2011 at 12:29 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 28, 2011 at 12:06 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Mar 23, 2011 at 12:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah, I had a private TODO about that. I'd like to see if we can refactor the grammar to eliminate some of the duplication there as well as the potential for oversights of this sort. I believe that USER MAPPINGs are missing from ObjectType as well as a bunch of other basic places ... Are you going to work on this? If not I can pick it up, at least insofar as making the comment stuff work across the board. I'm still up to my rear in collations, so feel free. OK. I'll work on it this week. Attached. Foreign tables are already OK, I believe; it's only foreign data wrappers and foreign servers that appear to need fixing. The fact that foreign data wrapper is sometimes abbreviated to fdw and sometimes not does nothing for the greppability of the code. I'm wondering if we should go through and fix the constants that abbreviate it: ACL_KIND_FDW ACL_ALL_RIGHTS_FDW OBJECT_FDW OCLASS_FDW It seems to me that it would be a whole lot clearer and easier if these all spelled it out FOREIGN_DATA_WRAPPER, as we do for similar object types. Other than a pretty minute back-patch hazard, I don't see much down side. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company foreign-comment.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] [GENERAL] Date conversion using day of week
On 1 April 2011 02:00, Adrian Klaver adrian.kla...@gmail.com wrote: On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote: If we wanted to make it work, then I think the thing to do would be to add a new set of formatting tokens IDY, IDAY etc. I don't like the idea of interpreting DY and co. differently depending on whether the other tokens happen to be ISO week or Gregorian. Just to play Devils advocate here, but why not? The day name is the same either way, it is the index that changes. I am not sure why that could not be context specific? To be perfectly honest, it's mostly because I was hoping not to spend very much more of my time in formatting.c. Every time I go in there I come out a little bit less sane. I'm concerned that if I do anything further to it, I might inadvertently summon Chattur'gha or something. But since you went to the trouble of calling me on my laziness, let's take a look at the problem. At the time when the day-of-week token gets converted into a numeric value and put into the TmFromChar.d field, the code has no knowledge of whether the overall pattern is Gregorian or ISO (the DY field could well be at the front of the pattern, for example). Later on, in do_to_timestamp, the code expects the 'd' value to make sense given the mode (it should be zero-based on Sunday for Gregorian, or one-based on Monday for ISO). That's all well and good *except* in the totally bizarre case raised by the OP. To resolve it, we could make TmFromChar.d always stored using the ISO convention (because zero then has the useful property of meaning not set) and converted to the Gregorian convention as necessary in do_to_timestamp. Cheers, BJ -- 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] Problem with pg_upgrade?
On 31.03.2011 17:55, Bruce Momjian wrote: I will work on code to allow autovacuum_max_workers to be set to zero in HEAD and 9.0, and have pg_upgrade us that. We've intentionally not allowed the user to disable anti-wraparound autovacuum before. Do we really want to allow it now for the sake of pg_upgrade? I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. Is the problem just that the clog files get removed too early, or is there something else? If it's just the clog files, we could simply copy them (again) after updating datfrozenxids. -- 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] [GENERAL] Date conversion using day of week
On Thu, 2011-03-31 at 08:00 -0700, Adrian Klaver wrote:
On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote:
On 31 March 2011 03:15, Steve Crawford scrawf...@pinpointresearch.com
wrote:
On 03/29/2011 04:24 PM, Adrian Klaver wrote:
...
Well the strange part is only fails for SUN:...
[. . .]
We *could* make the OP's query return the Sunday of ISO week 2011-13,
which would be properly written 2011-13-7, but I think the right move
here would be to throw the error for illegal mixture of format tokens.
This is a trivial change -- just a matter of changing the from_date
type on the DAY, Day, day, DY, Dy, dy keys.
[. . .]
Just to play Devils advocate here, but why not? The day name is the same
either
way, it is the index that changes. I am not sure why that could not be
context
specific?
Just to be clear, the reason I was mixing things in this way was that I
wanted to validate that the dayname being passed was valid for the
current locale, and I could find no easier way of doing it. FTR, I have
now resorted to finding the given dayname in the results of this query:
select day, to_char(day, 'dy') as dayname,
extract('dow' from day) as dayno
from (
select current_date + n as day
from generate_series(0, 6) as n) d;
If there is an easier way of doing this, please let me know. As far as
the postgres API goes, exposing a function that would validate a dayname
returning a day number would resolve all of this for considerably less
complexity. Also throwing an error in the to_date function for
unexpectedly mixed input formats seems quite reasonable.
Thanks for your time and attention. The commercial RDBMS vendors could
learn a lot about customer support from this forum.
__
Marc Munro
signature.asc
Description: This is a digitally signed message part
Re: [HACKERS] Libpq PGRES_COPY_BOTH - version compatibility
On Mon, Mar 28, 2011 at 7:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Now if we had a track record showing that we could tweak the protocol version without causing problems, it'd be fine with me to do it for this usage. But we don't, and this particular case doesn't seem like the place to start. And, btw, a moment's study of the protocol version checking code in postmaster.c shows that bumping the minor version number to 3.1 *would* break things: a client requesting 3.1 from a current postmaster would get a failure. Given that, it seems that there is far more downside than upside to this particular change, and we shouldn't do it. Accordingly, I'm going to mark the open item raise protocol version number closed. Maybe we oughta change that logic --- it's not clear to me that there's any meaningful difference between major and minor numbers given the current postmaster behavior. I don't think this would be a bad thing to do if we're fairly clear that it's correct and won't break anything, but I don't think it's worth delaying beta for, so I propose not to add it to the open items list unless someone else feels otherwise. -- 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] SSI bug?
On 31.03.2011 16:31, Kevin Grittner wrote: I've stared at the code for hours and have only come up with one race condition which can cause this, although the window is so small it's hard to believe that you would get this volume of orphaned locks. I'll keep looking, but if you could try this to see if it has a material impact, that would be great. I am very sure this patch is needed and that it is safe. It moves a LWLockAcquire statement up to cover the setup for the loop that it already covers. It also includes a fix to a comment that got missed when we switched from the pointer between lock targets to duplicating the locks. Ok, committed. Did we get anywhere with the sizing of the various shared memory structures? Did we find the cause of the out of shared memory warnings? Would it help if we just pre-allocated all the shared memory hash tables and didn't allow them to grow? It's bizarre that the hash table that requests the slack shared memory space first gets it, and it can't be reused for anything else without a server restart. I feel it would make better to not allow the tables to grow, so that you get consistent behavior across restarts. -- 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] SSI bug?
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Did we get anywhere with the sizing of the various shared memory structures? Did we find the cause of the out of shared memory warnings? The patch you just committed is related to that. Some tuple locks for summarized transactions were not getting cleaned up. I found one access to the list not protected by the appropriate LW lock, which is what this patch fixed. I'm not satisfied that was the only issue, though; I'm still looking. Would it help if we just pre-allocated all the shared memory hash tables and didn't allow them to grow? I've been thinking that it might be wise. It's bizarre that the hash table that requests the slack shared memory space first gets it, and it can't be reused for anything else without a server restart. I feel it would make better to not allow the tables to grow, so that you get consistent behavior across restarts. Agreed. I think it was OK in prior releases because there was just one HTAB in shared memory doing this. With multiple such tables, it doesn't seem sane to allow unbounded lazy grabbing of the space this way. The only thing I've been on the fence about is whether it makes more sense to allocate it all up front or to continue to allow incremental allocation but set a hard limit on the number of entries allocated for each shared memory HTAB. Is there a performance- related reason to choose one path or the other? -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] [GENERAL] Date conversion using day of week
On 1 April 2011 02:35, Marc Munro m...@bloodnok.com wrote: Just to be clear, the reason I was mixing things in this way was that I wanted to validate that the dayname being passed was valid for the current locale, and I could find no easier way of doing it. Ah, I see. In that case I think to_date would have disappointed you even if IYYY-IW-DY did work, since the inputs do not appear to be checked against the localised versions of the day names. They are only checked against the hard-coded English names. to_date and to_char are asymmetric in this sense -- localisation only happens on the way out. Cheers, BJ -- 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] Problem with pg_upgrade?
Heikki Linnakangas wrote: On 31.03.2011 17:55, Bruce Momjian wrote: I will work on code to allow autovacuum_max_workers to be set to zero in HEAD and 9.0, and have pg_upgrade us that. We've intentionally not allowed the user to disable anti-wraparound autovacuum before. Do we really want to allow it now for the sake of pg_upgrade? Not sure. I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. Well, is this going to show in SHOW ALL or pg_settings? Do we have the ability to easily disable display of this? Is the problem just that the clog files get removed too early, or is there something else? If it's just the clog files, we could simply copy them (again) after updating datfrozenxids. The problem is that pg_upgrade through pg_dumpall is setting pg_database/pg_class frozen xid values and I can't have autovacuum modifying the system while this is happening. -- 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] Problem with pg_upgrade?
On Thu, Mar 31, 2011 at 11:32 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. I like the idea of a command-line switch. -- 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] Problem with pg_upgrade?
Robert Haas wrote: On Thu, Mar 31, 2011 at 11:32 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: ?I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. I like the idea of a command-line switch. If you want to do that you should gereralize it as --binary-upgrade in case we have other needs for it. -- 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] Libpq PGRES_COPY_BOTH - version compatibility
On Thu, Mar 31, 2011 at 17:35, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 28, 2011 at 7:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Now if we had a track record showing that we could tweak the protocol version without causing problems, it'd be fine with me to do it for this usage. But we don't, and this particular case doesn't seem like the place to start. And, btw, a moment's study of the protocol version checking code in postmaster.c shows that bumping the minor version number to 3.1 *would* break things: a client requesting 3.1 from a current postmaster would get a failure. Given that, it seems that there is far more downside than upside to this particular change, and we shouldn't do it. Accordingly, I'm going to mark the open item raise protocol version number closed. +1. Maybe we oughta change that logic --- it's not clear to me that there's any meaningful difference between major and minor numbers given the current postmaster behavior. I don't think this would be a bad thing to do if we're fairly clear that it's correct and won't break anything, but I don't think it's worth delaying beta for, so I propose not to add it to the open items list unless someone else feels otherwise. Perhaps this part should go on the TODO list then? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Bug in autovacuum.c?
Looking over the autovacuum.c code, I see: /* * Determine the oldest datfrozenxid/relfrozenxid that we will allow to * pass without forcing a vacuum. (This limit can be tightened for * particular tables, but not loosened.) */ recentXid = ReadNewTransactionId(); xidForceLimit = recentXid - autovacuum_freeze_max_age; /* ensure it's a normal XID, else TransactionIdPrecedes misbehaves */ if (xidForceLimit FirstNormalTransactionId) xidForceLimit -= FirstNormalTransactionId; This last line doesn't look right to me; should it be: xidForceLimit = FirstNormalTransactionId; -- 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] Problem with pg_upgrade?
On Thu, Mar 31, 2011 at 12:11 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Thu, Mar 31, 2011 at 11:32 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: ?I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. I like the idea of a command-line switch. If you want to do that you should gereralize it as --binary-upgrade in case we have other needs for it. Yeah. Or we could do a binary_upgrade GUC which has the effect of forcibly suppressing autovacuum, and maybe other things later. I think that's a lot less hazardous than fiddling with the autovacuum GUC. -- 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] [GENERAL] Date conversion using day of week
On 03/31/2011 08:27 AM, Brendan Jurd wrote: On 1 April 2011 02:00, Adrian Klaveradrian.kla...@gmail.com wrote: On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote: If we wanted to make it work, then I think the thing to do would be to add a new set of formatting tokens IDY, IDAY etc. I don't like the idea of interpreting DY and co. differently depending on whether the other tokens happen to be ISO week or Gregorian. Just to play Devils advocate here, but why not? The day name is the same either way, it is the index that changes. I am not sure why that could not be context specific? To be perfectly honest, it's mostly because I was hoping not to spend very much more of my time in formatting.c. Every time I go in there I come out a little bit less sane. I'm concerned that if I do anything further to it, I might inadvertently summon Chattur'gha or something. But since you went to the trouble of calling me on my laziness, let's take a look at the problem. I understand, my foray into formatting.c has left an impression. At the time when the day-of-week token gets converted into a numeric value and put into the TmFromChar.d field, the code has no knowledge of whether the overall pattern is Gregorian or ISO (the DY field could well be at the front of the pattern, for example). Later on, in do_to_timestamp, the code expects the 'd' value to make sense given the mode (it should be zero-based on Sunday for Gregorian, or one-based on Monday for ISO). That's all well and good *except* in the totally bizarre case raised by the OP. Now I am confused the docs say: D day of the week, Sunday(1) to Saturday(7) ID ISO day of the week, Monday(1) to Sunday(7) This would seem to say they both are one-based but differ on the day that is 1. To resolve it, we could make TmFromChar.d always stored using the ISO convention (because zero then has the useful property of meaning not set) and converted to the Gregorian convention as necessary in do_to_timestamp. Since I am in this deep might as well go deeper. When I see the requirement: IYYY-IW-IDY(proposed) or YYY-WW-DY which is implied GYYY-GWW-GDY I see the constant being pulled out: I YYY-W-DY G YYY-W-DY I know this presents backwards compatibility issues. Also that the data formatting functions are supposed to track Oracle behavior. It just seems a way to simplify the formatting process. Thanks for taking the time to explain the process. Cheers, BJ -- Adrian Klaver adrian.kla...@gmail.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] Process local hint bit cache
On Wed, Mar 30, 2011 at 6:30 PM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Mar 30, 2011 at 2:35 PM, Merlin Moncure mmonc...@gmail.com wrote: btw I haven't forgotten your idea to move TransactionIdInProgress Down. I think this is a good idea, and will experiment with it pre and post cache. aside: Moving TransactionIdInProgress below TransactionIdDidCommit can help in once sense: TransactionIdDidCommit grabs the XidStatus but discards the knowledge if the transaction is known aborted. If it is in fact aborted you can immediately set the hint bits and punt. This should save an awful lot of calls to TransactionIdInProgress when scanning unhinted dead tuples. Yeah, it might make sense to have a function that returns commit/abort/unsure, where unsure can only happen if the transaction ID follows RecentXmin. You might also want to rearrange things so that that function starts by checking the passed-in XID against cachedFetchXid so we don't lose the benefit of that one-element cache. -- 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] Problem with pg_upgrade?
On Wed, Mar 30, 2011 at 10:57 AM, Bruce Momjian br...@momjian.us wrote: I am hearing only second-hand reports of this problem through Rhodiumtoad on IRC. I don't have IRC access this week If the firewalls allow port 80, you can use Freenode's web interface: webchat.freenode.net Regards, -- Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] Bug in autovacuum.c?
On Thu, Mar 31, 2011 at 12:17 PM, Bruce Momjian br...@momjian.us wrote: Looking over the autovacuum.c code, I see: /* * Determine the oldest datfrozenxid/relfrozenxid that we will allow to * pass without forcing a vacuum. (This limit can be tightened for * particular tables, but not loosened.) */ recentXid = ReadNewTransactionId(); xidForceLimit = recentXid - autovacuum_freeze_max_age; /* ensure it's a normal XID, else TransactionIdPrecedes misbehaves */ if (xidForceLimit FirstNormalTransactionId) xidForceLimit -= FirstNormalTransactionId; This last line doesn't look right to me; should it be: xidForceLimit = FirstNormalTransactionId; That would probably work, but the existing coding actually makes more sense. It's essentially trying to scan backwards by autovacuum_freeze_max_age XIDs through the circular XID space. But the XID space isn't actually circular, because there are 3 special values. So if we land on one of those values we want to skip backward by 3. Here FirstNormalTransactionId doesn't represent itself, but rather the number of special XIDs that exist. -- 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] [GENERAL] Date conversion using day of week
On 1 April 2011 03:32, Adrian Klaver adrian.kla...@gmail.com wrote: Now I am confused the docs say: D day of the week, Sunday(1) to Saturday(7) ID ISO day of the week, Monday(1) to Sunday(7) This would seem to say they both are one-based but differ on the day that is 1. That's correct for the user-facing interpretation. Internally, however, Gregorian day-of-week is represented with Sunday = 0. I can't see any good reason in the code for why that should be so, but it was like that when I found it, and until now I haven't had any cause to mess with it. My suggestion for moving forward basically still stands, though. We'd need to standardise the use of TmFromChar.d to either one of the 1-based conventions, and convert to the other one as required in do_to_timestamp. The Gregorian convention is probably the right choice for the standard, even though it has the week starting on a Sunday (ridiculous!) because it means less converting for the majority of cases. Cheers, BJ -- 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] [GENERAL] Date conversion using day of week
On 03/31/2011 08:00 AM, Adrian Klaver wrote:
On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote:
On 31 March 2011 03:15, Steve Crawfordscrawf...@pinpointresearch.com wrote:
On 03/29/2011 04:24 PM, Adrian Klaver wrote:
...
Well the strange part is only fails for SUN:...
test(5432)aklaver=select to_date('2011-13-SUN', 'IYYY-IW-DY');
to_date
2011-03-28
...
You specified Sunday as the day but the date returned is a Monday. I
would categorize that as a bug. (Hackers cc'd). Since Sunday is the last
day of an ISO week, it should have returned 2011-04-03.
My first inclination without consulting source or morning coffee is that
PostgreSQL is seeing Sunday as day zero. Note that while:
The relevant paragraphs in the docs are:
--
An ISO week date (as distinct from a Gregorian date) can be specified
to to_timestamp and to_date in one of two ways:
* Year, week, and weekday: for example to_date('2006-42-4',
'IYYY-IW-ID') returns the date 2006-10-19. If you omit the weekday it
is assumed to be 1 (Monday).
* Year and day of year: for example to_date('2006-291',
'IYYY-IDDD') also returns 2006-10-19.
Attempting to construct a date using a mixture of ISO week and
Gregorian date fields is nonsensical, and will cause an error. In the
context of an ISO year, the concept of a month or day of month has
no meaning. In the context of a Gregorian year, the ISO week has no
meaning. Users should avoid mixing Gregorian and ISO date
specifications.
--
We *could* make the OP's query return the Sunday of ISO week 2011-13,
which would be properly written 2011-13-7, but I think the right move
here would be to throw the error for illegal mixture of format tokens.
This is a trivial change -- just a matter of changing the from_date
type on the DAY, Day, day, DY, Dy, dy keys.
With the attached patch applied, this is what happens instead:
# select to_date('2011-13-SUN', 'IYYY-IW-DY');
ERROR: invalid combination of date conventions
HINT: Do not mix Gregorian and ISO week date conventions in a
formatting template.
If we wanted to make it work, then I think the thing to do would be
to add a new set of formatting tokens IDY, IDAY etc. I don't like the
idea of interpreting DY and co. differently depending on whether the
other tokens happen to be ISO week or Gregorian.
Just to play Devils advocate here, but why not? The day name is the same either
way, it is the index that changes. I am not sure why that could not be context
specific?
A week day represented as an int is ambiguous - as you mention, the
index is necessary to decode to the correct day. Sunday is unambiguous
so we could do something reasonable. But from everything I've read
(though I didn't actually shell out 130CHF for a full 33-page copy of
ISO8601:2004), the ISO *week* date format does not represent day-of-week
as other than a numeric value so it would not really be an ISO8601
formatted date and I would be tempted to thrown an error. However...
This whole discussion opens a #10 sized can o' worms. Admittedly, I
don't have good knowledge of any SQL-mandated interpretations of an ISO
date - but based on my reading of ISO formatting I see the following issues:
1. What we describe in the documentation as an ISO date is actually an
ISO *week* date - a special purpose format included within ISO8601.
2011-03-31 is also an ISO date as are 20110331, 20110331T013212 and
20110331T21.3344298. Fixing this is probably as simple as a
clarification in the documentation.
2. The ISO week-date format is defined as having the week-number
prefaced by a W as in 2011-W03-7. From the ISO8601 FAQ page: Week
date is an alternative date representation used in many commercial and
industrial applications. It is: -Www-D where is the Year in the
Gregorian calendar, ww is the week of the year between 01 (the first
week) and 52 or 53 (the last week), and D is the day in the week between
1 (Monday) and 7 (Sunday). Example: 2003-W14-2 represents the second day
of the fourteenth week of 2003. However PostgreSQL does *not* accept
that as input even as specified as an ISO date:
select to_date('2003-W14-2', 'IYYY-IW-ID');
ERROR: invalid value W1 for IW
DETAIL: Value must be an integer.
Fixing this would require both a coding change and a decision whether or
not to throw an error on incorrectly formatted input.
3. ISO8601 requires zero-padding. PostgreSQL, however, does not complain
if that padding is missing. The following should be 2011-04-2
(actually, 2011-W04-2 as noted above) but PostgreSQL accepts:
select to_date('2011-4-2', 'IYYY-IW-ID');
to_date
2011-01-25
However in ISO dates the hyphens are supposed to only be for easier
reading by humans. But if we just remove them:
select to_date('201142', 'IYYYIWID');
to_date
2011-10-17
(Monday of the 42nd week).
Fix it and throw an error (and suffer the howls of anguish when backward
compatibility is shattered) or tiptoe quietly
Re: [HACKERS] Bug in autovacuum.c?
Excerpts from Robert Haas's message of jue mar 31 13:58:41 -0300 2011: On Thu, Mar 31, 2011 at 12:17 PM, Bruce Momjian br...@momjian.us wrote: Looking over the autovacuum.c code, I see: /* * Determine the oldest datfrozenxid/relfrozenxid that we will allow to * pass without forcing a vacuum. (This limit can be tightened for * particular tables, but not loosened.) */ recentXid = ReadNewTransactionId(); xidForceLimit = recentXid - autovacuum_freeze_max_age; /* ensure it's a normal XID, else TransactionIdPrecedes misbehaves */ if (xidForceLimit FirstNormalTransactionId) xidForceLimit -= FirstNormalTransactionId; This last line doesn't look right to me; should it be: xidForceLimit = FirstNormalTransactionId; That would probably work, but the existing coding actually makes more sense. It's essentially trying to scan backwards by autovacuum_freeze_max_age XIDs through the circular XID space. But the XID space isn't actually circular, because there are 3 special values. So if we land on one of those values we want to skip backward by 3. Here FirstNormalTransactionId doesn't represent itself, but rather the number of special XIDs that exist. Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] [GENERAL] Date conversion using day of week
On 1 April 2011 04:16, Steve Crawford scrawf...@pinpointresearch.com wrote: This whole discussion opens a #10 sized can o' worms. Admittedly, I don't have good knowledge of any SQL-mandated interpretations of an ISO date - but based on my reading of ISO formatting I see the following issues: 1. What we describe in the documentation as an ISO date is actually an ISO *week* date - a special purpose format included within ISO8601. 2011-03-31 is also an ISO date as are 20110331, 20110331T013212 and 20110331T21.3344298. Fixing this is probably as simple as a clarification in the documentation. In the docs paragraph I quoted upthread, the full name ISO week date is given. Elsewhere the shorthand ISO or ISO date is used, in contrast to the ordinary Gregorian style. This is the only sense in which we refer to ISO in the context of to_date, but I have no real objection to expanding this to the full name ISO week date everywhere it is mentioned, if people find the current usage ambiguous. 2. The ISO week-date format is defined as having the week-number prefaced by a W as in 2011-W03-7. ... However PostgreSQL does *not* accept that as input even as specified as an ISO date: It does, but you must use the somewhat awkward quoting notation to indicate that the W is a literal character in the input string, not a formatting character: 'IYYY-WIW-ID' ... What I've concluded is that the root of the entire problem is providing ISO formatting options in pieces at all. The ISO date format has various requirements like ordering from largest temporal term to smallest, zero-padding, W prefacing an ISO week, no skipping of temporal terms (201105 is May 2011, never the 5th of an unknown month), etc. all intended to make an ISO date string unambiguous. As such, it should only require a single format option saying this is an ISO8601 date string and mixing of ISO and Gregorian date formatting becomes impossible. I agree with your summary of the ISO standards. Unfortunately, to_date and its cohorts are not targeting ISO. They are targeting quasi-compatibility with some Oracle functions of the same name, I suppose to make life easier for folks who are migrating from Oracle to Postgres. Any proposed reform of these (admittedly weird and kludgy) functions is viewed through that lens, and usually rejected on those grounds. I've been down that road before. There's not much point having compatibility functions if they aren't, well, compatible. In the big picture, to_date isn't meant to be the general entry point for parsing dates. If you wanted to make ISO8601 work as a syntax for inputting date type literals vis. SELECT date '2011-W14-01', you might have a better shot at getting that off the ground. Cheers, BJ -- 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] SSI bug?
On Thu, Mar 31, 2011 at 11:06:30AM -0500, Kevin Grittner wrote: The only thing I've been on the fence about is whether it makes more sense to allocate it all up front or to continue to allow incremental allocation but set a hard limit on the number of entries allocated for each shared memory HTAB. Is there a performance- related reason to choose one path or the other? Seems like it would be marginally better to allocate it up front -- then you don't have the cost of having to split buckets later as it grows. Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- 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] [GENERAL] Date conversion using day of week
On 03/31/2011 10:51 AM, Brendan Jurd wrote:
I agree with your summary of the ISO standards. Unfortunately,
to_date and its cohorts are not targeting ISO. They are targeting
quasi-compatibility with some Oracle functions of the same name, I
suppose to make life easier for folks who are migrating from Oracle to
Postgres. Any proposed reform of these (admittedly weird and kludgy)
functions is viewed through that lens, and usually rejected on those
grounds. I've been down that road before. There's not much point
having compatibility functions if they aren't, well, compatible.
In the big picture, to_date isn't meant to be the general entry point
for parsing dates. If you wanted to make ISO8601 work as a syntax for
inputting date type literals vis. SELECT date '2011-W14-01', you might
have a better shot at getting that off the ground.
Well, to return to the original issue, should we allow the day to be
spelled out and fix it (as noted in this thread it is non-standard but
also unambiguous and we already allow plenty of non-standard formats) or
throw an error? For me personally, either would be fine. What isn't
correct is the current behavior:
select to_date('2011-13-SUN', 'IYYY-IW-DY');
to_date
2011-03-28
Cheers,
Steve
--
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] SSI bug?
Dan Ports d...@csail.mit.edu wrote:
On Thu, Mar 31, 2011 at 11:06:30AM -0500, Kevin Grittner wrote:
The only thing I've been on the fence about is whether it
makes more sense to allocate it all up front or to continue to
allow
incremental allocation but set a hard limit on the number of
entries
allocated for each shared memory HTAB. Is there a performance-
related reason to choose one path or the other?
Seems like it would be marginally better to allocate it up front --
then
you don't have the cost of having to split buckets later as it
grows.
The attached patch should cover that.
-Kevin
*** a/src/backend/storage/lmgr/lock.c
--- b/src/backend/storage/lmgr/lock.c
***
*** 281,295 InitLocks(void)
{
HASHCTL info;
int hash_flags;
! longinit_table_size,
! max_table_size;
/*
* Compute init/max size to request for lock hashtables. Note these
* calculations must agree with LockShmemSize!
*/
max_table_size = NLOCKENTS();
- init_table_size = max_table_size / 2;
/*
* Allocate hash table for LOCK structs. This stores per-locked-object
--- 281,293
{
HASHCTL info;
int hash_flags;
! longmax_table_size;
/*
* Compute init/max size to request for lock hashtables. Note these
* calculations must agree with LockShmemSize!
*/
max_table_size = NLOCKENTS();
/*
* Allocate hash table for LOCK structs. This stores per-locked-object
***
*** 303,316 InitLocks(void)
hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
LockMethodLockHash = ShmemInitHash(LOCK hash,
!
init_table_size,
max_table_size,
info,
hash_flags);
/* Assume an average of 2 holders per lock */
max_table_size *= 2;
- init_table_size *= 2;
/*
* Allocate hash table for PROCLOCK structs. This stores
--- 301,313
hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
LockMethodLockHash = ShmemInitHash(LOCK hash,
!
max_table_size,
max_table_size,
info,
hash_flags);
/* Assume an average of 2 holders per lock */
max_table_size *= 2;
/*
* Allocate hash table for PROCLOCK structs. This stores
***
*** 323,329 InitLocks(void)
hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
LockMethodProcLockHash = ShmemInitHash(PROCLOCK hash,
!
init_table_size,
max_table_size,
info,
hash_flags);
--- 320,326
hash_flags = (HASH_ELEM | HASH_FUNCTION | HASH_PARTITION);
LockMethodProcLockHash = ShmemInitHash(PROCLOCK hash,
!
max_table_size,
max_table_size,
info,
hash_flags);
*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***
*** 959,966 InitPredicateLocks(void)
{
HASHCTL info;
int hash_flags;
! longinit_table_size,
! max_table_size;
SizerequestSize;
boolfound;
--- 959,965
{
HASHCTL info;
int hash_flags;
! longmax_table_size;
SizerequestSize;
boolfound;
***
*** 969,975 InitPredicateLocks(void)
* Note these calculations must agree with PredicateLockShmemSize!
*/
max_table_size = NPREDICATELOCKTARGETENTS();
- init_table_size = max_table_size / 2;
Re: [HACKERS] SSI bug?
On 31.03.2011 21:23, Kevin Grittner wrote: Dan Portsd...@csail.mit.edu wrote: On Thu, Mar 31, 2011 at 11:06:30AM -0500, Kevin Grittner wrote: The only thing I've been on the fence about is whether it makes more sense to allocate it all up front or to continue to allow incremental allocation but set a hard limit on the number of entries allocated for each shared memory HTAB. Is there a performance- related reason to choose one path or the other? Seems like it would be marginally better to allocate it up front -- then you don't have the cost of having to split buckets later as it grows. The attached patch should cover that. That's not enough. The hash tables can grow beyond the maximum size you specify in ShmemInitHash. It's just a hint to size the directory within the hash table. We'll need to teach dynahash not to allocate any more entries after the preallocation. A new HASH_NO_GROW flag to hash_create() seems like a suitable interface. -- 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] [GENERAL] Date conversion using day of week
On 1 April 2011 05:16, Steve Crawford scrawf...@pinpointresearch.com wrote: Well, to return to the original issue, should we allow the day to be spelled out and fix it (as noted in this thread it is non-standard but also unambiguous and we already allow plenty of non-standard formats) or throw an error? For me personally, either would be fine. What isn't correct is the current behavior: I started out thinking we should throw the error, but I am coming around to the idea of fixing it. I outlined how that might work in reply to Adrian Klaver elsewhere in the thread [1]. Cheers, BJ [1] http://archives.postgresql.org/pgsql-hackers/2011-03/msg01906.php -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in autovacuum.c?
Alvaro Herrera wrote: That would probably work, but the existing coding actually makes more sense. It's essentially trying to scan backwards by autovacuum_freeze_max_age XIDs through the circular XID space. But the XID space isn't actually circular, because there are 3 special values. So if we land on one of those values we want to skip backward by 3. Here FirstNormalTransactionId doesn't represent itself, but rather the number of special XIDs that exist. Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). I am now thinking the code is correct --- it maps values from 0 to FirstNormalTransactionId into the top of the (unsigned) xid range. Unless someone objects, I will add a C comment about this behavior so future readers are not confused. -- 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] Problem with pg_upgrade?
Robert Haas wrote: On Thu, Mar 31, 2011 at 12:11 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Thu, Mar 31, 2011 at 11:32 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: ?I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. I like the idea of a command-line switch. If you want to do that you should gereralize it as --binary-upgrade in case we have other needs for it. Yeah. Or we could do a binary_upgrade GUC which has the effect of forcibly suppressing autovacuum, and maybe other things later. I think that's a lot less hazardous than fiddling with the autovacuum GUC. I like the idea of a command-line flag because it forces everything to be affected, and cannot be turned on and off in sessions --- if you are doing a binary upgrade, locked-down is good. :-) -- 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] Bug in autovacuum.c?
Bruce Momjian wrote: Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). I am now thinking the code is correct --- it maps values from 0 to FirstNormalTransactionId into the top of the (unsigned) xid range. Unless someone objects, I will add a C comment about this behavior so future readers are not confused. OK, now I think it is wrong. :-) The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. -- 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] SSI bug?
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: That's not enough. The hash tables can grow beyond the maximum size you specify in ShmemInitHash. It's just a hint to size the directory within the hash table. We'll need to teach dynahash not to allocate any more entries after the preallocation. A new HASH_NO_GROW flag to hash_create() seems like a suitable interface. OK. If we're doing that, is it worth taking a look at the safety margin added to the size calculations, and try to make the calculations more accurate? Would you like me to code a patch for this? There are a couple other patches which I think should be applied, if you have time to deal with them. There was a fix for an assertion failure here: http://archives.postgresql.org/pgsql-bugs/2011-03/msg00352.php It just rechecks some conditions after dropping a shared LW lock and acquiring an exclusive LW lock. Without this recheck there is a window for the other transaction involved in the conflict to also detect a conflict and flag first, leading to the assertion. There's another area I need to review near there, but that is orthogonal. There is a patch to improve out-of-shared-memory error handling and reporting here: http://archives.postgresql.org/pgsql-hackers/2011-03/msg01170.php This one is due to my earlier failure to spot the difference between HASH_ENTER and HASH_ENTER_NULL. For a shared memory HTAB the HASH_ENTER_NULL will return a null if unable to allocate the entry, while HASH_ENTER will ereport ERROR with a generic message. This patch leaves HASH_ENTER on the can't happen cases, but replaces the ereport ERROR after it with an Assert because it's something which should never happen. The other cases are changed to HASH_ENTER_NULL so that the error message with the hint will be used instead of the more generic message. These patches are both in direct response to problems found during testing by YAMAMOTO Takashi. -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] Bug in autovacuum.c?
On Thu, Mar 31, 2011 at 2:59 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). I am now thinking the code is correct --- it maps values from 0 to FirstNormalTransactionId into the top of the (unsigned) xid range. Unless someone objects, I will add a C comment about this behavior so future readers are not confused. OK, now I think it is wrong. :-) The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. The XID space is *circular*. -- 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
[HACKERS] Windows build issues
I was trying to build the Windows msvc build for the first time and ran into some issues. The documentation talks about obtaining and using the Platform SDK. I understand that this is nowadays called the Windows SDK. Searching for the former on the Microsoft download site doesn't offer anything recent. If my understanding is correct, the documentation should be updated. The file src/tools/msvc/README contains some information that apparently augments the build instructions in the main documentation. It talks about Visual Studio 2005, which is probably outdated, and the section Notes about Visual Studio Express talks about some files that my installation doesn't contain, so probably also outdated. That last section should probably be moved into the main documentation, if it's still relevant. Between the main documentation and that README, it also becomes less clear whether the SDK or Visual Studio Express or both are needed. Then the real issue. The build.pl tries to call vcbuild, which neither the SDK version 7.1 nor the Visual Studio Express 2010 contain anywhere. (There is a vcbuild.dll, but that's it.) Some web searching suggests that vcbuild.exe might have been renamed in the Visual Studio 2010 version. I don't see any buildfarm coverage of that version. So is 2010 supported, and where is the vcbuild program supposed to come from? -- 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] corner case about replication and shutdown
[ sorry for not responding sooner ] On Wed, Mar 23, 2011 at 9:46 AM, Fujii Masao masao.fu...@gmail.com wrote: When I read the shutdown code to create the smart shutdown patch for sync rep, I found the corner case where shutdown can get stuck infinitely. This happens when postmaster reaches PM_WAIT_BACKENDS state before walsender marks itself as WAL sender process for streaming WAL (i.e., before walsender calls MarkPostmasterChildWalSender). In this case,CountChildren(NORMAL) in PostmasterStateMachine() returns non-zero because normal backend (i.e., would-be walsender) is running, and postmaster in PM_WAIT_BACKENDS state gets out of PostmasterStateMachine(). Then the backend receives START_REPLICATION command, declares itself as walsender and CountChildren(NORMAL) returns zero. The problem is; that declaration doesn't trigger PostmasterStateMachine() at all. So, even though there is no normal backends, postmaster cannot call PostmasterStateMachine() and move its state from PM_WAIT_BACKENDS. Good catch. I think this problem is harmless in practice since it doesn't happen too often. But that can happen... The simple fix is to change ServerLoop() so that it periodically calls PostmasterStateMachine() while shutdown is running. One idea I had was to have a backend that changes state from regular backend to walsender kick the postmaster in some way - for example by writing to a socket the other end of which the postmaster is holding open. Florian suggested that might be useful anyway as a means of detecting when the postmaster has gone belly-up, so maybe we could kill two birds with one stone. That seems like too much rejiggering to do this late in the release cycle, though. But I don't think the idea of calling PostmasterStateMachine() periodically is very appealing either - that's a significant change in how that code is being used now, and even if it doesn't break anything else, it'll allow for hangs of up to 60 seconds, which doesn't sound exciting either. The root of this problem in some sense is that we don't distinguish between regular backends and backends that haven't yet decided whether they are regular backends or walsenders. But even if we created such a distinction it won't fix the problem unless the postmaster somehow gets notified of the state change. And if we have that, then we're back to not needing to distinguish. Anyone have a good idea? -- 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] Windows build issues
On 03/31/2011 03:38 PM, Peter Eisentraut wrote: So is 2010 supported, and where is the vcbuild program supposed to come from? Not that I know of. But VS 2008 is, and should be readily available. In fact, I recently made patches to allow it to to be used to build all the live branches. 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] wal_buffers = -1 and SIGHUP
On Thu, Mar 31, 2011 at 8:38 AM, Bernd Helmle maili...@oopsware.de wrote:
This might be nitpicking (or i'm currently missing something), but i
recognized that setting wal_buffers = -1 always triggers the following on
reload, even if nothing to wal_buffers had changed:
$ pg_ctl reload
LOG: received SIGHUP, reloading configuration files
LOG: parameter wal_buffers cannot be changed without restarting the
server
This only happens when you have wal_buffers set to -1.
This is a bug. The root cause is that, on startup, we do this:
if (xbuffers != XLOGbuffers)
{
snprintf(buf, sizeof(buf), %d, xbuffers);
SetConfigOption(wal_buffers, buf, PGC_POSTMASTER,
PGC_S_OVERRIDE);
}
I had intended to commit Greg's patch with a show hook and an assign
hook and the calculated value stored separately from the GUC variable,
which I believe would avoid this is a problem, but Tom thought this
way was better. Unfortunately, my knowledge of the GUC machinery is
insufficient to think of a way to avoid it, other than the one Tom
already rejected.
--
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] Windows build issues
No, 2010 is not yet supported, though I beleive some initial work was done. I'm intending to work on it for 9.2. On 3/31/11, Peter Eisentraut pete...@gmx.net wrote: I was trying to build the Windows msvc build for the first time and ran into some issues. The documentation talks about obtaining and using the Platform SDK. I understand that this is nowadays called the Windows SDK. Searching for the former on the Microsoft download site doesn't offer anything recent. If my understanding is correct, the documentation should be updated. The file src/tools/msvc/README contains some information that apparently augments the build instructions in the main documentation. It talks about Visual Studio 2005, which is probably outdated, and the section Notes about Visual Studio Express talks about some files that my installation doesn't contain, so probably also outdated. That last section should probably be moved into the main documentation, if it's still relevant. Between the main documentation and that README, it also becomes less clear whether the SDK or Visual Studio Express or both are needed. Then the real issue. The build.pl tries to call vcbuild, which neither the SDK version 7.1 nor the Visual Studio Express 2010 contain anywhere. (There is a vcbuild.dll, but that's it.) Some web searching suggests that vcbuild.exe might have been renamed in the Visual Studio 2010 version. I don't see any buildfarm coverage of that version. So is 2010 supported, and where is the vcbuild program supposed to come from? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: 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] Windows build issues
On Thu, Mar 31, 2011 at 22:00, Andrew Dunstan and...@dunslane.net wrote: On 03/31/2011 03:38 PM, Peter Eisentraut wrote: So is 2010 supported, and where is the vcbuild program supposed to come from? Not that I know of. But VS 2008 is, and should be readily available. In fact, I recently made patches to allow it to to be used to build all the live branches. Yeah, we had an initial patch for VS2010 support, but it wasn't quite finished and it was decided to wait until 9.2. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Windows build issues
On tor, 2011-03-31 at 16:00 -0400, Andrew Dunstan wrote: On 03/31/2011 03:38 PM, Peter Eisentraut wrote: So is 2010 supported, and where is the vcbuild program supposed to come from? Not that I know of. But VS 2008 is, and should be readily available. In fact, I recently made patches to allow it to to be used to build all the live branches. The documentation appears to claim that the Platform/Windows SDK without any Visual Studio should be enough. Is there also an upper limit on the supported SDK version then? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in autovacuum.c?
Robert Haas wrote: On Thu, Mar 31, 2011 at 2:59 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. ?Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. ?Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). I am now thinking the code is correct --- it maps values from 0 to FirstNormalTransactionId into the top of the (unsigned) xid range. Unless someone objects, I will add a C comment about this behavior so future readers are not confused. OK, now I think it is wrong. ? :-) The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. The XID space is *circular*. Right but you would think that as the xid moves forward, the caculation of how far back to vacuum should move only forward. In this case, incrementing the xid by one would cause the vacuum horizon to move backward by two. -- 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] Problem with pg_upgrade?
Bruce Momjian wrote: Robert Haas wrote: On Thu, Mar 31, 2011 at 12:11 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Thu, Mar 31, 2011 at 11:32 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: ?I think the maintenance overhead of an invisible variable is too much. A simple GUC or command-line switch isn't much code. I like the idea of a command-line switch. If you want to do that you should gereralize it as --binary-upgrade in case we have other needs for it. Yeah. Or we could do a binary_upgrade GUC which has the effect of forcibly suppressing autovacuum, and maybe other things later. I think that's a lot less hazardous than fiddling with the autovacuum GUC. I like the idea of a command-line flag because it forces everything to be affected, and cannot be turned on and off in sessions --- if you are doing a binary upgrade, locked-down is good. :-) Someone emailed me mentioning we might want to use this flag to cause the server to use only local connections or lock it down in some other way in the future. -- 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] Bug in autovacuum.c?
On Thu, Mar 31, 2011 at 4:16 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: On Thu, Mar 31, 2011 at 2:59 PM, Bruce Momjian br...@momjian.us wrote: Bruce Momjian wrote: Yeah, I think this change would have the effect of moving the freeze limit by one (or two?) counts. ?Given the moving nature of values returned by ReadNewTransactionId this would probably have no practical effect. ?Still, the code as is seems more natural to me (Tom wrote this bit IIRC, not me). I am now thinking the code is correct --- it maps values from 0 to FirstNormalTransactionId into the top of the (unsigned) xid range. Unless someone objects, I will add a C comment about this behavior so future readers are not confused. OK, now I think it is wrong. ? :-) The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. The XID space is *circular*. Right but you would think that as the xid moves forward, the caculation of how far back to vacuum should move only forward. In this case, incrementing the xid by one would cause the vacuum horizon to move backward by two. I don't see how that would happen. The XID immediately preceding FirstNormalTransactionId is 2^32-1, and that's exactly what this calculation produces. -- 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
[HACKERS] cast from integer to money
On the open items list, we have: conversion from integer literals to money type http://archives.postgresql.org/pgsql-testers/2011-01/msg0.php What this is really complaining about is that we added a cast from numeric to money, but not from integer to money. This isn't really a bug: the fact that we added one cast doesn't oblige us to add two. On the other hand, the change is probably harmless and straightforward, and might reduce user confusion. Right now: rhaas=# select 1::money; ERROR: cannot cast type integer to money LINE 1: select 1::money; ^ rhaas=# select 1.0::money; money --- $1.00 (1 row) Does anyone care enough about this to put in the effort to fix it, or should we just let it go? Does anyone see a reason why we wouldn't want to do this, if someone's motivated to code it up? -- 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
[HACKERS] pg_upgrade exit_nicely()
While reading around in pg_upgrade code I came across the slightly
bizarre function
void exit_nicely(bool need_cleanup)
The parameter doesn't actually determine whether any cleanup is done.
The cleanup is done anyway and the parameter only controls the exit
code in a backwards way.
Also most of the cleanup appears to be useless, because you don't need
to close files or free memory before the program exits.
I figured this could be written more cleanly with an exit hook, so here
is a patch. I don't care much whether this patch is for now or later,
just wanted to throw it out there.
diff --git i/contrib/pg_upgrade/check.c w/contrib/pg_upgrade/check.c
index 1c4847a..05aac8f 100644
--- i/contrib/pg_upgrade/check.c
+++ w/contrib/pg_upgrade/check.c
@@ -131,7 +131,7 @@ report_clusters_compatible(void)
pg_log(PG_REPORT, \n*Clusters are compatible*\n);
/* stops new cluster */
stop_postmaster(false, false);
- exit_nicely(false);
+ exit(0);
}
pg_log(PG_REPORT, \n
diff --git i/contrib/pg_upgrade/option.c w/contrib/pg_upgrade/option.c
index c984535..857d652 100644
--- i/contrib/pg_upgrade/option.c
+++ w/contrib/pg_upgrade/option.c
@@ -77,12 +77,12 @@ parseCommandLine(int argc, char *argv[])
strcmp(argv[1], -?) == 0)
{
usage();
- exit_nicely(false);
+ exit(0);
}
if (strcmp(argv[1], --version) == 0 || strcmp(argv[1], -V) == 0)
{
pg_log(PG_REPORT, pg_upgrade PG_VERSION \n);
- exit_nicely(false);
+ exit(0);
}
}
@@ -125,7 +125,7 @@ parseCommandLine(int argc, char *argv[])
if ((log_opts.debug_fd = fopen(optarg, w)) == NULL)
{
pg_log(PG_FATAL, cannot open debug file\n);
- exit_nicely(false);
+ exit(1);
}
break;
@@ -141,7 +141,7 @@ parseCommandLine(int argc, char *argv[])
if ((old_cluster.port = atoi(optarg)) = 0)
{
pg_log(PG_FATAL, invalid old port number\n);
- exit_nicely(false);
+ exit(1);
}
break;
@@ -149,7 +149,7 @@ parseCommandLine(int argc, char *argv[])
if ((new_cluster.port = atoi(optarg)) = 0)
{
pg_log(PG_FATAL, invalid new port number\n);
- exit_nicely(false);
+ exit(1);
}
break;
diff --git i/contrib/pg_upgrade/pg_upgrade.h w/contrib/pg_upgrade/pg_upgrade.h
index 4461952..8f72ea8 100644
--- i/contrib/pg_upgrade/pg_upgrade.h
+++ w/contrib/pg_upgrade/pg_upgrade.h
@@ -363,7 +363,6 @@ void check_for_libpq_envvars(void);
/* util.c */
-void exit_nicely(bool need_cleanup);
char *quote_identifier(const char *s);
int get_user_info(char **user_name);
void check_ok(void);
diff --git i/contrib/pg_upgrade/server.c w/contrib/pg_upgrade/server.c
index 4d5f373..a155f90 100644
--- i/contrib/pg_upgrade/server.c
+++ w/contrib/pg_upgrade/server.c
@@ -23,7 +23,7 @@ static bool test_server_conn(ClusterInfo *cluster, int timeout);
*
* Connects to the desired database on the designated server.
* If the connection attempt fails, this function logs an error
- * message and calls exit_nicely() to kill the program.
+ * message and calls exit() to kill the program.
*/
PGconn *
connectToServer(ClusterInfo *cluster, const char *db_name)
@@ -45,7 +45,8 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
if (conn)
PQfinish(conn);
- exit_nicely(true);
+ printf(Failure, exiting\n);
+ exit(1);
}
return conn;
@@ -57,7 +58,7 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
*
* Formats a query string from the given arguments and executes the
* resulting query. If the query fails, this function logs an error
- * message and calls exit_nicely() to kill the program.
+ * message and calls exit() to kill the program.
*/
PGresult *
executeQueryOrDie(PGconn *conn, const char *fmt,...)
@@ -81,8 +82,8 @@ executeQueryOrDie(PGconn *conn, const char *fmt,...)
PQerrorMessage(conn));
PQclear(result);
PQfinish(conn);
- exit_nicely(true);
- return NULL; /* Never get here, but keeps compiler happy */
+ printf(Failure, exiting\n);
+ exit(1);
}
else
return result;
@@ -152,6 +153,18 @@ get_major_server_version(ClusterInfo *cluster)
}
+static void
+#ifdef HAVE_ATEXIT
+stop_postmaster_exit_hook(void)
+#else
+stop_postmaster_exit_hook(int exitstatus, void *arg)
+#endif
+{
+ stop_postmaster(true, true);
+
+}
+
+
void
start_postmaster(ClusterInfo *cluster, bool quiet)
{
@@ -159,11 +172,22 @@ start_postmaster(ClusterInfo *cluster, bool quiet)
const char *bindir;
const char *datadir;
unsigned short port;
+ bool exit_hook_registered = false;
bindir = cluster-bindir;
datadir = cluster-pgdata;
port = cluster-port;
+ if (!exit_hook_registered)
+ {
+#ifdef HAVE_ATEXIT
+ atexit(stop_postmaster_exit_hook);
+#else
+ on_exit(stop_postmaster_exit_hook);
+#endif
+ exit_hook_registered = true;
+ }
+
/*
* On Win32, we can't send both pg_upgrade output and pg_ctl output to the
* same file because we get the error: The process cannot access the file
diff --git i/contrib/pg_upgrade/util.c
Re: [HACKERS] Bug in autovacuum.c?
Robert Haas wrote: The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. The XID space is *circular*. Right but you would think that as the xid moves forward, the caculation of how far back to vacuum should move only forward. ?In this case, incrementing the xid by one would cause the vacuum horizon to move backward by two. I don't see how that would happen. The XID immediately preceding FirstNormalTransactionId is 2^32-1, and that's exactly what this calculation produces. OK, let me see if I understand --- the caculation is below: xidForceLimit = recentXid - autovacuum_freeze_max_age; if (xidForceLimit FirstNormalTransactionId) xidForceLimit -= FirstNormalTransactionId; The values: xidForceLimit Result --- max_xid-2 max_xid-2 max_xid-1 max_xid-1 max_xid max_xid 0 max_xid-3 - backward here 1 max_xid-2 2 max_xid-1 3 3 With the -= change to =, we get: xidForceLimit Result --- max_xid-2 max_xid-2 max_xid-1 max_xid-1 max_xid max_xid 0 3 1 3 2 3 3 3 -- 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
[HACKERS] GSoC 2011 Eager MV implementation proposal
Title: Implementation of Eager Materialized views in postgres Name of Proposer Email : Aamir Khan ak4u2...@gmail.com Synopsis: I would like to implement eager materialized view.An eager materialized view will be updated whenever the view changes. This is done with a system of triggers on all of the underlying tables. Benefits to the PostgreSQL Community: First of all, it would be the best if my work is helpful to everybody who misses materialized views in PostgreSQL, because PostgreSQL do not have still implemented materialized views. In addition, MV is mentioned as the top rated feature in TODO list. Deliverables: First of all, at the end of whole my project is not only finishing the patch if possible, get patch into next PostgreSQL release, or keep git repository actual to last PosgreSQL version. Bio I am student of one of the most premier institute in India namely Indian Institute of Technology Roorkee pursuing my Bachelor of Technology Computer Science Engineering. I, as a part of team manages institutes website www.iitr.ac.in . We have setup a intranet portal inside campus of IIT Rookee which has around 80 applications hosted on it. I have created ebooks sharing portal within the intranet. I have very good experience in web designing (e.g, I have created website during last year summer internship www.raysconsultants.com ). Regarding open source work : i have submitted many patches for phpmyadmin I like to contribute in open source communitites because it gives me experience,technical expertise and it teaches spirit of team work. And its my time to give back to community from which i gained a lot of experience and motivation. Project plan I am completely free this summer. I intend to indulge in some of my hobbies this summer, like Squash etc, leaving me plenty of time to complete my project. The tentative schedule as discussed with my mentor is : (1) Identify how an eager MV system would work, by labelling and identifying every table, query, trigger, etc... This would be a written document that has been checked by me for completeness, precision, and correctness. (1 week) (2) Identify the internal structure of a parsed query, the basic atomic parts of this structure, and how each atomic structure maps precisely to our MV system described in (1). Again, a written document approved by myself, and reviewed by the PostgreSQL community. (2 weeks) (3) Outline how a create_eager_mv function would work, starting with the interface (what parameters it takes), and working through the exact SQL commands that must be executed to complete the process. Identify which SQL commands depend on the structure of the query. (2 weeks) (4) Write unit tests that check a variety of queries against your (as of yet, unwritten) function. Ensure completeness by reviewing with me. (1 week) (5) Finally, given all of the above, translate (3) into working code, likely implemented in PL/Python or C, that passes the unit tests. Submit to PostgreSQL community for review. (2 weeks) Contact Information Name: Aamir Khan Country: Indian School and degree: Indian Institute of Technology, Roorkee, Bachelor of Technology Email: ak4u2...@gmail.com Phone: +91-9557647357
Re: [HACKERS] pg_upgrade exit_nicely()
Feel free to apply this to HEAD. --- Peter Eisentraut wrote: While reading around in pg_upgrade code I came across the slightly bizarre function void exit_nicely(bool need_cleanup) The parameter doesn't actually determine whether any cleanup is done. The cleanup is done anyway and the parameter only controls the exit code in a backwards way. Also most of the cleanup appears to be useless, because you don't need to close files or free memory before the program exits. I figured this could be written more cleanly with an exit hook, so here is a patch. I don't care much whether this patch is for now or later, just wanted to throw it out there. [ Attachment, skipping... ] -- 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] cast from integer to money
Robert Haas robertmh...@gmail.com wrote: On the open items list, we have: conversion from integer literals to money type http://archives.postgresql.org/pgsql-testers/2011-01/msg0.php What this is really complaining about is that we added a cast from numeric to money, but not from integer to money. This isn't really a bug: the fact that we added one cast doesn't oblige us to add two. On the other hand, the change is probably harmless and straightforward, and might reduce user confusion. There were reasonable arguments made why this could be a bad idea -- primarily around the question of whether '395' represented $3.95 or $395.00. Going the other way has issues with truncation of fractions and the number of digits which can be handled. I'm not convinced it's sane, and I feel strongly it's too late in the cycle to try to implement. -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] proposal: a validator for configuration files
Hi Selena, On Mar 30, 2011, at 11:42 PM, Selena Deckelmann wrote: Hi! On Wed, Mar 30, 2011 at 8:40 AM, Alexey Klyukin al...@commandprompt.com wrote: I did a little bit of work on this, and we discussed it here: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00345.php http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php Probably there's a bit of bitrot in there. Cool, I was not aware of your work in this direction. I've updated your patch to apply to the latest HEAD, implementing Tom Lane's suggestions (attached). I think I'll implement the other part (reporting all invalid parameters, as opposed to only the first one) tomorrow. The development plan consists of 2 parts. The first one is to add new code that would allow running the checks in both a stand-alone process, when postmaster is not running, and as a function call from a backend postgres process. Initially the code would simply loads configuration files, without performing any of the validation checks. The second part consists of adding specific checks. Cool! Mine was only going to work if the system started up or was reloaded. Well, I think a stand-alone check is an easy part :) As I said above, some of what you've suggested seems more like a non-postgres core thing.. maybe an extension? Or maybe offer the option to read Well, initially I'm going to start with just a check that configuration files are valid, and add other checks afterwards. I think it makes sense for them to be optional. My idea was to just check that settings were *valid* not that they met some other, more subjective criteria. Well, my definition of valid configuration is not only the one that server is able to parse and load, but also to actually apply (i.e. can bind to a listen_address or read SSL certificate files). I agree that's not always necessary (i.e. when checking configuration on a different server than the one it should be applied to), so we can add a flag to turn them off. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. parser_continue_on_errors.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
Re: [HACKERS] cast from integer to money
On Thu, Mar 31, 2011 at 4:58 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Robert Haas robertmh...@gmail.com wrote: On the open items list, we have: conversion from integer literals to money type http://archives.postgresql.org/pgsql-testers/2011-01/msg0.php What this is really complaining about is that we added a cast from numeric to money, but not from integer to money. This isn't really a bug: the fact that we added one cast doesn't oblige us to add two. On the other hand, the change is probably harmless and straightforward, and might reduce user confusion. There were reasonable arguments made why this could be a bad idea -- primarily around the question of whether '395' represented $3.95 or $395.00. That's not too hard to figure out, right? If 1.00 means $1.00, 1 had better not mean $0.01, or there will be riots in the streets. Going the other way has issues with truncation of fractions and the number of digits which can be handled. Notice I didn't propose that. I'm not convinced it's sane, and I feel strongly it's too late in the cycle to try to implement. Fair enough. Any contrary votes? -- 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] Bug in autovacuum.c?
On Thu, Mar 31, 2011 at 4:35 PM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: The effect is to map max xid + 1 to max xid - FirstNormalTransactionId(3) + 1, which makes the xid look like it is going backwards, less than max xid --- not good. The XID space is *circular*. Right but you would think that as the xid moves forward, the caculation of how far back to vacuum should move only forward. ?In this case, incrementing the xid by one would cause the vacuum horizon to move backward by two. I don't see how that would happen. The XID immediately preceding FirstNormalTransactionId is 2^32-1, and that's exactly what this calculation produces. OK, let me see if I understand --- the caculation is below: xidForceLimit = recentXid - autovacuum_freeze_max_age; if (xidForceLimit FirstNormalTransactionId) xidForceLimit -= FirstNormalTransactionId; The values: xidForceLimit Result --- max_xid-2 max_xid-2 max_xid-1 max_xid-1 max_xid max_xid 0 max_xid-3 - backward here 1 max_xid-2 2 max_xid-1 3 3 You have to consider those three lines all of a piece. Suppose autovacuum_freeze_age is 100. Then: 105 - 5 104 - 4 103 - 3 102 - max_xid 101 - max_xid - 1 100 - max_xid - 2 -- 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] Bug in autovacuum.c?
Robert Haas wrote: ? ?xidForceLimit = recentXid - autovacuum_freeze_max_age; ? ?if (xidForceLimit FirstNormalTransactionId) ? ? ? ?xidForceLimit -= FirstNormalTransactionId; The values: ? ? ? ?xidForceLimit ? Result ? ? ? ?--- ? ? ? ?max_xid-2 ? ? ? max_xid-2 ? ? ? ?max_xid-1 ? ? ? max_xid-1 ? ? ? ?max_xid ? ? ? ? max_xid ? ? ? ?0 ? ? ? ? ? ? ? max_xid-3 ? ? ? - backward here ? ? ? ?1 ? ? ? ? ? ? ? max_xid-2 ? ? ? ?2 ? ? ? ? ? ? ? max_xid-1 ? ? ? ?3 ? ? ? ? ? ? ? 3 You have to consider those three lines all of a piece. Suppose autovacuum_freeze_age is 100. Then: 105 - 5 104 - 4 103 - 3 102 - max_xid 101 - max_xid - 1 100 - max_xid - 2 OK, just keep going below 100: 105 - 5 104 - 4 103 - 3 102 - max_xid 101 - max_xid - 1 100 - max_xid - 2 99 - max_id 98 - max_id -1 Wouldn't you rather: 105 - 5 104 - 4 103 - 3 102 - 3 101 - 3 100 - 3 99 - max_id 98 - max_id -1 -- 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] cast from integer to money
Robert Haas robertmh...@gmail.com wrote: There were reasonable arguments made why this could be a bad idea -- primarily around the question of whether '395' represented $3.95 or $395.00. That's not too hard to figure out, right? If 1.00 means $1.00, 1 had better not mean $0.01, or there will be riots in the streets. Going the other way has issues with truncation of fractions and the number of digits which can be handled. Notice I didn't propose that. If you're just talking about going in the one direction, I might be persuaded that's sane, especially because of the case of literals, and especially since there are currencies where fractional amounts aren't used in the conventional representation. -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] Process local hint bit cache
On Wed, Mar 30, 2011 at 2:35 PM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Mar 30, 2011 at 11:23 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 30.03.2011 18:02, Robert Haas wrote: On Wed, Mar 30, 2011 at 10:40 AM, Greg Starkgsst...@mit.edu wrote: But one way or another the hint bits have to get set sometime. The sooner that happens the less clog i/o has to happen in the meantime. I talked about this with Merlin a bit yesterday. I think that his thought is that most transactions will access a small enough number of distinct CLOG pages, and the cache accesses might be fast enough, that we really wouldn't need to get the hint bits set, or at least that vacuum/freeze time would be soon enough. I'm not sure if that's actually true, though - I think the overhead of the cache might be higher than he's imagining. However, there's a sure-fire way to find out... code it up and see how it plays. I did a little experiment: I hacked SetHintBits() to do nothing, so that hint bits are never set. I then created a table with 10 rows in it: postgres=# \d foo Table public.foo Column | Type | Modifiers +-+--- a | integer | postgres=# INSERT INTO foo SELECT generate_series(1, 10); INSERT 0 10 And ran queries on the table: postgres=# do $$ declare i int4; begin loop perform COUNT(*) FROM foo; end loop; end; $$; This test case is designed to exercise the visibility tests as much as possible. However, all the tuples are loaded in one transaction, so the one-element cache in TransactionLogFetch is 100% effective. I ran oprofile on that. It shows that about 15% of the time is spent in HeapTupleSatisfiesMVCC and its subroutines. 6.6% is spent in HeapTupleSatisfiesMVCC itself. Here's the breakdown of that: $ opreport -c --global-percent CPU: Intel Architectural Perfmon, speed 2266 MHz (estimated) Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (No unit mask) count 10 samples % app name symbol name ... --- 2143 0.4419 postgres postgres heapgettup_pagemode 73277 15.1091 postgres postgres heapgetpage 31858 6.5688 postgres postgres HeapTupleSatisfiesMVCC 31858 6.5688 postgres postgres HeapTupleSatisfiesMVCC [self] 12809 2.6411 postgres postgres TransactionIdIsInProgress 12091 2.4931 postgres postgres XidInMVCCSnapshot 7150 1.4743 postgres postgres TransactionIdIsCurrentTransactionId 7056 1.4549 postgres postgres TransactionIdDidCommit 1839 0.3792 postgres postgres TransactionIdPrecedes 1467 0.3025 postgres postgres SetHintBits 1155 0.2382 postgres postgres TransactionLogFetch --- ... I then ran the same test again with an unpatched version, to set the hint bits. After the hint bits were set, I ran oprofile again: --- 275 0.4986 postgres heapgettup_pagemode 4459 8.0851 postgres heapgetpage 3005 5.4487 postgres HeapTupleSatisfiesMVCC 3005 5.4487 postgres HeapTupleSatisfiesMVCC [self] 1620 2.9374 postgres XidInMVCCSnapshot 110 0.1995 postgres TransactionIdPrecedes --- So with hint bits set, HeapTupleSatisfiesMVCC accounts for 8% of the total CPU time, and without hint bits, 15%. Even if clog access was free, hint bits still give a significant speedup thanks to skipping all the other overhead like TransactionIdIsInProgress() and TransactionIdIsCurrentTransactionId(). Speeding up clog access is important; when the one-element cache isn't saving you the clog access becomes a dominant factor. But you have to address that other overhead too before you can get rid of hint bits. Here is a patch demonstrating the caching action, but without the cache table, which isn't done yet -- It only works using the very last transaction id fetched. I used macros so I could keep the changes quite localized. While playing with the data structure to implement an xid cache inside TransactionLogFetch, I learned a nasty lesson. HeapTupleSatisifiesMVCC is super sensitive to any changes and even jumping through a couple of calls to TransactionLogFetch was measurable in a couple of performance test case I came up with. I hauntingly recalled Tom's warnings to beware unexpected performance downsides. In short, irregardless of implementation,
Re: [HACKERS] Process local hint bit cache
On Thu, Mar 31, 2011 at 5:33 PM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Mar 30, 2011 at 2:35 PM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Mar 30, 2011 at 11:23 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 30.03.2011 18:02, Robert Haas wrote: On Wed, Mar 30, 2011 at 10:40 AM, Greg Starkgsst...@mit.edu wrote: But one way or another the hint bits have to get set sometime. The sooner that happens the less clog i/o has to happen in the meantime. I talked about this with Merlin a bit yesterday. I think that his thought is that most transactions will access a small enough number of distinct CLOG pages, and the cache accesses might be fast enough, that we really wouldn't need to get the hint bits set, or at least that vacuum/freeze time would be soon enough. I'm not sure if that's actually true, though - I think the overhead of the cache might be higher than he's imagining. However, there's a sure-fire way to find out... code it up and see how it plays. I did a little experiment: I hacked SetHintBits() to do nothing, so that hint bits are never set. I then created a table with 10 rows in it: postgres=# \d foo Table public.foo Column | Type | Modifiers +-+--- a | integer | postgres=# INSERT INTO foo SELECT generate_series(1, 10); INSERT 0 10 And ran queries on the table: postgres=# do $$ declare i int4; begin loop perform COUNT(*) FROM foo; end loop; end; $$; This test case is designed to exercise the visibility tests as much as possible. However, all the tuples are loaded in one transaction, so the one-element cache in TransactionLogFetch is 100% effective. I ran oprofile on that. It shows that about 15% of the time is spent in HeapTupleSatisfiesMVCC and its subroutines. 6.6% is spent in HeapTupleSatisfiesMVCC itself. Here's the breakdown of that: $ opreport -c --global-percent CPU: Intel Architectural Perfmon, speed 2266 MHz (estimated) Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (No unit mask) count 10 samples % app name symbol name ... --- 2143 0.4419 postgres postgres heapgettup_pagemode 73277 15.1091 postgres postgres heapgetpage 31858 6.5688 postgres postgres HeapTupleSatisfiesMVCC 31858 6.5688 postgres postgres HeapTupleSatisfiesMVCC [self] 12809 2.6411 postgres postgres TransactionIdIsInProgress 12091 2.4931 postgres postgres XidInMVCCSnapshot 7150 1.4743 postgres postgres TransactionIdIsCurrentTransactionId 7056 1.4549 postgres postgres TransactionIdDidCommit 1839 0.3792 postgres postgres TransactionIdPrecedes 1467 0.3025 postgres postgres SetHintBits 1155 0.2382 postgres postgres TransactionLogFetch --- ... I then ran the same test again with an unpatched version, to set the hint bits. After the hint bits were set, I ran oprofile again: --- 275 0.4986 postgres heapgettup_pagemode 4459 8.0851 postgres heapgetpage 3005 5.4487 postgres HeapTupleSatisfiesMVCC 3005 5.4487 postgres HeapTupleSatisfiesMVCC [self] 1620 2.9374 postgres XidInMVCCSnapshot 110 0.1995 postgres TransactionIdPrecedes --- So with hint bits set, HeapTupleSatisfiesMVCC accounts for 8% of the total CPU time, and without hint bits, 15%. Even if clog access was free, hint bits still give a significant speedup thanks to skipping all the other overhead like TransactionIdIsInProgress() and TransactionIdIsCurrentTransactionId(). Speeding up clog access is important; when the one-element cache isn't saving you the clog access becomes a dominant factor. But you have to address that other overhead too before you can get rid of hint bits. Here is a patch demonstrating the caching action, but without the cache table, which isn't done yet -- It only works using the very last transaction id fetched. I used macros so I could keep the changes quite localized. While playing with the data structure to implement an xid cache inside TransactionLogFetch, I learned a nasty lesson. HeapTupleSatisifiesMVCC is super sensitive to any changes and even jumping through a couple of calls to TransactionLogFetch was measurable in a couple of performance test case I came up with. I hauntingly recalled Tom's warnings to
Re: [HACKERS] cast from integer to money
* Kevin Grittner (kevin.gritt...@wicourts.gov) wrote: If you're just talking about going in the one direction, I might be persuaded that's sane, especially because of the case of literals, and especially since there are currencies where fractional amounts aren't used in the conventional representation. Going just integer-money, with the 1 - $1.00, seems completely reasonable to me. As for being too late in the cycle.. if someone's willing to do the work, I can't imagine it breaking anything, so I wouldn't be against putting it in. It really should be before the first beta tho. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] GSoC 2011 Eager MV implementation proposal
On 03/31/2011 04:38 PM, AAMIR KHAN wrote: I would like to implement eager materialized view.An eager materialized view will be updated whenever the view changes. This is done with a system of triggers on all of the underlying tables. Last summer someone worked on snapshot materialized views. That didn't result in anything that could be committed. If you wanted to work on trying to actually finish that, that might go somewhere useful. There are a number of hard problems in getting a working implementation of materialized views that all get ignored by all of the student proposals we get, and what you're talking about doesn't address any of them. You really should read all of the messages in the following threads: http://archives.postgresql.org/pgsql-hackers/2010-04/msg00479.php http://archives.postgresql.org/pgsql-hackers/2010-06/msg00743.php http://archives.postgresql.org/pgsql-hackers/2010-07/msg00396.php And the following summaries: http://wiki.postgresql.org/wiki/Materialized_Views_GSoC_2010 http://rhaas.blogspot.com/2010/04/materialized-views-in-postgresql.html And then say how what you're suggesting fits into the issues raised last summer. The theory and way to implement eager MVs are interesting problems. But working on them won't lead toward code that can be committed to PostgreSQL this year. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
[HACKERS] Should psql support URI syntax?
Hello, Most database connectors/frameworks nowadays support a URI style connection string. Something like: pgsql://user:pass@host/database Do we think psql should support this style of connection string? Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- 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] Should psql support URI syntax?
On 03/31/2011 07:25 PM, Joshua D. Drake wrote: Hello, Most database connectors/frameworks nowadays support a URI style connection string. Something like: pgsql://user:pass@host/database Do we think psql should support this style of connection string? Syntactic sugar aside, what is the advantage of that over a conninfo string? 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] Should psql support URI syntax?
On Thu, 2011-03-31 at 19:32 -0400, Andrew Dunstan wrote: On 03/31/2011 07:25 PM, Joshua D. Drake wrote: Hello, Most database connectors/frameworks nowadays support a URI style connection string. Something like: pgsql://user:pass@host/database Do we think psql should support this style of connection string? Syntactic sugar aside, what is the advantage of that over a conninfo string? I would think it would be purely syntatic sugar really, which does incorporate a familiar interface for those who are working in different worlds (.Net/Drupal/JAVA) etc... Perhaps programability would also be useful from a shell/system script perspective. Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- 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] 2nd Level Buffer Cache
On 03/24/2011 03:36 PM, Jim Nasby wrote: On Mar 23, 2011, at 5:12 PM, Tom Lane wrote: Robert Haasrobertmh...@gmail.com writes: It looks like the only way anything can ever get put on the free list right now is if a relation or database is dropped. That doesn't seem too good. Why not? AIUI the free list is only for buffers that are totally dead, ie contain no info that's possibly of interest to anybody. It is *not* meant to substitute for running the clock sweep when you have to discard a live buffer. Turns out we've had this discussion before: http://archives.postgresql.org/pgsql-hackers/2010-12/msg01088.php and http://archives.postgresql.org/pgsql-hackers/2010-12/msg00689.php Investigating this has been on the TODO list for four years now: http://archives.postgresql.org/pgsql-hackers/2007-04/msg00781.php I feel that work in this area is blocked behind putting together a decent mix of benchmarks that can be used to test whether changes here are actually good or bad. All of the easy changes to buffer allocation strategy, ones that you could verify by inspection and simple tests, were made in 8.3. The stuff that's left has the potential to either improve or reduce performance, and which will happen is very workload dependent. Setting up systematic benchmarks of multiple workloads to run continuously on big hardware is a large, boring, expensive problem that few can justify financing (except for Jim of course), and even fewer want to volunteer time toward. This whole discussion of cache policy tweaks is fun, but I just delete all the discussion now because it's just going in circles without a good testing regime. The right way to start is by saying this is the benchmark I'm going to improve with this change, and it has a profiled hotspot at this point. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Foreign table permissions and cloning
Hi, I've noticed some weirdness when trying to grant various types of permissions on a foreign table and thought I'd report it here: postgres=# \d stuff Foreign table public.stuff Column | Type | Modifiers +-+--- id | integer | colour | text| animal | text| Server: file postgres=# GRANT SELECT (colour) ON FOREIGN TABLE stuff TO user_a; ERROR: column privileges are only valid for relations postgres=# GRANT SELECT (colour) ON TABLE stuff TO user_a; GRANT postgres=# GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_a; ERROR: syntax error at or near FOREIGN LINE 1: GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_... ^ Granting select for all tables in a schema to a user will affect foreign tables however. And column-level permissions work with foreign tables if you refer to them as regular tables in the GRANT/REVOKE statement. Using the term FOREIGN TABLE in a GRANT statement isn't documented. I suspect this will need its own entry in the syntax definition section of the GRANT and REVOKE reference pages. I also noticed this doesn't work: postgres=# CREATE TABLE animals (LIKE stuff); ERROR: inherited relation stuff is not a table Since LIKE doesn't maintain any sort of link with the table like INHERITS does, it would be nice if this could work in future. Thanks -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: 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] Should psql support URI syntax?
I would think it would be purely syntatic sugar really, which does incorporate a familiar interface for those who are working in different worlds (.Net/Drupal/JAVA) etc... I wouldn't mind having something more standard supported; I'm always looking up the conninfo for the options I don't use frequently. However, is there any standard for database URIs? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com San Francisco -- 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] Foreign table permissions and cloning
On 1 April 2011 00:54, Thom Brown t...@linux.com wrote: Hi, I've noticed some weirdness when trying to grant various types of permissions on a foreign table and thought I'd report it here: postgres=# \d stuff Foreign table public.stuff Column | Type | Modifiers +-+--- id | integer | colour | text | animal | text | Server: file postgres=# GRANT SELECT (colour) ON FOREIGN TABLE stuff TO user_a; ERROR: column privileges are only valid for relations postgres=# GRANT SELECT (colour) ON TABLE stuff TO user_a; GRANT postgres=# GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_a; ERROR: syntax error at or near FOREIGN LINE 1: GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_... ^ Granting select for all tables in a schema to a user will affect foreign tables however. And column-level permissions work with foreign tables if you refer to them as regular tables in the GRANT/REVOKE statement. Using the term FOREIGN TABLE in a GRANT statement isn't documented. I suspect this will need its own entry in the syntax definition section of the GRANT and REVOKE reference pages. I also noticed this doesn't work: postgres=# CREATE TABLE animals (LIKE stuff); ERROR: inherited relation stuff is not a table Since LIKE doesn't maintain any sort of link with the table like INHERITS does, it would be nice if this could work in future. Also, there probably needs to be some elaboration of how a NOT NULL declaration operates on a foreign table column on the CREATE FOREIGN TABLE reference page. From what I can see, if the foreign table cannot be modified such as those defined using the file_fdw handler, it bears no relevance, and if the foreign table can be written to, it won't prevent NULL values being returned if they're already in there, just prevent them being entered (presumably). It also won't validate data in the writable foreign table upon its creation. And another problem I've found is if you try to create a table named the same as a foreign table, and you use the IF NOT EXISTS clause: postgres=# CREATE TABLE IF NOT EXISTS animals (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence animals_id_seq1 for serial column animals.id NOTICE: relation animals already exists, skipping CREATE TABLE postgres=# CREATE TABLE IF NOT EXISTS stuff (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence stuff_id_seq for serial column stuff.id NOTICE: relation stuff already exists, skipping ERROR: referenced relation stuff is not a table The reverse doesn't error though, where you attempt to create a foreign table named the same as a regular table using IF NOT EXISTS. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: 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] Should psql support URI syntax?
An advantage to this uri form is that it allows applications to be configured uniformly - I do not need to ask is this using libpq, needing one sort of configuration, or Java, needing another? Rather, I may say, here is a uri I may use with any of my applications
Re: [HACKERS] Should psql support URI syntax?
On Thu, 2011-03-31 at 19:10 -0500, Joshua Berkus wrote: I would think it would be purely syntatic sugar really, which does incorporate a familiar interface for those who are working in different worlds (.Net/Drupal/JAVA) etc... I wouldn't mind having something more standard supported; I'm always looking up the conninfo for the options I don't use frequently. However, is there any standard for database URIs? There is an IETF RFC for generic URI: http://www.ietf.org/rfc/rfc2396.txt http://www.ietf.org/rfc/rfc3986.txt Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bug in autovacuum.c?
On Thu, Mar 31, 2011 at 10:59 PM, Bruce Momjian br...@momjian.us wrote: OK, just keep going below 100: 105 - 5 104 - 4 103 - 3 102 - max_xid 101 - max_xid - 1 100 - max_xid - 2 99 - max_id 98 - max_id -1 Yeah, I think this is what the code is doing. Wouldn't you rather: 105 - 5 104 - 4 103 - 3 102 - 3 101 - 3 100 - 3 99 - max_id 98 - max_id -1 I think I would expect 105 - 5 104 - 4 103 - 3 102 - max_id 101 - max_id-1 100 - max_id-2 99 - max_id-3 But it doesn't really matter either way, does it? We don't even allow setting vacuum_max_freeze_age to 2^31-1 or any value that would be close to triggering a problem here. -- 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] cast from integer to money
On Mar 31, 2011, at 6:39 PM, Stephen Frost sfr...@snowman.net wrote: * Kevin Grittner (kevin.gritt...@wicourts.gov) wrote: If you're just talking about going in the one direction, I might be persuaded that's sane, especially because of the case of literals, and especially since there are currencies where fractional amounts aren't used in the conventional representation. Going just integer-money, with the 1 - $1.00, seems completely reasonable to me. As for being too late in the cycle.. if someone's willing to do the work, I can't imagine it breaking anything, so I wouldn't be against putting it in. It really should be before the first beta tho. Agreed, emphatically. ...Robert -- 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] corner case about replication and shutdown
On Fri, Apr 1, 2011 at 4:48 AM, Robert Haas robertmh...@gmail.com wrote: I think this problem is harmless in practice since it doesn't happen too often. But that can happen... The simple fix is to change ServerLoop() so that it periodically calls PostmasterStateMachine() while shutdown is running. One idea I had was to have a backend that changes state from regular backend to walsender kick the postmaster in some way - for example by writing to a socket the other end of which the postmaster is holding open. Florian suggested that might be useful anyway as a means of detecting when the postmaster has gone belly-up, so maybe we could kill two birds with one stone. That seems like too much rejiggering to do this late in the release cycle, though. But I don't think the idea of calling PostmasterStateMachine() periodically is very appealing either - that's a significant change in how that code is being used now, and even if it doesn't break anything else, it'll allow for hangs of up to 60 seconds, which doesn't sound exciting either. The root of this problem in some sense is that we don't distinguish between regular backends and backends that haven't yet decided whether they are regular backends or walsenders. But even if we created such a distinction it won't fix the problem unless the postmaster somehow gets notified of the state change. And if we have that, then we're back to not needing to distinguish. Anyone have a good idea? Another simple fix is to make walsender send SIGUSR1 to postmaster so that it calls PostmasterStateMachine() in sigusr1_handler(), when it marks itself as walsender. The attached patch does this. Thought? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center sigusr1_kicks_state_machine_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
Re: [HACKERS] BUG #5856: pg_attribute.attinhcount is not correct.
On Thu, Mar 31, 2011 at 11:11:49AM -0400, Robert Haas wrote: On Thu, Mar 31, 2011 at 6:06 AM, Noah Misch n...@leadboat.com wrote: I think this is a manifestation the same problem mentioned here: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=31b6fc06d83c6de3644c8f2921eb7de0eb92fac3 I believe this requires some refactoring to fix. ?It would be good to do that. The best way I can see is to make ATExecAddColumn more like ATExecDropColumn, ATAddCheckConstraint, and ATExecDropConstraint. ?Namely, recurse at Exec-time rather than Prep-time, and cease recursing when we satisfy the ADD COLUMN with a merge. ?Did you have something else in mind? I had exactly what you just said in mind. Patch attached, then. Incidentally, when we satisfy an ADD COLUMN with a merge, we do not check or update attnotnull: details ... what should we do? Not sure. I think that anything we do here is bound to have some corner cases that are not quite right for so long as NOT NULL constraints aren't represented in pg_constraint, and it's way too late to dredge up that issue again for 9.1. I'm somewhat inclined to just defer fixing it until we get that work committed. OK. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 737ab1a..c64ffac 100644 *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *** *** 285,300 static void ATSimplePermissions(Relation rel, int allowed_targets); static void ATWrongRelkindError(Relation rel, int allowed_targets); static void ATSimpleRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode); - static void ATOneLevelRecursion(List **wqueue, Relation rel, - AlterTableCmd *cmd, LOCKMODE lockmode); static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static List *find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior behavior); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing, AlterTableCmd *cmd, LOCKMODE lockmode); ! static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel, ! ColumnDef *colDef, bool isOid, LOCKMODE lockmode); static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid, Oid collid); static void ATPrepAddOids(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd, LOCKMODE lockmode); --- 285,299 static void ATWrongRelkindError(Relation rel, int allowed_targets); static void ATSimpleRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode); static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static List *find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior behavior); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing, AlterTableCmd *cmd, LOCKMODE lockmode); ! static void ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, ! ColumnDef *colDef, bool isOid, ! bool recurse, bool recursing, LOCKMODE lockmode); static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid, Oid collid); static void ATPrepAddOids(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd, LOCKMODE lockmode); *** *** 2775,2789 ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_AddColumn: /* ADD COLUMN */ ATSimplePermissions(rel, ATT_TABLE|ATT_COMPOSITE_TYPE|ATT_FOREIGN_TABLE); - /* Performs own recursion */ ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode); pass = AT_PASS_ADD_COL; break; case AT_AddColumnToView:/* add column via CREATE OR REPLACE * VIEW */ ATSimplePermissions(rel, ATT_VIEW); - /* Performs own recursion */ ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode); pass = AT_PASS_ADD_COL; break;
Re: [HACKERS] Keywords in pg_hba.conf should be field-specific
On 29 October 2010 09:59, Brendan Jurd dire...@gmail.com wrote: On 18 October 2010 01:19, Tom Lane t...@sss.pgh.pa.us wrote: Brendan Jurd dire...@gmail.com writes: On 17 October 2010 09:59, Tom Lane t...@sss.pgh.pa.us wrote: Good point. Maybe the correct fix is to remember whether each token was quoted or not, so that keyword detection can be done safely after the initial lexing. I still think that the current method is impossibly ugly ... I'm happy to revise the patch on that basis. Any suggestions about how to communicate the 'quotedness' of each token? We could make each token a struct consisting of the token itself, plus a boolean flag to indicate whether it had been quoted. Does that work for you? Seems reasonable. I had the idea of a parallel list of booleans in the back of my mind, but a list of structs is probably easier to understand, and to extend further if necessary. Okay, I've taken the red pill and I'm finding out how deep the rabbit hole goes ... The logical structure of pg_hba.conf is a set of lines, each line containing a set of fields, each field containing a set of tokens. The way the existing implementation handles this is to create a list of lines containing sublists of fields, containing comma-separated strings for the set of tokens, with newlines embedded next to tokens which might be keywords. The tokeniser breaks apart the comma-separated tokens ... and then reassembles them into a comma-separated string. Which the db/role matching functions then have to break apart *again*. In order to keep track of whether each individual token was quoted, I first need to impose some sanity here. Rather than using a magical string for each field, I intend to use a List of HbaToken structs which explicitly note whether quoting was used. Introducing an extra List level does mean a bit more work copying and freeing, and it makes the patch really quite intrusive. I have to touch a lot of lines in hba.c, but I think the additional clarity is worth it. If nobody dissuades me from this approach I hope to post a patch in a couple of days. Hi folks, I've finally managed to get a patch together for the above. It ended up being a much more difficult project than I anticipated. Just to recap, the HEAD code in hba.c parses pg_hba.conf (and ident.conf) into a List of lines, each line being a List of strings. These strings have various magical properties, including commas to separate individual tokens and newlines to mark up keywords. I have replaced this behaviour with something which more closely represents the true logical structure of the file, and is (hopefully) much more intelligible. The patch introduces the HbaToken struct, which consists of a string and a boolean to indicate whether the string was parsed inside quotes. The parser now produces a triple-nested list -- a List of lines, each line being a List of fields, each field being a List of tokens. It sounds straightforward enough, but to make this happen I had to change a very substantial fraction of pg_hba.c. I have tested a few basic pg_hba.conf setups and everything seems to be working as intended. There is much more testing to be done, it probably leaks memory and there is a lot more room for cleanup in hba.c, but I would like to open the patch up for review and comment as a WIP. Posting it to the 2011-Next CF. Cheers, BJ hba-keywords.diff.bz2 Description: BZip2 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
