Re: [PATCHES] AllocSetReset improvement
On Thu, 2005-05-12 at 11:26 -0400, Tom Lane wrote: I have another idea though: in the case you are looking at, I think that the context in question never gets any allocations at all, which means its blocks list stays null. We could move the MemSet inside the if (blocks) test --- if there are no blocks allocated to the context, it surely hasn't got any chunks either, so the MemSet is unnecessary. Good point. There's same MemSet in AllocSetDelete() too. Karel -- Karel Zak [EMAIL PROTECTED] ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
[PATCHES] farsi faq has not been added yet in website,
would you please fix it, why farsi faq is not in web site?With Regards,--taghi Discover Yahoo! Have fun online with music videos, cool games, IM & more. Check it out!
Re: [PATCHES] AllocSetReset improvement
Tom Lane [EMAIL PROTECTED] writes: a_ogawa [EMAIL PROTECTED] writes: It is a reasonable idea. However, the majority part of MemSet was not able to be avoided by this idea. Because the per-tuple contexts are used at the early stage of executor. Drat. Well, what about changing that? We could introduce additional contexts or change the startup behavior so that the ones that are frequently reset don't have any data in them unless you are working with pass-by-ref values inside the inner loop. That might be possible. However, I think that we should change only aset.c about this article. I thought further: We can check whether context was used from the last reset even when blocks list is not empty. Please see attached patch. The effect of the patch that I measured is as follows: o Execution time that executed the SQL ten times. (1)Linux(CPU: Pentium III, Compiler option: -O2) - original: 24.960s - patched : 23.114s (2)Linux(CPU: Pentium 4, Compiler option: -O2) - original: 8.730s - patched : 7.962s (3)Solaris(CPU: Ultra SPARC III, Compiler option: -O2) - original: 37.0s - patched : 33.7s regards, --- Atsushi Ogawa aset.c.patch Description: Binary data ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] updated GiST patch
Neil Conway wrote:
This is an updated version of the GiST patch I posted a few months ago.
Attached is a revised version. This update fixes some newly-added bugs
in mark and restore (I forgot to save and restore the current buffer),
and replaces two ReleaseBuffer() + ReadBuffer() pairs with
ReleaseAndReadBuffer(). (Is this still worth doing? It seems it no
longer saves a lock acquire/release, but perhaps it will again be a win
in some future version of the bufmgr...)
BTW, this idiom occurs a few times:
if (BufferIsValid(buf))
{
ReleaseBuffer(buf);
buf = InvalidBuffer;
}
it would be nice to make this more concise. Perhaps:
InvalidateBuffer(buf);
although that doesn't make the modification of `buf' obvious. An
alternative would be to have ReleaseBuffer() always return
InvalidBuffer, so:
if (BufferIsValid(buf))
buf = ReleaseBuffer(buf);
Any thoughts on this? (I'm inclined to prefer InvalidateBuffer().)
-Neil
new_gist_patch-4.patch.gz
Description: application/tgz
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] updated GiST patch
Neil Conway [EMAIL PROTECTED] writes:
... replaces two ReleaseBuffer() + ReadBuffer() pairs with
ReleaseAndReadBuffer(). (Is this still worth doing? It seems it no
longer saves a lock acquire/release, but perhaps it will again be a win
in some future version of the bufmgr...)
I think there is no longer any noticeable win except in the case where
the old and new pages are actually the same. Which is probably unlikely
to be true inside an index AM (it is a useful win for random access into
a heap for instance). But as you say it might someday again be useful.
My advice is to combine if and only if you're not contorting the code
to do so.
BTW, this idiom occurs a few times:
if (BufferIsValid(buf))
{
ReleaseBuffer(buf);
buf = InvalidBuffer;
}
I'd leave it as-is; ISTM to be more easily understandable than the
alternatives you suggest.
regards, tom lane
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] Exec statement logging
OK, new patch. I used portal-sourceText as Tom suggested, and checked for NULL before printing. I put the original query in brackets in the log output, patch attached. I would like to also do this for server-side EXECUTE. I am have attached a second version that does it by using the existing loop we use to check for a match for log_statement patterns. The code checks for an ExecuteStmt node, and saves the prepare string that matches the portal. The system can't determine the actual prepare string, so I used the debug_query_string for a server-side PREPARE --- previously it just had a null for the prepare string. Also, rather than change the API for pg_parse_query(), I use a global variable that I check after the function call. Here is sample output with log_statement and log_min_duration_statement enabled: PREPARE xx AS SELECT 1; LOG: statement: PREPARE xx AS SELECT 1; LOG: duration: 1.102 ms statement: PREPARE xx AS SELECT 1; PREPARE EXECUTE xx; LOG: statement: EXECUTE xx; [client PREPARE: PREPARE xx AS SELECT 1;] LOG: duration: 0.990 ms statement: EXECUTE xx; [client PREPARE: PREPARE xx AS SELECT 1;] ?column? -- 1 (1 row) --- Simon Riggs wrote: On Sat, 2005-05-14 at 16:55 -0400, Bruce Momjian wrote: Uh, I am confused by this. Your code test is: + if ((log_statement == LOGSTMT_ALL save_log_duration) || + save_log_min_duration_statement) + gettimeofday(start_t, NULL); First, log_min_duration_statement == -1 turns off logging. Your test would enable it for -1. Also, you added log_statement == LOGSTMT_ALL, but don't have a similar test lower down where gettimeofday is used, so I don't understand its usage here, or why it would be used in this place. The original test is: + if (save_log_duration || save_log_min_duration_statement != -1) + gettimeofday(start_t, NULL); Yes, that looks wrong. Thanks for your diligence. I'm sorry that you've had to both spot an error and recode for it, I was expecting to do that. One thing you did was to log debug_query_string, but I don't see how that could be the right value. I assume it would be empty in a client-side execute, or be the previous query. I instead used EXECUTE portal_name because that is what we are passed from the client. I used the debug_query_string because even in the EXEC case it is set, so that the SQL statement appears correctly in pg_stat_activity. It may look wrong, but it is there. That part, at least, is correct, since I have used the patch in tuning. Perhaps it is only there when stats_command_string is set? Setting the SQL string to be only EXECUTE portal_name makes the log output almost useless for query tuning, so please reconsider that. Perhaps you could include both the portal name and the SQL statement? Best Regards, Simon Riggs -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/backend/tcop/postgres.c === RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v retrieving revision 1.443 diff -c -c -r1.443 postgres.c *** src/backend/tcop/postgres.c 21 Apr 2005 19:18:13 - 1.443 --- src/backend/tcop/postgres.c 16 May 2005 03:40:00 - *** *** 1011,1017 stop_t.tv_sec--; stop_t.tv_usec += 100; } ! usecs = (long) (stop_t.tv_sec - start_t.tv_sec) * 100 + (long) (stop_t.tv_usec - start_t.tv_usec); /* Only print duration if we previously printed the statement. */ if (statement_logged save_log_duration) --- 1011,1018 stop_t.tv_sec--; stop_t.tv_usec += 100; } ! usecs = (long) (stop_t.tv_sec - start_t.tv_sec) * 100 + ! (long) (stop_t.tv_usec - start_t.tv_usec); /* Only print duration if we previously printed the statement. */ if (statement_logged save_log_duration) *** *** 1579,1584 --- 1580,1590 boolis_trans_exit = false; boolcompleted; charcompletionTag[COMPLETION_TAG_BUFSIZE]; + struct timeval start_t, + stop_t; + boolsave_log_duration = log_duration; + int save_log_min_duration_statement = log_min_duration_statement; + boolsave_log_statement_stats = log_statement_stats;
Re: [PATCHES] Exec statement logging
Bruce Momjian pgman@candle.pha.pa.us writes: a null for the prepare string. Also, rather than change the API for pg_parse_query(), I use a global variable that I check after the function call. This is horribly ugly and will doubtless lead to pfree crashes. What was the point again? regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Exec statement logging
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: a null for the prepare string. Also, rather than change the API for pg_parse_query(), I use a global variable that I check after the function call. This is horribly ugly and will doubtless lead to pfree crashes. What was the point again? Can we change the API of pg_parse_query() to optionally return a PREPARE string? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
[PATCHES] Quick little \h enhancement for psql
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Attached is a patch that enhances the \h capability in psql. I often
find myself typing a command and then wanting to get the syntax for
it. So I do a ctrl-a and add a \h: but psql does not recognize the
command, because I have stuff attached to it (e.g. alter table
foobar),
so I have to scroll over and delete everything except the name of the
command itself. This patch gives \h three chances to match: if nothing
matches the complete string (current behavior), it tries to match the
first two words (e.g. ALTER TABLE). If that fails, it tries to match
the first word (e.g. DELETE).
- --
Greg Sabino Mullane [EMAIL PROTECTED]
PGP Key: 0x14964AC8 200505161840
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-
iD8DBQFCiSF3vJuQZxSWSsgRAumaAKCKd0xns4V+ITRSfxGJCrJUtZOrjgCfW8FC
qbFC2jdXGOWWP8cN3Um/1hY=
=kZeh
-END PGP SIGNATURE-
Index: help.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.101
diff -c -r1.101 help.c
*** help.c 22 Feb 2005 04:40:55 - 1.101
--- help.c 16 May 2005 20:59:51 -
***
*** 304,355
}
else
{
! int i;
bool help_found = false;
FILE *output;
! size_t len;
int nl_count = 0;
char *ch;
! /* don't care about trailing spaces or semicolons */
len = strlen(topic);
while (topic[len - 1] == ' ' || topic[len - 1] == ';')
! len--;
! /* Count newlines for pager */
! for (i = 0; QL_HELP[i].cmd; i++)
{
! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
! strcmp(topic, *) == 0)
! {
! nl_count += 5;
! for (ch = QL_HELP[i].syntax; *ch != '\0'; ch++)
! if (*ch == '\n')
! nl_count++;
! /* If we have an exact match, exit. Fixes \h SELECT */
! if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0)
! break;
! }
! }
!
! output = PageOutput(nl_count, pager);
!
! for (i = 0; QL_HELP[i].cmd; i++)
! {
! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
! strcmp(topic, *) == 0)
! {
! help_found = true;
! fprintf(output, _(Command: %s\n
! Description: %s\n
! Syntax:\n%s\n\n),
! QL_HELP[i].cmd,
! _(QL_HELP[i].help),
! _(QL_HELP[i].syntax));
! /* If we have an exact match, exit. Fixes \h SELECT */
! if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0)
! break;
! }
}
if (!help_found)
--- 304,381
}
else
{
! int i,j,x=0;
bool help_found = false;
FILE *output;
! size_t len, wordlen;
int nl_count = 0;
char *ch;
! /* User gets two chances: exact match, then the first word */
!
! /* First pass : strip trailing spaces and semicolons */
len = strlen(topic);
while (topic[len - 1] == ' ' || topic[len - 1] == ';')
! len--;
! for (x=1; x=3; x++) /* Three chances to guess that word... */
{
! if (x1) /* Nothing on first pass - try the opening words */
! {
! wordlen=j=1;
! while (topic[j] != ' ' j++len)
! wordlen++;
! if (x==2)
! {
! j++;
! while (topic[j] != ' ' j++=len)
! wordlen++;
! }
! if (wordlen = len) /* Don't try again if the same word */
! {
! output = PageOutput(nl_count, pager);
! break;
! }
! len = wordlen;
! }
!
! /* Count newlines for pager */
! for (i = 0; QL_HELP[i].cmd; i++)
! {
! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
! strcmp(topic, *) == 0)
! {
! nl_count += 5;
! for (ch = QL_HELP[i].syntax; *ch != '\0'; ch++)
! if (*ch == '\n')
! nl_count++;
! /* If we have an exact match, exit. Fixes \h SELECT */
! if (pg_strcasecmp(topic, QL_HELP[i].cmd) == 0)
! break;
! }
! }
!
! output = PageOutput(nl_count, pager);
!
! for (i = 0; QL_HELP[i].cmd; i++)
! {
! if (pg_strncasecmp(topic, QL_HELP[i].cmd, len) == 0 ||
! strcmp(topic, *) == 0)
! {
! help_found = true;
! fprintf(output, _(Command: %s\n
! Description: %s\n
! Syntax:\n%s\n\n),
! QL_HELP[i].cmd,
! _(QL_HELP[i].help),
! _(QL_HELP[i].syntax));
!
Re: [PATCHES] Exec statement logging
Bruce Momjian pgman@candle.pha.pa.us writes: What was the point again? Simon said that the EXECUTE is pretty useless for debugging unless we show the prepare query. His patch shows the prepare query for client-side prepare, but not for server side when using the PREPARE/EXECUTE commands --- seems we should display it in both cases. So we need to make sure that prepared statements always capture the originating query string. I thought you'd done that by using debug_query_string ... which is a bit ugly but not horrid. Why do we need more mechanism than that? regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Exec statement logging
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: What was the point again? Simon said that the EXECUTE is pretty useless for debugging unless we show the prepare query. His patch shows the prepare query for client-side prepare, but not for server side when using the PREPARE/EXECUTE commands --- seems we should display it in both cases. So we need to make sure that prepared statements always capture the originating query string. I thought you'd done that by using debug_query_string ... which is a bit ugly but not horrid. Why do we need more mechanism than that? The problem is we have to pass that string down to the completion of the query for log_min_duration_statement printing. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] updated GiST patch
Patch applied.
Tom Lane wrote:
Neil Conway [EMAIL PROTECTED] writes:
BTW, this idiom occurs a few times:
if (BufferIsValid(buf))
{
ReleaseBuffer(buf);
buf = InvalidBuffer;
}
I'd leave it as-is; ISTM to be more easily understandable than the
alternatives you suggest.
Yeah, fair enough.
-Neil
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [PATCHES] GiST header cleanup
Patch applied. Tom Lane wrote: One objection: I think the GiST amproc numbers (GIST_CONSISTENT_PROC and friends) *are* part of the API and should be in the public header, even if they happen not to be used by any C code at the moment. Ok, I've moved these back to gist.h GISTNStrategies seems inherently bogus, since there's no essential limit on the number of strategies in a gist index. I'd get rid of it. Done. The 100 in pg_am.h is pretty nasty too, because it is on the one hand theoretically insufficient and on the other hand in practice way too much. Yeah, I agree this is pretty ugly, but I'm not planning to fix it any time soon, either... -Neil ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
