Re: [PATCHES] Partial match in GIN (next vesrion)
Looking at this now. Wouldn't it be a good idea for comparePartial to get the strategy number of the operator? As you have it set up, I doubt that an opclass can support more than one partial-match operator. It might be useful, although I don't see any usage of that right now. I'll add this option. -- 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] [GENERAL] Making sure \timing is on
On Tue, May 13, 2008 at 10:47:40AM -0400, Alvaro Herrera wrote: > Tom Lane escribió: > > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > > David Fetter escribi?: > > >> Thanks for the heads-up :) > > >> > > >> Second patch attached, this time with some docs. > > > > > Added to July commitfest. > > > > Surely this is merely proof of concept and not a complete patch. > > David, ya heard da man :-) Next patch attached :) Cheers, David (free() the malloc()s!) -- 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 -- 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] [GENERAL] Making sure \timing is on
On Tue, May 13, 2008 at 08:14:51AM -0700, David Fetter wrote:
> On Tue, May 13, 2008 at 10:47:40AM -0400, Alvaro Herrera wrote:
> > Tom Lane escribió:
> > > Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > > > David Fetter escribi?:
> > > >> Thanks for the heads-up :)
> > > >>
> > > >> Second patch attached, this time with some docs.
> > >
> > > > Added to July commitfest.
> > >
> > > Surely this is merely proof of concept and not a complete patch.
> >
> > David, ya heard da man :-)
>
> Next patch attached :)
>
> Cheers,
> David (free() the malloc()s!)
*Sigh*
This time with the patch actually attached :P
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: doc/src/sgml/ref/psql-ref.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.203
diff -c -c -r1.203 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml 8 May 2008 17:04:26 - 1.203
--- doc/src/sgml/ref/psql-ref.sgml 13 May 2008 14:41:18 -
***
*** 1867,1876
!\timing
! Toggles a display of how long each SQL statement takes, in
milliseconds.
--- 1867,1879
!\timing [ON | OFF]
! Without parameter, toggles a display of how long each SQL
! statement takes, in milliseconds. With parameter, sets same.
Index: src/bin/psql/command.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.188
diff -c -c -r1.188 command.c
*** src/bin/psql/command.c 8 May 2008 17:04:26 - 1.188
--- src/bin/psql/command.c 13 May 2008 14:41:21 -
***
*** 884,890
/* \timing -- toggle timing of queries */
else if (strcmp(cmd, "timing") == 0)
{
! pset.timing = !pset.timing;
if (!pset.quiet)
{
if (pset.timing)
--- 903,914
/* \timing -- toggle timing of queries */
else if (strcmp(cmd, "timing") == 0)
{
! char *opt = psql_scan_slash_option(scan_state,
!
OT_NORMAL, NULL, false);
! if (opt)
! pset.timing = ParseVariableBool(opt);
! else
! pset.timing = !pset.timing;
if (!pset.quiet)
{
if (pset.timing)
***
*** 892,897
--- 916,922
else
puts(_("Timing is off."));
}
+ free(opt);
}
/* \unset */
--
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] [GENERAL] Making sure \timing is on
David Fetter <[EMAIL PROTECTED]> writes: >>> Surely this is merely proof of concept and not a complete patch. >> >> Next patch attached :) Uh, my point was that the agreement was to do this to *all* of psql's toggling backslash commands, not only \timing. 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] Replace offnum++ by OffsetNumberNext
Patch applied. Thanks.
---
Fujii Masao wrote:
> This is the patch replace offnum++ by OffsetNumberNext.
>
> According to off.h, OffsetNumberNext is the macro prepared to
> disambiguate the different manipulations on OffsetNumbers.
> But, increment operator was used in some places instead of the macro.
>
> --
> Fujii Masao
> NIPPON TELEGRAPH AND TELEPHONE CORPORATION
> NTT Open Source Software Center
> TEL (03)5860-5115
> FAX (03)5463-5490
> ? patch.diff
> Index: src/backend/access/heap/pruneheap.c
> ===
> RCS file: /projects/cvsroot/pgsql/src/backend/access/heap/pruneheap.c,v
> retrieving revision 1.9
> diff -c -r1.9 pruneheap.c
> *** src/backend/access/heap/pruneheap.c 26 Mar 2008 21:10:37 -
> 1.9
> --- src/backend/access/heap/pruneheap.c 4 Apr 2008 14:34:19 -
> ***
> *** 789,795
> MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
>
> maxoff = PageGetMaxOffsetNumber(page);
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
> {
> ItemId lp = PageGetItemId(page, offnum);
> HeapTupleHeader htup;
> --- 789,795
> MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber));
>
> maxoff = PageGetMaxOffsetNumber(page);
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum =
> OffsetNumberNext(offnum))
> {
> ItemId lp = PageGetItemId(page, offnum);
> HeapTupleHeader htup;
> Index: src/backend/executor/nodeBitmapHeapscan.c
> ===
> RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeBitmapHeapscan.c,v
> retrieving revision 1.25
> diff -c -r1.25 nodeBitmapHeapscan.c
> *** src/backend/executor/nodeBitmapHeapscan.c 26 Mar 2008 21:10:38 -
> 1.25
> --- src/backend/executor/nodeBitmapHeapscan.c 4 Apr 2008 14:34:19 -
> ***
> *** 301,307
> OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
> OffsetNumber offnum;
>
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum++)
> {
> ItemId lp;
> HeapTupleData loctup;
> --- 301,307
> OffsetNumber maxoff = PageGetMaxOffsetNumber(dp);
> OffsetNumber offnum;
>
> ! for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum =
> OffsetNumberNext(offnum))
> {
> ItemId lp;
> HeapTupleData loctup;
> Index: src/backend/storage/page/bufpage.c
> ===
> RCS file: /projects/cvsroot/pgsql/src/backend/storage/page/bufpage.c,v
> retrieving revision 1.78
> diff -c -r1.78 bufpage.c
> *** src/backend/storage/page/bufpage.c10 Feb 2008 20:39:08 -
> 1.78
> --- src/backend/storage/page/bufpage.c4 Apr 2008 14:34:19 -
> ***
> *** 533,539
>* Since this is just a hint, we must confirm
> that there is
>* indeed a free line pointer
>*/
> ! for (offnum = FirstOffsetNumber; offnum <=
> nline; offnum++)
> {
> ItemId lp =
> PageGetItemId(page, offnum);
>
> --- 533,539
>* Since this is just a hint, we must confirm
> that there is
>* indeed a free line pointer
>*/
> ! for (offnum = FirstOffsetNumber; offnum <=
> nline; offnum = OffsetNumberNext(offnum))
> {
> ItemId lp =
> PageGetItemId(page, offnum);
>
> ***
> *** 736,742
> totallen = 0;
> nused = 0;
> nextitm = 0;
> ! for (offnum = 1; offnum <= nline; offnum++)
> {
> lp = PageGetItemId(page, offnum);
> Assert(ItemIdHasStorage(lp));
> --- 736,742
> totallen = 0;
> nused = 0;
> nextitm = 0;
> ! for (offnum = FirstOffsetNumber; offnum <= nline; offnum =
> OffsetNumberNext(offnum))
> {
> lp = PageGetItemId(page, offnum);
> Assert(ItemIdHasStorage(lp));
>
>
> --
> Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches
--
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
Re: [PATCHES] libpq Win32 Mutex performance patch
Andrew Chernow wrote: > Tom Lane wrote: >> Silently not locking is surely >> not very safe. >> > > Here is the dump code version of the patch. If anyone wants the return > value idea, let me know. So is this a patch we want applied? -- 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] libpq Win32 Mutex performance patch
Alvaro Herrera wrote: > Andrew Chernow wrote: > > Tom Lane wrote: > >> Silently not locking is surely > >> not very safe. > >> > > > > Here is the dump code version of the patch. If anyone wants the > > return value idea, let me know. > > So is this a patch we want applied? Please see my other thread about libpq thread-locking which should be finished before this one, after which this patch will change. So no, this is not the version to be applied. //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] libpq Win32 Mutex performance patch
Magnus Hagander wrote: > Alvaro Herrera wrote: > > Andrew Chernow wrote: > > > Tom Lane wrote: > > >> Silently not locking is surely > > >> not very safe. > > >> > > > > > > Here is the dump code version of the patch. If anyone wants the > > > return value idea, let me know. > > > > So is this a patch we want applied? > > Please see my other thread about libpq thread-locking which should be > finished before this one, after which this patch will change. So no, > this is not the version to be applied. Hmm, AFAICT on that other thread you state > I'm leaning towards going with the simpler one, which is the patch > from Andrew that crashes on out of memory. which would seem to mean this patch? -- 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] [GENERAL] Making sure \timing is on
On Tue, May 13, 2008 at 11:36:57AM -0400, Tom Lane wrote:
> David Fetter <[EMAIL PROTECTED]> writes:
> >>> Surely this is merely proof of concept and not a complete patch.
> >>
> >> Next patch attached :)
>
> Uh, my point was that the agreement was to do this to *all* of
> psql's toggling backslash commands, not only \timing.
Done :)
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.168
diff -c -c -r1.168 describe.c
*** src/bin/psql/describe.c 2 May 2008 10:16:16 - 1.168
--- src/bin/psql/describe.c 4 May 2008 23:54:53 -
***
*** 307,315
"WHEN t.typlen < 0\n"
" THEN CAST('var' AS
pg_catalog.text)\n"
"ELSE CAST(t.typlen AS
pg_catalog.text)\n"
! " END AS \"%s\",\n",
gettext_noop("Internal name"),
! gettext_noop("Size"));
appendPQExpBuffer(&buf,
" pg_catalog.obj_description(t.oid, 'pg_type')
as \"%s\"\n",
gettext_noop("Description"));
--- 307,325
"WHEN t.typlen < 0\n"
" THEN CAST('var' AS
pg_catalog.text)\n"
"ELSE CAST(t.typlen AS
pg_catalog.text)\n"
! " END AS \"%s\",\n"
! "
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",
gettext_noop("Internal name"),
! gettext_noop("Size"),
! gettext_noop("Elements"));
appendPQExpBuffer(&buf,
" pg_catalog.obj_description(t.oid, 'pg_type')
as \"%s\"\n",
gettext_noop("Description"));
--
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] [GENERAL] Making sure \timing is on
On Tue, May 13, 2008 at 01:53:33PM -0700, David Fetter wrote:
> On Tue, May 13, 2008 at 11:36:57AM -0400, Tom Lane wrote:
> > David Fetter <[EMAIL PROTECTED]> writes:
> > >>> Surely this is merely proof of concept and not a complete patch.
> > >>
> > >> Next patch attached :)
> >
> > Uh, my point was that the agreement was to do this to *all* of
> > psql's toggling backslash commands, not only \timing.
>
> Done :)
Ugh. This time with the correct patch attached :P
Cheers,
David (not having much luck with attachments)
--
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: doc/src/sgml/ref/psql-ref.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.203
diff -c -c -r1.203 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml 8 May 2008 17:04:26 - 1.203
--- doc/src/sgml/ref/psql-ref.sgml 13 May 2008 20:52:29 -
***
*** 673,685
! \a
! If the current table output format is unaligned, it is switched to
aligned.
! If it is not unaligned, it is set to unaligned. This command is
! kept for backwards compatibility. See \pset for a
! more general solution.
--- 673,687
!\a [ ON |
! OFF ]
! Without parameter, toggle format between aligned and
! unaligned. With parameter, set it. This command is kept for
! backwards compatibility. See \pset for a more
! general solution.
***
*** 1292,1305
! \H
! Turns on HTML query output format. If the
! HTML format is already on, it is switched
! back to the default aligned text format. This command is for
! compatibility and convenience, but see \pset
! about setting other output options.
--- 1294,1308
!\H [ ON |
! OFF ]
! Without parameter, toggles between HTML and
! aligned query output format. With paramter, sets it.
! This command is for compatibility and convenience, but see
! \pset about setting other output options.
***
*** 1867,1876
!\timing
! Toggles a display of how long each SQL statement takes, in
milliseconds.
--- 1870,1882
!\timing [ON | OFF]
! Without parameter, toggles a display of how long each SQL
! statement takes, in milliseconds. With parameter, sets same.
Index: src/bin/psql/command.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.188
diff -c -c -r1.188 command.c
*** src/bin/psql/command.c 8 May 2008 17:04:26 - 1.188
--- src/bin/psql/command.c 13 May 2008 20:52:29 -
***
*** 180,189
*/
if (strcmp(cmd, "a") == 0)
{
! if (pset.popt.topt.format != PRINT_ALIGNED)
! success = do_pset("format", "aligned", &pset.popt,
pset.quiet);
else
! success = do_pset("format", "unaligned", &pset.popt,
pset.quiet);
}
/* \C -- override table title (formerly change HTML caption) */
--- 180,199
*/
if (strcmp(cmd, "a") == 0)
{
! char *opt = psql_scan_slash_option(scan_state,
!
OT_NORMAL, NULL, true);
! if (opt)
! success = do_pset("format",
!
ParseVariableBool(opt) ? "aligned" : "unaligned",
! &pset.popt,
pset.quiet);
else
! {
! if (pset.popt.topt.format != PRINT_ALIGNED)
! success = do_pset("format", "aligned",
&pset.popt, pset.quiet);
! else
! success = do_pset("format", "unaligned",
&pset.popt, pset.quiet);
! }
! free(opt);
}
/* \C -- override table title (formerly change HTML caption) */
***
*** 538,547
/* HTML mode */
else if (strcmp(cmd, "H") == 0 || strcmp(cmd, "html") == 0)
{
! if (pset.p
Re: [PATCHES] options for RAISE statement
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > I am sending enhanced version of original patch. Applied with syntax revisions as per pghackers discussion. I made a couple of other changes too: I took out the restriction against throwing codes that are category codes, and instead just documented that that might be a bad idea. I don't see a reason to prohibit that if people really want to do it. Also, for the few condition names that are duplicated in plerrcodes.h, I just made it throw the first sqlstate it finds, rather than complaining. This will work, in the sense that you can trap the error using the same condition name, and I don't think it's really important to make the user think about this. 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] options for RAISE statement
2008/5/14 Tom Lane <[EMAIL PROTECTED]>: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> I am sending enhanced version of original patch. > > Applied with syntax revisions as per pghackers discussion. thank you > > I made a couple of other changes too: I took out the restriction against > throwing codes that are category codes, and instead just documented that > that might be a bad idea. I don't see a reason to prohibit that if > people really want to do it. I didn't see a reason to allow it - but I don't see it important Also, for the few condition names that are > duplicated in plerrcodes.h, I just made it throw the first sqlstate it > finds, rather than complaining. This will work, in the sense that you > can trap the error using the same condition name, and I don't think it's > really important to make the user think about this. I am afraid so this can be surprise for some people - but again it's not necessary solve it now. Regards Pavel Stehule > >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] [GENERAL] Making sure \timing is on
David Fetter <[EMAIL PROTECTED]> writes: > On Tue, May 13, 2008 at 11:36:57AM -0400, Tom Lane wrote: >>> Uh, my point was that the agreement was to do this to *all* of >>> psql's toggling backslash commands, not only \timing. >> >> Done :) Hmm, I thought we had a lot more than three that were like this. But on looking closer, I guess all the other ones are \pset boolean options that already behave properly. Actually, \a and \H are fairly bogus anyway, because they are "toggling" a setting that has more than two values. I wonder whether defining the argument as a boolean is really very sane. Perhaps it would be better to take the argument if given as just a regular format setting --- ie, with an argument, either of these would just be a shorthand for \pset format. 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] stored procedure stats in collector
Martin Pihlak <[EMAIL PROTECTED]> writes: > Now I just realized that the current patch doesn't handle this quite > correctly. Modified patch attached. I'm starting to look through this now, and I notice that the various code paths in execQual.c are not too consistent about whether it counts as a call if a strict function is skipped over because of NULL arguments. I'm inclined to make it uniformly say that that's not a call and is not included in the stats --- any objections? 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] [GENERAL] Making sure \timing is on
Tom Lane escribió: > Actually, \a and \H are fairly bogus anyway, because they are "toggling" > a setting that has more than two values. I wonder whether defining the > argument as a boolean is really very sane. Perhaps it would be better to > take the argument if given as just a regular format setting --- ie, > with an argument, either of these would just be a shorthand for > \pset format. Agreed -- they are bogus. However, making "\H aligned" be an alias for "\pset format aligned" does not look very sane either (is that "html aligned" or what?) Seeing how these have been deprecated for a very long while, perhaps the thing to do is leave their behavior alone and throw a warning when they are used; in a couple more releases, remove them. However this isn't very sane either, because we'd break scripts that are using \H for no good reason. Another thing we could do is keep them as "toggles", but instead of aligned/HTML and aligned/unaligned, make them remember the state that was set at the time they were called, and toggle between HTML (or aligned) and the last state. ... except that since aligned is the default mode, \a should not toggle between last state and aligned, but rather between last state and unaligned. Which makes it a misnomer. Or maybe the thing to do is leave them damn well alone and just fix \timing. -- 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
Re: [PATCHES] stored procedure stats in collector
I wrote: > I'm starting to look through this now, I found another fairly big problem, which is that this stuff isn't even going to begin to compile on Windows. What I think we should do about that is forget tracking getrusage()'s user/system/real time and just track elapsed time. We have the technology to get that in a portable fashion (cf the well-proven instrument.c code). Such a decision would also alleviate two of the biggest negatives of this patch, which are the runtime overhead and the extent to which it's going to bloat the pgstats file. Thoughts? 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] posix advises ...
On Sun, 11 May 2008, Hans-Juergen Schoenig wrote: we also made some simple autoconf hack to check for broken posix_fadvise. Because of how you did that, your patch is extremely difficult to even test. You really should at least scan the output from diff you're about to send before submitting a patch to make sure it makes sense--yours is over 30,000 lines long just to implement a small improvement. The reason for that is that you regenerated "configure" using a later version of Autoconf than the official distribution, and the diff for the result is gigantic. You even turned off the check in configure.in that specifically prevents you from making that mistake so it's not like you weren't warned. You should just show the necessary modifications to configure.in in your patch, certainly shouldn't submit a patch that subverts the checks there, and leave out the resulting configure file if you didn't use the same version of Autoconf. I find the concept behind this patch very useful and I'd like to see a useful one re-submitted. I'm in the middle of setting up some new hardware this month and was planning to test the existing fadvise patches Greg Stark sent out as part of that. Having another one to test for accelerating multiple sequential scans would be extremely helpful to add to that, because then I think I can reuse some of the test cases Jeff Davis put together for the 8.3 improvements in that area to see how well it works. It wasn't as clear to me how to test the existing bitmap scan patch, yours seems a more straightforward patch to use as a testing ground for fadvise effectiveness. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] libpq object hooks
Merlin Moncure wrote: Attached is an updated version of 'libpq object hooks'. The primary purpose for libpq object hooks is to support our libpqtypes system (not attached), but could possibly some nice sideband features to libpq. We are hoping to sneak this into the May commit fest. I've had a preliminary look at this. The first thing it needs is lots and lots of documentation. I think it probably needs a Section in the libpq chapter all on its own, preferably with some examples. I think that lack alone is enough to keep it from being committed for now. Second, the hook names are compared case insensitively and by linear search. I don't see any justification for using case insensitive names for hooks in a C program, so I think that part should go. And if we expect to keep anything other than trivial numbers of hooks we should look at some sort of binary or hashed search. The thing that is a bit disturbing is that the whole style of this scheme is very different from the fairly simple APIs that the rest of libpq presents. It's going to make libpq look rather odd, I think. I would have felt happier if the authors had been able to come up with a simple scheme to add API calls to export whatever information they needed, rather than using this callback scheme. That said, this patch doesn't look that bad to me otherwise, at least on first inspection. One might say the the ability to add tuples to a resultset arbitrarily, or to change an attribute arbitrarily, might be footguns (and if you can add one, why can't you delete one?), but then this is data in the hands of the client anyway, so they can do what they like with it after they get it out of the resultset, so I guess there's no real danger. 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] libpq object hooks
Andrew Dunstan wrote: > The first thing it needs is lots and lots of documentation. I think it > probably needs a Section in the libpq chapter all on its own, preferably > with some examples. I think that lack alone is enough to keep it from > being committed for now. > > Second, the hook names are compared case insensitively and by linear > search. I don't see any justification for using case insensitive names > for hooks in a C program, so I think that part should go. And if we > expect to keep anything other than trivial numbers of hooks we should > look at some sort of binary or hashed search. > > The thing that is a bit disturbing is that the whole style of this > scheme is very different from the fairly simple APIs that the rest of > libpq presents. It's going to make libpq look rather odd, I think. I > would have felt happier if the authors had been able to come up with a > simple scheme to add API calls to export whatever information they > needed, rather than using this callback scheme. My personal opinion is still that I would like to see a more general usefulness for these functions before adding them to libpq. The complexity of the API just mirrors my gut feeling on this. -- 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] libpq object hooks
Andrew Dunstan escribió: > The thing that is a bit disturbing is that the whole style of this > scheme is very different from the fairly simple APIs that the rest of > libpq presents. It's going to make libpq look rather odd, I think. I > would have felt happier if the authors had been able to come up with a > simple scheme to add API calls to export whatever information they > needed, rather than using this callback scheme. I'm not sure I understand this point. Remember that this is here to support the libpqtypes library. There doesn't seem to be a way for an API such as you describe to work. > Second, the hook names are compared case insensitively and by linear > search. I don't see any justification for using case insensitive names > for hooks in a C program, so I think that part should go. And if we > expect to keep anything other than trivial numbers of hooks we should > look at some sort of binary or hashed search. Keep in mind that the original patch supported a single hook being registered. Perhaps we could dream about having a couple of hooks registered, but turning into hashed search would seem to be overkill, at least for now. (If hooking into libpq is truly successful we can always improve it later -- it's not an exported detail of the API after all.) -- 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
Re: [PATCHES] stored procedure stats in collector
Tom Lane wrote: I wrote: I'm starting to look through this now, I found another fairly big problem, which is that this stuff isn't even going to begin to compile on Windows. Where exactly is taht problem? In getrusage()? We have a getrusage() in src/port that works fine on Windows, no? What I think we should do about that is forget tracking getrusage()'s user/system/real time and just track elapsed time. We have the technology to get that in a portable fashion (cf the well-proven instrument.c code). Such a decision would also alleviate two of the biggest negatives of this patch, which are the runtime overhead and the extent to which it's going to bloat the pgstats file. Those argument makes a lot of sense, though. A bloated pgstats file can be a real problem. And I don't see that information as being all that helpful anyway. //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] stored procedure stats in collector
Magnus Hagander <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> I found another fairly big problem, which is that this stuff isn't even >> going to begin to compile on Windows. > Where exactly is taht problem? In getrusage()? We have a getrusage() in > src/port that works fine on Windows, no? Huh ... I'd forgotten about that ... although it seems to work only for rather small values of "work", since the WIN32 code path isn't paying attention to the "who" argument. >> What I think we should do about that is forget tracking getrusage()'s >> user/system/real time and just track elapsed time. > Those argument makes a lot of sense, though. Yeah, the real bottom line here is whether we are buying anything that's worth another two kernel calls per function. Given all the buffering and offloading of I/O to bgwriter that we try to do, it's hard to argue that stime/utime measurements for the current backend really mean a lot. 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] stored procedure stats in collector
Tom Lane wrote: Magnus Hagander <[EMAIL PROTECTED]> writes: Tom Lane wrote: I found another fairly big problem, which is that this stuff isn't even going to begin to compile on Windows. Where exactly is taht problem? In getrusage()? We have a getrusage() in src/port that works fine on Windows, no? Huh ... I'd forgotten about that ... although it seems to work only for rather small values of "work", since the WIN32 code path isn't paying attention to the "who" argument. True, but it works for this case :-) What I think we should do about that is forget tracking getrusage()'s user/system/real time and just track elapsed time. Those argument makes a lot of sense, though. Yeah, the real bottom line here is whether we are buying anything that's worth another two kernel calls per function. Given all the buffering and offloading of I/O to bgwriter that we try to do, it's hard to argue that stime/utime measurements for the current backend really mean a lot. Agreed. //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] stored procedure stats in collector
Magnus Hagander <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Huh ... I'd forgotten about that ... although it seems to work only for >> rather small values of "work", since the WIN32 code path isn't paying >> attention to the "who" argument. > True, but it works for this case :-) Shouldn't we at least make it fail with EINVAL if "who" doesn't match whichever semantics this code is able to implement? [ not relevant to the immediate patch, I suppose, but it might save some tears later. ] 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] stored procedure stats in collector
Tom Lane wrote: Magnus Hagander <[EMAIL PROTECTED]> writes: Tom Lane wrote: Huh ... I'd forgotten about that ... although it seems to work only for rather small values of "work", since the WIN32 code path isn't paying attention to the "who" argument. True, but it works for this case :-) Shouldn't we at least make it fail with EINVAL if "who" doesn't match whichever semantics this code is able to implement? [ not relevant to the immediate patch, I suppose, but it might save some tears later. ] Yeah, we only ever call it asking for our own process, but I guess we might at some point in the future change that, so it can't hurt.. Want me to do it, or will you? //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] stored procedure stats in collector
Magnus Hagander <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Shouldn't we at least make it fail with EINVAL if "who" doesn't match >> whichever semantics this code is able to implement? > Yeah, we only ever call it asking for our own process, but I guess we > might at some point in the future change that, so it can't hurt.. Want > me to do it, or will you? Please do, I'm going to bed ... 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
