Re: [PATCHES] WIP Join Removal
Tom Lane <[EMAIL PROTECTED]> writes: > Simon Riggs <[EMAIL PROTECTED]> writes: >> On Mon, 2008-09-01 at 22:23 +0300, Heikki Linnakangas wrote: >>> Did plan invalidation make it safe to rely on the presence of a unique >>> index for planning decisions? > >> My understanding was "Yes" and this case was the specific reason I >> originally wanted to pursue plan invalidation back in 2006. It may be worth considering what other cases might need this info and taking them into account to be sure the solution is usable for them too. I suspect we'll probably need a generic function for determining whether a PathKey list can be proved unique. Other cases off the top of three other cases where this could be useful -- but generally anywhere the planner introduces a Unique node could benefit from looking at this. a) Turn a UNION into UNION ALL if there are unique indexes for any column in each side and at least one column is a constant in each side and none of the constants are equal. b) Remove the aggregate on IN subqueries when there's a unique constraint so that: SELECT * from a where a.fk IN (select pk FROM b) Can do a semijoin without taking care to avoid duplicating records in "a" if there should be duplicate values of "pk" in "b". c) Turn bad mysqlish queries which are really semijoins (used to work around their historic lack of subqueries) such as: SELECT DISTINCT a.pk FROM a JOIN b USING (x) into SELECT a.pk FROM a WHERE x IN (SELECT x FROM b) -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] WIP Join Removal
Simon Riggs <[EMAIL PROTECTED]> writes: > Same answer, just slower. Removing the join makes the access to a into a > SeqScan, whereas it was a two-table index plan when both tables present. > The two table plan is added by the immediately preceding call add_... - > i.e. that plan is only added during join time not during planning of > base relations. Perhaps it would clearer to discuss a non-outer join here: select invoices.* from customer join invoices using (company_id,customer_id) where customer_id = ? where there's a foreign key relation guaranteeing that every invoice has a matching . If there's an index on customer(customer_id) but not on invoices(customer_id) then conceivably it would be faster to use that than scan all of the invoices. I wonder if it would be more worthwhile to remove them and have a subsequent phase where we look for possible joins to *add*. So even if the user writes "select * from invoices where customer_id=?" the planner might be able to discover that it can find those records quicker by scanning customer, finding the matching and then using an index to look them up in invoices. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] Explain XML patch v2
"Tom Raney" <[EMAIL PROTECTED]> writes: > This is an update to my EXPLAIN XML patch submitted a few days ago. > > I've added a documentation patch and modified some of the code per comments > by > Gregory Stark. You should update the wiki at http://wiki.postgresql.org/wiki/CommitFest:2008-07 so any reviewers look at the updated patch. (I certainly don't see any reason to wait two months for the next commit fest) -- 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
Re: [PATCHES] [HACKERS] Hint Bits and Write I/O
"Simon Riggs" <[EMAIL PROTECTED]> writes: > The default and minimum value for this parameter is 1, so very similar to > existing behaviour. Expected settings would be 2-5, possibly as high as 20, > though those are just educated guesses. So the maximum is set arbitrarily as > 100. Not a fan of arbitrary constants. ISTM this should just have a maximum of MaxHeapTuplesPerPage. I'm not really happy with having this parameter at all. It's not something a DBA can understand or have any hope of setting intelligently. I assume this is a temporary measure until we have a better understanding of what real-world factors affect the right values for this knob? > Temp buffers are never dirtied by hint bit setting. Most temp tables are > written in a single command, so that re-accessing clog for temp tuples > is seldom costly. This also changes current behaviour. I'm not sure I agree with this logic and it doesn't seem like temporary tables are an important enough case to start coming up with special cases which may help or may hurt. Most people use temporary tables the way you describe but I'm sure there's someone out there using temporary tables in a radically different fashion. I'm also a bit concerned that *how many hint bits* isn't enough information to determine how important it is to write out the page. What about how old the oldest transaction is which was hinted? Or how many *unhinted* xmin/xmax values were found? If HTSV can hint xmin for a tuple but finds xmax still in progress perhaps that's a good sign it's not worth dirtying the page? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- 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] Hint Bits and Write I/O
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > If only VACUUM is going to set "flexible" to off, maybe it's better to > leave the APIs as they are and have a global that's set by VACUUM only > (and reset in a PG_CATCH block). Ugh. Perhaps it would be simpler to have a wrapper function HTSV() macro which passes flexible=true to HTSV_internal(). Then vacuum can call HTSV_internal(). I'm not sure what the performance tradeoff is between having an extra argument to HTSV and having HTSV check a global which messes with optimizations. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Feature: give pg_dump a WHERE clause expression
"daveg" <[EMAIL PROTECTED]> writes: > The argument that one could just use copy and tar and gzip applies to the > whole existance of pg_dump itself. pg_dump is just a more convenient way to > copy out tables and as such does not NEED to exist at all. However it is > convenient to that is does exist and does have convenient features to > selectively handle schemas and tables. That's not really true, pg_dump does a *lot* more work than just group table data together. Its real value comes in reconstructing DDL for all the objects and understanding the dependencies between them. That's functionality that isn't available elsewhere and can't be reproduced without just reimplementing pg_dump. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] \d+ should display the storage options for columns
"Alvaro Herrera" <[EMAIL PROTECTED]> writes:
> This seems to be against an older version of psql ... with the
> printTable API stuff, we reworked this -- in particular the mbvalidate()
> call that's only on WIN32 is gone (actually it's the lack of it that's
> gone.)
Sorry. Here's a patch against a current sync of HEAD.
Incidentally how can this new API work? Calling _() on a function parameter
would work but how would the translation tools know what strings need to be
translated?
Index: src/bin/psql/describe.c
===
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.173
diff -c -r1.173 describe.c
*** src/bin/psql/describe.c 13 May 2008 00:23:17 - 1.173
--- src/bin/psql/describe.c 23 May 2008 18:52:57 -
***
*** 784,790
printTableContent cont;
int i;
char *view_def = NULL;
! char *headers[4];
char **modifiers = NULL;
char **ptr;
PQExpBufferData title;
--- 784,790
printTableContent cont;
int i;
char *view_def = NULL;
! char *headers[5];
char **modifiers = NULL;
char **ptr;
PQExpBufferData title;
***
*** 852,858
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
"\n a.attnotnull, a.attnum");
if (verbose)
! appendPQExpBuffer(&buf, ",
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
if (tableinfo.relkind == 'i')
appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
--- 852,858
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
"\n a.attnotnull, a.attnum");
if (verbose)
! appendPQExpBuffer(&buf, ", a.attstorage,
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
if (tableinfo.relkind == 'i')
appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
***
*** 918,924
--- 918,927
}
if (verbose)
+ {
+ headers[cols++] = "Storage";
headers[cols++] = "Description";
+ }
printTableInit(&cont, &myopt, title.data, cols, numrows);
***
*** 972,980
printTableAddCell(&cont, modifiers[i], false);
}
! /* Description */
if (verbose)
! printTableAddCell(&cont, PQgetvalue(res, i, 5), false);
}
/* Make footers */
--- 975,992
printTableAddCell(&cont, modifiers[i], false);
}
! /* Storage and Description */
if (verbose)
! {
! char *storage = PQgetvalue(res, i, 5);
! printTableAddCell(&cont, (storage[0]=='p' ? "plain" :
!
(storage[0]=='m' ? "main" :
!
(storage[0]=='x' ? "extended" :
!
(storage[0]=='e' ? "external" :
!
"???",
! false);
! printTableAddCell(&cont, PQgetvalue(res, i, 6), false);
! }
}
/* Make footers */
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL
training!
--
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] \d+ should display the storage options for columns
"Gregory Stark" <[EMAIL PROTECTED]> writes:
> Oleg pointed out to me here that while we have a command to *set* the toast
> storage characteristics there's no actual supported way to display the current
> settings.
>
> It seems like this would be a reasonable thing to add to \d+
Sorry, sent the wrong diff before. The previous diff didn't work due to an
array overflow.
Index: src/bin/psql/describe.c
===
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.170
diff -c -r1.170 describe.c
*** src/bin/psql/describe.c 5 May 2008 01:21:03 - 1.170
--- src/bin/psql/describe.c 21 May 2008 20:25:26 -
***
*** 791,797
printTableOpt myopt = pset.popt.topt;
int i;
char *view_def = NULL;
! const char *headers[5];
char **cells = NULL;
char **footers = NULL;
char **ptr;
--- 791,797
printTableOpt myopt = pset.popt.topt;
int i;
char *view_def = NULL;
! const char *headers[6];
char **cells = NULL;
char **footers = NULL;
char **ptr;
***
*** 865,871
if (verbose)
{
! cols++;
headers[cols - 1] = _("Description");
}
--- 865,872
if (verbose)
{
! cols+=2;
! headers[cols - 2] = _("Storage");
headers[cols - 1] = _("Description");
}
***
*** 877,883
"\n (SELECT
substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
"\n FROM pg_catalog.pg_attrdef d"
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
! "\n a.attnotnull, a.attnum");
if (verbose)
appendPQExpBuffer(&buf, ",
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
--- 878,884
"\n (SELECT
substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
"\n FROM pg_catalog.pg_attrdef d"
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
! "\n a.attnotnull, a.attnum,
a.attstorage");
if (verbose)
appendPQExpBuffer(&buf, ",
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
***
*** 957,967
/* Description */
if (verbose)
#ifdef WIN32
! cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res,
i, 5), myopt.encoding);
#else
! cells[i * cols + cols - 1] = PQgetvalue(res, i, 5);
#endif
}
/* Make title */
--- 958,978
/* Description */
if (verbose)
+ {
+ char *storage = PQgetvalue(res, i, 5);
+
+ cells[i * cols + cols -2] =
+ (storage[0]=='p' ? _("plain") :
+(storage[0]=='m' ? _("main") :
+ (storage[0]=='x' ? _("extended") :
+ (storage[0]=='e' ? _("external") :
+ "???";
#ifdef WIN32
! cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res,
i, 6), myopt.encoding);
#else
! cells[i * cols + cols - 1] = PQgetvalue(res, i, 6);
#endif
+ }
}
/* Make title */
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL
training!
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] \d+ should display the storage options for columns
Oleg pointed out to me here that while we have a command to *set* the toast
storage characteristics there's no actual supported way to display the current
settings.
It seems like this would be a reasonable thing to add to \d+
Index: src/bin/psql/describe.c
===
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.170
diff -c -r1.170 describe.c
*** src/bin/psql/describe.c 5 May 2008 01:21:03 - 1.170
--- src/bin/psql/describe.c 21 May 2008 18:07:13 -
***
*** 865,871
if (verbose)
{
! cols++;
headers[cols - 1] = _("Description");
}
--- 865,872
if (verbose)
{
! cols+=2;
! headers[cols - 2] = _("Storage");
headers[cols - 1] = _("Description");
}
***
*** 877,883
"\n (SELECT
substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
"\n FROM pg_catalog.pg_attrdef d"
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
! "\n a.attnotnull, a.attnum");
if (verbose)
appendPQExpBuffer(&buf, ",
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
--- 878,884
"\n (SELECT
substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
"\n FROM pg_catalog.pg_attrdef d"
"\n WHERE d.adrelid = a.attrelid
AND d.adnum = a.attnum AND a.atthasdef),"
! "\n a.attnotnull, a.attnum,
a.attstorage");
if (verbose)
appendPQExpBuffer(&buf, ",
pg_catalog.col_description(a.attrelid, a.attnum)");
appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
***
*** 957,967
--- 958,979
/* Description */
if (verbose)
+ {
+ char *storage = PQgetvalue(res, i, 6);
+
+ cells[i * cols + cols -2] =
+ (storage[0]=='p' ? "PLAIN" :
+(storage[0]=='m' ? "MAIN" :
+ (storage[0]=='x' ? "EXTENDED" :
+ (storage[0]=='e' ? "EXTERNAL" :
+ "???";
+
#ifdef WIN32
cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res,
i, 5), myopt.encoding);
#else
cells[i * cols + cols - 1] = PQgetvalue(res, i, 5);
#endif
+ }
}
/* Make title */
--
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: [HACKERS] [PATCHES] WITH RECURSIVE patch V0.1
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: >> >> Couldn't we just have it pay attention to the existing >> >> max_stack_depth? >> > >> > Recursive query does not consume stack. The server enters an infinite >> > loop without consuming stack. Stack-depth error does not happen. >> >> We could have a separate guc variable which limits the maximum number of >> levels of recursive iterations. That might be a useful feature for DBAs that >> want to limit their users from issuing an infinite query. > > statement_timeout :) Good point. Though it occurs to me that if you set FETCH_COUNT in psql (or do the equivalent in your code ) statement_timeout becomes much less useful. -- 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: [HACKERS] [PATCHES] WITH RECURSIVE patch V0.1
"Yoshiyuki Asaba" <[EMAIL PROTECTED]> writes: > Hi, > > From: David Fetter <[EMAIL PROTECTED]> > Subject: Re: [HACKERS] [PATCHES] WITH RECURSIVE patch V0.1 > Date: Mon, 19 May 2008 04:36:30 -0700 > >> > > I think it's the other way around. The server should not emit >> > > infinite number of records. >> > >> > How about adding new GUC parameter "max_recursive_call"? >> >> Couldn't we just have it pay attention to the existing >> max_stack_depth? > > Recursive query does not consume stack. The server enters an infinite > loop without consuming stack. Stack-depth error does not happen. We could have a separate guc variable which limits the maximum number of levels of recursive iterations. That might be a useful feature for DBAs that want to limit their users from issuing an infinite query. Note that users can always construct their query to limit the number of recursive iterations. So this would only be useful for DBAs that don't trust their users and want to impose a limit. It doesn't add any actual expressive power that SQL doesn't have already. The recursive query syntax in the spec actually does include the ability to assign an output column to show what level of recursive iteration you're on. So alternately we could have a GUC variable which just allows the DBA to prohibit any recursive query without such a column and a fiter imposing a maximum value on it. That's probably the most appropriate option. -- 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] [HACKERS] WITH RECURSIVE patch V0.1
"Martijn van Oosterhout" <[EMAIL PROTECTED]> writes: > From an implementation point of view, the only difference between > breadth-first and depth-first is that your tuplestore needs to be LIFO > instead of FIFO. I think it's not so simple. How do you reconcile that concept with the join plans like merge join or hash join which expect you to be able to be able to process the records in a specific order? It sounds like you might have to keep around a stack of started executor nodes or something but hopefully we can avoid anything like that because, well, ick. -- 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
Re: [PATCHES] WITH RECURSIVE patch V0.1
This is indeed really cool. I'm sorry I haven't gotten to doing what I promised in this area but I'm glad it's happening anyways. "Zoltan Boszormenyi" <[EMAIL PROTECTED]> writes: > Can we get the rows in tree order, please? >... > After all, I didn't specify any ORDER BY clauses in the base, recursive or the > final queries. The standard has a clause to specify depth-first order. However doing a depth-first traversal would necessitate quite a different looking plan and it's far less obvious (to me anyways) how to do it. > Also, it seems there are no infinite recursion detection: > > # with recursive x(level, parent, child) as ( >select 1::integer, * from test_connect_by where parent is null >union all >select x.level + 1, base.* from test_connect_by as base, x where base.child > = x.child > ) select * from x; > ... it waits and waits and waits ... Well, psql might wait and wait but it's actually receiving rows. A cleverer client should be able to deal with infinite streams of records. I think DB2 does produce a warning if there is no clause it can determine will bound the results. But that's not actually reliable. It's quite possible to have clauses which will limit the output but not in a way the database can determine. Consider for example a tree-traversal for a binary tree stored in a recursive table reference. The DBA might know that the data contains no loops but the database doesn't. -- 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
Re: [PATCHES] WITH RECURSIVE patch V0.1
"David Fetter" <[EMAIL PROTECTED]> writes: > On Mon, May 19, 2008 at 12:21:20AM -0400, Gregory Stark wrote: >> "Zoltan Boszormenyi" <[EMAIL PROTECTED]> writes: >> > Also, it seems there are no infinite recursion detection: >> > >> > # with recursive x(level, parent, child) as ( >> >select 1::integer, * from test_connect_by where parent is null >> >union all >> >select x.level + 1, base.* from test_connect_by as base, x where >> > base.child >> > = x.child >> > ) select * from x; >> > ... it waits and waits and waits ... >> >> Well, psql might wait and wait but it's actually receiving rows. A >> cleverer client should be able to deal with infinite streams of >> records. > > That would be a very good thing for libpq (and its descendants) to > have :) I think you can do it in libpq. In psql you can use \set FETCH_COUNT or somrthing like this (I can't look it up just now) to use a cursor to do this too. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] Patch to change psql default banner v6
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: > >> I know we decided not to do that, but I am trying to figure out what the >> goal if 'help' is? To display the most frequently-used help commands? >> Aren't they at the top of \?. > > The purpose of 'help' is to provide useful help. If it only says "see \?" > then it's just redirecting you somewhere else, which isn't useful. Haven't been following this thread. Has the idea of making "help" just a synonym for \? not come up? Or has it been rejected? It seems simplest. -- 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] create or replace language
"Tom Lane" <[EMAIL PROTECTED]> writes: > The equivalent problem for views and functions is handled by restricting > CREATE OR REPLACE to not change the output column set of a view or the > type signature of a function, independently of whether there are any > actual references to the object. So maybe the right thing is that > CREATE OR REPLACE LANGUAGE can change "inessential" properties of an > existing language, but not the core properties --- which might only be > the handler function, though you could make a case for the validator and > the trusted flag as well. I'm not so sure. What about if a PL language wants to include a version number in the language handler? Or if a new version has to change the name for some reason -- perhaps they discover that the old name doesn't work on some linkers for some reason. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- 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] Proposed patch - psql wraps at window width
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > The fact that it's wrapped is not a new format in itself, just a property of > the aligned format. Well I agree I just don't see any benefit to presenting it that way. I mean, sure "wrapped" means "aligned and wrapped", but it's just shorthand notation anyways. In fact, it seems the only reason to separate the two like you're describing would be if it *were* possible to have a wrapped html. Then your notation could handle things "\pset format wrapped" couldn't: \pset format html:autowrap But I think we agree that isn't happening so why spell it "aligned:autowrap" instead of just "wrap"? -- 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] [HACKERS] Proposed patch - psql wraps at window width
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: >> Bryce Nesbitt wrote: >> > Bruce Momjian wrote: >> > > I have updated the documentation for this patch. I consider it ready to >> > > apply. I think it is as close to a middle ground as we are going to >> > > get. Further adjustment will have to happen when we have more reports >> > > from the field. >> > >> > I heard a pretty compelling argument to make "wrapped" part of "aligned", >> > and thus I think the patch is ready to go only after adjusting the >> > user-facing syntax: > > I think this is what makes the most sense of what I've seen so far. > Wrapped is a special case of aligned anyway (there's no "wrapped > unaligned" or "wrapped HTML" for example, nor would we want there to > be.) Well there's no unaligned HTML or aligned unaligned either... > I think that could be fixed easily by having the syntax be something > like > > \pset format aligned:80 > \pset format aligned:autowrap I suppose. It seems kind of inconvenient though, what advantage does it have? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] float4/float8/int64 passed by value with tsearchfixup
"Tom Lane" <[EMAIL PROTECTED]> writes: > BTW, I trolled the contrib files for other v0 functions taking or > returning float4 or float8. I found seg_size (fixed it) and a whole > bunch of functions in earthdistance. Those use float8 not float4, > so they are not broken by this patch, but that module will have to > be v1-ified before we can consider applying the other part of the > patch. So are you killing V0 for non-integral types? Because if not we should keep some sacrificial module to the regression tests to use to test for this problem. I don't see any way not to kill v0 for non-integral types if we want to make float4 and float8 pass-by-value. We could leave those pass-by-reference and just make bigint pass-by-value. -- 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] float4/float8/int64 passed by value with tsearchfixup
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >>> Tom Lane wrote: >>>> Specifically, I think what you missed is that on some platforms C >>>> functions pass or return float values differently from similar-sized >>>> integer or pointer values (typically, the float values get passed in >>>> floating-point registers). > >> But I'm skeptical that it would hit such a wide swathe of the build farm. In >> particular AFAIK the standard ABI for i386 does no such thing. > > I did some digging, and it seems you're mistaken. The standard gcc ABI > for both i386 and x86_64 returns floats in float registers (387 > registers in the first case, and SSE registers in the second case). > This appears to have been the case for a very long time. I quote from > the manual for gcc 2.95: Ah, return values. I accidentally glossed over that point and was looking for how parameters were passed. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] float4/float8/int64 passed by value with tsearchfixup
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: > >> Specifically, I think what you missed is that on some platforms C >> functions pass or return float values differently from similar-sized >> integer or pointer values (typically, the float values get passed in >> floating-point registers). > > Argh ... I would have certainly missed that. Hum. That's a valid concern for some platforms, Sparc I think? But I'm skeptical that it would hit such a wide swathe of the build farm. In particular AFAIK the standard ABI for i386 does no such thing. You can get behaviour like that from GCC using function attributes like regparam but it's not the default. -- 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] Proposed patch - psql wraps at window width
"Bryce Nesbitt" <[EMAIL PROTECTED]> writes: > I asked the folks over at "Experts Exchange" to test the behavior of the ioctl I always thought that was a joke domain name, like Pen Island.com. -- 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
Re: [PATCHES] Proposed patch - psql wraps at window width
"Tom Lane" <[EMAIL PROTECTED]> writes: > Bryce Nesbitt <[EMAIL PROTECTED]> writes: >> I checked the use of COLUMNS and it seems bash updates the >> environment >> variable when a window is resized. > > [ Please get rid of the HTML formatting ... ] > > Bash can update the environment all it wants, but that will not affect > what is seen by a program that's already running. Personally I often > resize the window while psql is running, and I expect that to work. Hm, then having COLUMNS override the ioctl isn't such a good idea. Checking GNU ls source the ioctl overrides COLUMNS if it works, but there's a --width option which trumps both of the other two. I guess just a psql variable would be the equivalent. > I'm with Peter on this one: we have used ioctl, and nothing else, to > determine the vertical window dimension for many years now, to the tune > of approximately zero complaints. It's going to take one hell of a > strong argument to persuade me that determination of the horizontal > dimension should not work exactly the same way. Well the cases are not analogous. Firstly, the window height doesn't actually alter the output. Secondly there's really no downside in a false positive since most pagers just exit if they decide the output fit on the screen -- which probably explains why no ssh users have complained... And in any case you can always override it by piping the output to a pager yourself -- which is effectively all I'm suggesting doing here. So here are your two hella-strong arguments: a) not all terminals support the ioctl. Emacs shell users may be eccentric but surely using psql over ssh isn't especially uncommon. Falling back to COLUMNS is standard, GNU ls is not alone, Solaris and FreeBSD both document supporting COLUMNS. b) you don't necessarily *want* the output formatted to fill the screen. You may be generating a report to email and want to set the width to the RFC recommended 72 characters. You may just have a full screen terminal but not enjoy reading 200-character long lines -- try it, it's really hard: MANWIDTH If $MANWIDTH is set, its value is used as the line length for which manual pages should be formatted. If it is not set, manual pages will be formatted with a line length appropriate to the current terminal (using an ioctl(2) if available, the value of $COLUMNS, or falling back to 80 characters if neither is available). Cat pages will only be saved when the default formatting can be used, that is when the terminal line length is between 66 and 80 characters. -- 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] Proposed patch - psql wraps at window width
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> Also, how would you suggest figuring the width to use for output going to a >> file? ioctl is irrelevant in that case, imho it should just default to 80 >> columns if COLUMNS is unset. > > It would be a spectacularly awful idea for this patch to affect the > output to a file at all. It's a compromise of convenience over principle to allow the default output format to vary but I would still want to have the same set of output formats _available_ to me regardless of whether I'm redirecting to a file or not. Much like ls -C is available even if you're redirecting to a file and -1 is available if you're on a terminal. It sucks to run a program, decide you want to capture that output and find you get something else. It *really* sucks to find there's just no way to get the same output short of heroic efforts. I also have the converse problem occasionally. I run everything under emacs and occasionally run into programs which default to awkward output formats. Usually it's not too bad because it's still on a pty but the window width is a particular one which confuses programs. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Proposed patch - psql wraps at window width
"Peter Eisentraut" <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: >> I checked the use of COLUMNS and it seems bash updates the environment >> variable when a window is resized. I added ioctl(TIOCGWINSZ) if COLUMNS >> isn't set. We already had a call in print.c for detecting the >> number of rows on the screen to determine if the pager should >> be used. Seems COLUMNS should take precedence over ioctl(), right? > > Considering that the code to determine the row count is undisputed so far, > the > column count detection should work the same. That is, we might not need to > look at COLUMNS at all. Unless there is a use case for overriding the column > count (instead of just turning off the wrapping). I do that all the time. I normally am running under an emacs terminal so I don't know what width the ioctl's going to get back but it's unlikely to be right. In any case I may want to format the output to a width narrower than the window because I'm going to narrow it. Also, how would you suggest figuring the width to use for output going to a file? ioctl is irrelevant in that case, imho it should just default to 80 columns if COLUMNS is unset. -- 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] Improve shutdown during online backup
"Simon Riggs" <[EMAIL PROTECTED]> writes: > Personally, I think "smart" shutdown could be even smarter. It should > kick off unwanted sessions, such as an idle pgAdmin session - maybe a > rule like "anything that has been idle for >30 seconds". That's not a bad idea in itself but I don't think it's something the server should be in the business of doing. One big reason is that the server shouldn't be imposing arbitrary policy. That should be something the person running the shutdown is in control over. What you could do is have a separate program (I would write a client but a server-side function would work too) to kick off users based on various criteria you can specify. Then you can put in your backup scripts two commands, one to kick off idle users and then do a smart shutdown. -- 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
Re: [PATCHES] datum passed to macro which expects a pointer
"Gavin Sherry" <[EMAIL PROTECTED]> writes: > On Sat, Apr 12, 2008 at 07:07:48PM -0400, Tom Lane wrote: >> Gavin Sherry <[EMAIL PROTECTED]> writes: >> > I wish. It was actually thrown up when we (Greenplum) changed the macros >> > to be inline functions as part of changing Datum to be 8 bytes. >> >> Hmmm ... Datum has been 8 bytes for many years, on 64-bit machines. >> What is it you're trying to accomplish by making it wider on 32-bitters? > > I miss stated there. This was actually about making key 64 bit types > pass by value instead of pass by reference. There was a patch to do this posted recently here as well. http://archives.postgresql.org/pgsql-patches/2008-03/msg00335.php Hm. I suppose it's true that you could make Datum 64-bit even on 32-bit machines and make int8 and float8 pass-by-value. Seems unlikely to be a net win though. -- 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] EXPLAIN progress info
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> I think a better way to get a real "percentage done" would be to add a method >> to each node which estimates its percentage done based on the percentage done >> its children report and its actual and expected rows and its costs. > > You can spend a week inventing some complicated method, and the patch > will be rejected because it adds too much overhead. Anything we do here > has to be cheap enough that no one will object to having it turned on > all the time --- else it'll be useless exactly when they need it. Actually Dave made a brilliant observation about this when I described it. Most nodes can actually estimate their progress without any profiling overhead at all. In fact they can do so more accurately than using the estimated rows. Sequential scans, for example, can base a report on the actual block they're on versus the previously measured end of the file. Bitmap heap scans can report based on the number of blocks queued up to read. Index scans are the obvious screw case. I think they would have to have a counter that they increment on every tuple returned and reset to zero when restarted. I can't imagine that's really a noticeable overhead though. Limit and sort would also be a bit tricky. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] EXPLAIN progress info
"Heikki Linnakangas" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> 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. Note that this doesn't include the gettimeofdays. It's just a couple integer increments and assigments per tuple. >> 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. > I think there's two different use cases here. The one that Greg's proposal > would be good for is a GUI, like pgAdmin. It would be cool to see how a query > progresses through the EXPLAIN tree when you run it from the query tool. That > would be great for visualizing the executor; a great teaching tool. It also means if a query takes suspiciously long you don't have to run explain in another session (possibly getting a different plan) and if it takes way too long such that it's too long to wait for results you can get an explain analyze for at least partial data. > Yeah, something like this would be better for monitoring a live system. > > The number of rows processed by execMain.c would only count the number of rows > processed by the top node of the tree, right? For a query that for example > performs a gigantic sort, that would be 0 until the sort is done, which is not > good. It's hard to come up with a single counter that's representative :-(. Alternately you could count the number of records which went through ExecProcNode. That would at least get something which gives you a holistic view of the query. I don't see how you would know what the expected end-point would be though. I think a better way to get a real "percentage done" would be to add a method to each node which estimates its percentage done based on the percentage done its children report and its actual and expected rows and its costs. So for example a nested loop would calculate P1-(1-P2)/ER1 where P1 is the percentage done of the first child and P2 is the percentage done of the second child and ER1 is the expected number of records from the first child. Hash Join would calculate (P1*C1 + P2*C2)/(C1+C2). That could get a very good estimate of the percentage done, basically as good as the estimated number of records. -- 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
Re: [PATCHES] EXPLAIN progress info
"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
[PATCHES] EXPLAIN progress info
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
Re: [PATCHES] Partial match in GIN
"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] Indexam API changes
"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] Database owner installable modules patch
"Tom Dunstan" <[EMAIL PROTECTED]> writes: > - I'd like to add pg_depend entries for stuff installed by the module > on the pd_module entry, so that you can't drop stuff required by the > module without uninstalling the module itself. There would have to be > either a function or more syntax to allow a script to do that, or some > sort of module descriptor that let the backend do it itself. > > - Once the issue of loading a dll from inside the module's directory > is done, I'd like to look for an e.g. module_install() function inside > there, and execute that rather than the install.sql if found. Ditto > for uninstall. I wonder if there's much of a use case for any statements aside from CREATE statements. If we restrict it to CREATE statements we could hack things to create pg_depend entries automatically. In which case we wouldn't need an uninstall script at all. The hacks to do this seem pretty dirty but on the other hand the idea of having modules consist of a bunch of objects rather than arbitrary SQL actually seems cleaner and more robust. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Ordered Append WIP patch v1
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > Alvaro Herrera wrote: >> Gregory Stark wrote: >> > >> > Here's the WIP patch I described on -hackers to implemented "ordered" >> > append >> > nodes. >> >> Did you ever publish an updated version of this patch? No, it's been kind of on the back burner. > I don't think so. I think we just need to tell Greg if he should > continue in this direction. I think the executor side of things is pretty straightforward. Where I'm really uncertain is on the planner side of things. To be honest I didn't follow at all what Tom was saying to do with the equivalence classes. What it's doing now is basically just lying and saying the child columns are equivalent to the parent columns -- I'm not sure what the consequences of that are. Tom seemed to think that would be bad but I don't see any real problems with it. -- 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
Re: [PATCHES] Expose checkpoint start/finish times into SQL.
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Greg Smith <[EMAIL PROTECTED]> writes: >> > ... If they'd have noticed it while the server was up, perhaps because the >> > "last checkpoint" value hadn't changed in a long time (which seems like it >> > might be available via stats even if, as you say, the background writer is >> > out of its mind at that point), they could have done such a kill and >> > collected some actual useful information here. That's the theory at >> > least. >> >> Well, mebbe, but that still seems to require a lot of custom monitoring >> infrastructure that is not present in this patch, and furthermore that >> this patch doesn't especially aid the development of. > > These kind of things can be monitored externally very easily, say by > Nagios, when the values are available via the database. If you have to > troll the logs, it's quite a bit harder to do it. I can see Tom's reluctance out of fear that really this is going to be the first of hundreds of dials which have to be monitored so a single function to handle that single dial is kind of short sighted. I would think eventually it should be part of the Postgres SNMP MIB. But I would say from my experience on the not-really-a-sysadmin side I think the time of the last checkpoint is probably the *most* important thing to be monitoring. Effectively it's monitoring how stale your data on disk is potentially becoming by showing how much recovery will be necessary. > I'm not sure about the right values to export -- last checkpoint start > time is the most obvious idea, but I would also suggest exporting last > checkpoint end, or NULL if the checkpoint is ongoing; and also previous- > to-last checkpoint start and end. I'm surprised y'all want the time of the last checkpoint *start* though. It seems to me that only the last checkpoint *end* is actually interesting. A checkpoint which has started but not ended yet is not really a checkpoint at all. It's nothing. In the future there could be multiple checkpoints which have "started" but not finished. Or we could add support for lazy checkpoints in which case there could be an infinite number of "potential" checkpoints which haven't finished. Worse, the time the last checkpoint actually finished isn't actually interesting either. What you want to know is what time the record which the last finished checkpoint *checkpointed up to* was inserted. That is, the time of the record that the last checkpoint record *points to*. That represents the "guarantee" that the database is making to the sysadmin about data integrity. Everything before that time is guaranteed to have reached data files already. Everything after it may or may not be in the data files and has to be checked against the WAL logs. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] psql command aliases support
"Bernd Helmle" <[EMAIL PROTECTED]> writes: > --On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark > <[EMAIL PROTECTED]> wrote: > >>> \o foo instead of \i foo -- check your keyboard layout >> >> The point is here you've typed a different command entirely. >> Unsurprisingly it's going to do something different. > > Uh well, you surely don't know that if you accidently hit o instead of > ione > reason more for an alias \output and \input ;) > > To be serious, i take your point, but i'm under the impression that you simply > can't safe the user against all mistakes they are going to make ever. Your > concerns apply to every existing alias implementation as well and you aren't > safe against decisions of your distributor to add conflicting aliases to their > default bashrc as well. I think I'm not managing to express myself clearly here. Generalizing the complaint into simply "if you typo something different happens" is losing too much information. With shell aliases if you typo "foo" into "bar" then, yes, something different will happen than what you expected. However the new behaviour will still be of the same "kind" -- that is it runs a shell command, just the wrong shell command. If there's no command by that name you get an error not some unrelated shell functionality. In the proposed alias syntax if you typo \cold into \coldd it will print a mysterious error about trying to connect to a database or if you typo \old into \oldd it will write out a file and stop printing query output. The user might not even realize that \c and \o are commands since they think they're running commands \colld and \oldd. I think you have to find a syntax where the current commands continue to mean exactly what they always meant and where typos can't result in an entirely different kind of behaviour. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] psql command aliases support
"Peter Eisentraut" <[EMAIL PROTECTED]> writes: > This is a valid concern, but it is orthogonal to the alias feature. You have > the same problem already if you mistype > > \oo instead of \o > \ofoo instead of \obar Not really. In these cases you know what \o is going to do, you've just typo'd the filename. The case I was saying was a "conflict" was because you were using an entirely different feature and might not never have even met \o. > \o instead of \p > \oset instead of \pset Sure, if you typo \o instead of select * from pg_class you might be surprised too. You can't protect against typing an entirely different command than intended. At least you can then go look up what \o does and figure out what happened. If you type \ofoo instead of \obar I think there's a big difference between having it accidentally do what you wanted it to do but to the wrong file and having it do something entirely unrelated to what you intended and end up overwriting a file instead of run a simple select. > or even more amusingly > > \o foo instead of \i foo -- check your keyboard layout The point is here you've typed a different command entirely. Unsurprisingly it's going to do something different. \old means something *today*. In the proposed syntax by creating the alias you're changing what it means. You're not changing other \ofoo arguments though which is where the possibility for confusion arises. Consider instead if Debian decided to include a convenient \deb alias for a select query against the package repository. Now if I happen to have an "eb" table I will be surprised when \deb doesn't work. And lengthening the alias doesn't really help (aside from defeating the purpose of having aliases). If they define \debian they would still be interfering if I should have a table named "ebian". As rule of thumb, I think if you try to execute an alias which doesn't exist, you should get an "alias does not exist" command. You should not get an "Invalid command" nor "Did not find relation" and certainly not some random command you've never met before being run reading or overwriting random files. I repeat my suggestion of having a \query command so you could do: \setquery deb select * from debian_packages where packagename like :1 \setquery bbdb select * from bbdb where name like :1 \query deb postgres \query bbdb peter to run a saved query. I'm not attached to "setquery" though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] psql command aliases support
"Bernd Helmle" <[EMAIL PROTECTED]> writes: >> Picture a newbie typoing on their \old alias and trying to figure out >> where all their data is going... Hopefully they weren't too attached to >> whatever was in their "ldd" file yesterday. > > Of course, the patch doesn't work this way. Only complete tokens delivered by > the parser are substituted, illustrated here with your example: To be more explicit what I meant was someone doing #= \alias old select version(); ... #= \oldd #= \old #= select 'where is all my output going?' #= select 'what happened to my ldd file?' -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Consistent \d commands in psql
"Tom Lane" <[EMAIL PROTECTED]> writes: > "Greg Sabino Mullane" <[EMAIL PROTECTED]> writes: >>> \df Lists all user functions >>> \df [pattern] Lists both system and user functions matching [pattern] >>> \df * Lists all system and user functions > >> I don't like this for two reasons: the items returned changes based on >> the existence of args, rather than on the command itself, and more >> importantly, this would make it inconsistent with the other backslash >> commands. > > I think you misunderstood the context of the discussion. Whatever we do > will be done to the whole family of \d commands --- we are just using > \df as an exemplar. Hm, I didn't realize that. I thought the reason \df was special was that users often need to refer to "system" functions. Whereas they never need to refer to system tables or system sequences etc unless they know that's what they're looking for. However, now that I look at the list of \d commands that argument kind of falls flat. Users also need to find "system" operators, data types, etc. And I think the same logic as \df applies for those other things too. \dt pg_class should "just work". And if you create a macaddr data type it doesn't seem like too much of an imposition to play it safe and have \dT macaddr show the user that there are now two matching data types. So I guess I should just play along and pretend that's what I meant all along :) -- 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
Re: [PATCHES] psql command aliases support
"Peter Eisentraut" <[EMAIL PROTECTED]> writes: > Am Dienstag, 1. April 2008 schrieb Tom Lane: >> Do we really want such a thing? > > Yes! > >> The space of backslash command names >> is so densely populated already that it's hard to imagine creating >> aliases without conflicting with existing (or future) command names >> --- as indeed your example does. It seems like mostly a recipe for >> confusion. > > This is a standard feature and effect on shells. Shells have even more > likely > commands and conflicts, and still aliases are widely used. If people are > concerned about conflicts, they shouldn't use aliases. I think you're crazy to think shells are more likely to have conflicts. Shells require a whole token match, not just the first letter. In other words, any alias *starting with* the letters c, d, e, f, g, h, i, o, s, w, z would be a conflict. Just for maximum confusion the list of letters which cause conflicts when capitalized would be entirely different. Picture a newbie typoing on their \old alias and trying to figure out where all their data is going... Hopefully they weren't too attached to whatever was in their "ldd" file yesterday. I could see it being handy being able to save commonly executed queries and access them with a shortcut but I think you need to use a separate namespace from psql's backslash commands. Perhaps `query` or invent a single backslash command to execute them like "\query current_query". Actually I like that last idea the most. One thing to think about is how to pass arguments to the aliases. Csh put us in the soup by hacking in arguments in a terrible way. As a result quoting in csh aliases is awful. Even if it's not implemented right away I think we should figure out what the syntax would be for passing arguments to be interpolated into the query before backing ourselves into a corner. I can't imagine much of a use case for being able to alias backslash commands themselves. They're already ridiculously terse anyways. How many keystrokes can you possibly save? -- 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] Consistent \d commands in psql
"Tom Lane" <[EMAIL PROTECTED]> writes: > "Brendan Jurd" <[EMAIL PROTECTED]> writes: >> If I read Greg's latter proposal correctly, he was suggesting > >> \df Lists all user functions >> \df [pattern] Lists both system and user functions matching [pattern] >> \df * Lists all system and user functions > > Hmm, I must've misread it, because I didn't understand it quite like > that. That seems like a nice simple minimal-featuritis approach. Sorry if was confusing but yes, that is what I intended by my second proposal. I prefer it to my own first proposal or any of the others. I admit I was thinking primarily of non-globbing cases for pattern. As in, I would want \df rtrim to "work". I suppose it could be annoying to have to type \df public.* -- there's nothing stopping us from having \dfU and \dfS too I suppose, though I doubt most people would find them. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Consistent \d commands in psql
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> One --perhaps nice, perhaps not-- property of this is that if you defined a >> function named "rtrim" and then did "\df rtrim" it would show you _both_ the >> system and user function and make it easier to see the conflict. Whereas the >> other behaviour I proposed would hide the system function which might >> exacerbate the user's confusion. > > Yeah, that is a very good point indeed. > > Another way we could approach this is > > \df -> all functions > \dfS-> sys functions only > \dfU-> user functions only > > which avoids falling into the trap Greg mentions. That doesn't satisfy the original source of the annoyance which is that \df spams your terminal with ten screens of system functions with your user functions hidden amongst them. -- 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
Re: [PATCHES] Consistent \d commands in psql
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> It might be cute to see if the pattern matches any user functions and if not >> try again with system functions. So you would still get results if you did >> "\df rtrim" for example. > > Interesting idea. IIUC, \df would give you either all user functions > *or* all system functions depending on the actual catalog contents, > while \dfS would always give you just system functions. That means > there'd be no way to replicate the all-functions-of-both-types behavior > that has been the default in every prior release. That sounds like > a recipe for getting complaints --- changing the default behavior is > one thing, but making it so that that behavior isn't available at > all is surely going to break somebody's code or habitual usage. Actually on further thought I wonder if it wouldn't be simpler (and perhaps more consistent with \d) to just list *all* matches iff a pattern is provided but list only user functions if *no* pattern is provided. That would effectively be exactly the current behaviour except that you would have to do \dfS to get a list of system functions. And yeah, you wouldn't be able to get a list of all functions whether system or user functions. I suppose you could do \df * One --perhaps nice, perhaps not-- property of this is that if you defined a function named "rtrim" and then did "\df rtrim" it would show you _both_ the system and user function and make it easier to see the conflict. Whereas the other behaviour I proposed would hide the system function which might exacerbate the user's confusion. > BTW, should we remove the special hack that discriminates against > showing I/O functions (or really anything that touches cstring) in \df? > ISTM that was mostly there to reduce clutter, and this proposal solves > that problem more neatly. I know I've cursed that behavior under my > breath more than once, but again maybe my usage isn't typical. . o O Ohh! That's why I can never find them! -- 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] Consistent \d commands in psql
"Tom Lane" <[EMAIL PROTECTED]> writes: > Hmm. Personally, most of my uses of \df are for the purpose of looking > for built-in functions, and so this'd be a step backwards for my usage. > Likewise for operators. Maybe I'm in the minority or maybe not. > The only one of these things for which the argument seems entirely > compelling is comments. I do understand the appeal of consistency but > sometimes it's not such a great thing. The problem is that there's absolutely no way to do the equivalent of a plain \dt and get a list of just your user functions. That's a real headache and it gets worse as we add more and more system functions too. It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did "\df rtrim" for example. -- 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] [WIP] Keeping track of snapshots
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > The other question is about CommitTransactionCommand. Currently my > EOXact routine barfs for every snapshot not unregistered on main > transaction commit -- a leak. I see this as a good thing, however it > forced me to be more meticulous about not having ActiveSnapshot be set > in commands that have multiple transactions like VACUUM, multitable > CLUSTER and CREATE INDEX CONCURRENTLY. I believe ActiveSnapshot has to be set during CREATE INDEX CONCURRENTLY if it's an expression index which calls a function which needs a snapshot... AFAICT VACUUM had better not ever need a snapshot because its xmin isn't included in other vacuum commands' globalxmin so there's no guarantee that if it had a snapshot that the tuples visible in that snapshot wouldn't disappear out from under it. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
"Zoltan Boszormenyi" <[EMAIL PROTECTED]> writes: > Zoltan Boszormenyi írta: >> Gregory Stark írta: >>> 4) Your problems with tsearch and timestamp etc raise an interesting >>> problem. >>>We don't need to mark this in pg_control because it's a purely a run-time >>>issue and doesn't affect on-disk storage. However it does affect ABI >>>compatibility with modules. Perhaps it should be added to >>>PG_MODULE_MAGIC_DATA. >> >> I am looking into it. > > Do you think it's actually needed? > PG_MODULE_MAGIC_DATA contains the server version > the external module was compiled for. This patch won't go to > older versions, so it's already protected from the unconditional > float4 change. And because you can't mix server and libraries > with different bitsize, it's protected from the conditional int64, > float8, etc. changes. Right, it does seem like we're conservative about adding things to PG_MODULE_MAGIC. As long as this is hard coded based on HAVE_LONG_INT_64 then we don't strictly need it. And I don't see much reason to make this something the user can override. If there are modules which use the wrong macros and assume certain other data types are pass-by-reference when they're not then they're going to fail regardless of what version of postgres they're compiled against anyways. So I would say in response to your other query to _not_ use pg_config_manual.h which is intended for variables which the user can override. If you put anything there then we would have to worry about incompatibilities. -- 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Ok, ignore my previous message. I've read the patch now and that's not an issue. The old code path is not commented out, it's #ifdef'd conditionally on HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in patch form). A few comments: 1) Please don't include configure in your patch. I don't know why it's checked into CVS but it is so that means manually removing it from any patch. It's usually a huge portion of the diff so it's worth removing. 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie OSX). I don't really see an alternative so it's just something to note for the folks setting that up (Hi Dave). Actually there is an alternative but I prefer the approach you've taken. The alternative would be to have a special value in the catalogs for 8-bit maybe-pass-by-value data types and handle the check at run-time. Another alternative would be to have initdb fix up these values in C code instead of fixing them directly in the bki scripts. That seems like more hassle than it's worth though and a bigger break with the rest. 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having a #define like INT64PASSBYVALUE which is defined to be either "true" or "false". It might start getting confusing having three different defines for the same thing though. But personally I hate having more #ifdefs than necessary, it makes it hard to read the code. 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. Actually, why isn't sizeof(Datum) in there already? Do we have any protection against loading 64-bit compiled modules in a 32-bit server or vice versa? But generally this is something I've been wanting to do for a while and basically the same approach I would have taken. It seems sound to me. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
"Zoltan Boszormenyi" <[EMAIL PROTECTED]> writes: > - the int8inc(), int2_sum() and int4_sum() used pointers directly from the > Datums > for performance, that code path is now commented out, the other code path > is correct for the AggState and !AggState runs and correct every time and now > because of the passbyval nature of int8, the !AggState version is not slower > than using the pointer directly. Does this mean count() and sum() are slower on a 32-bit machine? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! - 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] Logging conflicted queries on deadlocks
"Gregory Stark" <[EMAIL PROTECTED]> writes: > There's no way the other transaction's timeout could fire while we're doing > this is there? Are we still holding the lw locks at this point which would > prevent that? Ah, reading the patch I see comments indicating that yes that's possible and no, we don't really care. So, ok. If the backend disappears entirely could the string be empty? Perhaps it would be safer to copy out st_activity inside the loop checking st_changecount? It is a really nice feature though. Note that there was an unrelated demand for just this info on one of the other lists just today. Thanks very much Itagaki-san! -- 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] Logging conflicted queries on deadlocks
"Tom Lane" <[EMAIL PROTECTED]> writes: > ITAGAKI Takahiro <[EMAIL PROTECTED]> writes: >> Here is a patch to log conflicted queries on deadlocks. Queries are dumped >> at CONTEXT in the same sorting order as DETAIL messages. Those queries are >> picked from pg_stat_get_backend_activity, as same as pg_stat_activity, >> so that users cannot see other user's queries. > > Applied with revisions. It's a cute idea --- I first thought it > couldn't work reliably because of race conditions, but actually we > haven't aborted our own transaction at this point, so everyone else > involved in the deadlock is still waiting and it should be safe to > grab their activity strings. There's no way the other transaction's timeout could fire while we're doing this is there? Are we still holding the lw locks at this point which would prevent that? > One thing that I worried about for a little bit is that you can imagine > privilege-escalation scenarios. Suppose that the user is allowed to > call some SECURITY DEFINER function that runs as superuser, and a > deadlock occurs inside that. As the patch stands, every query involved > in the deadlock will be reported, which might be undesirable. We could > make the test use the outermost session user's ID instead of current > ID, but that might only move the security risks someplace else. > Thoughts? Perhaps we should only do this if the current user's ID is the same as the outermost session user's ID? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] Proposed patch to change TOAST compression strategy
"Tom Lane" <[EMAIL PROTECTED]> writes: > * Adds an early-failure path to the compressor as suggested by Jan: > if it's been unable to find even one compressible substring in the > first 1KB (parameterizable), assume we're looking at incompressible > input and give up. (There are various ways this could be done, but > this way seems to add the least overhead to the compressor's inner > loop.) I'm not sure how to test the rest of it, but this bit seems testable. I fear this may be too conservative. Even nigh incompressible data will find a few backreferences. I'll try some tests and see. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Proposed patch for 8.3 VACUUM FULL crash
"Tom Lane" <[EMAIL PROTECTED]> writes: > On investigation the problem occurs because we changed vacuum.c's > PageGetFreeSpaceWithFillFactor() to use PageGetHeapFreeSpace() > instead of just computing pd_upper - pd_lower as it had done in > every previous release. This was *not* a good idea: VACUUM FULL > does its own accounting for line pointers and does not need "help". Fwiw this change appears to have crept in when the patch was merged. Ironically while most of us have been complaining about patches not being widely visible and tested outside of CVS in this case we perhaps suffered from the opposite problem. The patch was fairly heavily tested on this end before it was posted and I'm not sure those tests have been repeated since the merge. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Proposed patch for bug #3921
"Tom Lane" <[EMAIL PROTECTED]> writes: > An issue that this patch doesn't address is what happens if the source > index is in a non-default tablespace that the current user does not have > CREATE permission for. With both current CVS HEAD and this patch, that > will result in an error. Is that okay? I was going to say "we could add a hint suggesting how to specify the tablespace" but as you pointed out earlier there really isn't a way to specify the tablespace. Hm, looking at the grammar we already have something like this for constraints. We could add an OptConsTableSpace after INCLUDING INDEXES. I just tested it and it didn't cause any conflicts which makes sense since like options appear in the column list. So the syntax would be CREATE TABLE foo (..., LIKE bar INCLUDING INDEXES USING INDEX TABLESPACE foo_ts, ...) Kind of verbose but nice that it's the same syntax as constraints. Not sure how easy it would be to shoehorn into t he like processing, I could look at that if you want. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Bitmap index scan preread using posix_fadvise
Attached is the correct patch, sorry for the confusion.
If anyone's interested in testing it you can do so without the randomarray.c
file by using Pavel Stehule's solution for generating arrays:
postgres=# create table h as (select (random()*100)::integer as r,
repeat('x',512)::text as t from generate_series(1,100));
SELECT
postgres=# create index hri on h(r);
CREATE INDEX
postgres=# analyze h;
ANALYZE
postgres=# \timing
Timing is on.
postgres=# set preread_pages = 0; explain analyze select * from h where r =
any (array(select (1+random()*100)::integer from generate_series(1,100)));
postgres=# set preread_pages = 100; explain analyze select * from h where r =
any (array(select (1+random()*100)::integer from generate_series(1,100)));
postgres=# set preread_pages = 0; explain analyze select * from h where r =
any (array(select (1+random()*100)::integer from generate_series(1,100)));
postgres=# set preread_pages = 100; explain analyze select * from h where r =
any (array(select (1+random()*100)::integer from generate_series(1,100)));
postgres=# set preread_pages = 0; explain analyze select * from h where r =
any (array(select (1+random()*100)::integer from generate_series(1,100)));
bitmap-preread-v8.diff.gz
Description: Binary data
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [PATCHES] Bitmap index scan preread using posix_fadvise
"Gregory Stark" <[EMAIL PROTECTED]> writes: > Aside from some autoconf tests and the documentation for the GUC I > think it's all in there. I'm sorry, it seems I accidentally grabbed an old tree to generate this patch. I'll send along a better more recent version. Argh. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Bitmap index scan preread using posix_fadvise (Was: There's random access and then there's random access)
Here's the WIP patch for doing prereading when doing bitmap index scans.
I was performance testing it as I was developing it here:
http://archives.postgresql.org/pgsql-hackers/2007-12/msg00395.php
Note that this only kicks in for bitmap index scans which are kind of tricky
to generate. I used the attached function to generate them in the post above.
Also note I wouldn't expect to see much benefit unless you're on a raid array,
even a small one. But if you are on a raid array then the benefit should be
immediately obvious or else posix_fadvise just isn't working for you. I would
be interested in hearing on which OSes it does or doesn't work.
*If* this is the approach we want to take rather than restructure the buffer
manager to avoid taking two trips by marking the buffer i/o-in-progress and
saving the pinned buffer in the bitmap heap scan then this is more or less in
final form. Aside from some autoconf tests and the documentation for the GUC I
think it's all in there.
bitmap-preread-v5.diff.gz
Description: Binary data
#include
#include
#include
PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(random_array);
Datum
random_array(PG_FUNCTION_ARGS)
{
int32 n = PG_GETARG_INT32(0);
int32 lobound = PG_GETARG_INT32(1);
int32 hibound = PG_GETARG_INT32(2);
Datum *elems = palloc(sizeof(Datum) * n);
ArrayType *retval;
int i;
for (i=0; i
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] [8.4] Updated WITH clause patch (non-recursive)
"Neil Conway" <[EMAIL PROTECTED]> writes: > Remaining work is to review the guts of the patch (which shouldn't take > long), and write documentation and regression tests. I'm personally > hoping to see this get into the tree fairly early in the 8.4 cycle, > pending discussion of course. Looking back at this I've realized (putting aside whether we want to apply the patch as is which is another question) that to get the CTEs materialized so they perform the way a user might expect them to would actually require the same infrastructure that recursive queries will require. Basically what I think we really want down the line is for something like: WITH (select * from complex_view) AS x SELECT * FROM x JOIN x as x2 ON (x.id=x2.id2) to run the view once, materialize the results and then join the resulting data with itself. At least that's what the user is likely expecting. Now it may be that we have a better plan by inlining the two calls which in an ideal world we would go ahead and try as well. But it's more likely that users would write the WITH clause because they specifically want to avoid re-evaluating a complex subquery. To do this though we would need the same capability that recursive queries would need. Namely the ability to have a single tuplestore with multiple readers reading from different positions in the tuplestore. So what I'm imagining doing is to add a flag to the RelOptInfo (Alternatively we could create a new rtekind, RTE_CTE, but that would require most sites which check for RTE_SUBQUERY to check for that as well). Then (I think) in create_subqueryscan_plan we would have to check for this flag and introduce the Memoize node I previously mentioned. That's basically a Materialize node which keeps track of its position within the tuplestore in its own state. It would also have to introuduce the one-time node with the Materialize node which the Memoize would point to. I'm getting a bit vague here as I haven't entirely absorbed how one-time plans work. That would allow the query above to, for example, generate something like: InitPlan -> Memoize x (cost=0.00..34.00 rows=2400 width=4) -> Seq scan on complex_view (cost=0.00..34.00 rows=2400 width=4) Merge Join (cost=337.50..781.50 rows=28800 width=8) Merge Cond: (x.id = x2.id) -> Sort (cost=168.75..174.75 rows=2400 width=4) Sort Key: x.id -> MemoizeRead x (cost=0.00..34.00 rows=2400 width=4) -> Sort (cost=168.75..174.75 rows=2400 width=4) Sort Key: x2.id -> MemoizeRead x x2 (cost=0.00..34.00 rows=2400 width=4) Does this sound like the right track? Should I be doing this at the RelOptInfo level or at some point higher up? Do I misunderstand anything about how InitPlan is handled? Other ideas: it might be interesting to note that we're sorting the same Memoize node twice and push that down into the initial plan. Or somehow to check whether it wouldn't be faster to just inline the memoized node directly because perhaps there's a path available which would work for this read of it. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Proposed patch: synchronized_scanning GUC variable
"Kris Jurka" <[EMAIL PROTECTED]> writes: > On Mon, 28 Jan 2008, Jeff Davis wrote: > >> I think that pg_dump is a reasonable use case for synchoronized scans >> when the table has not been clustered. It could potentially make pg_dump >> have much less of a performance impact when run against an active >> system. > > One of the advantages I see with maintaining table dump order is that rsyncing > backups to remote locations will work better. I can't see what scenario you're talking about here. pg_dump your live database, restore it elsewhere, then shut down the production database and run rsync from the live database to the restored one? Why not just run rsync for the initial transfer? I can't see that working well for a real database and a database loaded from a pg_dump anyways. Every dead record will introduce skew, plus page headers, and every record will have a different system data such as xmin for one. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] WIP: plpgsql source code obfuscation
"Tom Lane" <[EMAIL PROTECTED]> writes: > My recollection is that certain cryptography laws make hooks for crypto > just as problematic as actual crypto code. We'd have to tread very > carefully --- "general purpose" hooks are OK but anything narrowly > tailored to encryption purposes would be a hazard. Afaik the US was the only country with such a scheme with the ITAR export regulations and that's long since gone, at least as it applied to crypto. The current US export regulations don't have any of the stuff about hooks in them and exempt free software from any crypto export licenses. Doesn't stop some other country from coming up with the same idea of course but we don't generally worry about what laws some hypothetical country might introduce at some point in the future. That way lies madness. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] WIP: plpgsql source code obfuscation
"Pavel Stehule" <[EMAIL PROTECTED]> writes: > Do you thing some binary module that load some encrypted sources from > files? It can be possible too. But if source code will be stored in > pg_proc, then we need third method. Some like "obfuscate" (prev. are > validate and call"), because we can't to store plain text to prosrc > col. Is there a reason you couldn't, for instance, provide a function which takes source code and encrypts it. Then you would write dump the data it spits into your function declaration like: CREATE FUNCTION foo() returns integer AS $$ ... base64 encoded data $$ language "obfuscated:plperl"; "obfuscated:plperl"'s handler function would just decrypt it and pass it off to plperl. There is a validator function which gets called when you create a function but I don't think it has any opportunity to substitute its result for the original in prosrc. That might be interesting for other applications like compiled languages, though I think they would still want to save the source in prosrc and the bytecode in probin. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] WIP: plpgsql source code obfuscation
"Pavel Stehule" <[EMAIL PROTECTED]> writes: >> In such a scheme I think you would put the key in an attribute of the >> language. Either in pg_lang or some configuration location which the >> obfuscate:plperl interpreter knows where to find. >> > > what is advantage? It wouldn't require any core changes. It would be just another PL language to load which can be installed like other ones. This could be a big advantage because it doesn't look like there is a lot of support for putting th obfuscation directly into the core code. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] WIP: plpgsql source code obfuscation
Someone along the way suggested doing this as a kind of "wrapper" PL language. So you would have a PL language like "obfuscate:plperl" which would obfuscate the source code on the way in. Then when you execute a function it would deobfuscate the source code and then just pass it to the normal plperl. In such a scheme I think you would put the key in an attribute of the language. Either in pg_lang or some configuration location which the obfuscate:plperl interpreter knows where to find. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Proposed patch: synchronized_scanning GUC variable
"Jonah H. Harris" <[EMAIL PROTECTED]> writes: > On Jan 27, 2008 3:07 PM, Tom Lane <[EMAIL PROTECTED]> wrote: >> Per today's -hackers discussion, add a GUC variable to allow clients to >> disable the new synchronized-scanning behavior, and make pg_dump disable >> sync scans so that it will reliably preserve row ordering. This is a >> pretty trivial patch, but seeing how late we are in the 8.3 release >> cycle, I thought I'd better post it for comment anyway. > > +1 I liked the "synchronized_sequential_scans" idea myself. But otherwise, sure. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [8.4] Updated WITH clause patch (non-recursive)
"Neil Conway" <[EMAIL PROTECTED]> writes: > Remaining work is to review the guts of the patch (which shouldn't take > long), and write documentation and regression tests. I'm personally > hoping to see this get into the tree fairly early in the 8.4 cycle, > pending discussion of course. Note that as it stands it directly inlines the subquery into the query everywhere you use it. So if the user was hoping to save database work by avoiding duplicate subqueries he or she may be disappointed. On the other hand inlining it can allow the planner to produce better plans. Tom's feeling at the time was that even though it was providing something from the standard, it wasn't actually allowing the user to do anything he couldn't before. If it doesn't provide any additional expressive capabilities then I think he didn't like taking "with" as a reserved word. I still hope to do recursive queries for 8.4 so I don't have strong feelings for this part either way. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] OUTER JOIN performance regression remains in 8.3beta4
"Alvaro Herrera" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: > >> Comparing the behavior of this to my patch for HEAD, I am coming to the >> conclusion that this is actually a *better* planning method than >> removing the redundant join conditions, even when they're truly >> rendundant! The reason emerges as soon as you look at cases involving >> more than a single join. If we strip the join condition from just one >> of the joins, then we find that the planner insists on doing that join >> last, whether it's a good idea or not, because clauseful joins are >> always preferred to clauseless joins in the join search logic. > > Would it be a good idea to keep removing redundant clauses and rethink > the preference for clauseful joins, going forward? I don't understand what's going on here. The planner is choosing one join order over another because one join has more join clauses than the other? Even though some of those joins are entirely redundant and have no selectivity? That seems like a fortuitous choice made on entirely meaningless data. Is there some other source of data we could use to make this decision instead of the number of clauses? I would suggest the selectivity but from the sound of it that's not going to help at all. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Proposed patch to make mergejoin cost estimationmore symmetric
"Simon Riggs" <[EMAIL PROTECTED]> writes: > I think we all accept that you're the master here. The question is how > will we know to report problems to you? Hm, I think I agree. The problem is where to draw the line. Ultimately everything in the statistics tables and more could potentially be relevant. The other problem is that currently our explain output is a bit of a hack. It's just text we print out and we're limited in how much data we can put in that without it being unwieldy. When we get the table-based or xml-based or some other machine-readable explain plan we could stuff a lot more data in there and have it be the UI's responsibility to decide what data to display. When that happens it would be nice to have the raw data used to generate the cost estimations. At least the most important factors. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric
"Tom Lane" <[EMAIL PROTECTED]> writes: > Any objections to applying the patch? I like applying it. You don't include any negative tests and corner cases as well; cases where the new code shouldn't be disturbing the results such as a symmetric join or a join against a single-row table. The comments make me think you ran them but just didn't show them though. What about a merge join against an empty table? I suppose there would just be no statistics? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Better default_statistics_target
"Chris Browne" <[EMAIL PROTECTED]> writes: > - Any columns marked "unique" could keep to having somewhat smaller >numbers of bins in the histogram because we know that uniqueness >will keep values dispersed at least somewhat. I think you're on the wrong track. It's not dispersal that's significant but how evenly the values are dispersed. If the values are evenly spread throughout the region from low to high bound then we just need the single bucket telling us the low and high bound and how many values there are. If they're unevenly distributed then we need enough buckets to be able to distinguish the dense areas from the sparse areas. Perhaps something like starting with 1 bucket, splitting it into 2, seeing if the distributions are similar in which case we stop. If not repeat for each bucket. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
"Brendan Jurd" <[EMAIL PROTECTED]> writes: > The patch is very invasive (at least compared to any of my previous > patches), but so far I haven't managed to find any broken behaviour. I'm sorry to suggest anything at this point, but... would it be less invasive if instead of requiring the immediate cast you created a special case in the array code to allow a placeholder object for "empty array of unknown type". The only operation which would be allowed on it would be to cast it to some specific array type. That way things like UPDATE foo SET col = array[]; INSERT INTO foo (col) VALUES (array[]); could be allowed if they could be contrived to introduce an assignment cast. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: > If you have lots of data it doesn't mean you are modifying lots of > data. It sure can. How do you modify lots of data if you *don't* have lots data in the first place? Certainly it doesn't mean you necessarily are, some databases are OLTP which do no large updates. But data warehouses with oodles of data also often have to do large updates or deletions. > I don't think anyone here (good lord I hope not) would say that firing > a trigger over 500k rows is fast. Instead you should likely just work the data > outside the partition and then move it directly into the target > partition. Well you don't even have to do that. You can issue the updates directly against the partitions. In fact, that's precisely what the rules effectively do... Rules rewrite the query to be a query directly against the partitions. Come to think of it I think there actually is a correct way to use rules which wouldn't suffer from the problems that have come up. Instead of putting a WHERE clause on the rule just expand deletes and updates to expand to deletes and updates against *all* partitions. Then let constraint_exclusion kick in to narrow down which partitions should actually receive the updates and deletes. I think triggers are the only solution for insert though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: > Heh, o.k. that was an ambiguous sentence. In a partitioned environment > you are likely not moving millions of rows around. Thus the "rule" > benefit is lost. You are instead performing many (sometimes > lots-o-many) inserts and updates that involve a small number of rows. I'm still not following at all. If you're partitioning it's because you have a *lot* of data. It doesn't say anything about what you're doing with that data. Partitioning is useful for managing large quantities of data for both OLTP and DSS systems. I tend to be happier recommending triggers over rules if only because rules are just harder to understand. Arguably they don't really work properly for this use anyways given what happens if you use volatile functions like random() in your where clause. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: > >> A trigger will probably beat a rule for inserts/updates involving a small >> number of rows. > > Which is exactly what partitioning is doing. Say what? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] Ordered Append WIP patch v1
Here's the WIP patch I described on -hackers to implemented "ordered" append nodes. merge-append-v1.diff.gz Description: Binary data -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Better default_statistics_target
"Simon Riggs" <[EMAIL PROTECTED]> writes: > Long text fields that might use LIKE should be set to 100. CHAR(1) and > general fields should be set to 10. I could see arguing that either way. Longer fields are capable of more precision and so may need more buckets to predict. On the other hand longer fields take more space and take longer to compare so to make consistent use of resources you would want to avoid storing and comparing large numbers of them whereas you could afford much larger targets for small quick columns. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] V0.2 patch for TODO Item: SQL-language referenceparameters by name.
"David Fetter" <[EMAIL PROTECTED]> writes: > What I mean by "kinda" is that it's a standard way of handling > parameters in Oracle and in DBI. That's a good reason *not* to use them for other purposes. Users trying to create procedures through DBI or other interfaces like it will run into problems when the driver misinterprets the parameters. > I think it would be a very bad idea > to require that people use the function name in parameters, I think were talking about only allowing it to disambiguate if the name is shadowed by a variable in an inner scope. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Autovacuum cancellation
"Tom Lane" <[EMAIL PROTECTED]> writes: >> I think there's a window where the process waiting directly on >> autovacuum could have already fired its deadlock check before it was >> waiting directly on autovacuum. > > I think you don't understand what that code is doing. If there's an > autovac anywhere in the dependency graph, it'll find it. That'll teach me to try to read code from a patch directly without trying to apply it or at least read the original source next to it. I thought I had seen this code recently enough to apply the patch from memory -- clearly not. I assume the right thing happens if multiple deadlock check signals fire for the same autovacuum? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Autovacuum cancellation
"Tom Lane" <[EMAIL PROTECTED]> writes: > There's some things still to be desired here: if an autovac process is > involved in a hard deadlock, the patch doesn't favor zapping it over > anybody else, nor consider cancelling the autovac as an alternative to > rearranging queues for a soft deadlock. But dealing with that will open > cans of worms that I don't think we want to open for 8.3. Can autovacuum actually get into a hard deadlock? Does it take more than one lock that can block others at the same time? I think there's a window where the process waiting directly on autovacuum could have already fired its deadlock check before it was waiting directly on autovacuum. But the only way I can see it happening is if another process is cancelled before its deadlock check fires and the signals are processed out of order. I'm not sure that's a case we really need to worry about. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Win32: Minimising desktop heap usage
"Tom Lane" <[EMAIL PROTECTED]> writes:
>> ! /*
>> ! * Note: We use getenv here because the more modern
>> SHGetSpecialFolderPath()
>> ! * will force us to link with shell32.lib which eats valuable desktop
>> heap.
>> ! */
>> ! tmppath = getenv("APPDATA");
>
> Hmm, is there any functional downside to this? I suppose
> SHGetSpecialFolderPath does some things that getenv does not...
The functional difference appears to be that the environment variable is set
on startup (or login?) and doesn't necessarily have the most up to date value
if it's been changed. But it's not something you're likely to change and you
can always adjust the environment variable manually to fix the problem.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Assertion failure with small block sizes
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the >> same >> line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the >> assertion then it passes all regression tests even if I push >> TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as >> possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as >> well. > > Hmm. I'm inclined to reverse the tests (there are 3 not just 1) in > heapam.c, so that it explicitly tries to toast only in plain tables, > rather than adding more exclusion cases. Thoughts? Well RELKIND_UNCATALOGED can be usefully toasted in that data can be compressed internally. I suppose we know none normally receive such treatment at normal block sizes. If we want to make the tuple threshold configurable down the line would we want it affecting initdb bootstrapping? My experiments show it isn't a problem but I don't particularly see any reason why it's advantageous. We may want some future relkinds to be toastable but I don't see a problem adding new cases to the test. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Assertion failure with small block sizes
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> Testing Postgres with a small block size runs into an assertion failure when >> it tries to toast a pg_proc tuple during initdb. I think the assertion is >> just >> wrong and RELKIND_UNCATALOGUED is valid here. > > Uh, what makes you think the assertion is the only problem? The toast > table won't exist yet. Well tuptoaster doesn't try to store anything external if there's no toast table anyways. With the assertion modified it passes the regression tests (modulo row orderings). > How small is "small" anyway? This was 1k with 256 toast target/threshold. If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the same line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the assertion then it passes all regression tests even if I push TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as well. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Assertion failure with small block sizes
Testing Postgres with a small block size runs into an assertion failure when
it tries to toast a pg_proc tuple during initdb. I think the assertion is just
wrong and RELKIND_UNCATALOGUED is valid here. Other places in the code check
for both before, for example, creating toast tables.
creating template1 database in
/home/stark/src/local-HEAD/pgsql/src/test/regress/./tmp_check/data/base/1 ...
TRAP: FailedAssertion("!(rel->rd_rel->relkind == 'r')", File: "tuptoaster.c",
Line: 440)
--- tuptoaster.c13 Oct 2007 22:34:06 +0100 1.78
+++ tuptoaster.c14 Oct 2007 15:37:17 +0100
@@ -437,7 +437,8 @@
* We should only ever be called for tuples of plain relations ---
* recursing on a toast rel is bad news.
*/
- Assert(rel->rd_rel->relkind == RELKIND_RELATION);
+ Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
+ rel->rd_rel->relkind == RELKIND_UNCATALOGED);
/*
* Get the tuple descriptor and break down the tuple(s) into fields.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
[PATCHES] Packed varlena tuptoaster.c oops
Caught this in my testing with enhanced debugging checks. Index: src/backend/access/heap/tuptoaster.c === RCS file: /home/stark/src/REPOSITORY/pgsql/src/backend/access/heap/tuptoaster.c,v retrieving revision 1.77 diff -u -r1.77 tuptoaster.c --- src/backend/access/heap/tuptoaster.c1 Oct 2007 16:25:56 - 1.77 +++ src/backend/access/heap/tuptoaster.c11 Oct 2007 14:47:17 - @@ -813,7 +813,6 @@ pfree(DatumGetPointer(old_value)); toast_free[i] = true; - toast_sizes[i] = VARSIZE(toast_values[i]); need_change = true; need_free = true; -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] 'Waiting on lock'
"Simon Riggs" <[EMAIL PROTECTED]> writes: > On Tue, 2007-09-25 at 09:16 -0400, Tom Lane wrote: >> Simon Riggs <[EMAIL PROTECTED]> writes: >> > SQLServer and DB2 have more need of this than PostgreSQL, but we do >> > still need it. >> >> Why? What does it do that statement_timeout doesn't do better? > > If the execution time is negligible, then setting statement_timeout is > the same thing as setting a lock timeout. To make this explicit, I think the typical scenario where it would make a difference is where you're running some large job in a plpgsql function. You might be processing millions of records but want for a single step of that process to not wait for a lock. You still want to process all the records you can though. So for example if you're updating all the user profiles on your system but don't want to block on any user-profiles which are locked by active users -- especially if you use database locks for user-visible operations which users can drag out for long periods of time. (Not saying I agree with that design but there are arguments for it and people do do it) -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] Eliminate more detoast copies for packed varlenas
Ok, this removes what should be most if not all of the call sites where we're detoasting text or byteas. In particular it gets all the regexp/like functions and all the trim/pad functions. It also gets hashtext and hash_any. $ zcat packed-varlena-efficiency_v0.patch.gz | diffstat backend/access/hash/hashfunc.c| 12 !! backend/utils/adt/like.c | 80 !!! backend/utils/adt/oracle_compat.c | 157 !! backend/utils/adt/regexp.c| 119 include/fmgr.h|1 5 files changed, 5 insertions(+), 364 modifications(!) packed-varlena-efficiency_v0.patch.gz Description: Binary data -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [COMMITTERS] pgsql: Avoid possibly-unportable initializer, per buildfarm warning.
"Tom Lane" <[EMAIL PROTECTED]> writes:
> Log Message:
> ---
> Avoid possibly-unportable initializer, per buildfarm warning.
This was lost in the tsearch2 merge.
Index: src/backend/tsearch/dict_thesaurus.c
===
RCS file:
/home/stark/src/REPOSITORY/pgsql/src/backend/tsearch/dict_thesaurus.c,v
retrieving revision 1.3
diff -c -r1.3 dict_thesaurus.c
*** src/backend/tsearch/dict_thesaurus.c25 Aug 2007 00:03:59 -
1.3
--- src/backend/tsearch/dict_thesaurus.c17 Sep 2007 13:30:27 -
***
*** 653,663
static LexemeInfo *
findTheLexeme(DictThesaurus * d, char *lexeme)
{
! TheLexeme key = {lexeme, NULL}, *res;
if (d->nwrds == 0)
return NULL;
res = bsearch(&key, d->wrds, d->nwrds, sizeof(TheLexeme), cmpLexemeQ);
if (res == NULL)
--- 653,666
static LexemeInfo *
findTheLexeme(DictThesaurus * d, char *lexeme)
{
! TheLexeme key, *res;
if (d->nwrds == 0)
return NULL;
+ key.lexeme = lexeme;
+ key.entries = NULL;
+
res = bsearch(&key, d->wrds, d->nwrds, sizeof(TheLexeme), cmpLexemeQ);
if (res == NULL)
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] PL/TCL Patch to prevent postgres from becoming multithreaded
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > This has been saved for the 8.4 release: > > http://momjian.postgresql.org/cgi-bin/pgpatches_hold > > --- > > Marshall, Steve wrote: >> There is a problem in PL/TCL that can cause the postgres backend to >> become multithreaded. Postgres is not designed to be multithreaded, so >> this causes downstream errors in signal handling. Um, this is a bug fix. Unless you had some problem with it? Do we have anyone actively maintaining pltcl these days? I'm intentionally quite unfamiliar with Tcl or I would be happy to verify it's reasonable. But the explanation seems pretty convincing. If we don't have anyone maintaining it then we're pretty much at the mercy of applying whatever patches come in from people who are more familiar with it than us. In my experience that's how new maintainers for modules of free software are often found anyways. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] WIP patch for latestCompletedXid method of computing snapshot xmax
"Tom Lane" <[EMAIL PROTECTED]> writes: > Gregory Stark <[EMAIL PROTECTED]> writes: >> "Tom Lane" <[EMAIL PROTECTED]> writes: >>> This patch implements Florian's idea about how to manage snapshot xmax >>> without the ugly and performance-losing tactic of taking XidGenLock and >>> ProcArrayLock at the same time. I had to do a couple of slightly klugy >>> things to get bootstrap and prepared transactions to work, but on the >>> whole it seems at least as clean as the code we have now. Comments? > >> Just that it will be fascinating to see what effects this has on the >> benchmarks. > > Yeah, I was hoping to get some benchmarks before deciding whether it's > worth the risk of pushing this into 8.3. I'm off trying pgbench now, > but if anyone wants to try something more serious like DBT2 ... I ran some DBT2 tests but haven't been able to show any performance effects either in average or worst-case response times. I think that's for a few reasons: 1) This is only a dual-processor machine I'm playing with and we scale well on two processors already. 2) TPC-C doesn't have many read-only transactions 3) The deadlocks I found earlier cause enough noise in the response times to hide any subtler effects. We may have to wait until the next time Sun runs their 1,000-process monster Java benchmark to see if it helps there. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Yet more tsearch refactoring
"Teodor Sigaev" <[EMAIL PROTECTED]> writes: > Did you check it on 64-bit boxes with strict alignment? I remember that was a > headache for me. Is there a regression test which would fail if this kind of problem crops up? Not every developer can test every type of hardware but (aside from believing the code will work of course) we should at a minimum be certain that the build farm will detect the problem. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] HOT patch - version 15
"Tom Lane" <[EMAIL PROTECTED]> writes: > What it sounds is utterly unsafe. You can get away with not WAL-logging > individual bit flips (that is, hint-bit-setting) because either state of > the page is valid. If I read this proposal correctly it is to change > t_ctid without WAL-logging, which means that a partial page write (torn > page syndrome) could leave the page undetectably corrupted --- t_ctid > is 6 bytes and could easily cross a hardware sector boundary. Well we would never be overwriting the blockid, only the posid which is 2 bytes. And the ctid (and posid) should always be 4-byte aligned. So actually it would never cross a hardware sector boundary. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] HOT patch - version 15
"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > On 9/11/07, Bruce Momjian <[EMAIL PROTECTED]> wrote: > >> Right. My point is that pruning only modifies item pointers. It does >> not change the actual heap tuples. In the quote above, how is Heikki's >> "quick pruning" different from the pruning you are describing? > > I think the only difference is that the quick pruning does not mark > intermediate tuples ~LP_USED and hence we may avoid WAL logging. > > Btw, I am not too enthusiastic about quick pruning because it leaves > behind dead heap-only tuples which are not reachable from any root > heap tuples. Not that we can't handle them, but I am worried about > making such changes right now unless we are sure about the benefits. > We can always tune and tweak in 8.4 You could mark such tuples with LP_DELETE. That would also let other transactions quickly tot up how much space would be available if they were to run PageRepairFragmentation. But if you don't WAL log setting LP_DELETE then you would still have to deal with unreachable HOT tuples who lost their LP_DELETE flag. I suppose that as long as PageRepairFragmentation first looks for any dead HOT tuples without depending on LP_DELETE that would be enough. I wonder if you could do the quick prune without even taking the exclusive page lock? You're overwriting 16 bits and you know nobody else will be modifying any of the line pointers in question to anything else. (because your pin prevents the vacuum lock from coming in and trying to mark it unused). -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] HOT patch - version 15
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > Looking at the patch I see: > > + /* > +* Mark the page as clear of prunable tuples. If we find a tuple which > +* may become prunable, we shall set the hint again. > +*/ > + PageClearPrunable(page); > > I like the idea of the page hint bit, but my question is if there is a > long-running transaction, isn't each SELECT going to try to defragment a > page over and over again because there is still something prunable on > the page? Well it'll try to prune the chains over and over again. If it doesn't find anything it won't defragment, but yes. I think we could tackle that by storing on the page the oldest xmax which would allow us to prune a tuple. Then you could compare RecentGlobalXmin against that and not bother looking at any other chains if it hasn't been passed yet. It would be hard to do that with single-chain pruning though, once you the limiting tuple you would then wouldn't know what the next limiting xmax is until the next time you do a full-page prune. Still that gets us down to at most two full-page prunes per update instead of a potentially unbounded number of prunes. This seems like a further optimization to think about after we have a place to trigger the pruning where it'll do the most good. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] WIP patch for latestCompletedXid method of computing snapshot xmax
"Tom Lane" <[EMAIL PROTECTED]> writes: > This patch implements Florian's idea about how to manage snapshot xmax > without the ugly and performance-losing tactic of taking XidGenLock and > ProcArrayLock at the same time. I had to do a couple of slightly klugy > things to get bootstrap and prepared transactions to work, but on the > whole it seems at least as clean as the code we have now. Comments? Just that it will be fascinating to see what effects this has on the benchmarks. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] HOT patch - version 15
"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > On 9/6/07, Gregory Stark <[EMAIL PROTECTED]> wrote: > >> Ah, as I understand it you can't actually do the pruning then because the >> executor holds references to source tuple on the page. In other words you >> can never "get the vacuum lock" there because you already have the page >> pinned yourself. > > I don't think executor holding a reference is a problem because when > we check for vacuum lock, we have already pinned the page anyways. But that's the point. Even though the pin-count is 1 and it may look like we have the vacuum lock we really don't. The fact that the buffer was already pinned by us means that there are already pointers around referencing tuples on that page. If we move them around those pointers become garbage. In fact Postgres avoids copying data if it can get by with a pointer at the original tuple on disk so some of the pointers will be Datums pointing at individual columns in the tuples in the page. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] HOT patch - version 15
"Tom Lane" <[EMAIL PROTECTED]> writes: > Bruce Momjian <[EMAIL PROTECTED]> writes: >> Tom Lane wrote: >>> Uh, why would any of this code at all execute during a pure lookup? > >> No idea. It seems an index lookup tries to prune a heap chain, and he >> was asking if it should look at other chains on the page; I said not. >> Whether the index lookup should prune the heap chain is another issue. > > ISTM the only time we should be doing HOT cleanup work is when we are > trying to insert a new tuple on that page --- and maybe even only when > we try and it doesn't fit straight off. Otherwise you're pushing > maintenance work into foreground query pathways, which is exactly where > I don't think we should be headed. Ah, as I understand it you can't actually do the pruning then because the executor holds references to source tuple on the page. In other words you can never "get the vacuum lock" there because you already have the page pinned yourself. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] HOT patch - version 15
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Bruce Momjian <[EMAIL PROTECTED]> writes: >> > I am thinking we are best just doing the index chain we already have to >> > read. >> >> > If you are modifying the table (like with UPDATE) it makes sense to be >> > more aggressive and do the whole page because you know there are >> > probably other table modifications, but for an index lookup there isn't >> > any knowledge of whether the table is being modified so looking at more >> > than we need seems like overkill. >> >> Uh, why would any of this code at all execute during a pure lookup? > > No idea. It seems an index lookup tries to prune a heap chain, and he > was asking if it should look at other chains on the page; I said not. > Whether the index lookup should prune the heap chain is another issue. Pruning chains is kind of the whole point of the exercise no? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] HOT documentation README
"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > It would be worth mentioning that columns appearing in predicates > of partial indexes and expressions of expression indexes are also > checked. If any of these columns are changed, HOT update is not done. In discussion a question arose. I don't think we currently compare these columns before when we're building an index and find a visible hot update. We just set hot_visible even if the chain might still be a valid hot chain for the new index right? Should we consider checking the columns in that case too? Or would it be too much extra overhead? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] HOT documentation README
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > I have taken this, and Pavan's documentation about CREATE INDEX, and > worked up an updated README. Comments? Corrections? Oh, you I think the CREATE INDEX documentation you refer to was actually the one I suggested. A few tweaks: > (If we find any HOT-updated tuples with RECENTLY_DEAD or > DELETE_IN_PROGRESS we ignore it assuming that we will also come across > the _end_ of the update chain and index that instead.) There's more to this. We build a mapping telling us the Root tuple for each tuple in the page. Then when we scan tuples looking for the Head of each HOT chain (ie, a tuple that wasn't HOT updated) and index the corresponding Root from the map using the key value from the Head tuple. We treat DELETE_IN_PROGRESS the same way we treat RECENTLY_DEAD (and INSERT_IN_PROGRESS the same as LIVE) because we assume it's been deleted (or inserted) by our own transaction. So while it's not actually committed yet we can assume it is since if its transaction aborts the index creation itself will be aborted. Other transactions cannot be deleting or inserting tuples without having committed or aborted already because we have a lock on the table and the other transactions normally keep their locks until they exit. NOTE: This is something likely to change. Current discussions are leading towards handling DELETE_IN_PROGRESS and INSERT_IN_PROGRESS from other transactions. We would do this by waiting until the transactions owning those tuples exit. This would allow us to index tables being used by transactions which release their locks early to work. In particular this happens for system tables. > The tricky case arises with queries executed in the same transaction as > CREATE INDEX. In the case of a new table created within the same > transaction (such as with pg_dump), the index will be usable because > there will never be any HOT update chains so the indcreatexid will never > be set. This is unclear and perhaps misleading. I think it needs to be more like "In the case of a new table in which rows were inserted but none updated (such as with pg_dump) the index will be usable because ..." > Also in the case of a read-committed transaction new queries will be able to > use the index. A serializable transaction building an index on an existing > table with HOT updates cannot not use the index. I don't think this is clear and I'm not sure it's right. Currently the transaction that actually did the CREATE INDEX has to follow the same rules as other transactions. This means if there were any visible hot updated tuples and the index is therefore marked with our xid in indcreatexid we will *not* be able to use it in the same transaction as our xid is never in our serializable snapshot. This is true even if we're not in serializable mode as we cannot know what earlier snapshots are still in use and may be used with the new plan. NOTE: This again is something likely to change. In many cases it ought to be possible to have the transaction use the index it just built even if there were visible HOT updated tuples in it. In particular in READ COMMITTED transactions which have no outstanding commands using early snapshots then subsequent planning ought to be able to use the index. Even if outstanding commands are using old snapshots if we can be sure they can't use the new plan then it would still be safe to use the index in the new plan. Also in SERIALIZABLE mode those same statements hold for temporary tables. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] HOT documentation README
"Bruce Momjian" <[EMAIL PROTECTED]> writes: > Heikki Linnakangas wrote: > >> Here's an updated version of the README I posted earlier. It now >> reflects the changes to how pruning works. > > I have taken this, and Pavan's documentation about CREATE INDEX, and > worked up an updated README. Comments? Corrections? You should also take the appendix to Heikki's README about CREATE INDEX that I wrote. > > I plan to put this in src/backend/access/heap/README.HOT. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Buglet in "Sort Method" explain output in degenerate case
"Tom Lane" <[EMAIL PROTECTED]> writes:
> Setting it at conclusion is correct, I think, since if we ever changed
> the code to abandon TSS_BOUNDED state in the face of unexpected memory
> growth, it would be wrong to have set it in make_bounded_sort.
Actually that would be pretty easy to do and strikes me as worth doing. It
isn't hard to contrive an example that over-runs work_mem though I'm not sure
how easy it would be to actually run out of RAM.
One thing though, we would have to let up on switching to bounded sort if we
run out of memory accumulating tuples. We wouldn't want to switch to bounded
sort and then spill to disk right afterward. Here I just added 10% assuming
usually the remaining tuples won't be 10% larger than the first batch.
Introducing floating point math in here kind of bothers me though.
Index: src/backend/utils/sort/tuplesort.c
===
RCS file: /home/stark/src/REPOSITORY/pgsql/src/backend/utils/sort/tuplesort.c,v
retrieving revision 1.77
diff -c -r1.77 tuplesort.c
*** src/backend/utils/sort/tuplesort.c 7 Jun 2007 19:19:57 - 1.77
--- src/backend/utils/sort/tuplesort.c 1 Sep 2007 20:31:53 -
***
*** 940,946
*/
if (state->bounded &&
(state->memtupcount > state->bound * 2 ||
! (state->memtupcount > state->bound && LACKMEM(state
{
#ifdef TRACE_SORT
if (trace_sort)
--- 940,946
*/
if (state->bounded &&
(state->memtupcount > state->bound * 2 ||
! (state->memtupcount > state->bound * 1.1 && LACKMEM(state
{
#ifdef TRACE_SORT
if (trace_sort)
***
*** 970,975
--- 970,976
break;
case TSS_BOUNDED:
+
/*
* We don't want to grow the array here, so check whether the
* new tuple can be discarded before putting it in. This should
***
*** 991,996
--- 992,1009
tuplesort_heap_siftup(state, false);
tuplesort_heap_insert(state, tuple, 0, false);
}
+
+ /* If the later tuples were larger than the first batch we could be
+ * low on memory in which case we have to give up on the bounded
+ * sort and fail over to a disk sort
+ */
+ if (LACKMEM(state))
+ {
+ REVERSEDIRECTION(state);
+ inittapes(state);
+ dumptuples(state, false);
+ }
+
break;
case TSS_BUILDRUNS:
***
*** 1068,1075
* in memory, using a heap to eliminate excess tuples. Now we have
* to transform the heap to a properly-sorted array.
*/
! if (state->memtupcount > 1)
! sort_bounded_heap(state);
state->current = 0;
state->eof_reached = false;
state->markpos_offset = 0;
--- 1081,1087
* in memory, using a heap to eliminate excess tuples. Now we have
* to transform the heap to a properly-sorted array.
*/
! sort_bounded_heap(state);
state->current = 0;
state->eof_reached = false;
state->markpos_offset = 0;
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
