[PATCHES] Indexam API changes

2008-04-08 Thread Heikki Linnakangas
There's a bunch of mails in the patch queue about the indexam API, so we 
need to discuss that.


The first question is: do we want to refactor the bitmap index scan API, 
if we don't have any immediate use for it? Namely, since we don't have 
anyone actively working on the bitmap index patch nor the git patch.


There was also discussion on adding support for candidate matches, 
mainly for GIT, but GiST could possibly also take advantage of that.


If people think it's worth it, I can fix the bit-rot in the patch and 
work on it, but personally I don't think it is.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [PATCHES] Indexam API changes

2008-04-08 Thread Oleg Bartunov

On Tue, 8 Apr 2008, Heikki Linnakangas wrote:

There's a bunch of mails in the patch queue about the indexam API, so we need 
to discuss that.


The first question is: do we want to refactor the bitmap index scan API, if 
we don't have any immediate use for it? Namely, since we don't have anyone 
actively working on the bitmap index patch nor the git patch.


There was also discussion on adding support for candidate matches, mainly 
for GIT, but GiST could possibly also take advantage of that.


we talked about GIN. It'd be great if we eliminate @@@ operator in 8.4 !



If people think it's worth it, I can fix the bit-rot in the patch and work on 
it, but personally I don't think it is.



Regards,
Oleg
_
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: [EMAIL PROTECTED], http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83

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


Re: [PATCHES] Improve shutdown during online backup

2008-04-08 Thread Albe Laurenz
[what should happen if a smart shutdown request is received during online 
backup mode?
 I'll cc: the hackers list, maybe others have something to say to this]

Heikki Linnakangas wrote:
 Albe Laurenz wrote:
 Moreover, if Shutdown == SmartShutdown, new connections won't be accepted,
 and nobody can connect and call pg_stop_backup().
 So even if I'd add a check for
 (pmState == PM_WAIT_BACKENDS)  !BackupInProgress() somewhere in the
 ServerLoop(), it wouldn't do much good, because the only way for somebody
 to cancel online backup mode would be to manually remove the file.
 
 Good point.
 
 So the only reasonable thing to do on smart shutdown during an online
 backup is to have the shutdown request fail, right? The only alternative 
 being
 that a smart shutdown request should interrupt online backup mode.
 
 Or we can add another state, PM_WAIT_BACKUP, before PM_WAIT_BACKENDS, 
 that allows new connections, and waits until the backup ends.

That's an option. Maybe it is possible to restrict connections to superusers
(who are the only ones who can call pg_stop_backup() anyway).

Or, we could allow superuser connections in state PM_WAIT_BACKENDS...

Opinions?

Yours,
Laurenz Albe

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


Re: [PATCHES] wal_sync_method as enum

2008-04-08 Thread Magnus Hagander
Alvaro Herrera wrote:
 Magnus Hagander wrote:
  Attached patch implements wal_sync_method as an enum. I'd like
  someone to look it over before I apply it - I don't have the
  machines to test all codepaths (and some of it is hard to test -
  better to read it and make sure it's right).
  
  In order to implement the enum guc, I had to break out a new
  SYNC_METHOD_OPEN_DSYNC out of SYNC_METHOD_OPEN where it was
  previously overloaded. This is one of the parts where I'm slightly
  worried I have made a mistake.
 
 I took a look at this and I like it -- it seems correct as far as I
 can tell, and it has the nice property that we can display the list of
 values that the current platform actually support.

Yeah, this was one of the most helpful ones I thought of when I
started working on the enum stuff. It just also happened to be the one
that required the most changes :-S It'll be very good for tools like
[php]pgadmin to be able to show which values are actually supported.


 This thing looked a bit dubious to me at first:
 
  !   switch (new_sync_method)
  {
  !   case SYNC_METHOD_FSYNC:
  !   case SYNC_METHOD_FSYNC_WRITETHROUGH:
  !   case SYNC_METHOD_FDATASYNC:
 
 because at least some of the values are only present on some
 platforms. However, I think realized that the actual values are
 present on all platforms, independent of whether they represent a
 supported fsync method, so this is OK.  Perhaps it would be good to
 add a comment on top of the switch statement explaining this.
 Otherwise looks fine.

Will add comment. I used to have #ifdefs there from the old code, but
it's a lot more readable this way... Even more readable with the
comment, of course.


 Well, and the ereport(FIXME) needs to be improved :-)

Pah, details, details :-) It was just a copy/paste after all...

//Magnus

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Teodor Sigaev
Looking at the patch, you require that the TIDBitmap fits in work_mem in 
non-lossy format. I don't think that's acceptable, it can easily exceed 
work_mem if you search for some very common word. Failing to execute a 
valid query is not good.
But way is better than nothing. In really, that way was chosen to have fast 
merge of (potentially) hundreds of sorted lists of ItemPointers. Other ways is 
much slower.


Some calculations: with 8Mb of mem_work TIDBimap in non-lossy mode can store at 
least 20 pages, which gives to us no less than 20 tuples. For frequent 
word, that number should multiplied to 10 or 100, because practically every 
tuple will contain it. Practical limit to number of articles/document served by 
one servers is about 10 millions.


There are no so many alternatives:
- collect all needed ItemPointers and sort then unique them.
- merge each posting list with already collected ones
- N-way merge, where N can be very big
- Rerun index scan with all possible combinations

All this ways will be much slower even for not very big collections.

I don't think the storage size of tsquery matters much, so whatever is 
the best solution in terms of code readability etc.
That was about tsqueryesend/recv format? not a storage on disk. We don't require 
compatibility of binary format of db's files, but I have some doubts about 
binary dump.




Hmm. match_special_index_operator() already checks that the index's 
opfamily is pattern_ops, or text_ops with C-locale. Are you reusing the 
same operator families for wildspeed? Doesn't it then also get confused 
if you do a WHERE textcol  'foo' query by hand?

No, wildspeed use the same operator ~~
match_special_index_operator() isn't called at all: in 
match_clause_to_indexcol() function is_indexable_operator() is called before 
match_special_index_operator() and returns true.


expand_indexqual_opclause() sees that operation is a OID_TEXT_LIKE_OP and calls 
prefix_quals() which fails because it wishes only several Btree opfamilies.





NOTICE 2: it seems to me, that similar technique could be implemented 
for ordinary BTree to eliminate hack around LIKE support.
LIKE expression. I wonder what the size and performance of that would be 
like, in comparison to the proposed GIN solution?


GIN speeds up '%foo%' too - which is impossible for btree. But I don't like a 
hack around LIKE support in BTree. This support uses outflank ways missing 
regular one.


I'm thinking about add new strategy to Btree and allow directly support of 
prefix LIKE search. And BTree will scan index while compare method with option 
returns true.


--
Teodor Sigaev   E-mail: [EMAIL PROTECTED]
   WWW: http://www.sigaev.ru/

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


Re: [PATCHES] [HACKERS] TRUNCATE TABLE with IDENTITY

2008-04-08 Thread Zoltan Boszormenyi

Zoltan Boszormenyi írta:

Decibel! írta:

On Apr 3, 2008, at 12:52 AM, Zoltan Boszormenyi wrote:

Where is the info in the sequence to provide restarting with
the _original_ start value?


There isn't any. If you want the sequence to start at some magic 
value, adjust the minimum value.


There's the START WITH option for IDENTITY columns and this below
is paragraph 8 under General rules of 14.10 truncate table statement
in 6WD2_02_Foundation_2007-12.pdf (page 902):

8) If RESTART IDENTITY is specified and the table descriptor of T 
includes a column descriptor IDCD of

  an identity column, then:
  a) Let CN be the column name included in IDCD and let SV be the 
start value included in IDCD.
  b) The following alter table statement is effectively executed 
without further Access Rule checking:

  ALTER TABLE TN ALTER COLUMN CN RESTART WITH SV

This says that the original start value is used, not the minimum value.
IDENTITY has the same options as CREATE SEQUENCE. In fact the
identity column specification links to 11.63 sequence generator 
definition

when it comes to IDENTITY sequence options. And surprise, surprise,
11.64 alter sequence generator statement now defines
ALTER SEQUENCE sn RESTART [WITH newvalue]
where omitting the WITH newval part also uses the original start value.

Best regards,
Zoltán Böszörményi


Attached patch implements the extension found in the current SQL200n draft,
implementing stored start value and supporting ALTER SEQUENCE seq RESTART;
Some error check are also added to prohibit CREATE SEQUENCE ... RESTART ...
and ALTER SEQUENCE ... START ...

Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig  Schönig GmbH
http://www.postgresql.at/

diff -dcrpN pgsql.orig/src/backend/commands/sequence.c pgsql/src/backend/commands/sequence.c
*** pgsql.orig/src/backend/commands/sequence.c	2008-01-01 20:45:49.0 +0100
--- pgsql/src/backend/commands/sequence.c	2008-04-08 10:51:27.0 +0200
*** static Relation open_share_lock(SeqTable
*** 88,94 
  static void init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel);
  static Form_pg_sequence read_info(SeqTable elm, Relation rel, Buffer *buf);
  static void init_params(List *options, bool isInit,
! 			Form_pg_sequence new, List **owned_by);
  static void do_setval(Oid relid, int64 next, bool iscalled);
  static void process_owned_by(Relation seqrel, List *owned_by);
  
--- 88,94 
  static void init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel);
  static Form_pg_sequence read_info(SeqTable elm, Relation rel, Buffer *buf);
  static void init_params(List *options, bool isInit,
! 			Form_pg_sequence new, Form_pg_sequence old, List **owned_by);
  static void do_setval(Oid relid, int64 next, bool iscalled);
  static void process_owned_by(Relation seqrel, List *owned_by);
  
*** DefineSequence(CreateSeqStmt *seq)
*** 116,122 
  	NameData	name;
  
  	/* Check and set all option values */
! 	init_params(seq-options, true, new, owned_by);
  
  	/*
  	 * Create relation (and fill *null  *value)
--- 116,122 
  	NameData	name;
  
  	/* Check and set all option values */
! 	init_params(seq-options, true, new, NULL, owned_by);
  
  	/*
  	 * Create relation (and fill *null  *value)
*** DefineSequence(CreateSeqStmt *seq)
*** 143,148 
--- 143,153 
  namestrcpy(name, seq-sequence-relname);
  value[i - 1] = NameGetDatum(name);
  break;
+ 			case SEQ_COL_STARTVAL:
+ coldef-typename = makeTypeNameFromOid(INT8OID, -1);
+ coldef-colname = start_value;
+ value[i - 1] = Int64GetDatumFast(new.start_value);
+ break;
  			case SEQ_COL_LASTVAL:
  coldef-typename = makeTypeNameFromOid(INT8OID, -1);
  coldef-colname = last_value;
*** AlterSequence(AlterSeqStmt *stmt)
*** 336,342 
  	memcpy(new, seq, sizeof(FormData_pg_sequence));
  
  	/* Check and set new values */
! 	init_params(stmt-options, false, new, owned_by);
  
  	/* Clear local cache so that we don't think we have cached numbers */
  	/* Note that we do not change the currval() state */
--- 341,347 
  	memcpy(new, seq, sizeof(FormData_pg_sequence));
  
  	/* Check and set new values */
! 	init_params(stmt-options, false, new, seq, owned_by);
  
  	/* Clear local cache so that we don't think we have cached numbers */
  	/* Note that we do not change the currval() state */
*** read_info(SeqTable elm, Relation rel, Bu
*** 967,973 
   */
  static void
  init_params(List *options, bool isInit,
! 			Form_pg_sequence new, List **owned_by)
  {
  	DefElem*last_value = NULL;
  	DefElem*increment_by = NULL;
--- 972,978 
   */
  static void
  init_params(List *options, bool isInit,
! 			Form_pg_sequence new, Form_pg_sequence old, List **owned_by)
  {
  	DefElem*last_value = NULL;
  	DefElem*increment_by = NULL;
*** init_params(List *options, bool isInit,
*** 995,1003 
  		/*
  		 * 

Re: [PATCHES] Indexam API changes

2008-04-08 Thread Gregory Stark
Heikki Linnakangas [EMAIL PROTECTED] writes:

 There's a bunch of mails in the patch queue about the indexam API, so we need
 to discuss that.

 The first question is: do we want to refactor the bitmap index scan API, if we
 don't have any immediate use for it? Namely, since we don't have anyone
 actively working on the bitmap index patch nor the git patch.

I haven't read the patch. My understanding from the discussion is that it
would allow callers of the indexam to receive a hunk of index pointers and
process them rather than have to wait for the complete index scan to finish
before processing any. Is that it?

In general I think we need to be more open to incremental improvements. I
think there are several fronts on which we refuse patches to do X because it's
useless without Y and have nobody working on Y because they would have to
solve X first.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Gregory Stark
Teodor Sigaev [EMAIL PROTECTED] writes:

  - compare function has third (optional) argument, of boolean type, it points 
 to
kind of compare: partial or exact match. If argument is equal to 'false',
function should produce comparing as usual, else function's result is
treated as:
= 0  - match
 0  - doesn't match but continue scan
 0  - stop scan

Perhaps a minor point but I think this would be cleaner as a separate opclass
function with a separate support number rather than overloading the comparison
function. 

Then if the support function is there it supports that type of scan and if it
doesn't then it doesn't, rather than depending on a magic third argument to
completely change behaviour. 

You can always share code using an internal function or if the behaviour is
identical just register the same function twice.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Heikki Linnakangas

Teodor Sigaev wrote:
Looking at the patch, you require that the TIDBitmap fits in work_mem 
in non-lossy format. I don't think that's acceptable, it can easily 
exceed work_mem if you search for some very common word. Failing to 
execute a valid query is not good.
But way is better than nothing. In really, that way was chosen to have 
fast merge of (potentially) hundreds of sorted lists of ItemPointers. 
Other ways is much slower.


How about forcing the use of a bitmap index scan, and modify the indexam 
API so that GIN could a return a lossy bitmap, and let the bitmap heap 
scan do the rechecking?


I don't think the storage size of tsquery matters much, so whatever is 
the best solution in terms of code readability etc.
That was about tsqueryesend/recv format? not a storage on disk. We don't 
require compatibility of binary format of db's files, but I have some 
doubts about binary dump.


We generally don't make any promises about cross-version compatibility 
of binary dumps, though it would be nice not to break it if it's not too 
much effort.


Hmm. match_special_index_operator() already checks that the index's 
opfamily is pattern_ops, or text_ops with C-locale. Are you reusing 
the same operator families for wildspeed? Doesn't it then also get 
confused if you do a WHERE textcol  'foo' query by hand?

No, wildspeed use the same operator ~~
match_special_index_operator() isn't called at all: in 
match_clause_to_indexcol() function is_indexable_operator() is called 
before match_special_index_operator() and returns true.


expand_indexqual_opclause() sees that operation is a OID_TEXT_LIKE_OP 
and calls prefix_quals() which fails because it wishes only several 
Btree opfamilies.


Oh, I see. So this assumption mentioned in the comment there:

/*
 * LIKE and regex operators are not members of any index opfamily,
 * so if we find one in an indexqual list we can assume that it
 * was accepted by match_special_index_operator().
 */

is no longer true with wildspeed. So we do need to check that in 
expand_indexqual_opclause() then.


NOTICE 2: it seems to me, that similar technique could be implemented 
for ordinary BTree to eliminate hack around LIKE support.
LIKE expression. I wonder what the size and performance of that would 
be like, in comparison to the proposed GIN solution?


GIN speeds up '%foo%' too - which is impossible for btree. But I don't 
like a hack around LIKE support in BTree. This support uses outflank 
ways missing regular one.


You could satisfy '%foo%' using a regular and a reverse B-tree index, 
and a bitmap AND. Which is interestingly similar to the way you proposed 
to use a TIDBitmap within GIN.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Alvaro Herrera
Heikki Linnakangas wrote:
 Teodor Sigaev wrote:

 GIN speeds up '%foo%' too - which is impossible for btree. But I don't  
 like a hack around LIKE support in BTree. This support uses outflank  
 ways missing regular one.

 You could satisfy '%foo%' using a regular and a reverse B-tree index,  
 and a bitmap AND. Which is interestingly similar to the way you proposed  
 to use a TIDBitmap within GIN.

Huh, can you?  I can see doing col LIKE 'foo%' OR reverse(col) LIKE
reverse('%foo') with two btree indexes, but not a true %foo% ...

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Heikki Linnakangas

Alvaro Herrera wrote:

Heikki Linnakangas wrote:

Teodor Sigaev wrote:


GIN speeds up '%foo%' too - which is impossible for btree. But I don't  
like a hack around LIKE support in BTree. This support uses outflank  
ways missing regular one.
You could satisfy '%foo%' using a regular and a reverse B-tree index,  
and a bitmap AND. Which is interestingly similar to the way you proposed  
to use a TIDBitmap within GIN.


Huh, can you?  I can see doing col LIKE 'foo%' OR reverse(col) LIKE
reverse('%foo') with two btree indexes, but not a true %foo% ...


That should be AND, not OR..

Hmm. It is the same as far as I can see. Am I missing something?

You couldn't support more complex patterns directly, like 'foo%bar%foo', 
but you could still split that into 'foo%' AND '%bar%' AND '%foo', and 
recheck the matches against the original pattern


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [PATCHES] Partial match in GIN

2008-04-08 Thread Alvaro Herrera
Heikki Linnakangas wrote:
 Alvaro Herrera wrote:
 Heikki Linnakangas wrote:

 You could satisfy '%foo%' using a regular and a reverse B-tree index, 
  and a bitmap AND. Which is interestingly similar to the way you 
 proposed  to use a TIDBitmap within GIN.

 Huh, can you?  I can see doing col LIKE 'foo%' OR reverse(col) LIKE
 reverse('%foo') with two btree indexes, but not a true %foo% ...

 That should be AND, not OR..

 Hmm. It is the same as far as I can see. Am I missing something?

Well, LIKE %foo% is supposed to match foo unanchored, but with a btree
(or two btrees) you can only get 'foo' anchored to either end of the
string (or both).

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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


[PATCHES] EXPLAIN progress info

2008-04-08 Thread Gregory Stark

Not to be opened before May 1st!


I know I should still be looking at code from the March Commitfest but I was
annoyed by this *very* FAQ:

 http://archives.postgresql.org/pgsql-general/2008-04/msg00402.php 

This also came up at the UKUUG-Pg conference so it was already on my mind. I
couldn't resist playing with it and trying to solve this problem.



explain-progress.diff.gz
Description: Binary data

I'm not sure what the right way to get the data back to psql would be.
Probably it should be something other than what it's doing now, an INFO
message. It might even be a special message type? Also need some thought about
what progress info could be reported in other situations aside from the
executor. VACUUM, for example, could report its progress pretty easily.

To use it run a long query in psql and hit C-\ in (unless you're on a system
with SIGINFO support such as BSD where the default will probably be C-t).

But no reviewing it until we finish with the March patches! 
Do as I say, not as I do :(



explain-progress.diff.gz
Description: Binary data


-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS support!

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


[PATCHES] Fix \dT enum in psql

2008-04-08 Thread David Fetter
Folks,

In psql, \dT doesn't show the elements for enums.  Please find patch
vs. CVS TIP attached which fixes this per the following TODO item:

http://archives.postgresql.org/pgsql-hackers/2008-01/msg00826.php

Cheers,
David.
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Index: src/bin/psql/describe.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.166
diff -c -c -r1.166 describe.c
*** src/bin/psql/describe.c 30 Mar 2008 18:10:20 -  1.166
--- src/bin/psql/describe.c 8 Apr 2008 20:29:01 -
***
*** 310,317 
--- 310,328 
END AS \%s\,\n,
  gettext_noop(Internal name),
  gettext_noop(Size));
+ 
appendPQExpBuffer(buf,
+ pg_catalog.array_to_string(\n
+   ARRAY(\n
+   SELECT e.enumlabel\n
+ FROM pg_catalog.pg_enum e\n
+ WHERE e.enumtypid = t.oid\n
+ ORDER BY e.oid\n
+ ),\n
+ E'\\n'\n
+ ) AS \%s\,\n
  pg_catalog.obj_description(t.oid, 'pg_type') 
as \%s\\n,
+ gettext_noop(Elements),
  gettext_noop(Description));
  
appendPQExpBuffer(buf, FROM pg_catalog.pg_type t\n

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


Re: [PATCHES] EXPLAIN progress info

2008-04-08 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 I know I should still be looking at code from the March Commitfest but I was
 annoyed by this *very* FAQ:

  http://archives.postgresql.org/pgsql-general/2008-04/msg00402.php 

Seems like pg_relation_size and/or pgstattuple would solve his problem
better, especially since he'd not have to abort and restart the long
query to find out if it's making progress.  It'd help if pgstattuple
were smarter about dead vs uncommitted tuples, though.

regards, tom lane

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


Re: [PATCHES] EXPLAIN progress info

2008-04-08 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 Gregory Stark [EMAIL PROTECTED] writes:
 I know I should still be looking at code from the March Commitfest but I was
 annoyed by this *very* FAQ:

  http://archives.postgresql.org/pgsql-general/2008-04/msg00402.php 

 Seems like pg_relation_size and/or pgstattuple would solve his problem
 better, especially since he'd not have to abort and restart the long
 query to find out if it's making progress.  It'd help if pgstattuple
 were smarter about dead vs uncommitted tuples, though.

I specifically didn't go into detail because I thought it would be pointed out
I should be focusing on the commitfest, not proposing new changes. I just got
caught up with an exciting idea.

But it does *not* abort the current query. It spits out an explain tree with
the number of rows and loops executed so far for each node and returns to
processing the query. You can hit the C-t or C-\ multiple times and see the
actual rows increasing. You could easily imagine a tool like pgadmin
displaying progress bars based on the estimated and actual rows.

There are downsides: 

a) the overhead of counting rows and loops is there for every query execution,
even if you don't do explain analyze. It also has to palloc all the
instrumentation nodes.

b) We're also running out of signals to control backends. I used SIGILL but
really that's not exactly an impossible signal, especially for user code from
contrib modules. We may have to start looking into other ways of having the
postmaster communicate with backends. It could open a pipe before it starts
backends for example.

c) It's not easy to be sure that every single CHECK_FOR_INTERRUPTS() site
throughout the backend is a safe place to be calling random node output
functions. I haven't seen any problems and realistically it seems all the node
output functions *ought* to be safe to call from anywhere but it warrants a
second look.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Bruce Momjian

This has been saved for the next commit-fest:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---

Gregory Stark wrote:
 Andrew Dunstan [EMAIL PROTECTED] writes:
 
  Gregory Stark wrote:
  Attached is an updated patch.
 
 
  This patch appears to add a nonexistent test to the regression schedules.
 
 I must have forgotten to cvs add it. Sorry.
 
 Also, I forgot to mention previously there is an unrelated trivial hunk in
 here. I noticed we free the password early, presumably for security reasons,
 but don't actually overwrite the memory at that point. I added a memset in
 there, otherwise the free seems kind of pointless. I normally don't bother
 with security features like that since they don't really add any security
 but as long as it's there it may as well do something vaguely useful.
 

[ Attachment, skipping... ]

 
 -- 
   Gregory Stark
   EnterpriseDB  http://www.enterprisedb.com
 
 ---(end of broadcast)---
 TIP 4: Have you searched our list archives?
 
http://archives.postgresql.org

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] EXPLAIN progress info

2008-04-08 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 There are downsides: 

Insurmountable ones at that.  This one already makes it a non-starter:

 a) the overhead of counting rows and loops is there for every query execution,
 even if you don't do explain analyze.

and you are also engaging in a flight of fantasy about what the
client-side code might be able to handle.  Particularly if it's buried
inside, say, httpd or some huge Java app.  Yeah, you could possibly make
it work for the case that the problem query was manually executed in
psql, but that doesn't cover very much real-world territory.

You'd be far more likely to get somewhere with a design that involves
looking from another session to see if anything's happening.  In the
case of queries that are making database changes, pgstattuple is
certainly a usable option.  For SELECT-only queries, I agree it's
harder, but it's still possible.  I seem to recall some discussion of
including a low-overhead progress counter of some kind in the
pg_stat_activity state exposed by a backend.  The number of rows so far
processed by execMain.c in the current query might do for the
definition.

regards, tom lane

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 This has been saved for the next commit-fest:
   http://momjian.postgresql.org/cgi-bin/pgpatches_hold

Er, why saved?  Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.

regards, tom lane

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  This has been saved for the next commit-fest:
  http://momjian.postgresql.org/cgi-bin/pgpatches_hold
 
 Er, why saved?  Until there's a new patch submission there's not going
 to be more work to do on this in the next fest.
 
 I think maybe you need to think a bit harder about the distinction
 between your TODO-items-in-waiting list and the commit fest queue.
 I was willing to wade through a pile of TODO-items-in-waiting this
 time, because I pressed you to make the list available before having
 sorted through it; but I'm not going to be pleased to see those same
 threads in the fest queue next time, unless someone's done some actual
 work in between.

It is in the next fest so I will remember to ask if people have done any
work on them --- if not they are either deleted or moved to the next
commit fest.

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 Are you suggesting we just delete the threads and let them die if they
 don't submit a new version?

I am suggesting that they are not material for another commit fest until
some new work has been done.  (And the appearance of that new work would
trigger an entry being made in the pending-commit-fest list.)

I've surely got no objection to you hanging on to such threads in your
personal almost-TODO list, and prodding people when appropriate.  But
the commit fest queue is not for that purpose.

regards, tom lane

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Andrew Dunstan



Bruce Momjian wrote:

Tom Lane wrote:
  

Bruce Momjian [EMAIL PROTECTED] writes:


This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
  

Er, why saved?  Until there's a new patch submission there's not going
to be more work to do on this in the next fest.

I think maybe you need to think a bit harder about the distinction
between your TODO-items-in-waiting list and the commit fest queue.
I was willing to wade through a pile of TODO-items-in-waiting this
time, because I pressed you to make the list available before having
sorted through it; but I'm not going to be pleased to see those same
threads in the fest queue next time, unless someone's done some actual
work in between.



It is in the next fest so I will remember to ask if people have done any
work on them --- if not they are either deleted or moved to the next
commit fest.

Are you suggesting we just delete the threads and let them die if they
don't submit a new version?

  


My understanding was that all items in a commit-fest have one of these 
three dispositions:


. committed
. rejected
. referred back to author for more work

We're really only interested in the third one here, and so, yes, the 
ball should be in the author's court, not yours. I don't see any reason 
for you to move items from one queue to another like that. It just looks 
like it's making work.


cheers

andrew


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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Tom Lane
Andrew Dunstan [EMAIL PROTECTED] writes:
 My understanding was that all items in a commit-fest have one of these 
 three dispositions:

 . committed
 . rejected
 . referred back to author for more work

Right.  But Bruce's personal queue has got a different lifecycle:
items get removed when they are resolved by a committed patch, or
by being rejected as not wanted, or by being summarized on the public
TODO list.  For what he's doing that's a very good definition ---
things don't get forgotten just because nothing has happened lately.
But it's becoming clearer to me that the commit-fest queue has to be
a separate animal.  We used Bruce's queue as the base this time around,
because we had no other timely-available source of the raw data.
Seems like it's time to split them, though.

If we do split them then there is going to be some added effort to
maintain the commit fest queue.  Bruce has made it pretty clear
that he doesn't want to put in any extra cycles here.  So someone
else has to step up to the plate if this is going to work.
Any volunteers out there?

regards, tom lane

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Bruce Momjian
Tom Lane wrote:
 Andrew Dunstan [EMAIL PROTECTED] writes:
  My understanding was that all items in a commit-fest have one of these 
  three dispositions:
 
  . committed
  . rejected
  . referred back to author for more work
 
 Right.  But Bruce's personal queue has got a different lifecycle:
 items get removed when they are resolved by a committed patch, or
 by being rejected as not wanted, or by being summarized on the public
 TODO list.  For what he's doing that's a very good definition ---
 things don't get forgotten just because nothing has happened lately.
 But it's becoming clearer to me that the commit-fest queue has to be
 a separate animal.  We used Bruce's queue as the base this time around,
 because we had no other timely-available source of the raw data.
 Seems like it's time to split them, though.

Right, if the patch author stops working on it, but it is a feature we
want, the thread goes on the TODO list (or we complete the patch), so
yes, it is a different life-cycle.

 If we do split them then there is going to be some added effort to
 maintain the commit fest queue.  Bruce has made it pretty clear
 that he doesn't want to put in any extra cycles here.  So someone
 else has to step up to the plate if this is going to work.
 Any volunteers out there?

I assumed the wiki was going to be the official patch list from now on
and my web pages were just going to be a public display of things I was
tracking.

Frankly, I haven't been putting anything on the queue for the next
commit fest now except stuff that was already in-process for this commit
fest.  The ideas is that we can commit stuff that has appeared since the
commit fest started before the next commit fest starts.  I also moved
the emails to the next commit fest queue because that preserves the
comments made too.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] Concurrent psql patch

2008-04-08 Thread Bruce Momjian
Andrew Dunstan wrote:
  Are you suggesting we just delete the threads and let them die if they
  don't submit a new version?
 

 
 My understanding was that all items in a commit-fest have one of these 
 three dispositions:
 
 . committed
 . rejected
 . referred back to author for more work
 
 We're really only interested in the third one here, and so, yes, the 
 ball should be in the author's court, not yours. I don't see any reason 
 for you to move items from one queue to another like that. It just looks 
 like it's making work.

True.  I could move the emails back to my private mailbox and just track
them there too.  I moved them so it would be visible we were waiting for
some people.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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