Re: [HACKERS] Sampling profiler updated
On Sat, Jul 18, 2009 at 5:36 PM, Jaime Casanova wrote: > On Sat, Jul 18, 2009 at 3:38 PM, Robert Haas wrote: >> On Sat, Jul 18, 2009 at 4:09 PM, Dimitri Fontaine >> wrote: >>> So I'm going to change patch state to "Returned with Feedback" as I guess >>> we'll need to talk about the issue and find a way to solve it, and I don't >>> think this state prevent from getting back to the patch in this same fest. >> >> In general I would prefer patches to be set to Returned with Feedback >> only when we think they are probably done for this CommitFest. > > why? it seems very simple as is: > Returned with Feedback means there is something to clean, when the > author fixes the problem he can update it to Needs review. No, that's what "waiting on author" means. "Returned with feedback" means that it might be acceptable in some CommitFest, but not this one. "Rejected" means we don't want it. The distinction is important because it affects the review process. If I have reviewed 8 patches and they are all in the status "waiting on author", then I am reluctant to take on any more patches because I might have to re-review as many as 8 patches, and that might be as much (or more) than I'm prepared to take on. But if all of those patches are returned with feedback, then I know that they are not coming back, and I can take on a few more without worrying that I'm suddenly going to get slammed by the need to re-review a bunch of stuff. It is also very hard to close the CommitFest if things that are dead keep coming back to life again. To take an example, suppose that a patch is reviewed on July 16th and the patch author is asked to make some changes. Then, on August 10th, the author resubmits. If we take the view that this is legitimate, then we're going to have a whole slough of resubmits just prior to whatever we set as the last day for resubmits, which will mean that the CommitFest is not going to be closed in a month, and we need that to happen so that people can get back to working on their own patches. What we need to say is if the patch author can't resubmit within a few days (say, four, maybe a bit more if it's a major patch or early in the CommitFest), then we move it from waiting to author to returned with feedback and move on. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-threaded pgbench
Greg (Smith), do you have time to review this version? If not, I will assign a round-robin reviewer when one becomes available. I can do a concurrency test of this next week. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sampling profiler updated
On Sat, Jul 18, 2009 at 5:28 PM, Tom Lane wrote: > Robert Haas writes: >> On Sat, Jul 18, 2009 at 4:09 PM, Dimitri Fontaine >> wrote: >>> So I'm going to change patch state to "Returned with Feedback" as I guess >>> we'll need to talk about the issue and find a way to solve it, and I don't >>> think this state prevent from getting back to the patch in this same fest. > >> In general I would prefer patches to be set to Returned with Feedback >> only when we think they are probably done for this CommitFest. >> Otherwise, it's hard to tell which are really done and which are maybe >> done. > > What do you want to use then ... Waiting on Author? Yeah, that's what I was thinking. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GEQO vs join order restrictions
On Sat, Jul 18, 2009 at 12:49 PM, Tom Lane wrote:
> We could refrain from collapsing the sub-problem during joinlist
> formation. But the trouble with that is it creates a "hard" join order
> restriction. Most of the restrictions are "soft" to some extent, ie,
> you can do some rearrangements but not others. It might be worth
> looking at though; in the cases where there is no chance of a
> rearrangement, it would save cycles for either regular or GEQO planning.
This seems very promising, although the proof of the pudding is in the
eating. As a slight refinement of this idea, it's not exactly that
the join order is totally fixed, but rather that in a large join nest
there may be groups of tables that can be planned as independent
subproblems. The obvious example of this is something like:
(...lots of stuff...) FULL JOIN (...lots of things...)
...where there isn't any point in considering any joins between stuff
and things, regardless of whether you are using GEQO or the standard
planner (and maybe we handle this case already?). My brain is a
little foggy at the moment, but I think this would also apply to
Andres' example query, because I believe with anything of the form...
A LEFT JOIN (B1 INNER JOIN B2 INNER JOIN B3 ... INNER JOIN Bn) LEFT JOIN C
...the set of all Bi can be planned as an independent subproblem and
then joined to A either before or after C. But (I think):
A LEFT JOIN (B1 INNER JOIN B2 LEFT JOIN B3) LEFT JOIN C
= A LEFT JOIN (B1 INNER JOIN B2) LEFT JOIN C LEFT JOIN B3
Here {B1, B2} is an independent subproblem, but {B1, B2, B3} is not.
Still, given that the planning time for the standard planner at least
can grow exponentially in the number of relations, even a small
reduction in the problem size is potentially a big win.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using results from INSERT ... RETURNING
Jaime Casanova writes: > On Sat, Jul 18, 2009 at 6:25 PM, Merlin Moncure wrote: >> Being able to use 'returning' in a subquery is probably the #1 most >> requested feature for postgresql (it's also a todo). Solving it for >> 'with' queries is a nice step in the right direction, and sidesteps >> some of the traps that result from the general case. > ah! that's why i asked: 'if we will support this, shouldn't we > supporting INSERT RETURNING inside subqueries too?' We've been over that: when will you fire triggers? What happens if the outer query doesn't want to read the whole output of the DML command, or wants to read it more than once? If the DML command is in a single-evaluation WITH clause at the top level of the command, then it's reasonable to identify the outer command's own begin and end as the times to fire triggers; and there is no issue about partial or repeated evaluation. If it's in a subquery then things get much messier and more poorly defined. Note that it can't just be "a WITH clause". It has to be one at the top query level, or the problem comes right back. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic explain options v3 - RR Review
Hi Robert, Hi All,
Patch applies with some offset changes, code changes look sensible, I
personally like the new syntax and the features it may allow in future. One,
possibly big, gripe remains though:
The formerly valid statement which cannot be written without the parentheses
and stay semantically equivalent:
EXPLAIN (SELECT 1 ORDER BY 1) UNION ALL (SELECT 2 ORDER BY 1);
is now not valid anymore (The added %prec UMINUS causes the first '(' to be
recognize as start of the option list as intended).
This currently can only be resolved by using an option list like:
EXPLAIN (VERBOSE OFF) ...
Its also currently impossible to use an empty set of parentheses to resolve
this - this could easily be changed though.
I have to admit I don't see a nice solution here except living with the
incompatibility... Perhaps somebody has a better idea?
Andres
PS: The 'offset corrected' version I tested with is attached
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index b7c0ef9..956af94 100644
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
***
*** 14,19
--- 14,20
#include "commands/explain.h"
#include "executor/instrument.h"
+ #include "nodes/makefuncs.h"
#include "utils/guc.h"
PG_MODULE_MAGIC;
*** explain_ExecutorEnd(QueryDesc *queryDesc
*** 196,207
msec = queryDesc->totaltime->total * 1000.0;
if (msec >= auto_explain_log_min_duration)
{
StringInfoData buf;
initStringInfo(&buf);
! ExplainPrintPlan(&buf, queryDesc,
! queryDesc->doInstrument && auto_explain_log_analyze,
! auto_explain_log_verbose);
/* Remove last line break */
if (buf.len > 0 && buf.data[buf.len - 1] == '\n')
--- 197,210
msec = queryDesc->totaltime->total * 1000.0;
if (msec >= auto_explain_log_min_duration)
{
+ ExplainStmt *stmt = makeExplain(NIL, NULL);
StringInfoData buf;
initStringInfo(&buf);
! stmt->analyze =
! (queryDesc->doInstrument && auto_explain_log_analyze);
! stmt->verbose = auto_explain_log_verbose;
! ExplainPrintPlan(&buf, queryDesc, stmt);
/* Remove last line break */
if (buf.len > 0 && buf.data[buf.len - 1] == '\n')
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index ea9b3a6..afcab08 100644
*** a/doc/src/sgml/ref/explain.sgml
--- b/doc/src/sgml/ref/explain.sgml
*** PostgreSQL documentation
*** 31,36
--- 31,37
+ EXPLAIN [ ( [ { ANALYZE | VERBOSE | COSTS } [ boolean_value ] ] [, ...] ) ] statement
EXPLAIN [ ANALYZE ] [ VERBOSE ] statement
*** EXPLAIN [ ANALYZE ] [ VERBOSE ]
+
+Only the ANALYZE and VERBOSE options
+can be specified, and only in the order, without surrounding the option list
+in parentheses. Prior to PostgreSQL 8.5, the
+unparenthesized syntax was the only one supported. It is expected that
+all new options will be supported only when using the parenthesized syntax,
+which also allows a value to be specified for each option
+(e.g. TRUE or FALSE).
+
+
Keep in mind that the statement is actually executed when
*** ROLLBACK;
*** 99,105
ANALYZE
! Carry out the command and show the actual run times.
--- 110,117
ANALYZE
! Carry out the command and show the actual run times. This
! parameter defaults to OFF.
*** ROLLBACK;
*** 108,114
VERBOSE
! Include the output column list for each node in the plan tree.
--- 120,152
VERBOSE
! Include the output column list for each node in the plan tree. This
! parameter defaults to OFF.
!
!
!
!
!
! COSTS
!
!
! Include information on the estimated startup and total cost of each
! plan node, as well as the estimated number of rows and the estimated
! width of each row. This parameter defaults to ON.
!
!
!
!
!
! boolean_value
!
!
! Specifies whether the named parameter should be turned on or off. You
! can use the values TRUE, ON,
! YES, or 1 to request the stated
! option, and FALSE, OFF,
! NO, or 0. If the Boolean value
! is omitted, it defaults to TRUE.
*** EXPLAIN SELECT * FROM foo WHERE i = 4;
*** 202,207
--- 240,259
+Here is the same plan with costs suppressed:
+
+
+ EXPLAIN (COSTS OFF) SELECT * FROM foo WHERE i = 4;
+
+ QUERY PLAN
+
+ Index Scan using fi on foo
+Index Cond: (i = 4)
+ (2 rows)
+
+
+
+
Here is an example of a query plan for a query using an aggregate
function:
diff --git a/src/backend/commands/explain.c b/src/backend/command
Re: [HACKERS] explain refactoring v4 - RR Review
Hi Robert, Hi all, On Friday 12 June 2009 06:32:11 Robert Haas wrote: > OK, here it is again. Changes are the same as the previous version, > but this one should apply cleanly after today's pgindent run. Not much to say here. Patch applies cleanly, the changes look sensible and as far as I can see they did not introduce visible changes. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using results from INSERT ... RETURNING
On Sat, Jul 18, 2009 at 6:25 PM, Merlin Moncure wrote: > On Sat, Jul 18, 2009 at 5:21 PM, Jaime > Casanova wrote: >> my questions first: >> - what's the use case for this? > > Being able to use 'returning' in a subquery is probably the #1 most > requested feature for postgresql (it's also a todo). Solving it for > 'with' queries is a nice step in the right direction, and sidesteps > some of the traps that result from the general case. ah! that's why i asked: 'if we will support this, shouldn't we supporting INSERT RETURNING inside subqueries too?' i'm not too confident with the code but i think the problems for both cases have to be similar so if we solve one, why not the other? > > move records from one table to another: > with foo as (delete from bar where something returning *) insert > insert into baz select foo.*: > seems like a corner case... > gather defaulted values following an insert for later use: > with foo as (insert into bar(field) select 'hello' from > generate_series(1,n) returning *) insert into baz select foo.*; > ok -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using results from INSERT ... RETURNING
On Sat, Jul 18, 2009 at 5:21 PM, Jaime Casanova wrote: > my questions first: > - what's the use case for this? Being able to use 'returning' in a subquery is probably the #1 most requested feature for postgresql (it's also a todo). Solving it for 'with' queries is a nice step in the right direction, and sidesteps some of the traps that result from the general case. There are many obvious ways this feature is helpful...here's a couple: move records from one table to another: with foo as (delete from bar where something returning *) insert insert into baz select foo.*: gather defaulted values following an insert for later use: with foo as (insert into bar(field) select 'hello' from generate_series(1,n) returning *) insert into baz select foo.*; merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] navigation menu for documents
On Jul 18, 2009, at 9:54 AM, Richard Huxton wrote: No, it doesn't. David Wheeler's navigation (see upthread) that he uses for the Bricolage docs does, however. Ah, if you can change the overall layout then the world is your shellfish of choice. Would it be possible to include jquery? It's GPL/MIT dual-licence. That's what I used for the Bricolage stuff. I still need to fix the encoding issues on Windows, though… Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
David Fetter asked: > > 1. Among the committers who could maintain the features, whatever > they turn out to be, who is actually volunteering to do so? I am not a committer, but I saw a message about issues with documentation -- I would be willing to help on making documentation and comments more robust; I don't speak Japanese but have a couple of friends who do that I can call upon for some help. > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? This feature would be of interest in some corporate circles as well as government and health care. That said it seems like two issues are biting those working on it: (a) it's a lot of work even to get a simple version and (b) the model differs from the more tradition ACL and trying to wrap collective heads around the implications is an exercise in and of itself. My gut feeling is this is so much work that it needs a paying sponsor to see it through. Trying to do it without more hands is likely to distract from other items on the to-do list. Greg W. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Index AM API changes for deferability
On Sat, 2009-07-18 at 11:09 +0100, Dean Rasheed wrote: > Thinking about this a bit more, perhaps it would be better if I added > an out parameter to the AM for the uniqueness result, rather than > overloading the return value, which is quite ugly: Sounds reasonable to me. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE ... ALTER COLUMN ... SET DISTINCT
Le 18 juil. 09 à 20:55, Robert Haas a écrit : that indicate the base rev of the patch. In fact that rev is BEFORE the one I based the patch on, which was 64b2da3f7778bc6fd3d213ceb9e73ff3b6e03890, and it actually applies OK up until 0e3abe7ec78a3d51032d684598f188b0b0304fe9, the commit immediately preceding the 8.4 pgindent run. Robert told me on rrreviewers it's all whitespace stuff, which I could confirmed after having trained my eyes to recognize the pattern and mix and match whitespace and new column in the middle. Patch applied, I'll have to update the version check, but the following paste means I'm the one having to work on it now: Table "pg_catalog.pg_attribute" Column | Type| Modifiers ---+---+--- ... attstattarget | integer | not null attdistinct | real | not null attlen| smallint | not null Hmm, I didn't know that (my man page doesn't contain that language). It looks like strtod() is more widely used in the existing source code than atof(), so I'll change it (atof() is however used in the floatVal() macro). Considering I don't have an updated version when I start again, I'll have a try at it: don't rush into it yourself :) What about adding the following before the switch, to do like surrounding code? Assert(IsA(newValue, Integer) || IsA(newValue, Float)); Not a good plan. In my experience, gcc doesn't like switch () statements over enums that don't list all the values, unless they have a default clause; it masterminds by giving you a warning that you've "inadvertently" left out some values. Maybe this could still be a supplementary barrier for cassert builds in order to match existing code? -- dim I'm going to update the status of patch and resume work on it next week. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Deferrable unique constraints
Review feedback: 1. Compiler warning: index.c: In function ‘index_create’: index.c:784: warning: implicit declaration of function ‘SystemFuncName’ 2. I know that the GIN, GiST, and Hash AMs don't use the uniqueness_check argument at all -- in fact it is #ifdef'ed out. However, for the sake of documentation it would be good to change those unused arguments (in, e.g., gininsert()) to be IndexUniqueCheck enums rather than bools. 3. The unique constraint no longer waits for concurrent transactions to finish if the unique constraint is deferrable anyway (and it's not time to actually check the constraint yet). That makes sense, because the whole point is to defer the constraint. However, that means there are a few degenerate situations that were OK before, but can now get us into trouble. For instance, if you have a big delete and a concurrent big insert, the current code will just block at the first conflicting tuple and wait for the delete to finish. With deferrable constraints, it would save all of those tuples up as potential conflicts, using a lot of memory, when it is not strictly necessary because the tuples will be gone anyway. I'm not particularly worried about this situation -- because I think it's a reasonable thing to expect when using deferred constraints -- but I'd like to bring it up. 4. Requiring DEFERRABLE after UNIQUE in order to get commands like "UPDATE ... SET i = i + 1" to work isn't following the spec. I'm not sure what we should do here, because the 8.4 behavior is not following the spec at all, but people may still want it. 5. In the docs, 50.2: "This is the only situation ...": it's a little unclear what "this" is. 6. Missing support for CREATE INDEX CONCURRENTLY is unfortunate. What would be involved in adding support? 7. It looks like deferrable unique indexes can only be created by adding a constraint, not as part of the CREATE INDEX syntax. One consequence is that CIC can't be supported (right?). If you don't plan to support CIC, then maybe that's OK. 8. Code like the following: is_vacuum ? UNIQUE_CHECK_NO : deferUniqueCheck ? UNIQUE_CHECK_PARTIAL : relationDescs[i]->rd_index->indisunique ? UNIQUE_CHECK_YES : UNIQUE_CHECK_NO); Is a little difficult to read, at least for me. It's a style thing, so you don't have to agree with me about this. 9. Passing problemIndexList to AfterTriggerSaveEvent seems a little awkward. I don't see an obvious way to make it cleaner, but I thought it's worth mentioning. 10. You're overloading tgconstrrelid to hold the constraint's index's oid, when normally it holds the referenced table. You should probably document this a little better, because I don't think that field is used to hold an index oid anywhere else. The rest of the patch seems fairly complete. Tests, documentation, psql, and pg_dump support look good. And the patch works, of course. Code and comments look good to me as well. I like the patch. It solves a problem, brings us closer to the SQL standard, and the approach seems reasonable to me. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sampling profiler updated
On Sat, Jul 18, 2009 at 3:38 PM, Robert Haas wrote: > On Sat, Jul 18, 2009 at 4:09 PM, Dimitri Fontaine > wrote: >> So I'm going to change patch state to "Returned with Feedback" as I guess >> we'll need to talk about the issue and find a way to solve it, and I don't >> think this state prevent from getting back to the patch in this same fest. > > In general I would prefer patches to be set to Returned with Feedback > only when we think they are probably done for this CommitFest. why? it seems very simple as is: Returned with Feedback means there is something to clean, when the author fixes the problem he can update it to Needs review. > Otherwise, it's hard to tell which are really done and which are maybe > done. > when the rrr thinks the patch is in good shape mark it as Ready for committer then the commiter could apply, reject or return with feedback again... at the end of the commitfest if there are patches on returned with feedback and the author respond those will go to the next commitfest... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sampling profiler updated
Robert Haas writes: > On Sat, Jul 18, 2009 at 4:09 PM, Dimitri Fontaine > wrote: >> So I'm going to change patch state to "Returned with Feedback" as I guess >> we'll need to talk about the issue and find a way to solve it, and I don't >> think this state prevent from getting back to the patch in this same fest. > In general I would prefer patches to be set to Returned with Feedback > only when we think they are probably done for this CommitFest. > Otherwise, it's hard to tell which are really done and which are maybe > done. What do you want to use then ... Waiting on Author? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using results from INSERT ... RETURNING
On Tue, Jul 7, 2009 at 3:31 PM, Marko Tiikkaja wrote: > Hello. > > Here's a patch(WIP) that implements INSERT .. RETURNING inside a CTE. Should > apply cleanly against CVS head. > > The INSERT query isn't rewritten so rules and default values don't work. > Recursive CTEs don't work either. > my questions first: - what's the use case for this? - why you need a node InsertReturning (see nodeInsertReturning.c) at all? - if we will support this, shouldn't we supporting INSERT RETURNING inside subqueries too? and it crashes for triggers (example using regression's int4_tbl) create function trg_int4_tbl() returns trigger as $$ begin raise notice 'ejecutando'; return new; end; $$ language plpgsql; create trigger trig_int4_tbl before insert on int4_tbl for each row execute procedure trg_int4_tbl(); with q as (insert into int4_tbl select generate_series(1, 5) returning *) select * from q; NOTICE: ejecutando LOG: server process (PID 20356) was terminated by signal 11: Segmentation fault LOG: terminating any other active server processes server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: FATAL: the database system is in recovery mode Failed. !> LOG: all server processes terminated; reinitializing and for defaults (even if i'm providing the values, actually is worse in that case) CREATE TABLE t(i SERIAL PRIMARY KEY); with t1 as (insert into t values (default), (default), (default) returning 'INSERT', i) select * from t1; ERROR: unrecognized node type: 337 with t1 as (insert into t values (1), (2), (3) returning 'INSERT', i) select * from t1; LOG: server process (PID 21604) was terminated by signal 11: Segmentation fault LOG: terminating any other active server processes LOG: all server processes terminated; reinitializing LOG: database system was interrupted; last known up at 2009-07-18 15:29:28 ECT LOG: database system was not properly shut down; automatic recovery in progress server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: FATAL: the database system is in recovery mode Failed. !> LOG: redo starts at 0/32A0310 -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Revise parallel pg_restore's scheduling heuristic
"Kevin Grittner" wrote: > Performance tests to follow in a day or two. I'm looking to beg another week or so on this to run more tests. What I can have by the end of today is pretty limited, mostly because I decided it made the most sense to test this with big complex databases, and it just takes a fair amount of time to throw around that much data. (This patch didn't seem likely to make a significant difference on smaller databases.) My current plan is to test this on a web server class machine and a distributed application class machine. Both database types have over 300 tables with tables with widely ranging row counts, widths, and index counts. It would be hard to schedule the requisite time on our biggest web machines, but I assume an 8 core 64GB machine would give meaningful results. Any sense what numbers of parallel jobs I should use for tests? I would be tempted to try 1 (with the -1 switch), 8, 12, and 16 -- maybe keep going if 16 beats 12. My plan here would be to have the dump on one machine, and run pg_restore there, and push it to a database on another machine through the LAN on a 1Gb connection. (This seems most likely to be what we'd be doing in real life.) I would run each test with the CVS trunk tip with and without the patch applied. The database is currently 1.1TB. The application machine would have 2 cores and about 4GB RAM. I'm tempted to use Milwaukee County's database there, as it has the most rows per table, even though some of the counties doing a lot of document scanning now have bigger databases in terms of disk space. It's 89GB. I'd probably try job counts starting at one and going up by one until performance starts to drop off. (At one I would use the -1 switch.) In all cases I was planning on using a "conversion" postgresql.conf file, turning off fsync, archiving, statistics, etc. Does this sound like a sane approach to testing whether this patch actually improves performance? Any suggestions before I start this, to ensure most meaningful results? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sampling profiler updated
On Sat, Jul 18, 2009 at 4:09 PM, Dimitri Fontaine wrote: > So I'm going to change patch state to "Returned with Feedback" as I guess > we'll need to talk about the issue and find a way to solve it, and I don't > think this state prevent from getting back to the patch in this same fest. In general I would prefer patches to be set to Returned with Feedback only when we think they are probably done for this CommitFest. Otherwise, it's hard to tell which are really done and which are maybe done. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sampling profiler updated
Hi,
Le 14 juil. 09 à 11:47, Itagaki Takahiro a écrit :
I updated Sampling profiler patch to be applied to HEAD cleanly.
Which I confirm, as I just spent some time to reviewing the patch (it
was left unassigned in the commit fest). Reviewing code didn't strike
anything so obvious that I'd notice...
Basic concept of the patch is same as DTrace probes:
Call pgstat_push_condition(condition) before a operation and call
pgstat_pop_condition() at the end of the operation. Those functions
should be light-weight because they only change a variable on shared
memory without any locks.
...except that the typical integration is like this:
+ pgstat_push_condition(PGCOND_XLOG_OPEN);
fd = BasicOpenFile(path, O_RDWR | PG_BINARY |
get_sync_bit(sync_method),
S_IRUSR | S_IWUSR);
+ pgstat_pop_condition();
And we have this:
! void
! pgstat_push_condition(PgCondition condition)
! {
! volatile PgBackendStatus *beentry = MyBEEntry;
! PgCondition prev;
!
! if (profiling_interval <= 0 || !beentry)
! return;
I'm wondering if it'll be enough for people not interested into
profiling not to bother. The patch contains a lot of added call sites.
I'm not sure if it's possible to arrange for not calling the function
at all when profiling is disabled (GUC profiling_interval = 0).
On the same vein:
+ static void
+ LockPageContent(volatile BufferDesc *buf, LWLockMode mode)
+ {
+ pgstat_push_condition(PGCOND_LWLOCK_PAGE);
+ LWLockAcquire(buf->content_lock, mode);
+ pgstat_pop_condition();
+ }
+
+ static void
+ LockPageIO(volatile BufferDesc *buf, LWLockMode mode)
+ {
+ pgstat_push_condition(PGCOND_LWLOCK_IO);
+ LWLockAcquire(buf->io_in_progress_lock, mode);
+ pgstat_pop_condition();
+ }
With the call site being (in src/backend/storage/buffer/bufmgr.c,
FlushRelationBuffers(Relation rel), FlushDatabaseBuffers(Oid dbid)):
! LWLockAcquire(bufHdr->content_lock, LW_SHARED);
...
! LockPageContent(bufHdr, LW_SHARED);
Maybe there's nothing to worry about, but I figured I'd better raise
the concert for further reviewing.
Oh, and this too:
*** LockBuffer(Buffer buffer, int mode)
*** 2372,2380
if (mode == BUFFER_LOCK_UNLOCK)
LWLockRelease(buf->content_lock);
else if (mode == BUFFER_LOCK_SHARE)
! LWLockAcquire(buf->content_lock, LW_SHARED);
else if (mode == BUFFER_LOCK_EXCLUSIVE)
! LWLockAcquire(buf->content_lock, LW_EXCLUSIVE);
Now the build went fine, but the testing (default configuration) not
so much:
dim=# create table series(i integer);
dim=# insert into series select generate_series(1, 1000);
LOG: checkpoints are occurring too frequently (4 seconds apart)
HINT: Consider increasing the configuration parameter
"checkpoint_segments".
WARNING: condition stack overflow: 10
...
WARNING: condition stack overflow: 11
WARNING: condition stack overflow: 11
WARNING: condition stack overflow: 11
ERROR: canceling statement due to user request
dim=# select count(*) from series;
count
---
0
(1 row)
Time: 9504.624 ms
dim=# select * from pg_profiles;
profid | profname | profnum
+-+-
83000 | LWLock:Page | 15
58000 | Data:Extend | 7
80018 | LWLock:BgWriterComm | 1
1 | CPU | 85
22000 | Network:Send| 128
80009 | LWLock:WALWrite | 6
55000 | Data:Read | 1
45000 | XLog:Write | 1
15000 | CPU:Utility | 5
15100 | CPU:Commit | 1
57000 | Data:Write | 15
42000 | XLog:Insert | 24
46000 | XLog:Flush | 4
(13 rows)
Time: 11.372 ms
The error I got is matching this part of the patch:
! void
! pgstat_push_condition(PgCondition condition)
! {
...
! if (condition_stack_top < MAX_COND_STACK)
! condition_stack[condition_stack_top] = prev;
! else
! elog(WARNING, "condition stack overflow: %d",
condition_stack_top);
So I'm going to change patch state to "Returned with Feedback" as I
guess we'll need to talk about the issue and find a way to solve it,
and I don't think this state prevent from getting back to the patch in
this same fest.
Regards,
--
dim
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
On Sat, Jul 18, 2009 at 1:19 PM, Josh Berkus wrote:
> b) Efficient constraint-based row-level security (as opposed to individual
> row labelling)[1]
[...]
> [1] For an explanation of the two ways to do row-level security, see here:
> http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732
> http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757
Yeah. That would be teh awesome (though admittedly I'm a sucker for a
cool feature), but it's quite beside the point of the original email
here, which is whether there's any point in continuing with the
current SE-Linux patches. I think there isn't.
Quite beyond the fact that we never seem to be able to get a patch
that implements a reasonable first set of features, the amount of work
that's going to be required to get these patches committable is going
to be enormous. Just to cite a few examples, here is the
documentation for the "sepostgresql_mctrans" documentation.
% Enables to turn on/off Mcstrans feature on SE-PostgreSQL. It is on
by default. Every users can set this
% parameter on their sessision without any limitations. SELinux
provides a feature to translate a part of security
% context between raw format and human-readable format, called
Mcstrans. If the parameter is turned on,
% SE-PostgreSQL translate the security context when it is exported to
or imported from users.
So, one problem is that this is written in poor English. While I'm
willing to do a certain amount of copy-editing as part of the review
process, SE-PostgreSQL is a massive feature and it's unfair to put the
entire burden of making the documentation readable on the shoulders of
the reviewers. I already proofread and corrected these docs once,
back in December, but now they need more reworking because they've
been extensively revised since then, and lots of non-idiomatic
constructs have crept back in. It's worse than that, though: the
above is not only bad English, it's also not a very good description
of the feature. I certainly can't tell from reading it what that GUC
actually does, and there are no references to anyplace where I can
turn for followup reading.
And then there's this, which is unduly RedHat-centric:
% Please note that SELinux requires installed files, directories and
others should be labeled properly. RPM
% installation do it implicitly.
And then look at this patch hunk:
pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode)
{
if (pg_database_aclmask(db_oid, roleid, mode, ACLMASK_ANY) != 0)
+ {
+ /* SELinux checks db_database:{connect} permission */
+ if ((mode & ACL_CONNECT) &&
+ !sepgsqlCheckDatabaseConnect(db_oid))
+ return ACLCHECK_NO_PRIV;
+
return ACLCHECK_OK;
+ }
else
return ACLCHECK_NO_PRIV;
}
I don't think I'm stepping too far out on a limb to say that this
looks like a very poor interface design. Surely we don't want a
separate sepgsqlCheck function for every
combination of an object type and a privilege, and shouldn't the check
be inside pg_database_aclmask anyway?
These are just a few examples from reading through the first bit of
the patch. I have no doubt that Kaigai is good at what he does, but I
don't think he understands what the community is looking for from this
patch in terms of features and coding style. I think the question is
not so much whether anyone is interested in the features (I know some
are) but whether anyone who understands what will be acceptable for
PostgreSQL is willling to do the necessary amount of work to get a
committable patch that implements them. I would be willing to work
with someone to fine-tune such a patch set, but the ratio of reviewer
effort to patch quality improvement on this patch set is well above
what I am prepared to put in.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-threaded pgbench
On Sat, Jul 18, 2009 at 8:25 PM, Robert Haas wrote: > On Thu, Jul 9, 2009 at 4:51 AM, Itagaki > Takahiro wrote: >> Here is an updated version of multi-threaded pgbench patch. > > Greg (Smith), do you have time to review this version? If not, I will > assign a round-robin reviewer when one becomes available. Incidentally you could assign me something if you want. I gave feedback on Simon/Your join removal and the Append min/max patch. I don't think either has really reached any conclusive "finished" state though. I suppose I should mark your patch as "returned with feedback" even if it's mostly just "good work, keep going"? And the other patch isn't actually in this commitfest but I think we're still discussing what it should do. -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-threaded pgbench
On Thu, Jul 9, 2009 at 4:51 AM, Itagaki Takahiro wrote: > Here is an updated version of multi-threaded pgbench patch. Greg (Smith), do you have time to review this version? If not, I will assign a round-robin reviewer when one becomes available. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MIN/MAX optimization for partitioned table
On Sat, Jul 18, 2009 at 6:35 PM, Alan Li wrote: > Yeah, are you running into the same issue as well? I tried to figure out a > way around the O(n^2) behavior, but there were no existing direct > relationship between the child subpath and its associated AppendRelInfo. I > think an AppenRelInfo dynahash would work and just have it hash by the > child_relid. I don't see any place in my patch where I had to do anything like this. I do have to loop through all the appendrel elements and skip over the ones which don't belong to this appendrel which seems weird to me but it isn't n^2. Generating a normal Append node you're generating a brand new path for each child and attaching them to the append path. It looks like you're allowing the append rel to generate paths and then digging around looking at those paths. I wonder if that's the right approach. >> The other thing is it would be nice if we could avoid making separate >> subplans for each child and instead make one for the whole structure >> including the aggregate. It would at the very least make the explain >> output prettier, but I think it would avoid repeated execution of the >> aggregate in some cases as well. > > How would this plan look? I think the repeated execution of the aggregate > comes from having to process the output of each child. So it's O(n) > executions where n is the number of children, assuming each child has the > appropriate index for this optimization. No I just mean instead of generating InitPlan 1 Limit Index Scan InitPlan 2 Limit Index Scan Aggregate Append Result Result I think it would be better to generate this: InitPlan 1 Aggregate Append Limit 1 IndexScan Limit 1 IndexScan Result The reason I think this is better is because if the subquery is executed multiple times it will just return the one precalculated result. In your plan it will have all the child minimums precalculated but will still have to re-execute the aggregate and append node. That's not terribly expensive but we might as well generate the simpler more efficient plan. > The other optimization that I thought was that the optimizer might use the > check constraints (in the range partitioning) on the column that I want to > do a MIN/MAX on. Then it'll only have to execute the subplan in one child > partition and the parent partition. But it was more of a wild idea on my > part, not sure if that's possible. Yes, I think this is the long-term goal. That's the whole "better representation of partitions" plotline. To do this efficiently the planner should know what the partition key is and what the partitioning structure is. In an ideal world we would be able to collapse the whole structure and eliminate the append relation entirely. To do that we need some way to indicate that the parent relation is provably empty. I had in mind to mark it as read-only and the statistics as authoritative since that seems more useful than just being able to mark it empty. I think there are a lot of other interesting things we could do with statistics if we knew they were authoritative for a read-only table. (The read-only property we would need here is very strong. There would have to be a vacuum freeze and moreover we would have to know that all tuples were successfully frozen.) -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Comments on automatic DML routing and explicit partitioning subcommands
On Tue, Jul 14, 2009 at 10:14 AM, Alvaro Herrera wrote: >> Thank you very much for the patch. Well done for getting to this stage. >> There is definitely much support for your work. > > Thanks for the thorough review. Please when you add a comment to the > commitfest app, make sure you specify the message-id where you posted > the review to pgsql-hackers on the comment so that there's a link to the > archives. I added a new comment with the message-id, but please edit > your comment and add it, at which point I'll remove mine. Thanks. Since Simon did not fix this, I have done so. Also, I have given you (Alvaro) admin privileges (as I have with other committers and commitfest management folks) so that you will be able to fix things like this yourself in the future. Finally, since it does not seem likely that this patch can be reworked in time for the current CommitFest, I have changed the status of the patch to "Returned with Feedback". ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER TABLE ... ALTER COLUMN ... SET DISTINCT
On Fri, Jul 17, 2009 at 11:10 AM, Dimitri Fontaine I
couldn't apply it to current head because of bitrot. It applies fine
to
> git commit bcaef8b5a0e2d5c143dabd8516090a09e39b27b8 [1] but
This is one of the things that I hate about the requirement to post
context diffs: filterdiff, at least for me, strips out the git tags
that indicate the base rev of the patch. In fact that rev is BEFORE
the one I based the patch on, which was
64b2da3f7778bc6fd3d213ceb9e73ff3b6e03890, and it actually applies OK
up until 0e3abe7ec78a3d51032d684598f188b0b0304fe9, the commit
immediately preceding the 8.4 pgindent run.
> Now I've had time to read the code, here are my raw notes:
>
> pg_dump.c:
> tbinfo->attstattarget[j] = atoi(PQgetvalue(res, j,
> i_attstattarget));
> + tbinfo->attdistinct[j] = strdup(PQgetvalue(res, j,
> i_attdistinct));
> ...
> + if (atof(tbinfo->attdistinct[j]) != 0 &&
> + !tbinfo->attisdropped[j])
>
> - style issue, convert at PQgetvalue() time
Ah, good catch.
> - prefer strtod() over atof? Here's what my local man page has to say about
> the case:
>
> The atof() function has been deprecated by strtod() and should not be
> used in
> new code.
Hmm, I didn't know that (my man page doesn't contain that language).
It looks like strtod() is more widely used in the existing source code
than atof(), so I'll change it (atof() is however used in the
floatVal() macro).
> tablecmds.c:
> + switch (nodeTag(newValue))
> + {
> + case T_Integer:
> ...
> + case T_Float:
> ...
> + default:
> + elog(ERROR, "unrecognized node type: %d",
> + (int) nodeTag(newValue));
>
> What about adding the following before the switch, to do like surrounding
> code?
> Assert(IsA(newValue, Integer) || IsA(newValue, Float));
Not a good plan. In my experience, gcc doesn't like switch ()
statements over enums that don't list all the values, unless they have
a default clause; it masterminds by giving you a warning that you've
"inadvertently" left out some values.
> Given your revised version I'll try to play around with ndistinct behavior
> and exercise dump and restore, etc, but for now I'll pause my work.
>
> I guess I'll have a second look at the code to check that it's all in the
> spirit of surrounding code, which I didn't complete yet (wanted to exercise
> my abilities to apply the patch from a past commit and forward-merge from
> there).
OK. My one concern about updating this is that Peter is currently
reviewing my patch to autogenerate headers & bki stuff. If that gets
committed it's going to break this all again, and I'd rather just fix
it once (especially since that patch would reduce the amount of fixing
needed here by 50%). Also if this gets applied after being fixed it
will break that one.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make check failure for 8.4.0
On Sat, July 18, 2009 1:35 pm, Kevin Grittner wrote: > > Out of curiosity, where is the make log to which you refer? > Just the output from make. e.g. make > make.log 2>&1 cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] make check failure for 8.4.0
"Kevin Grittner" writes: > Bingo! A few weeks back I had been experimenting with using the PGXS > compiles for our extensions, rather than expanding our tarballs in the > build tree and just doing make and sudo make install there. On the > failing machine, the session I used has USE_PGXS defined. Hah. I wonder whether it's possible for an in-tree build to intentionally undefine that? We could do something like override USE_PGXS := in contrib/Makefile but I'm not sure if that results in the variable being "undefined". regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MIN/MAX optimization for partitioned table
On Sat, Jul 18, 2009 at 3:13 AM, Greg Stark wrote:
> This part:
>
> ! /* only try to optimize children rel's */
> ! foreach (lc, root->append_rel_list)
> ! {
> ! AppendRelInfo *a = (AppendRelInfo *) lfirst(lc);
> !
> ! if (a->child_relid == childrel->relid &&
> ! a->parent_relid == parentrel->relid)
> ! {
> ! appinfo = a;
> ! break;
> ! }
> ! }
>
> Looks like O(n^2). I guess that's a bigger problem than with just this
> patch. Perhaps append_rel_list should be a dynahash in general. I
> never really understood why it was simpler to have a single global
> append_rel_list anyways.
>
Yeah, are you running into the same issue as well? I tried to figure out a
way around the O(n^2) behavior, but there were no existing direct
relationship between the child subpath and its associated AppendRelInfo. I
think an AppenRelInfo dynahash would work and just have it hash by the
child_relid.
>
> The other thing is it would be nice if we could avoid making separate
> subplans for each child and instead make one for the whole structure
> including the aggregate. It would at the very least make the explain
> output prettier, but I think it would avoid repeated execution of the
> aggregate in some cases as well.
How would this plan look? I think the repeated execution of the aggregate
comes from having to process the output of each child. So it's O(n)
executions where n is the number of children, assuming each child has the
appropriate index for this optimization.
The other optimization that I thought was that the optimizer might use the
check constraints (in the range partitioning) on the column that I want to
do a MIN/MAX on. Then it'll only have to execute the subplan in one child
partition and the parent partition. But it was more of a wild idea on my
part, not sure if that's possible.
>
>
> --
> greg
> http://mit.edu/~gsstark/resume.pdf
>
>
>
Re: [HACKERS] make check failure for 8.4.0
Andrew Dunstan wrote: > That's why I asked to see the make log. Maybe some environment > setting affected things? Bingo! A few weeks back I had been experimenting with using the PGXS compiles for our extensions, rather than expanding our tarballs in the build tree and just doing make and sudo make install there. On the failing machine, the session I used has USE_PGXS defined. I unset that and out of paranoia I did a make distclean and started over. This time the same steps worked fine. Out of curiosity, where is the make log to which you refer? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
On Sat, Jul 18, 2009 at 10:19:16AM -0700, Josh Berkus wrote: > David, > >> 2. Apart from Kohei-san and Stephen Frost, is anybody actually >> interested in having this feature at all? > > I'm interested in a version of the feature. Not, so far, one congruent to anything Kohei-san has actually sent, though. I'd say this one's a bust. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
David, 2. Apart from Kohei-san and Stephen Frost, is anybody actually interested in having this feature at all? I'm interested in a version of the feature. That is, I'm specifically interested in an SEPostgres which delivers: a) SELinux-label control (pluggable with TrustedSolaris and other frameworks) of the existing PostgreSQL privileges. b) Efficient constraint-based row-level security (as opposed to individual row labelling)[1] I also believe that an SEPostgres which delivered row masking and data substitution would be of interest to a significant number of new users, but that these features are complex and unintuitive enough that they should always be an optional module. Secondarily, I believe that having integrated SEPostgres support woudl bring us new users from the government security sector and the health care sector who do not currently use PostgreSQL. Whether any of these users would contribute substantially to maintaining it is an open question; they certainly have funding, though, and the NSA has contributed a substantial amount of resources to Linux, and the Japanese Security Agency has contributed to PostgreSQL before. [1] For an explanation of the two ways to do row-level security, see here: http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732 http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757 -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fmgroids.h not installed by "make install" in VPATH
Peter Eisentraut wrote: > On Wednesday 08 July 2009 02:09:20 Alvaro Herrera wrote: > > It seems our makefiles fail to install fmgroids.h by "make install" in a > > VPATH build. > > > I think the solution is to treat fmgroids.h just like pg_config_os.h, > > i.e. add a line like this: > > > > $(INSTALL_DATA) utils/fmgroids.h '$(DESTDIR)/$(includedir_server)/utils' > > The fix looks right. Has it been applied? Nope, and I'm away right now so feel free. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] navigation menu for documents
Andrew Dunstan wrote: Peter Eisentraut wrote: This looks very cool, but should probably be implemented via a stylesheet change instead of some Perl parsing some HTML. :-) I'm not sure if this actually addresses Andrew's original concern, though. No, it doesn't. David Wheeler's navigation (see upthread) that he uses for the Bricolage docs does, however. Ah, if you can change the overall layout then the world is your shellfish of choice. Would it be possible to include jquery? It's GPL/MIT dual-licence. -- Richard Huxton Archonet Ltd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
On Saturday 18 July 2009 12:31:34 Andres Freund wrote: > > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > > interested in having this feature at all? > > I would definitely be interested. > +1 Best regards, Jesper -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GEQO vs join order restrictions
Andres Freund writes: > This also explains why I saw nearly no improvement during the genetic search > itself. The paths out of random_init_pool were already hugely selected, so > there were not that many improvements to find and a change was relatively > like > to yield a impossible ordering. Yeah, I suspect most of the variants tried during that phase simply failed. > I do even less know how feasible this is, but given that joins in the right > hand side of a LEFT JOIN are not really useful to study from the outside in > the general case, would it be possible to "hide" them below the join during > join order planning? We could refrain from collapsing the sub-problem during joinlist formation. But the trouble with that is it creates a "hard" join order restriction. Most of the restrictions are "soft" to some extent, ie, you can do some rearrangements but not others. It might be worth looking at though; in the cases where there is no chance of a rearrangement, it would save cycles for either regular or GEQO planning. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SE-PostgreSQL?
On Saturday 18 July 2009 18:06:00 David Fetter wrote: > Folks, > > At this point, SE-PostgreSQL has taken up a *lot* of community > resources, not to mention an enormous and doubtless frustrating amount > of Kohei-san's time and effort, thus far without a single committed > patch, or even a consensus as to what it should (or could) do. > > Rather than continuing to blunder into the future, I think we need to > do a reality check in the form of a couple of questions: > > 1. Among the committers who could maintain the features, whatever > they turn out to be, who is actually volunteering to do so? > 2. Apart from Kohei-san and Stephen Frost, is anybody actually > interested in having this feature at all? I would definitely be interested. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GEQO vs join order restrictions
On Saturday 18 July 2009 17:48:14 Tom Lane wrote: > I spent a bit of time looking into why GEQO behaves so miserably on the > test case Andres Freund presented here: > http://archives.postgresql.org/message-id/200907091700.43411.and...@anaraze >l.de > > The difficulty seems to be that the problem query is full of join order > restrictions; in particular lots of joins inside the right side of a > LEFT JOIN. So those sub-joins have to occur before their relations > can be joined to anything else. When GEQO generates a random join > sequence, it is very likely indeed that such pairs of relations will > not be adjacent in the raw join sequence. There is some code in > gimme_tree() (the "stack" business I added here: > http://archives.postgresql.org/pgsql-committers/2004-01/msg00186.php > ) that was meant to try to deal with that, but I now realize that it > entirely fails to do so. Given two relations that have to be joined > to each other first, if they are not already adjacent in the input > then they just create two separate stack entries and the algorithm > never tries to join them to each other. So the failure rate in the > presence of join order restrictions is very high, and it gets rapidly > worse as the join problem size increases. This explains Andres' > observation of random_init_pool() reporting complete failure at high > collapse_limit settings. We can't really expect a random search process > to be efficient at discovering that two out of a hundred items must be > adjacent --- especially not if the problem has multiple restrictions > like that and the only feedback it gets is overall success or failure. Yes, this sound sensible and follows the same line of thought I started to have after you suggested the problem is in random_init_pool. This also explains why I saw nearly no improvement during the genetic search itself. The paths out of random_init_pool were already hugely selected, so there were not that many improvements to find and a change was relatively like to yield a impossible ordering. > I'm inclined to address this by rewriting gimme_tree so that it *always* > finds a valid join order based on the given tour. This would involve > searching the whole stack for a possible join partner instead of > considering only the topmost stack entry. It sounds inefficient, but > it's not nearly as bad as wasting the entire cycle by failing near > the end, which is what happens now. I guess this should be relatively easy to implement and test? > A different line of thought is to add some knowledge about join order > restrictions to tour generation, such that the code never generates an > invalid join order to start with. Unfortunately it's not at all clear > how to do that. I haven't really thought much about that yet, but wouldn't it be possible to iteratively check the current path during tour generation for every relation added? If the next relation yields a impossible ordering, choose another relation as next, if no more left, restart? I do even less know how feasible this is, but given that joins in the right hand side of a LEFT JOIN are not really useful to study from the outside in the general case, would it be possible to "hide" them below the join during join order planning? If yes, that should be applicable for the normal planner as well. I do realize that this is nothing one could fastly cook up and that it would require changes in lots of places, but as a general idea... Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SE-PostgreSQL?
Folks, At this point, SE-PostgreSQL has taken up a *lot* of community resources, not to mention an enormous and doubtless frustrating amount of Kohei-san's time and effort, thus far without a single committed patch, or even a consensus as to what it should (or could) do. Rather than continuing to blunder into the future, I think we need to do a reality check in the form of a couple of questions: 1. Among the committers who could maintain the features, whatever they turn out to be, who is actually volunteering to do so? 2. Apart from Kohei-san and Stephen Frost, is anybody actually interested in having this feature at all? I would submit that if we get fewer than three enthusiastic, "me!"s on the first, or fewer people than five on the second, we just need to bounce this feature and move on. As I see it, those numbers are a bare minimum, although one could fairly argue that I've underestimated the minimum for the second. What say? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] pg_migrator not setting values of sequences?
Alvaro Herrera wrote: > Bruce Momjian wrote: > > > Something is certainly wrong. Did we change sequence table format from > > 8.3 to 8.4? > > 8.3 does not have start_value. Looking at an invalidly-migrated sequence's columns: regression=> \d serialtest_f2_foo Sequence "public.serialtest_f2_foo" Column | Type |Value ---+-+- sequence_name | name| serialtest_f2_foo last_value| bigint | 3 start_value | bigint | 1 increment_by | bigint | 9223372036854775807 max_value | bigint | 1 min_value | bigint | 1 cache_value | bigint | 0 log_cnt | bigint | 25387551686912 is_cycled | boolean | f is_called | boolean | Should pg_migrator just pull the misaligned values and do an ALTER SEQUENCE/seval() to fix it, or create a script to do that? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] GEQO vs join order restrictions
I spent a bit of time looking into why GEQO behaves so miserably on the test case Andres Freund presented here: http://archives.postgresql.org/message-id/200907091700.43411.and...@anarazel.de The difficulty seems to be that the problem query is full of join order restrictions; in particular lots of joins inside the right side of a LEFT JOIN. So those sub-joins have to occur before their relations can be joined to anything else. When GEQO generates a random join sequence, it is very likely indeed that such pairs of relations will not be adjacent in the raw join sequence. There is some code in gimme_tree() (the "stack" business I added here: http://archives.postgresql.org/pgsql-committers/2004-01/msg00186.php ) that was meant to try to deal with that, but I now realize that it entirely fails to do so. Given two relations that have to be joined to each other first, if they are not already adjacent in the input then they just create two separate stack entries and the algorithm never tries to join them to each other. So the failure rate in the presence of join order restrictions is very high, and it gets rapidly worse as the join problem size increases. This explains Andres' observation of random_init_pool() reporting complete failure at high collapse_limit settings. We can't really expect a random search process to be efficient at discovering that two out of a hundred items must be adjacent --- especially not if the problem has multiple restrictions like that and the only feedback it gets is overall success or failure. I'm inclined to address this by rewriting gimme_tree so that it *always* finds a valid join order based on the given tour. This would involve searching the whole stack for a possible join partner instead of considering only the topmost stack entry. It sounds inefficient, but it's not nearly as bad as wasting the entire cycle by failing near the end, which is what happens now. A different line of thought is to add some knowledge about join order restrictions to tour generation, such that the code never generates an invalid join order to start with. Unfortunately it's not at all clear how to do that. Ideas, comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] navigation menu for documents
Peter Eisentraut wrote: This looks very cool, but should probably be implemented via a stylesheet change instead of some Perl parsing some HTML. :-) I'm not sure if this actually addresses Andrew's original concern, though. No, it doesn't. David Wheeler's navigation (see upthread) that he uses for the Bricolage docs does, however. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] navigation menu for documents
On Friday 17 July 2009 15:58:27 Richard Huxton wrote: > 1. Fixed navigation > Copy STYLING/stylesheet.css over the existing one and you will have > static navigation links top and bottom of the page. Looks good. > 2. Titles on navigation links. > Run ./STYLING/title_links.pl and it should add title attributes to the > navigation links. This means hovering over the top links gives the title > of the page they will go to. Presumably we could do this directly from > the sgml source, and I think it's probably worthwhile. Yes, the DSSSL stylesheet could do that. > 3. Javascript popup menu. > This uses jquery, but that's just for convenience during discussion. You > could rework this without it. > Copy STYLING/*.js and STYLING/menu.inc to the docs dir and then run > ./STYLING/include_javascript.pl to include the popup script. > The central "chapter heading" section of the top navigation area should > now be a link that toggles the menu on/off. > The menu could be as simple/complex as you like - this is just what I > hacked together by parsing the TOC on index.html This looks very cool, but should probably be implemented via a stylesheet change instead of some Perl parsing some HTML. :-) I'm not sure if this actually addresses Andrew's original concern, though. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] SE-PgSQL/tiny rev.2193
On Sat, Jul 18, 2009 at 7:10 AM, Martijn van Oosterhout wrote: > On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote: >> > I'm starting to think that there's just no hope of this matching up >> > well enough with the way PostgreSQL already works to have a chance of >> > being accepted. >> >> What I'm understanding here is the apparent requirement that the SEPostgreSQL >> implementation be done in a way that a generic SELinux policy that has been >> written for an operating system and file system can be applied to PostgreSQL >> without change and do something useful. I can see merits for or against >> that. >> But in any case, this needs to be clarified, if I understand this requirement >> correctly anyway. > > Indeed. My impression was also that there are existing SELinux rules > and permission concepts already in use and people would like to apply > them to PostgreSQL, which puts the translation layer inside postgres. > However, from the postgres side there appears to be a push to make > unique postgresql SELinux rules and requiring every user of SELinux to > do the mapping of rights to postgresql specific terms. I think this is only semi-accurate. My impression is that a supposedly generic policy has been written for database objects and merged into model SE-Linux policy, but I think that was done largely in the hops of implementing SE-PostgreSQL. It would be one think if KaiGai showed up and said, see, there are three other databases that do this, now we want you to do it too, that would be one thing. But I don't think that's the case: I believe that we are the first, which makes the argument that we have to conform to the "standard" ways of doing this a lot weaker. > Specifically, creating SELinux permissions for CREATE LANGUAGE seems > particularly useless since that's not a data protection issue. The same > with aggregates, operator classes, etc. ISTM the goal of SELinux is not > primarily to restrict DDL but mostly to protect the data. I'd actually be willing to buy that. If KaiGai wants to take the list of objects for which SE-PostgreSQL supports grantable permissions and the list of objects for which $COMPETITOR supports permissions and implement only the intersection of the two, I think that would be reasonable. What I don't think is reasonable is trying to implement, in the first version of the patch, 50 types of permissions that we never had before, or on the other hand such a trivial percentage of what we already do that it's totally useless. > Personally I find the idea that SELinux permissions need to meet parity with > the existing permission structure > crazy, since it's not going to replace the existing permissions, just > supplement them. Maybe it is crazy, but here are my concerns... 1. If the permissions work in a way that is very different than the existing permissions, then they need lots and lots of hooks all over the code. And nobody except KaiGai understands why those hooks are where they are instead of somewhere else. That's going to make it difficult to maintain. 2. If we add a lot of new kinds of permission checks that PostgreSQL has never done before, using a framework that none of the committers understand, how do we verify that they are well-designed (correct, secure, etc)? I am fairly well convinced that the design for row-level security was a really bad design. Whether I'm right or not, I think something like that is far too large a feature to add "by the wayside" in the context of another patch. 3. As far as I can tell, there is a lot of resistance from the committers who have looked at this patch (Tom, Peter, and maybe Bruce, though I think his review was not quite so unfavorable) to the idea of committing it at all. I don't think they're going to be willing to work extra-hard to implement security features that are only going to be useful to people using SE-Linux. For what it's worth, this problem is not confined to the committers: SE-PostgreSQL is the only patch that I had people specifically tell me they didn't want to review because they just didn't care about it. Frankly, I don't care about it much either: ordinarily, the first and last thing I do with SE-Linux is shut it off. What is making me care even less is the fact that after many revisions we still don't have anything that can be reviewed with any seriousness. The initial versions had so many extra bells and whistles (row-level security, complex DDL permissions) that they went way beyond basic SE-Linux support, and now we have a version that is stripped down to the point where it does barely anything. I feel like I'm spinning my wheels on a patch that nobody in the PostgreSQL community really wants anyway. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Split-up ECPG patches
On Wed, Jul 15, 2009 at 07:17:17PM +0200, Böszörményi Zoltán wrote: > as asked by Michael Meskes, I have split up our ECPG patchset: Just a couple quick comments. It appears to me (without much testing) that the patches still partly rely on each other. But I cannot see a reason for this. > 1. dynamic cursorname (DECLARE :cursorname ..., etc) > 2. SQLDA support in Informix compat mode (C structure used for > descriptor and data query) One file has this: * (C) 2009 Cybertec GmbH * Zolt??n B??sz??rm??nyi * Hans-J??rgen Sch??nig Shouldn't this also list a license? In general I wonder whether we need some statement for every patch submitted? Anyone more into licensing might comment here. > 3. DESCRIBE OUTPUT support for named and sqlda descriptors I don't think we have to add ECPGdescribe2 to keep the old API. The old ECPGdescribe function does nothing, so it's not worth being kept. > 4. "string" pseudo-type in Informix compat mode There is still a lot of stuff being done when not in compatibility mode. I thought you wanted to change that? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ECPG support for struct in INTO list
On Fri, Jul 17, 2009 at 03:58:21PM +0200, Boszormenyi Zoltan wrote: > Attached is the short example I can reproduce with. > The version I used was final PostgreSQL 8.4.0, without our > extensions posted already. I added an indication to ecpg_type_name(): > > [z...@db00 ecpg-test]$ ecpg -C INFORMIX test28.pgc > ... > But you are right about the "supposed to work" part, > if I modify it the way below, it works: Thanks for spotting this. It appears to be a bug in Informix compatibility mode. Without -C INFORMIX it works nicely. Thus I guess we have to find and fix the bug. Your patch probably/hopefully is not needed. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org ICQ: 179140304, AIM/Yahoo/Skype: michaelmeskes, Jabber: mes...@jabber.org Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] DefaultACLs
Joshua Tolley wrote: First, as you've already pointed out, this needs documentation. /me looks at Stephen ;) Once I added the missing semicolon mentioned earlier, it applies and builds As we discussed outside of list, my compiler didn't picked that one because I didn't use --enable-cassert . fine. The regression tests, however, seem to assume that they'll be run as the postgres user, and the privileges test failed. Oh I thought they are always run under *database user* postgres, my bad, I reworked them and the are probably better as a result. Very minor stylistic or comment issues: * There's a stray newline added in pg_class.h (no other changes were made to that file by this patch) Fixed, probably I either pressed enter by mistake while viewing that file or it was some merging problem when updating my working copy. * It feels to me like the comment "Continue with standard grant" in aclchk.c interrupts the flow of the code, though such a comment was likely useful when the patch was being written. Ok I removed that comment. * pg_namespace_default_acl.h:71 should read "objects stored *in* pg_class" Fixed. * The comment at the beginning of InsertPgClassTuple() in catalog/heap.c should probably be updated to say that relation's ACLs aren't always NULL by default Done. * copy_from in gram.y got changed to to_from, but to_from isn't ever used in the default ACL grammar. I wondered if this was changed so you could use the same TO/FROM code as COPY uses, and then you decided to hardcode TO and FROM? Yes, that's more or less what happened. In my perusal of the patch, I didn't see any code that screamed at me as though it were a bad idea; quite likely there weren't any really bad ideas but I can't say with confidence I'd have spotted them if there were. The addition of both the NSPDEFACLOBJ_* defines alongside the NSP_ACL_OBJ_* defines kinda made me think there were too many sets of constants that had to be kept in sync, but I'm not sure that's much of an issue in reality, given how unlikely it is that schema object types to which default ACLs should apply are likely to be added or removed. Well the problem you see is not really a problem IMHO because most of code I've seen does not use actual catalog values inside gram.y/parser so I didn't use them either. But there is one problem there that also affects GRANT ON ALL patch and that was discussed previously (see http://archives.postgresql.org/pgsql-hackers/2009-07/msg00957.php and the rest of the thread from there). One (or both) of those patches will have to be adjusted but only commiter can decide that IMHO (I am happy to make any necessary changes but I really don't know which of the 3 solutions is better). I don't know how patches that require catalog version changes are generally handled; should the patch include that change? Than can reasonably be done only at commit time so it's up to commiter. I attached updated version of the patch per your comments. Let's hope I didn't introduce new problems :) Thanks for your time. -- Regards Petr Jelinek (PJMODOS) defaultacls.diff.gz Description: Unix tar archive -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] SE-PgSQL/tiny rev.2193
On Fri, Jul 17, 2009 at 03:59:29PM +0300, Peter Eisentraut wrote: > > I'm starting to think that there's just no hope of this matching up > > well enough with the way PostgreSQL already works to have a chance of > > being accepted. > > What I'm understanding here is the apparent requirement that the SEPostgreSQL > implementation be done in a way that a generic SELinux policy that has been > written for an operating system and file system can be applied to PostgreSQL > without change and do something useful. I can see merits for or against > that. > But in any case, this needs to be clarified, if I understand this requirement > correctly anyway. Indeed. My impression was also that there are existing SELinux rules and permission concepts already in use and people would like to apply them to PostgreSQL, which puts the translation layer inside postgres. However, from the postgres side there appears to be a push to make unique postgresql SELinux rules and requiring every user of SELinux to do the mapping of rights to postgresql specific terms. Specifically, creating SELinux permissions for CREATE LANGUAGE seems particularly useless since that's not a data protection issue. The same with aggregates, operator classes, etc. ISTM the goal of SELinux is not primarily to restrict DDL but mostly to protect the data. Personally I find the idea that SELinux permissions need to meet parity with the existing permission structure crazy, since it's not going to replace the existing permissions, just supplement them. Merely my opinion ofcourse, which doesn't count for much :) Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] WIP patch for TODO Item: Add prompt escape to display the client and server versions
Hi, Le 17 juil. 09 à 23:24, Tom Lane a écrit : It seems unlikely that the DB version number would be worth the prompt space. In situations like that you'd much more likely need identifying info like the DB hostname and port number. At work we have a fair number of database servers, some 8.2, lots of 8.3, and still some 8.1 I think. Triple the count for dev and preprod environments. Of course as the DBA I tend not to forget easily which server runs which version, and anyway when unsure I read the psql banner. But developers too will connect to those servers, and in the same project they can handle both 8.2 and 8.3 databases. And they already struggle to think about connecting with the right psql client version. I'm often called for a "\d is broken, see?". All of this to say "from the field" how much I think this could help us to have a rich prompt with database name and major version. Oh and they could refer to the right documentation, too, before asking me about why it doesn't work as intended... Regards, -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MIN/MAX optimization for partitioned table
This part:
! /* only try to optimize children rel's */
! foreach (lc, root->append_rel_list)
! {
! AppendRelInfo *a = (AppendRelInfo *) lfirst(lc);
!
! if (a->child_relid == childrel->relid &&
! a->parent_relid == parentrel->relid)
! {
! appinfo = a;
! break;
! }
! }
Looks like O(n^2). I guess that's a bigger problem than with just this
patch. Perhaps append_rel_list should be a dynahash in general. I
never really understood why it was simpler to have a single global
append_rel_list anyways.
The other thing is it would be nice if we could avoid making separate
subplans for each child and instead make one for the whole structure
including the aggregate. It would at the very least make the explain
output prettier, but I think it would avoid repeated execution of the
aggregate in some cases as well.
--
greg
http://mit.edu/~gsstark/resume.pdf
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Index AM API changes for deferability
2009/7/15 Tom Lane : > There is no reason at all to avoid an index AM API change if one is > useful. Thinking about this a bit more, perhaps it would be better if I added an out parameter to the AM for the uniqueness result, rather than overloading the return value, which is quite ugly: bool index_insert(Relation indexRelation, Datum *values, bool *isnull, ItemPointer heap_t_ctid, Relation heapRelation, IndexUniqueCheck uniqueness_check, bool *is_unique); This would allow me to tidy up some of the code I added to ExecInsertIndexTuples() which is a bit of a kludge to support the hash indexes enforcing uniqueness in the future: http://archives.postgresql.org/pgsql-hackers/2009-07/msg00812.php Also I could then move the ereport() for unique key violations from _bt_check_unique() into index_insert() which would allow the Duplicate key value error patch to be non-btree-specific: http://archives.postgresql.org/message-id/20090403161658.afb2.52131...@oss.ntt.co.jp http://archives.postgresql.org/message-id/24655.1238885...@sss.pgh.pa.us Thoughts? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] mixed, named notation support
2009/7/18 Bernd Helmle : > --On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule > wrote: > >> Hello >> >> I did some cleaning on this feature, and I hope so I solve some Tom's >> objections > > Attached is a cleaned up version of the patch, the one linked from the > commit fest has some hunks failing to be applied to current CVS. This is > mainly for reporting and backup purposes, i'm proceeding looking into the > code deeper now. The patch passes make check and functionality check looks > fine. > > Pavel, should I adjust the docs already (you've asked for volunteers...)? If > yes, have you something specific in mind? sure, please. Doc needs mainly language correction - my English is bad. And maybe some others - from me is too short and too technical. Pavel > > -- > Thanks > > Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] mixed, named notation support
--On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule
wrote:
Hello
I did some cleaning on this feature, and I hope so I solve some Tom's
objections
Attached is a cleaned up version of the patch, the one linked from the
commit fest has some hunks failing to be applied to current CVS. This is
mainly for reporting and backup purposes, i'm proceeding looking into the
code deeper now. The patch passes make check and functionality check looks
fine.
Pavel, should I adjust the docs already (you've asked for volunteers...)?
If yes, have you something specific in mind?
--
Thanks
Bernddiff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 3c8ce43..c6ece84 100644
*** a/doc/src/sgml/xfunc.sgml
--- b/doc/src/sgml/xfunc.sgml
*** CREATE FUNCTION test(int, int) RETURNS i
*** 1101,1106
--- 1101,1222
+
+Positional, Mixed and Named notation
+
+
+ notation
+ functions
+
+
+
+ Functions with named parameters should by called with mixed
+ or named notation. Any typical use expects posiotional
+ notation, when an order of parameters is important. For named notation
+ is important name of parameters. You can mix both notation - result is mixed notation.
+ Some first n parameters should be entered in positional parameters
+ and others in named notation. You don't need pass parameters than has
+ default value.
+
+ CREATE FUNCTION fx(a int, b int, m int = 1, o int = 0) RETURNS int AS $$
+ SELECT ($1 + $2) * $3 + $4
+ $$ LANGUAGE SQL;
+
+Function fx has obligatory parameters: a and
+b and optional parameters: m and o.
+This function should be called with positional parameters. See for a more detailed explanation of calling
+ function with default values.
+
+
+
+ Using positional notation
+
+
+ function
+ positional notation
+
+
+
+
+ SELECT fx(10,20);
+ fx
+
+ 30
+ (1 row)
+
+ SELECT fx(10,20,2,50);
+ fx
+ -
+ 110
+ (1 row)
+
+
+
+
+
+ Using named notation
+
+
+ function
+ named notation
+
+
+
+
+ SELECT fx(10 as a, 20 as b);
+ fx
+
+ 30
+ (1 row)
+
+ SELECT fx(20 as b, 10 as a);
+ fx
+
+ 30
+ (1 row)
+
+ SELECT fx(20 as b, 10 as a, 50 as o);
+ fx
+
+ 80
+ (1 row)
+
+
+
+
+
+ Using mixed notation
+
+
+ function
+ mixed notation
+
+
+
+
+ SELECT fx(10,20, 50 as o);
+ fx
+
+ 80
+ (1 row)
+
+ SELECT fx(10,20, 10 as m);
+ fx
+ -
+ 300
+ (1 row)
+
+ SELECT fx(10,20, 50 as o, 2 as m);
+ fx
+ -
+ 110
+ (1 row)
+
+
+
+
+
Function Volatility Categories
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 2b0cb35..714d337 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
***
*** 36,41
--- 36,42
#include "catalog/pg_ts_template.h"
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
+ #include "funcapi.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "parser/parse_func.h"
*** TypeIsVisible(Oid typid)
*** 603,610
* The caller might end up discarding such an entry anyway, but if it selects
* such an entry it should react as though the call were ambiguous.
*/
FuncCandidateList
! FuncnameGetCandidates(List *names, int nargs,
bool expand_variadic, bool expand_defaults)
{
FuncCandidateList resultList = NULL;
--- 604,723
* The caller might end up discarding such an entry anyway, but if it selects
* such an entry it should react as though the call were ambiguous.
*/
+ static bool
+ VerifyCandidateNamedNotation(HeapTuple proctup, int pronargs, int nargs, List *argnames, short int pronargdefaults,
+ bool *use_defaults, short int **param_map)
+ {
+ Datum proargnames;
+ Oid *p_argtypes;
+ char **p_argnames;
+ char *p_argmodes;
+ bool isnull;
+ int pronallargs;
+ int i;
+ int pp; /* proargs position */
+ int ap; /* args position */
+ ListCell *lc;
+ FuncCallNotation used_notation = POSITIONAL_NOTATION; /* used only for asertion */
+
+ #define UNDEFINED_PARAMETER -1
+
+ Assert(argnames != NIL);
+
+ /* Ignore if not enough default expressions */
+ if (nargs + pronargdefaults < pronargs)
+ return false;
+
+ /* check proargnames */
+ proargnames = SysCacheGetAttr(PROCOID, proctup,
+ Anum_pg_proc_proargnames,
+ &isnull);
+ if (isnull)
+ return false;
+
+ pronallargs = get_func_arg_info(proctup, &p_argtypes, &p_argnames, &p_argmodes);
+ Assert(p_argnames != NULL);
+
+ /*
+ * An number less or equal nargs means some arg,
+ * an number greather than nargs means some default.
+ */
+ *param_map = palloc(pronargs * sizeof(short int));
+ for (i = 0; i < pronargs; i++)
+ (*param_map)[i] = UNDEFINED_PARAMETER;
+
+ a
