Re: [HACKERS] Updated posix fadvise patch v19
On Fri, 14 Nov 2008, Gregory Stark wrote: In particular I was hoping Zoltan who original reported the sequential file strategy stuff being helpful might be able to see if this works for him. The original message there suggested it was particularly valuable when working with a somewhat broken kernel that was handling multiple simultaenous reads badly. I think the idea here is that if you've got an OS that handles that situation well, then the fadvise calls don't help much. But having them in there does improve the odds that the kernel will do the right thing with the sequential reads. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
Tom Lane <[EMAIL PROTECTED]> writes: > "Robert Haas" <[EMAIL PROTECTED]> writes: >> What's particularly unfortunate is Greg's comment that this would work >> if the planner chose an index scan. Had I defined an index on the >> columns in question, my code might have worked and been deployed to >> production - and then broken if the planner changed its mind and >> decided to use a seqscan after all. > >> ISTM any cursor that's going to be updated should specify WHERE UPDATE >> in the query, but maybe that's not a hard requirement as of now. > > Well, it's contrary to SQL spec, which says that sufficiently simple > cursors should be updatable even if they don't say FOR UPDATE. > > However ... the more I think about it, the more I wonder if we shouldn't > rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF > *only* for cursors that say FOR UPDATE. It is tempting since it would make application code which looped updating WHERE CURRENT OF semantically equivalent to UPDATE FROM. It seems better to have one set of functionality rather than two similar but subtly different sets of functionality available depending on the coding style. > Aside from the above issue, there's an already known and documented risk if > you omit FOR UPDATE, which is that your WHERE CURRENT OF update silently > becomes a no-op if someone else has already updated the target row since > your query started. It seems like not using FOR UPDATE is sufficiently > dangerous practice that requiring it wouldn't be doing our users a > disservice. Could we implicitly add FOR UPDATE when planning and executing a cursor of a sufficiently simple query? How simple does the spec say the query needs to be? > There is one thing we lack in order to go that far, though: the current > implementation of WHERE CURRENT OF can cope with inheritance queries, How would this implementation relate to the issues described in inheritance_planner (which always seemed strange): * inheritance_planner *Generate a plan in the case where the result relation is an *inheritance set. * * We have to handle this case differently from cases where a source relation * is an inheritance set. Source inheritance is expanded at the bottom of the * plan tree (see allpaths.c), but target inheritance has to be expanded at * the top. The reason is that for UPDATE, each target relation needs a * different targetlist matching its own column set. Also, for both UPDATE * and DELETE, the executor needs the Append plan node at the top, else it * can't keep track of which table is the current target table. Fortunately, * the UPDATE/DELETE target can never be the nullable side of an outer join, * so it's OK to generate the plan this way. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- 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] libpq-events windows gotcha
Tom Lane wrote: Andrew Chernow <[EMAIL PROTECTED]> writes: On the whole I vote for #4 out of these. I attached a patch for the docs. Its documented as a NOTE to the PGEventProc. Applied, but I editorialized on the wording a bit. Let me know if you think this is wrong ... It's correct, the wording is clearer now. I wasn't aware there was a caution tag, which is better than using . -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] contrib/pg_stat_statements
Tom Lane <[EMAIL PROTECTED]> wrote: > I'm trying to figure out what is the status of this patch? I'm not sure > if there's any point in applying it, when contrib/pg_stat_statements > hasn't been updated to make use of it. Here is an updated patch to use QueryDesc.queryText and supports nested statements. The patch needs to be applied after contrib/auto_explain patch. Now we can choose statistics.track_statements from 'none', 'top' and 'all'. Nested statements are collected in the case of 'all'. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center pg_stat_statements.1115.tar.gz Description: Binary data -- 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] Synchronous replication patch v2
> This is the whole patch of Synch Rep against head. This also contain > "Infrastructure changes for recover" patch by Simon because I'd like to > make walreceiver work in consistent recovery mode. Given that both this patch and Simon's hot standby patches need this infrastructure, it seems like it would be awfully good if we could get that piece into final form and committed. Tom Lane and Heikki both reviewed this for the September commitfest and even into October, but then, at least as I understand it, the reviewing train kind of ran out of steam. I'm not sure what the best way forward is. Do we need to assign a new reviewer, or do either of Tom and Heikki feel like picking it up again? ...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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
> However ... the more I think about it, the more I wonder if we shouldn't > rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF > *only* for cursors that say FOR UPDATE. Aside from the above issue, > there's an already known and documented risk if you omit FOR UPDATE, > which is that your WHERE CURRENT OF update silently becomes a no-op > if someone else has already updated the target row since your query > started. It seems like not using FOR UPDATE is sufficiently dangerous > practice that requiring it wouldn't be doing our users a disservice. Yes, I was reading that documentation today and scratching my head. It didn't quite dawn on me that the solution was to just always use FOR UPDATE, but certainly if it is then you may as well enforce it. I've encountered that no-op behavior in other situations in the past, specifically BEFORE triggers, and it's really weird and confusing, because you typically have to be doing something fairly complicated before the problem case arises, and then it mysteriously fails to work. ...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] auto_explain contrib moudle
Tom Lane <[EMAIL PROTECTED]> wrote: > Now that I look closer, the "contrib infrastructure" item is just a > combination of the auto_explain and pg_stat_statements items, and I > guess the reason you and Matthew were shown as reviewers was that > you'd each been assigned one of those two items. As far as I can see > this is just confusing and duplicative. That's right. Sorry for your confusing. > I think we can > proceed with the two other items separately. If there's any conflict > in the two patches we can resolve it after the first one gets applied. contrib/auto_explain patch is "Ready for committer" even without the QueryDesc patch. So please apply it if there are no problems. I'll rewrite contrib/pg_stat_statements to use the new feature in the QueryDesc patch and avoid confliction from the first one. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
> Here's a draft patch (no docs, no regression test) for that. It doesn't > look as ugly as I expected. Comments? I'm hesitant to call this a bug > fix, and we are past feature freeze ... Considering the number of and invasiveness of the patches remaining in the queue, I'm inclined to consider this a drop in the bucket, but then I'm biased since I just got bitten by it. ...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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
"Robert Haas" <[EMAIL PROTECTED]> writes: > What's particularly unfortunate is Greg's comment that this would work > if the planner chose an index scan. Had I defined an index on the > columns in question, my code might have worked and been deployed to > production - and then broken if the planner changed its mind and > decided to use a seqscan after all. > ISTM any cursor that's going to be updated should specify WHERE UPDATE > in the query, but maybe that's not a hard requirement as of now. Well, it's contrary to SQL spec, which says that sufficiently simple cursors should be updatable even if they don't say FOR UPDATE. However ... the more I think about it, the more I wonder if we shouldn't rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF *only* for cursors that say FOR UPDATE. Aside from the above issue, there's an already known and documented risk if you omit FOR UPDATE, which is that your WHERE CURRENT OF update silently becomes a no-op if someone else has already updated the target row since your query started. It seems like not using FOR UPDATE is sufficiently dangerous practice that requiring it wouldn't be doing our users a disservice. There is one thing we lack in order to go that far, though: the current implementation of WHERE CURRENT OF can cope with inheritance queries, that is you can say DECLARE c CURSOR ... FROM parent_table ... UPDATE parent_table ... WHERE CURRENT OF c and the right things will happen even if the cursor is currently showing a row from some child table. SELECT FOR UPDATE does not presently support inheritance, so we'd really have to fix that in order to not have a loss of functionality. This is something that was on my private to-do list for 8.4 but I hadn't thought of an easy way to do it. But, revisiting the issue just now, I have an idea: when a FOR UPDATE target is an inheritance tree, make the plan return not only ctid as a junk column, but also tableoid. The executor top level could easily use that to determine which table to actually try to do heap_lock_tuple in. I haven't looked at the planner code for this lately, but I'm thinking it is probably less than a day's work to make it happen. 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] Windowing Function Patch Review -> Performance Comparison.
Hitoshi Harada wrote: > > david=# explain select date,lag(date,1) over (order by date) from > > meter_Readings order by date; > > QUERY PLAN > > > > > > > Sort (cost=1038.73..1063.74 rows=10001 width=4) > > Sort Key: date > > -> Window (cost=0.00..374.27 rows=10001 width=4) > > -> Index Scan using meter_readings_pkey on meter_readings > > (cost=0.00..299.27 rows=10001 width=4) > > (4 rows) > > > > Is the final sort still required? Is it not already sorted in the > window? > > > > Oh, I forgot to mention about it. This behavior is also fixed and it > works without sort on the window now. I don't remember at all why I > did so and there's no comment around that but regression tests showed > there is no preblem without it. > > Regards, > > -- > Hitoshi Harada Fantastic news! That will speed up the few test queries I have quite a bit the sort was splitting to disk so performance was dropping quite a bit. This might be a good time to say that the hardware that I'm testing on is ultra slow. 600 Mhz Pentium III with only 28MB shared buffers. I've done some more performance tests with the patch. Realising that I really need to be comparing the performance with something else I decided to have a query process a large table with then without a windowing function just to see how much the query slows. Of course there is going to be an overhead using a tuple store. I just wanted to see how much. My results are not really very interesting, so I'll just include them in the bottom of the email for those who want to see. Casting my mind back to when the patch was always doing a sort before a window with an order by even when a btree index was there, it's really come a long way. I've little idea how difficult it would be to implement better planning for the following. I suppose if it's difficult then it's probably better to wait for the patch to be commited first, or maybe it's something for 8.5. SELECT department, SUM(Salary), ROW_NUMBER() OVER (ORDER BY department), SUM(SUM(salary)) OVER (ORDER BY department DESC) FROM employees GROUP BY department ORDER BY department; This query performs more sorts than really is needed. I suppose the most efficient way to process it would be to process the 2nd window first then the 1st before returning the results in the same order as the 1st window. Currently the plan looks like: QUERY PLAN - Sort (cost=1.33..1.34 rows=3 width=9) Sort Key: department -> Window (cost=1.25..1.31 rows=3 width=9) -> Sort (cost=1.25..1.26 rows=3 width=9) Sort Key: department -> Window (cost=1.17..1.23 rows=3 width=9) -> Sort (cost=1.17..1.18 rows=3 width=9) Sort Key: department -> HashAggregate (cost=1.10..1.15 rows=3 width=9) -> Seq Scan on employees (cost=0.00..1.06 rows=6 width=9) Maybe it's possible to sort the processing order of the windows based on the ORDER BY clauses putting any that match the ORDER BY of the final results last. I've not looked into this in much detail. Currently I cannot see any scenario where it would be bad. What do you think? David CREATE TABLE bigtable ( id SERIAL NOT NULL PRIMARY KEY, timestamp TIMESTAMP NOT NULL ); -- about 383MB of data INSERT INTO bigtable (timestamp) SELECT NOW() + (CAST(RANDOM() * 10 AS INT) || ' secs')::INTERVAL FROM generate_series(1,1000); CREATE INDEX bigtable_timestamp_idx ON bigtable (timestamp); VACUUM ANALYZE bigtable; -- base line test david=# SELECT COUNT(*) FROM (select id,timestamp from bigtable order by id) t; count -- 1000 (1 row) Time: 72862.238 ms -- lag test david=# SELECT COUNT(*) FROM (select id,LAG(timestamp,1) OVER (order by id) from bigtable order by id) t; count -- 1000 (1 row) Time: 257713.350 ms david=# SELECT COUNT(*) FROM (select id,NTILE(10) OVER (order by id) from bigtable order by id) t; count -- 1000 (1 row) Time: 183131.425 ms -- 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
I wrote:
> [ dept of second thoughts... ] Actually, given that he said FOR UPDATE,
> the plan should be propagating the tuple identity through to top level
> so that execMain can do its thing. So in principle we could probably
> get the information without widespread changes. This would fit
> reasonably well with the spec's requirements too --- any but trivial
> cursors are not deemed updatable unless you say FOR UPDATE. But it
> would mean two completely independent implementations within
> execCurrent.c...
Here's a draft patch (no docs, no regression test) for that. It doesn't
look as ugly as I expected. Comments? I'm hesitant to call this a bug
fix, and we are past feature freeze ...
regards, tom lane
Index: src/backend/executor/execCurrent.c
===
RCS file: /cvsroot/pgsql/src/backend/executor/execCurrent.c,v
retrieving revision 1.7
diff -c -r1.7 execCurrent.c
*** src/backend/executor/execCurrent.c 12 May 2008 00:00:48 - 1.7
--- src/backend/executor/execCurrent.c 15 Nov 2008 00:04:17 -
***
*** 46,51
--- 46,53
char *table_name;
Portal portal;
QueryDesc *queryDesc;
+ ExecRowMark *erm;
+ ListCell *lc;
ScanState *scanstate;
boollisnull;
Oid tuple_tableoid;
***
*** 86,91
--- 88,140
cursor_name)));
/*
+* If the query uses FOR UPDATE, look through the executor's state for
that
+* to see if we can identify the target row that way. We succeed if
there
+* is exactly one FOR UPDATE item for the requested table. (Note:
+* presently, FOR UPDATE is not allowed on inheritance trees, so there
is
+* no need to worry about whether a FOR UPDATE item is currently valid.)
+*/
+ erm = NULL;
+ foreach(lc, queryDesc->estate->es_rowMarks)
+ {
+ ExecRowMark *thiserm = (ExecRowMark *) lfirst(lc);
+
+ if (RelationGetRelid(thiserm->relation) == table_oid)
+ {
+ if (erm)
+ {
+ /* multiple references to desired relation */
+ erm = NULL;
+ break;
+ }
+ erm = thiserm;
+ }
+ }
+
+ if (erm)
+ {
+ /*
+* Okay, we were able to identify the target FOR UPDATE item.
+*
+* The cursor must have a current result row: per the SQL spec,
it's
+* an error if not.
+*/
+ if (portal->atStart || portal->atEnd)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_CURSOR_STATE),
+errmsg("cursor \"%s\" is not
positioned on a row",
+ cursor_name)));
+ /* Return the currently scanned TID */
+ if (ItemPointerIsValid(&(erm->curCtid)))
+ {
+ *current_tid = erm->curCtid;
+ return true;
+ }
+ /* Inactive scan? Probably can't happen at the moment */
+ return false;
+ }
+
+ /*
* Dig through the cursor's plan to find the scan node. Fail if it's
not
* there or buried underneath aggregation.
*/
Index: src/backend/executor/execMain.c
===
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.315
diff -c -r1.315 execMain.c
*** src/backend/executor/execMain.c 6 Nov 2008 20:51:14 - 1.315
--- src/backend/executor/execMain.c 15 Nov 2008 00:04:17 -
***
*** 602,607
--- 602,608
erm->noWait = rc->noWait;
/* We'll set up ctidAttno below */
erm->ctidAttNo = InvalidAttrNumber;
+ ItemPointerSetInvalid(&(erm->curCtid));
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
}
***
*** 1442,1447
--- 1443,1451
elog(ERROR,
"unrecognized heap_lock_tuple status: %u",
test);
}
+
+ /* Remember tuple TID for WHERE CURRENT
OF */
+ erm->curCtid = tuple.t_self;
}
}
Index: src/include/nodes/execnodes.h
===
RCS file: /cvsroot/pgsql/src/include/
Re: [HACKERS] Column reordering in pg_dump
On Friday 14 November 2008 13:37:05 hernan gonzalez wrote: > On Fri, Nov 14, 2008 at 4:12 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > > "hernan gonzalez" <[EMAIL PROTECTED]> writes: > >> I've added an option to pg_dump to reorder > >> columns in the ouput "CREATE TABLE" dump. > > > > This doesn't seem like a particularly good idea to me. In the first > > place, pg_dump is a tool for reproducing your database, not altering it, > > so it seems like basically the wrong place to be inserting this type of > > feature. (There's been some talk of a Postgres ETL tool, which would be > > the right place, but so far it's only talk :-(.) In the second place, > > column order is actually a pretty delicate affair when you start to > > think about table inheritance situations and tables that have been > > altered via ADD/DROP COLUMN. We had bugs in pg_dump in the past with > > its ability to deal with column order in such cases. So I'm not nearly > > as optimistic as you are that such a feature is incapable of causing > > problems. > > > >regards, tom lane > > > > In the first placeplace, pg_dump is a tool for reproducing your database, > > not altering it > > Yes, but the standard/recommended procedure for reorder columns in > postgresql is "pg_dump , edit , restore". I just didn't want to mess > editing a dump. > it's one method, but not the only method, see http://wiki.postgresql.org/wiki/Alter_column_position > Of couse, the standard behavior of pg_dump is not altered when the > "reorder" option > is not use. And bear in mind that the reordering hook is guaranteed to > alter only the order > of the "CREATE TABLE" fields. (The original and the modified dump > will differ only in that; > even in the case of dropped columns and inherited tables). The only > possible troubling scenario > I can imagine: a dump using a COPY without columns names in the data > dump; but that > only arises with version < 7.3. > yeah, i remember using that trick a lot... ah the good ole days :-P -- Robert Treat Conjecture: http://www.xzilla.net Consulting: http://www.omniti.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] libpq-events windows gotcha
Andrew Chernow <[EMAIL PROTECTED]> writes: >> On the whole I vote for #4 out of these. > I attached a patch for the docs. Its documented as a NOTE to the > PGEventProc. Applied, but I editorialized on the wording a bit. Let me know if you think this is wrong ... regards, tom lane Index: libpq.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.269 diff -c -r1.269 libpq.sgml *** libpq.sgml 13 Nov 2008 09:45:24 - 1.269 --- libpq.sgml 14 Nov 2008 22:57:05 - *** *** 5255,5260 --- 5255,5273 PGconn. This is because the address of the procedure is used as a lookup key to identify the associated instance data. + + + + On Windows, functions can have two different addresses: one visible + from outside a DLL and another visible from inside the DLL. One + should be careful that only one of these addresses is used with + libpq's event-procedure functions, else confusion will + result. The simplest rule for writing code that will work is to + ensure that event procedures are declared static. If the + procedure's address must be available outside its own source file, + expose a separate function to return the address. + + -- 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] Updated posix fadvise patch v19
> As this hasn't happened and I haven't been able to demonstrate it being useful > myself I guess it makes more sense to separate the two now and set the > sequential scan stuff aside until someone can demonstrate it being useful. Sounds good. How soon do you think you can post updated patches? > But as I said, it was mostly so I could tell what the slow start algorithm was > doing and make sure it was doing anything at all in testing. In that case, I think you should just rip it all out for now. >> Department of nitpicking: > > Will clean these up, they all look valid. I thought I did clean things up > already -- I guess you should be happy you're looking at it *after* that > cleanup :/ Maybe it's you who should be glad... :-) ...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] Updated posix fadvise patch v19
"Robert Haas" <[EMAIL PROTECTED]> writes: > I was pretty leery about reviewing this one due to the feeling that I > might well be in over my head, but they talked me into it, so here > goes nothin'. Apologies in advance for any deficiencies in this > review. > > - Overall, this looks pretty clean. > > - However, having said that, it looks as if there is still a bit of > experimentation going on in terms of what you actually want the patch > to do. There are a couple of things that say FIXME or XXX, and at > least one diff hunk that adds code surrounded by #if 0 (in FileRead). > Maybe committing FIXME is OK, but certainly #if 0 isn't, so there's at > least a bit of work needed here to put this in its final form, though > I think perhaps not that much. As you mentioned when posting this > version of the patch, it does two unrelated things only one of which > you are confident is beneficial. I suggest splitting this into two > patches and trying to get the prefetch part committed first. I think > that would make for easier review, too. When I posted the plan was that people would run experiments with other systems. In particular I was hoping Zoltan who original reported the sequential file strategy stuff being helpful might be able to see if this works for him. As this hasn't happened and I haven't been able to demonstrate it being useful myself I guess it makes more sense to separate the two now and set the sequential scan stuff aside until someone can demonstrate it being useful. > - I dislike cluttering up EXPLAIN ANALYZE with even more crap. On the > flip side, I am loathe to take away any sort of instrumentation that > might be helpful. I put it in for debugging since it was hard to know if it was actually doing anything otherwise. I figured whoever was reviewing the patch would run into the same problem. It would be trivial to strip out though. > - It's not clear to me whether there is a reason why we are only > adding instrumentation for bitmap heap scan; would not other scan > types benefit from something similar? If we're not going to add > instrumentation for all the cases that can benefit from it, then we > should just rip it out. Well there is a difference because bitmap heap scans do that slow-start thing. They ramp up the prefetching as you fetch more tuples from the bitmap up to the user configurable GUC so the point was to instrument what level they get to. Regular index scans prefetch whatever they find in the index leaf pages which will vary from page to page and isn't really something the user can influence anyways. But as I said, it was mostly so I could tell what the slow start algorithm was doing and make sure it was doing anything at all in testing. > - The WARNING at the top of PrefetchBuffer() seems strange. Is that > really only a WARNING? We just continue anyway? Er, uh, yeah that's bogus. The RelationIsValid is pointless as RelationOpenSmgr will just crash if that's not true. But I'm not nearly as convinced that BlockNumberIsValid() isn't worth asserting for. I don't see anything in the buffer tag lookup code which will fail in that case. What confuses me is that there's no corresponding check in the ReadBuffer code. Tom? Heikki? Should we have an assert in ReadBuffer asserting BlockNumberIsValid -- especially for the zero-page case where we're not actually going to do I/O? Or am I missing where that will seg fault? > Department of nitpicking: Will clean these up, they all look valid. I thought I did clean things up already -- I guess you should be happy you're looking at it *after* that cleanup :/ -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
> [ dept of second thoughts... ] Actually, given that he said FOR UPDATE, > the plan should be propagating the tuple identity through to top level > so that execMain can do its thing. So in principle we could probably > get the information without widespread changes. This would fit > reasonably well with the spec's requirements too --- any but trivial > cursors are not deemed updatable unless you say FOR UPDATE. But it > would mean two completely independent implementations within > execCurrent.c... What's particularly unfortunate is Greg's comment that this would work if the planner chose an index scan. Had I defined an index on the columns in question, my code might have worked and been deployed to production - and then broken if the planner changed its mind and decided to use a seqscan after all. ISTM any cursor that's going to be updated should specify WHERE UPDATE in the query, but maybe that's not a hard requirement as of now. ...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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
Gregory Stark <[EMAIL PROTECTED]> writes: > "Robert Haas" <[EMAIL PROTECTED]> writes: >> I found the following behavior surprising. Is there a reason for it? >> This is 8.3.5. ...Robert >> >> rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR >> UPDATE; > Skimming the code we don't support WHERE CURRENT OF on a query which involves > a Sort above the table specified. The way CURRENT OF works depends on the > nature of the plan and depends on there being an active scan of the specified > table. Since sort reads all its inputs in advance that information is lost by > the time the results are output. [ dept of second thoughts... ] Actually, given that he said FOR UPDATE, the plan should be propagating the tuple identity through to top level so that execMain can do its thing. So in principle we could probably get the information without widespread changes. This would fit reasonably well with the spec's requirements too --- any but trivial cursors are not deemed updatable unless you say FOR UPDATE. But it would mean two completely independent implementations within execCurrent.c... 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] Okay, DLLIMPORT is making me crazy
Tom Lane wrote: > ... meanwhile, the MSVC port has got its own issues: > > Generating win32ver.rc for src\backend > Building src\pl\plperl\SPI.c... > Could not determine contrib module type for intagg > at build.pl line 37 > > I am not sure what if anything that script needs to do for a contrib > module with no .c files. It just needs to be excluded from the build step (it should still be included in install, AFAICS). Will fix right away. //Magnus -- 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
"Robert Haas" <[EMAIL PROTECTED]> writes: > I found the following behavior surprising. Is there a reason for it? Yes. regards, tom lane (Oh, you wanted to know what the reason is? It's because a sort step is not going to pass through any tuple identity information.) -- 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] Okay, DLLIMPORT is making me crazy
... meanwhile, the MSVC port has got its own issues: Generating win32ver.rc for src\backend Building src\pl\plperl\SPI.c... Could not determine contrib module type for intagg at build.pl line 37 I am not sure what if anything that script needs to do for a contrib module with no .c files. 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] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
"Robert Haas" <[EMAIL PROTECTED]> writes: > I found the following behavior surprising. Is there a reason for it? > This is 8.3.5. ...Robert > > rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR > UPDATE; > DECLARE CURSOR Skimming the code we don't support WHERE CURRENT OF on a query which involves a Sort above the table specified. The way CURRENT OF works depends on the nature of the plan and depends on there being an active scan of the specified table. Since sort reads all its inputs in advance that information is lost by the time the results are output. If you had an index it would work. That this is tied to the nature of the plan does seem kind of unfortunate. I'm not sure the alternatives are very appealing though -- they would involve a lot of code to track a lot more information for queries that might never need it. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- 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] Okay, DLLIMPORT is making me crazy
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Your second go at fixing this seems to have almost worked. Is there a > reason why, alone of the utility programs, pg_resetxlog.c uses > postgres.h rather than postgres_fe.h? It doesn't compile otherwise. We could possibly refactor the backend includes enough that it could get the info it needs from frontend-safe headers. I think Zdenek was working on that awhile back, in fact. For the moment what I'm trying is to #define FRONTEND anyway ... 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] Okay, DLLIMPORT is making me crazy
Tom Lane wrote: I did this: http://archives.postgresql.org/pgsql-committers/2008-11/msg00156.php to try to fix this: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=narwhal&dt=2008-11-12%2021:00:01 only to get this: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=narwhal&dt=2008-11-13%2015:00:01 Anybody know how to get that stuff to act sanely? Or should we just toss the mingw build overboard? Your second go at fixing this seems to have almost worked. Is there a reason why, alone of the utility programs, pg_resetxlog.c uses postgres.h rather than postgres_fe.h? 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] WIP: Column-level Privileges
Alvaro Herrera wrote:
> I didn't check the rest of the code, so don't count this as a review.
I had a look at aclchk.c and didn't like your change to
objectNamesToOids; seems rather baroque. I changed it per the attached
patch.
Moreover I didn't very much like the way aclcheck_error_col is dealing
with two or one % escapes. I think you should have a separate routine
for the column case, and prepend a dummy string to no_priv_msg.
Why is there a InternalGrantStmt.rel_level? Doesn't it suffice to
check whether col_privs is NIL?
Is there enough common code in ExecGrant_Relation to justify the way you
have it? Can the common be refactored in a better way that separates
the two cases more clearly?
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
diff -u src/backend/catalog/aclchk.c src/backend/catalog/aclchk.c
--- src/backend/catalog/aclchk.c 14 Nov 2008 12:24:15 -
+++ src/backend/catalog/aclchk.c 14 Nov 2008 20:46:06 -
@@ -55,7 +55,8 @@
static void ExecGrant_Namespace(InternalGrant *grantStmt);
static void ExecGrant_Tablespace(InternalGrant *grantStmt);
-static List *objectNamesToOids(GrantObjectType objtype, List *objnames, Oid in_relOid);
+static List *objectNamesToOids(GrantObjectType objtype, List *objnames);
+static List *columnNamesToAttnums(List *colnames, Oid relid);
static AclMode string_to_privilege(const char *privname);
static const char *privilege_to_string(AclMode privilege);
static AclMode restrict_and_check_grant(bool is_grant, AclMode avail_goptions,
@@ -264,7 +265,7 @@
*/
istmt.is_grant = stmt->is_grant;
istmt.objtype = stmt->objtype;
- istmt.objects = objectNamesToOids(stmt->objtype, stmt->objects, 0);
+ istmt.objects = objectNamesToOids(stmt->objtype, stmt->objects);
/* all_privs to be filled below */
/* privileges to be filled below */
istmt.col_privs = NIL;
@@ -402,23 +403,24 @@
foreach(cell_obj, istmt.objects)
{
Oid relOid = lfirst_oid(cell_obj);
- List *cols_oids = NIL;
+ List *colnums;
ListCell *cell_coloid;
/* Get the attribute numbers for this relation for the columns affected */
- cols_oids = objectNamesToOids(ACL_OBJECT_COLUMN, privnode->cols, relOid);
+ colnums = columnNamesToAttnums(privnode->cols, relOid);
/* Loop through the columns listed for this privilege and
* add them to the col_privs list of privileges */
- foreach(cell_coloid, cols_oids)
+ foreach(cell_coloid, colnums)
{
- ListCell *cell_colprivs;
- int found = false;
- AttrNumber curr_attnum = lfirst_int(cell_coloid);
+ AttrNumber curr_attnum = lfirst_int(cell_coloid);
+ ListCell *cell_colprivs;
+ int found = false;
/* Check if we have already seen this column, and if
* so then just add to its aclmask. */
- foreach(cell_colprivs, istmt.col_privs) {
+ foreach(cell_colprivs, istmt.col_privs)
+ {
ColPrivs *col_privs = (ColPrivs *) lfirst(cell_colprivs);
if (col_privs->relOid == relOid && col_privs->attnum == curr_attnum)
@@ -487,50 +489,18 @@
* objectNamesToOids
*
* Turn a list of object names of a given type into an Oid list.
- * For columns, turn a list of column names into a list of
- * attribute numbers, for a given relation in_relOid.
*/
static List *
-objectNamesToOids(GrantObjectType objtype, List *objnames, Oid in_relOid)
+objectNamesToOids(GrantObjectType objtype, List *objnames)
{
List *objects = NIL;
ListCell *cell;
Assert(objnames != NIL);
+ AssertArg(objtype != ACL_OBJECT_COLUMN);
switch (objtype)
{
- case ACL_OBJECT_COLUMN:
- foreach(cell, objnames)
- {
-AttrNumber attnum;
-
-attnum = get_attnum(in_relOid,strVal(lfirst(cell)));
-if (attnum == InvalidAttrNumber)
-{
- HeapTuple tuple;
- Form_pg_class pg_class_tuple;
-
- tuple = SearchSysCache(RELOID,
- ObjectIdGetDatum(in_relOid),
- 0, 0, 0);
-
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for relation %u", in_relOid);
-
- pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
-
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_COLUMN),
- errmsg("column \"%s\" of relation %s does not exist.",
- strVal(lfirst(cell)), NameStr(pg_class_tuple->relname;
-
- ReleaseSysCache(tuple);
-}
-
-objects = lappend_int(objects, attnum);
- }
- break;
case ACL_OBJECT_RELATION:
case ACL_OBJECT_SEQUENCE:
foreach(cell, objnames)
@@ -647,6 +617,39 @@
}
/*
+ * columnNamesToAttnums
+ *
+ * Turn a list of column names into a list of attribute numbers, for the given
+ * relation.
+ */
+static List *
+columnNamesToAttnums(List *colnames, Oid relid)
+{
+ ListCell *cell;
+ List *colnums = NIL;
+
+ foreach(cell, colnames)
+ {
+ AttrNumber attnum;
+
+ attnum = get_attnum(relid, strVal(lfirst(cell)));
+ if (attnum == Inv
[HACKERS] "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
I found the following behavior surprising. Is there a reason for it? This is 8.3.5. ...Robert rhaas=# BEGIN WORK; BEGIN rhaas=# CREATE TABLE test_table (id serial, foo integer, bar integer); NOTICE: CREATE TABLE will create implicit sequence "test_table_id_seq" for serial column "test_table.id" CREATE TABLE rhaas=# INSERT INTO test_table VALUES (DEFAULT, 1, 1); INSERT 0 1 rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR UPDATE; DECLARE CURSOR rhaas=# FETCH c; id 1 (1 row) rhaas=# UPDATE test_table SET bar = NULL WHERE CURRENT OF c; ERROR: cursor "c" is not a simply updatable scan of table "test_table" rhaas=# \q -- 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] Signal handling patch (v2) for Synch Rep
Fujii Masao wrote: Attached is a patch of signal handling changes for Synch Rep. It seems that we wouldn't need to use the BackendPidGetProc function, nor the new AuxiliaryPidGetProc function, if we stored a PGPROC * instead of the pid in ProcState.procPid. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Simple postgresql.conf wizard
On Fri, 14 Nov 2008, Richard Huxton wrote: Is it only me that thinks this should be a service on the website too (or even first)? Fill in web form, click button, get sample postgresql.conf (with comments) back. Sounds familiar...ah, here it is: http://archives.postgresql.org/pgsql-performance/2007-06/msg00377.php I'm ignoring the temptation of working on the UI first and instead staying focused on getting the parameter model slimmed down to a readable set of code. Ports to other interfaces should follow--I've already got two people who want to build alternate ones lined up. To give you an idea how overdiscussed this general topic is, I just sent a message to Josh suggesting we might put database size into tiers and set some parameters based on that. Guess what? That was his idea the last time around, I subconsciously regurgitated it: http://archives.postgresql.org/pgsql-performance/2007-06/msg00602.php Add a tick-box asking if we can keep a copy of their answers and you might get some useful usage info too. It's rare I work with a company that wants their internal use of PostgreSQL to be public knowledge. Putting a collection flag on the page is just going to spook such commercial users, even if it defaults to off. The privacy issues are one reason I put the web-based port far down relative to my priorities; another is that I don't do much web application development. But if somebody else wants to run with that, fine by me. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- 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] contrib/pg_stat_statements
Martin Pihlak <[EMAIL PROTECTED]> writes: > ITAGAKI Takahiro wrote: >>> But is the idea of extending QueryDesc generally acceptable? Is it OK >>> to make a copy of the query string? >> >> The only thing I'm worried about is that QueryDesc lives longer than >> its queryText. Can I assume it never occurs? > I just finished validating this -- it seems OK. All of the query strings > that make it to CreateQueryDesc are either pstrdup-ed to Portal, in long > lived memory context or just literals. So no need to make extra copies :) I'm trying to figure out what is the status of this patch? I'm not sure if there's any point in applying it, when contrib/pg_stat_statements hasn't been updated to make use of it. 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] auto_explain contrib moudle
Jeff Davis <[EMAIL PROTECTED]> writes: > On Thu, 2008-11-13 at 14:31 -0500, Tom Lane wrote: >> This patch seems to contain a subset of the "contrib infrastructure" >> patch that's listed separately on the commitfest page. While I have >> no strong objection to what's here, I'm wondering what sort of process >> we want to follow. Is the infrastructure stuff getting separately >> reviewed or not? > I can review it, but not until this weekend. It looks like someone > already added me to the list of reviewers on that patch. I'm not sure if > Matthew Wetmore has already started reviewing it or not. Now that I look closer, the "contrib infrastructure" item is just a combination of the auto_explain and pg_stat_statements items, and I guess the reason you and Matthew were shown as reviewers was that you'd each been assigned one of those two items. As far as I can see this is just confusing and duplicative. I've removed the "infrastructure" item from the commitfest page; I think we can proceed with the two other items separately. If there's any conflict in the two patches we can resolve it after the first one gets applied. 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] Assorted contrib infrastructures patch
ITAGAKI Takahiro <[EMAIL PROTECTED]> writes: > I'm submitting 2 contrib modules and there 3 patches to core for them > from me and Martin, but they confict each other and there are some hunks > and rejections already. Here is an assorted patch of them. > Can I ask you to review the patches in this form? I've removed this item from the commitfest listing because it seems to just confuse matters --- all the suggested patches are listed separately under the auto_explain and pg_stat_statements entries. 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] Simple postgresql.conf wizard
On Thu, Nov 13, 2008 at 11:53 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > Heikki Linnakangas <[EMAIL PROTECTED]> writes: >> A lot of people have suggested raising our default_statistics target, >> and it has been rejected because there's some O(n^2) behavior in the >> planner, and it makes ANALYZE slower, but it's not that crazy. > > I think everyone agrees it ought to be raised. Where the rubber meets > the road is deciding just *what* to raise it to. We've got no > convincing evidence in favor of any particular value. > > If someone actually wanted to put some effort into this, I'd suggest > taking some reasonably complex benchmark (maybe TPCH or one of the DBT > series) and plotting planner runtime for each query as a function of > statistics_target, taking care to mark the breakpoints where it shifted > to a better (or worse?) plan due to having better stats. Almost there... I have a MSA70 plugged into the DL380 I have from HP and I'm trying to find time to get my scripts updated to deal with how tools have changed over the years... I'm updating the DBT-2 (tpc-c kit) I have first Regards, Mark -- 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] Column reordering in pg_dump
On Fri, Nov 14, 2008 at 4:12 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > "hernan gonzalez" <[EMAIL PROTECTED]> writes: >> I've added an option to pg_dump to reorder >> columns in the ouput "CREATE TABLE" dump. > > This doesn't seem like a particularly good idea to me. In the first > place, pg_dump is a tool for reproducing your database, not altering it, > so it seems like basically the wrong place to be inserting this type of > feature. (There's been some talk of a Postgres ETL tool, which would be > the right place, but so far it's only talk :-(.) In the second place, > column order is actually a pretty delicate affair when you start to > think about table inheritance situations and tables that have been > altered via ADD/DROP COLUMN. We had bugs in pg_dump in the past with > its ability to deal with column order in such cases. So I'm not nearly > as optimistic as you are that such a feature is incapable of causing > problems. > >regards, tom lane > In the first placeplace, pg_dump is a tool for reproducing your database, not > altering it Yes, but the standard/recommended procedure for reorder columns in postgresql is "pg_dump , edit , restore". I just didn't want to mess editing a dump. Of couse, the standard behavior of pg_dump is not altered when the "reorder" option is not use. And bear in mind that the reordering hook is guaranteed to alter only the order of the "CREATE TABLE" fields. (The original and the modified dump will differ only in that; even in the case of dropped columns and inherited tables). The only possible troubling scenario I can imagine: a dump using a COPY without columns names in the data dump; but that only arises with version < 7.3. Anyway, I know you know better. Best regards! Hernán J. González http://hjg.com.ar/ -- 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] Simple postgresql.conf wizard
Tom Lane wrote: "Jonah H. Harris" <[EMAIL PROTECTED]> writes: On Fri, Nov 14, 2008 at 11:43 AM, Tom Lane <[EMAIL PROTECTED]> wrote: IMHO, the only thing worse than an unstable plan is a stable one. Your opinion contradicts the majority of the industry then, I'm afraid. Like query hints, people are sometimes smarter than the optimizer. And, very often, they're not --- or more likely, they were smarter than the optimizer last year, but now conditions have changed. Failing to adapt to new conditions is exactly the problem with query hints, and in general with any insistence that plans should be "stable". Those are both very simplistic views. Yes, the planner often knows better. Yes, the DBA also often knows better. Stable plans are by no means a silver bullet. If a table suddenly changes dramatically in size, you certainly do want to replan your queries. The typical scenario that people want to avoid with stable plans is where a table grows slowly, until it reaches a critical point, and the planner chooses a different plan, and the new plan happens to be very bad. The scary thing about that is that it can happen at an unpredictable time, and it can be really hard to convince the planner to choose the old plan again. It's a tradeoff, for sure. Ideally the planner would be infallible. Failing that, it would be nice to provide the DBA some tools to control when new plans are taken into use. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] Synchronous replication patch v2
On Fri, 2008-11-14 at 19:23 +0200, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Fri, 2008-11-14 at 19:00 +0200, Heikki Linnakangas wrote: > >> Why do we need a separate XLogsndRqst variable in shared memory? Don't > >> we always want to send the WAL up to the same point as we flush it? > > > > If we're doing synch rep and we're committing. > > You flush and send the WAL, up to the same point? Yes, but you may make progress towards it in different size steps. > > What happens when we're > > doing async rep or running something like a large load. > > You don't flush, and you don't request the WAL to be sent? The > background writer and WAL sender can still wake up periodically, and > write and send the WAL as they find convenient. With WAL writes we write and flush at the same time. With WAL sending that doesn't sound such a good plan. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Column reordering in pg_dump
"hernan gonzalez" <[EMAIL PROTECTED]> writes: > I've added an option to pg_dump to reorder > columns in the ouput "CREATE TABLE" dump. This doesn't seem like a particularly good idea to me. In the first place, pg_dump is a tool for reproducing your database, not altering it, so it seems like basically the wrong place to be inserting this type of feature. (There's been some talk of a Postgres ETL tool, which would be the right place, but so far it's only talk :-(.) In the second place, column order is actually a pretty delicate affair when you start to think about table inheritance situations and tables that have been altered via ADD/DROP COLUMN. We had bugs in pg_dump in the past with its ability to deal with column order in such cases. So I'm not nearly as optimistic as you are that such a feature is incapable of causing problems. 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] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 12:07 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > And, very often, they're not --- or more likely, they were smarter than > the optimizer last year, but now conditions have changed. Failing to > adapt to new conditions is exactly the problem with query hints, and > in general with any insistence that plans should be "stable". Well, at least they didn't have to wait a year to fix the problem. Similarly, whether or not the plan changed due to bad hints or bad plans, detecting the change is relatively easy, so I don't really see an argument based on *why* the plan failed. In my, and many others opinion, if you decide to take your query plan into your own hands, it's your problem if it fails. I do agree that hints are a little too nice and simple, and generally get people into trouble because they're hard-coded in an app, tend to cause issues later, and are then difficult to track down. Oracle solved that years ago as well, which is why they support more advanced plan stability features than just hints. However, given the number of large-scale OLTP sites I've been to, I can tell you from experience that post-ANALYZE plan changes wreak complete havoc on a system and in many cases, bring it to its knees. In those cases, the proper query plan is well-known, and a hint (or some other form of plan stability) is all that would be required to prevent it from happening. This is pretty off-topic for this thread, so I'll postpone the discussion for 8.5. -- Jonah H. Harris, Senior DBA myYearbook.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] Synchronous replication patch v2
Simon Riggs wrote: On Fri, 2008-11-14 at 19:00 +0200, Heikki Linnakangas wrote: Why do we need a separate XLogsndRqst variable in shared memory? Don't we always want to send the WAL up to the same point as we flush it? If we're doing synch rep and we're committing. You flush and send the WAL, up to the same point? What happens when we're doing async rep or running something like a large load. You don't flush, and you don't request the WAL to be sent? The background writer and WAL sender can still wake up periodically, and write and send the WAL as they find convenient. I wouldn't want to presume that the network packet size and the disk write size are always identical. Huh? No-one's presuming that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Column reordering in pg_dump
Hi. As I wanted to change the order of columns of some tables (I know, I know, my code does not depend on that; but I prefer that \dt gives me a more organized ouput...) I've added an option to pg_dump to reorder columns in the ouput "CREATE TABLE" dump. The specified order is given in a external file (one column by line, format "scheme.tablename.colname"), you only need to write the desired columns to reorder. I've done it for my own use, minimal testing, etc; but the code is quite simple and has little coupling with the rest; and very little risk, IMO. The code (reoder.c and patched pg_dump.c) is here: http://hjg.com.ar/tech/prog/pg_dump_reorder/ See the README.txt, and the comments at reorder.c. If the dev team is interested in adding this little feature to the core, delighted (in that case, I guess the code needs some polishing, and perhaps revise terminology, options names, modify man page, etc) If not, there it is so the people interested can use it. BTW: I've been using Postgresql since ten years ago, and it rocks. Congrats. -- Hernán J. González http://hjg.com.ar/ Buenos Aires, Argentina -- 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] Simple postgresql.conf wizard
"Jonah H. Harris" <[EMAIL PROTECTED]> writes: > On Fri, Nov 14, 2008 at 11:43 AM, Tom Lane <[EMAIL PROTECTED]> wrote: >> IMHO, the only thing worse than an unstable plan is a stable one. > Your opinion contradicts the majority of the industry then, I'm > afraid. Like query hints, people are sometimes smarter than the > optimizer. And, very often, they're not --- or more likely, they were smarter than the optimizer last year, but now conditions have changed. Failing to adapt to new conditions is exactly the problem with query hints, and in general with any insistence that plans should be "stable". 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] Synchronous replication patch v2
On Fri, 2008-11-14 at 19:00 +0200, Heikki Linnakangas wrote: > Fujii Masao wrote: > > Attached is the latest version of Synch Rep patch. > > Why do we need a separate XLogsndRqst variable in shared memory? Don't > we always want to send the WAL up to the same point as we flush it? If we're doing synch rep and we're committing. What happens when we're doing async rep or running something like a large load. I wouldn't want to presume that the network packet size and the disk write size are always identical. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] SSL cleanups/hostname verification
On Thu, Nov 13, 2008 at 01:05, Magnus Hagander <[EMAIL PROTECTED]> wrote: > It means I will go ahead and apply it once I have looked it over once more. > > Thanks for review+testing! > > You may now move on to the next ssl patch if you're interested ;) Sure thing probably wont get to it till tomorrow... -- 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] Synchronous replication patch v2
Fujii Masao wrote: Attached is the latest version of Synch Rep patch. Why do we need a separate XLogsndRqst variable in shared memory? Don't we always want to send the WAL up to the same point as we flush it? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] gram.y => preproc.y
Michael Meskes <[EMAIL PROTECTED]> writes: > Added. Looks alright to me now --- you might as well commit and let the buildfarm find any remaining problems. 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] Brittleness in regression test setup
Peter Eisentraut <[EMAIL PROTECTED]> writes: > For some historic reasons, I have my local scripts set up so that they > build development instances using the hardcoded port 65432. This will > cause a temp install regression test to attempt to use port 565432 which > will be rejected silently by pg_regress, which will then use its > hardcoded default 65432 (no relation to my 65432). One thing we should do is have pg_regress.c, not the Makefile, select the default port to use. The concatenate-5 behavior is just not intelligent enough. 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] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 11:43 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > IMHO, the only thing worse than an unstable plan is a stable one. Your opinion contradicts the majority of the industry then, I'm afraid. Like query hints, people are sometimes smarter than the optimizer. -- Jonah H. Harris, Senior DBA myYearbook.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] Simple postgresql.conf wizard
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > You're right, I haven't, but yes I know that's a problem. We've chatted > about that with Greg sometimes. It would be nice to have more stable > plans. My favorite idea is to stop using the current relation size in > the planner, and use the value snapshotted at ANALYZE instead. Sorry, that's a nonstarter. We used to act that way and it was completely fatal to plan quality in very many typical scenarios. IMHO, the only thing worse than an unstable plan is a stable one. 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] CREATE AGGREGATE disallows STYPE = internal
"Robert Haas" <[EMAIL PROTECTED]> writes: > On Fri, Nov 14, 2008 at 10:12 AM, Tom Lane <[EMAIL PROTECTED]> wrote: >> "Robert Haas" <[EMAIL PROTECTED]> writes: >>> I would feel a lot better about it if we could invent some way of >>> distinguishing between different types of "internal", based on what is >>> actually being pointed to. >> >> Yeah, we discussed that awhile ago, but nothing's been done about it. >> At this point I doubt it will happen for 8.4. > That's a shame. Do you happen to have a pointer to the relevant > discussions in the archives? http://archives.postgresql.org/pgsql-hackers/2008-08/msg00644.php > Without that, it seems that the change you're proposing is totally > unsafe. It's not just a question of malicious activity: a clueless > admin (a category we've all been in from time to time...) could > relatively easily create a configuration that crashes the backend. It's no more dangerous than allowing the admin to create functions taking or returning internal to begin with. Basically, if you are superuser, there are no training wheels. 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] gram.y => preproc.y
On Fri, Nov 14, 2008 at 10:44:08AM -0500, Tom Lane wrote:
> preproc.y should be removed by make maintainer-clean. For consistency
> it probably ought to be listed in the distprep target too, even though
> it would be indirectly updated by the preproc.c target. I suspect it
> would be a good idea to list preproc.c's antecedent as
> $(srcdir)/preproc.y not just preproc.y. Also, you'll need to add
> preproc.y to .cvsignore.
Added.
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: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!
diff --exclude CVS -ruN pgsql/src/interfaces/ecpg/preproc/.cvsignore pgsql/src.mm/interfaces/ecpg/preproc/.cvsignore
--- pgsql/src/interfaces/ecpg/preproc/.cvsignore 2008-05-20 17:41:12.0 +0200
+++ pgsql/src.mm/interfaces/ecpg/preproc/.cvsignore 2008-11-14 17:35:09.0 +0100
@@ -1,3 +1,4 @@
+preproc.y
preproc.c
preproc.h
pgc.c
diff --exclude CVS -ruN pgsql/src/interfaces/ecpg/preproc/Makefile pgsql/src.mm/interfaces/ecpg/preproc/Makefile
--- pgsql/src/interfaces/ecpg/preproc/Makefile 2008-11-14 11:01:55.0 +0100
+++ pgsql/src.mm/interfaces/ecpg/preproc/Makefile 2008-11-14 17:34:38.0 +0100
@@ -38,27 +38,30 @@
$(srcdir)/preproc.h: $(srcdir)/preproc.c ;
-$(srcdir)/preproc.c: preproc.y
+$(srcdir)/preproc.c: $(srcdir)/preproc.y
ifdef BISON
$(BISON) -d $(BISONFLAGS) -o $@ $<
else
@$(missing) bison $< $@
endif
-$(srcdir)/pgc.c: pgc.l
+$(srcdir)/pgc.c: $(srcdir)/pgc.l
ifdef FLEX
$(FLEX) $(FLEXFLAGS) -o'$@' $<
else
@$(missing) flex $< $@
endif
+$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
+ $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@
+
ecpg_keywords.o c_keywords.o keywords.o preproc.o parser.o: preproc.h
# instead of maintaining our own list, take the one from the backend
keywords.c: % : $(top_srcdir)/src/backend/parser/%
rm -f $@ && $(LN_S) $< .
-distprep: $(srcdir)/preproc.c $(srcdir)/preproc.h $(srcdir)/pgc.c
+distprep: $(srcdir)/preproc.y $(srcdir)/preproc.c $(srcdir)/preproc.h $(srcdir)/pgc.c
install: all installdirs
$(INSTALL_PROGRAM) ecpg$(X) '$(DESTDIR)$(bindir)'
@@ -78,4 +81,4 @@
# want to ship those files in the distribution for people with
# inadequate tools.
maintainer-clean: distclean
- rm -f $(srcdir)/preproc.c $(srcdir)/preproc.h $(srcdir)/pgc.c
+ rm -f $(srcdir)/preproc.y $(srcdir)/preproc.c $(srcdir)/preproc.h $(srcdir)/pgc.c
diff --exclude CVS -ruN pgsql/src/tools/msvc/Solution.pm pgsql/src.mm/tools/msvc/Solution.pm
--- pgsql/src/tools/msvc/Solution.pm 2008-08-17 09:09:16.0 +0200
+++ pgsql/src.mm/tools/msvc/Solution.pm 2008-11-13 15:15:50.0 +0100
@@ -239,6 +239,19 @@
if (
IsNewer(
+'src\interfaces\ecpg\preproc\preproc.y',
+'src\backend\parser\gram.y'
+)
+ )
+{
+print "Generating preproc.y...\n";
+chdir('src\interfaces\ecpg\preproc');
+system('perl parse.pl < ..\..\..\backend\parser\gram.y > preproc.y');
+chdir('..\..\..\..');
+}
+
+if (
+IsNewer(
'src\interfaces\ecpg\include\ecpg_config.h',
'src\interfaces\ecpg\include\ecpg_config.h.in'
)
--
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] Updates of SE-PostgreSQL 8.4devel patches (r1197)
On Sat, 2008-11-15 at 00:58 +0900, KaiGai Kohei wrote: > Sorry, it seems to me you misunderstand something. Yep, seems so. Thank goodness for that. Thanks for putting me straight. > > I would also like to see the feature part of normal Postgres, rather > > than as a compile time option. The per-row overhead would then be > > optional, just as WITH OIDS is optional. This would allow many > > applications to take advantage of row level security, without the need > > for switching to a different executable and without the need to enable > > it for every table. For high security applications, default_row_security > > = on would obviously be a requirement. With a single executable on all > > distros we will have more robust software and it will be easier to > > configure and use. > > An issue is who can enable or disable the row-level security option. > If the owner of table can do it discretionary, we don't call it a > "mandatory" access control feature. It seems fairly easy to do that with a GUC, or at least an option on CREATE DATABASE, with no equivalent ALTER DATABASE option. Once created with security, a table would not be able to turn off security. So nobody would be able to turn off security for existing data. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Sometimes pg_dump generates dump which is not restorable
"Dmitry Koterov" <[EMAIL PROTECTED]> writes: > Thank you for a possible solution. > But what about the database which exists and works correctly (and conforms > all the standards from the documentation), but dump+restore sequence is > failed for it? Does it mean that pg_dump should be improved to pass > dump+restore sequence? No matter what pg_dump does, it can never guarantee that a non-immutable check constraint will still pass at restore time ... and that's basically what you've got, if the check function is search-path-sensitive. 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] CREATE AGGREGATE disallows STYPE = internal
On Fri, Nov 14, 2008 at 10:12 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Robert Haas" <[EMAIL PROTECTED]> writes: >> I would feel a lot better about it if we could invent some way of >> distinguishing between different types of "internal", based on what is >> actually being pointed to. > > Yeah, we discussed that awhile ago, but nothing's been done about it. > At this point I doubt it will happen for 8.4. That's a shame. Do you happen to have a pointer to the relevant discussions in the archives? Without that, it seems that the change you're proposing is totally unsafe. It's not just a question of malicious activity: a clueless admin (a category we've all been in from time to time...) could relatively easily create a configuration that crashes the backend. ...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] gram.y => preproc.y
Michael Meskes wrote: Does anyone see a problem with these changes? Or else I will commit. FWIW, I have looked at this a bit. I want to refactor it at some stage because it's ugly and fragile and rather obtuse, but the refactoring won't be happening for a while. 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] Brittleness in regression test setup
Peter Eisentraut wrote: So even if I configured my local scripts to use ports that are all different from each other and from 65432, this problem would still exist. Only if you choose the private port to be above . The standard buildfarm config file contains a warning against using such ports for exactly this reason. Is using a private 4-digit port terribly difficult for you? So, also, 2a) It has an undocumented hardcoded default port. Any thoughts on how to fix this? a) document it b) make it a lot noisier if it falls back on 65432. 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] Updates of SE-PostgreSQL 8.4devel patches (r1197)
Simon Riggs wrote: > On Thu, 2008-11-13 at 10:44 +0900, KaiGai Kohei wrote: > >> It's unclear for me what is the point you said. >> I guess you concern the fixed length field is always allocated in >> the case when any security feature is not enabled also, or performance >> degradation on the large scale databases. >> If incorrect, please tell me in another expression. >> >> At first, the fixed length 4 byte field is allocated only when >> the SE-PostgreSQL (or other security feature) is enabled. It can be >> controlled via PGACE hook. The pgaceSecurityAttributeNecessary() can >> return bool value, and it indicates the necessity of the security >> field. If SE-PostgreSQL is disabled on compile-time or run-time, >> the fixed length 4 byte value is not allocated. > > I'm sorry for not making my thoughts clearer. Let me try again: > > As I understand it, when enabled, the overhead for each row is more than > 4 bytes because you include a text field also, which you say has a > restricted number of values. IMHO the overhead is unacceptable, given > that our row overhead is already high. I would prefer to make the > maximum overhead per row 4 bytes only, which matches the maximum number > of required labels. This will allow very large databases to use this > feature. Indeed, the additional storage comsumption is not just only fixed 4 bytes per tuple, because these security identifiers require its text representation. However, the rate of them are nearly zero in very large databases, especially. Please consider the following example: postgres=# SELECT security_context, * FROM t1; security_context | a | b -+---+- unconfined_u:object_r:sepgsql_table_t:s0| 1 | aaa unconfined_u:object_r:sepgsql_table_t:s0| 2 | bbb unconfined_u:object_r:sepgsql_table_t:s0| 3 | ccc unconfined_u:object_r:sepgsql_table_t:s0:c0 | 4 | ddd unconfined_u:object_r:sepgsql_table_t:s0:c0 | 5 | eee unconfined_u:object_r:sepgsql_table_t:s0:c1 | 6 | fff unconfined_u:object_r:sepgsql_table_t:s0:c1 | 7 | ggg unconfined_u:object_r:sepgsql_table_t:s0:c2 | 8 | hhh (8 rows) postgres=# SELECT oid, * FROM pg_security; oid | seclabel ---+- 3397 | unconfined_u:object_r:sepgsql_table_t:s0 3398 | unconfined_u:object_r:sepgsql_proc_t:s0 3399 | unconfined_u:object_r:sepgsql_db_t:s0 16390 | unconfined_u:object_r:sepgsql_table_t:s0:c0 16391 | unconfined_u:object_r:sepgsql_table_t:s0:c1 16392 | unconfined_u:object_r:sepgsql_table_t:s0:c2 (6 rows) The table "t1" contains eight tuples, and there are four kind of security context. Every tuples has fixed 4-bytes identifier to indicate oid of pg_security which contains text representation. (Its output handler translate it into human readable text form.) The first three tuples have "3397" as its security identifier which indicates the tuple of "unconfined_u:object_r:sepgsql_table_t:s0" within pg_security. This entry is shared by them. If a table has 1,000,000 of tuples labeled by 100 kind of security contexts, we only need 100 tuples on the pg_security system catalog. In this case, the sum of text representation length is 5,000 byte at maximum, it is small enough. In other word, the relationship between a security identifier and an entry of pg_security is n:1, not 1:1. Sorry, it seems to me you misunderstand something. > I would also like to see the feature part of normal Postgres, rather > than as a compile time option. The per-row overhead would then be > optional, just as WITH OIDS is optional. This would allow many > applications to take advantage of row level security, without the need > for switching to a different executable and without the need to enable > it for every table. For high security applications, default_row_security > = on would obviously be a requirement. With a single executable on all > distros we will have more robust software and it will be easier to > configure and use. An issue is who can enable or disable the row-level security option. If the owner of table can do it discretionary, we don't call it a "mandatory" access control feature. Thanks, -- KaiGai Kohei <[EMAIL PROTECTED]> -- 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] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 10:50 AM, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: >> Oracle already thought of that a long time ago, which is why the plan >> has to come out better for it to take effect. > > Huh? We would never willingly choose a worse plan, of course, but the point > is that what looks like a better plan, with a smaller cost estimate, is > sometimes actually worse. Oracle bases it on cost and elapsed execution time. >> As for bad plans, you >> obviously haven't used Postgres in production enough to deal with it >> continually changing plans for the worse due to index bloat, data >> skew, phase of the moon, etc. :) > > You're right, I haven't, but yes I know that's a problem. We've chatted > about that with Greg sometimes. It would be nice to have more stable plans. > My favorite idea is to stop using the current relation size in the planner, > and use the value snapshotted at ANALYZE instead. That way, the planner > would be completely deterministic, based on the statistics. Then, we could > have tools to snapshot the statistics, move them to a test system, store > them, revert back to old statistics etc. Yes, plan stability would be a Good Thing(tm) IMO. -- Jonah H. Harris, Senior DBA myYearbook.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] Block-level CRC checks
Martijn van Oosterhout <[EMAIL PROTECTED]> writes: > But I understand the problem is that you want to continue in the face > of torn pages, something which is AFAICS ambitious. At least MS-SQL > just blows up on a torn page, havn't found results for other > databases... I don't think it's too "ambitious" to demand that this patch preserve a behavior we have today. In fact, if the patch were to break torn-page handling, it would be 100% likely to be a net *decrease* in system reliability. It would add detection of a situation that is not supposed to happen (ie, storage system fails to return the same data it stored) at the cost of breaking one's database when the storage system acts as it's expected and documented to in a routine power-loss situation. So no, I don't care that MSSQL is unable to handle this. This patch must, or it doesn't go in. 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] Simple postgresql.conf wizard
Jonah H. Harris wrote: Oracle already thought of that a long time ago, which is why the plan has to come out better for it to take effect. Huh? We would never willingly choose a worse plan, of course, but the point is that what looks like a better plan, with a smaller cost estimate, is sometimes actually worse. As for bad plans, you obviously haven't used Postgres in production enough to deal with it continually changing plans for the worse due to index bloat, data skew, phase of the moon, etc. :) You're right, I haven't, but yes I know that's a problem. We've chatted about that with Greg sometimes. It would be nice to have more stable plans. My favorite idea is to stop using the current relation size in the planner, and use the value snapshotted at ANALYZE instead. That way, the planner would be completely deterministic, based on the statistics. Then, we could have tools to snapshot the statistics, move them to a test system, store them, revert back to old statistics etc. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] gram.y => preproc.y
Michael Meskes <[EMAIL PROTECTED]> writes: > Does anyone see a problem with these changes? Or else I will commit. preproc.y should be removed by make maintainer-clean. For consistency it probably ought to be listed in the distprep target too, even though it would be indirectly updated by the preproc.c target. I suspect it would be a good idea to list preproc.c's antecedent as $(srcdir)/preproc.y not just preproc.y. Also, you'll need to add preproc.y to .cvsignore. 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
[HACKERS] xmlconcat(NULL) crash
Due to some code reshuffling, xmlconcat(NULL) will crash in 8.3 and 8.4.
The pfree(buf.data) in ExecEvalXml() frees a buffer that is actuall
only initialized in the XMLFOREST case. So then the xmlconcat(NULL)
falls through to the pfree() it crashes. I have attached a patch for
8.3 and 8.4 to clean this up.
diff -cr ../cvs-pgsql/src/backend/executor/execQual.c
./src/backend/executor/execQual.c
*** ../cvs-pgsql/src/backend/executor/execQual.c2008-11-03
16:31:21.0 +0200
--- ./src/backend/executor/execQual.c 2008-11-14 16:31:52.0 +0200
***
*** 3163,3175
bool *isNull, ExprDoneCond *isDone)
{
XmlExpr*xexpr = (XmlExpr *) xmlExpr->xprstate.expr;
- text *result;
- StringInfoData buf;
Datum value;
boolisnull;
ListCell *arg;
ListCell *narg;
- int i;
if (isDone)
*isDone = ExprSingleResult;
--- 3163,3172
***
*** 3195,3206
*isNull = false;
return
PointerGetDatum(xmlconcat(values));
}
}
break;
case IS_XMLFOREST:
initStringInfo(&buf);
- i = 0;
forboth(arg, xmlExpr->named_args, narg,
xexpr->arg_names)
{
ExprState *e = (ExprState *) lfirst(arg);
--- 3192,3207
*isNull = false;
return
PointerGetDatum(xmlconcat(values));
}
+ else
+ return (Datum) 0;
}
break;
case IS_XMLFOREST:
+ {
+ StringInfoData buf;
+
initStringInfo(&buf);
forboth(arg, xmlExpr->named_args, narg,
xexpr->arg_names)
{
ExprState *e = (ExprState *) lfirst(arg);
***
*** 3215,3225
argname);
*isNull = false;
}
- i++;
}
break;
- /* The remaining cases don't need to set up buf */
case IS_XMLELEMENT:
*isNull = false;
return PointerGetDatum(xmlelement(xmlExpr, econtext));
--- 3216,3240
argname);
*isNull = false;
}
}
+
+ if (*isNull)
+ {
+ pfree(buf.data);
+ return (Datum) 0;
+ }
+ else
+ {
+ text *result;
+
+ result = cstring_to_text_with_len(buf.data,
buf.len);
+ pfree(buf.data);
+
+ return PointerGetDatum(result);
+ }
+ }
break;
case IS_XMLELEMENT:
*isNull = false;
return PointerGetDatum(xmlelement(xmlExpr, econtext));
***
*** 3354,3366
break;
}
! if (*isNull)
! result = NULL;
! else
! result = cstring_to_text_with_len(buf.data, buf.len);
!
! pfree(buf.data);
! return PointerGetDatum(result);
}
/*
--- 3369,3376
break;
}
! elog(ERROR, "unrecognized XML operation");
! return (Datum) 0;
}
/*
diff -cr ../cvs-pgsql/src/test/regress/expected/xml.out
./src/test/regress/expected/xml.out
*** ../cvs-pgsql/src/test/regress/expected/xml.out 2008-09-02
15:11:11.0 +0300
--- ./src/test/regress/expected/xml.out 2008-11-14 16:50:02.0 +0200
***
*** 77,82
--- 77,94
(1 row)
+ SELECT xmlconcat(NULL);
+ xmlconcat
+ ---
+
+ (1 row)
+
+ SELECT xmlconcat(NULL, NULL);
+ xmlconcat
+ ---
+
+ (1 row)
+
SELECT xmlelement(name element,
xmlattributes (1 as one, 'deuce' as two),
'content');
diff -cr ../cvs-pgsql/src/test/regress/expected/xml_1.
Re: [HACKERS] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 2:52 AM, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: >> Other systems do it. For example, Oracle tracks column usage and >> attempts to determine the optimal statistics for that column (based on >> the queries that used it) on an iterative basis. We don't track >> column usage at all, so that option wouldn't be quite that easy to >> implement. Though, there are certain things ANALYZE would be able to >> determine with a little help, such as knowing to collect more samples >> for columns it finds extremely skewed data in. > > That kind of feedback loops are a bit dangerous. For starters, it would mean > that your test system would behave differently than your production system, > just because you run different queries on it. There's also all kinds of > weird dynamic behaviors that could kick in. For example, a query could run > fine for the first few hundred times, but then the analyzer notices that a > certain column is being accessed frequently and decides to increase the > stats target for it, which changes the plan, for worse. Usually the new plan > would be better, but the planner isn't perfect. Oracle already thought of that a long time ago, which is why the plan has to come out better for it to take effect. As for bad plans, you obviously haven't used Postgres in production enough to deal with it continually changing plans for the worse due to index bloat, data skew, phase of the moon, etc. :) -- Jonah H. Harris, Senior DBA myYearbook.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Brittleness in regression test setup
I have experienced some brittleness in the regression test setup that causes the tests to be run against a different server instance or fail in confusing ways when you have multiple instances running. For some historic reasons, I have my local scripts set up so that they build development instances using the hardcoded port 65432. This will cause a temp install regression test to attempt to use port 565432 which will be rejected silently by pg_regress, which will then use its hardcoded default 65432 (no relation to my 65432). If I have some other instance already running on 65432, then this will fail in non-reassuring ways such as == removing existing temp installation== == creating temporary installation== == initializing database system == == starting postmaster== running on port 65432 with pid 94178 == creating database "regression" == ERROR: database "regression" already exists It evidently failed to realize that there is another postmaster already running at that port and just ran its test setup routines against that other instance. If there is no database named "regression" on that other instance, then it will happily go ahead and run its full test suite against that other instance. I see two problems here: 1) It fails to realize that it could not start its own temp instance when another instance is already running. 2) It ignores the port specification almost silently. Since ports above 49152 are for private use, I think it is valid to configure test instances in that port range, but our regression test setup does not handle that port range very well. So even if I configured my local scripts to use ports that are all different from each other and from 65432, this problem would still exist. So, also, 2a) It has an undocumented hardcoded default port. Any thoughts on how to fix this? -- 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] CREATE AGGREGATE disallows STYPE = internal
"Robert Haas" <[EMAIL PROTECTED]> writes: > I would feel a lot better about it if we could invent some way of > distinguishing between different types of "internal", based on what is > actually being pointed to. Yeah, we discussed that awhile ago, but nothing's been done about it. At this point I doubt it will happen for 8.4. 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] CREATE AGGREGATE disallows STYPE = internal
Teodor Sigaev <[EMAIL PROTECTED]> writes: > Does that mean that it's possible to use as STYPE pointer to complex > C-structure > with internal pointers? That's exactly what array_agg is doing. You have to be careful about which context you keep the data in ... 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] Simple postgresql.conf wizard
On Fri, 2008-11-14 at 02:21 +, Gregory Stark wrote: > Simon Riggs <[EMAIL PROTECTED]> writes: > > > Your factual comments are accurate, but for Josh's stated target of Data > > Warehousing, a stats target of 400 is not unreasonable in some cases. > > What you forget to mention is that sample size is also determined by > > stats target and for large databases this can be a more important > > consideration than the points you mention. > > Even for data warehousing I would not recommend setting it as a *default* > statistics target, at least not without verifying that it doesn't cause any > problems. I agree with you that it would be likely misused by many people, simply because the term "data warehouse" is a very loosely defined concept. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Updates of SE-PostgreSQL 8.4devel patches (r1197)
On Thu, 2008-11-13 at 10:44 +0900, KaiGai Kohei wrote: > It's unclear for me what is the point you said. > I guess you concern the fixed length field is always allocated in > the case when any security feature is not enabled also, or performance > degradation on the large scale databases. > If incorrect, please tell me in another expression. > > At first, the fixed length 4 byte field is allocated only when > the SE-PostgreSQL (or other security feature) is enabled. It can be > controlled via PGACE hook. The pgaceSecurityAttributeNecessary() can > return bool value, and it indicates the necessity of the security > field. If SE-PostgreSQL is disabled on compile-time or run-time, > the fixed length 4 byte value is not allocated. I'm sorry for not making my thoughts clearer. Let me try again: As I understand it, when enabled, the overhead for each row is more than 4 bytes because you include a text field also, which you say has a restricted number of values. IMHO the overhead is unacceptable, given that our row overhead is already high. I would prefer to make the maximum overhead per row 4 bytes only, which matches the maximum number of required labels. This will allow very large databases to use this feature. I would also like to see the feature part of normal Postgres, rather than as a compile time option. The per-row overhead would then be optional, just as WITH OIDS is optional. This would allow many applications to take advantage of row level security, without the need for switching to a different executable and without the need to enable it for every table. For high security applications, default_row_security = on would obviously be a requirement. With a single executable on all distros we will have more robust software and it will be easier to configure and use. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] CREATE AGGREGATE disallows STYPE = internal
> You can certainly screw yourself up by connecting some incompatible > internal-accepting functions together this way. So what I propose is > that we allow STYPE = internal to be specified in CREATE AGGREGATE, > but only by superusers, who are trusted not to create security holes > anyway. > > Comments? To me, that sounds like a foot-gun you could take out a tank with. I would feel a lot better about it if we could invent some way of distinguishing between different types of "internal", based on what is actually being pointed to. ...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] Sometimes pg_dump generates dump which is not restorable
Thank you for a possible solution. But what about the database which exists and works correctly (and conforms all the standards from the documentation), but dump+restore sequence is failed for it? Does it mean that pg_dump should be improved to pass dump+restore sequence? Besides that, for pg_dump has corresponding behaviour CONSTRAINT = FOREIGN KEY. For CONSTRAINT = CHECK - it hasn't. On Thu, Nov 13, 2008 at 9:07 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Dmitry Koterov" <[EMAIL PROTECTED]> writes: > > 3. The function a() calls any OTHER function b() from OTHER namespace (or > > uses operators from other namespaces), but does not specify the schema > name, > > because it is in database search_path: > > > CREATE FUNCTION a(i integer) RETURNS boolean AS $$ > > BEGIN > > PERFORM b(); -- b() is is from "nsp" schema > > RETURN true; > > END;$$ LANGUAGE plpgsql IMMUTABLE; > > I think your function is broken. You might want to fix it by attaching > a local search_path setting to it. > >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] WIP: Column-level Privileges
Alvaro Herrera wrote: > Alvaro Herrera wrote: > > > I'll probably fix both things and submit an updated version tomorrow. > > Here it is. Really attached this time. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. colprivs_wip.2008111401.patch.gz Description: Binary data -- 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: Column-level Privileges
Alvaro Herrera wrote:
> I'll probably fix both things and submit an updated version tomorrow.
Here it is. This applies cleanly to current CVS HEAD and passes the
regression tests. Apart from fixing the conflicts, I updated psql's \z
with the new array aggregate, and changed the Schema_pg_* declarations
in pg_attribute.h to contain initializators for attacl (I'm not sure
that { 0 } is the correct initializator, but it seems better than
omitting it completely). Also tacked _null_ at the end of the DATA
lines.
I didn't check the rest of the code, so don't count this as a review.
--
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] gram.y => preproc.y
On Fri, Nov 14, 2008 at 09:03:34AM -0300, Alvaro Herrera wrote: > > +$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y > > + $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@ > > Why are you passing $(srcdir) as a parameter here? It doesn't look like > the script uses it at all, or does it? The new version does. I need to tell the script where to find the other files in a VPATH build system. 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: michaelmeskes, Jabber: [EMAIL PROTECTED] 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] libpq-events windows gotcha
4. what we do now, but document loudly that PGEventProc must be static. If it can't be referenced outside the DLL directly then the issue can't arise. If you need the function address for calls to PQinstanceData, an implementor can create a public function that returns their private PGEventProc address. Do you have a preference form the list above, or an another idea? If not, we would probably implement #1. Although, the simplest thing is #4 which leaves everything as is and updates the sgml docs with a strong warning to implementors. On the whole I vote for #4 out of these. I attached a patch for the docs. Its documented as a NOTE to the PGEventProc. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ Index: doc/src/sgml/libpq.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.269 diff -C6 -r1.269 libpq.sgml *** doc/src/sgml/libpq.sgml 13 Nov 2008 09:45:24 - 1.269 --- doc/src/sgml/libpq.sgml 14 Nov 2008 12:08:07 - *** *** 5252,5263 --- 5252,5275 A particular event procedure can be registered only once in any PGconn. This is because the address of the procedure is used as a lookup key to identify the associated instance data. + + + + On windows, functions can have two different addresses: one accessed + from outside a DLL, obtained from the import library, and another from + inside a DLL. For this reason, implementors should never directly expose + an event procedure. If the event procedure is needed by an API user, + then its address should be returned by a library function; ie. + PGEventProc MyGetEventProc(void);. This ensures that + the application and DLL are always using the same address. + + -- 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] gram.y => preproc.y
Michael Meskes wrote: > +$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y > + $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@ Why are you passing $(srcdir) as a parameter here? It doesn't look like the script uses it at all, or does it? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1216)
I updated the patch set of SE-PostgreSQL (revision 1216). There are no new features and bugfixes, but it was rebased to the latest CVS HEAD, because the previous r1206 got a few conflictions. These patches are ready for reviewing. We need your help to make progress them so much! [1/6] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1216.patch [2/6] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r1216.patch [3/6] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1216.patch [4/6] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1216.patch [5/6] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1216.patch [6/6] http://sepgsql.googlecode.com/files/sepostgresql-row_acl-8.4devel-3-r1216.patch Draft of the SE-PostgreSQL documentation is here: http://wiki.postgresql.org/wiki/SEPostgreSQL I could get a help from a native English speaker who works in SELinux community as a documentation maintainer, so I'm also updating the description now. If you notice anything, please feel free to comment also. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <[EMAIL PROTECTED]> -- 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] CREATE AGGREGATE disallows STYPE = internal
internal-accepting functions together this way. So what I propose is that we allow STYPE = internal to be specified in CREATE AGGREGATE, but only by superusers, who are trusted not to create security holes anyway. Does that mean that it's possible to use as STYPE pointer to complex C-structure with internal pointers? If yes then performance of ts_stat() could be significantly increased. -- Teodor Sigaev E-mail: [EMAIL PROTECTED] WWW: http://www.sigaev.ru/ -- 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] Simple postgresql.conf wizard
Josh Berkus wrote: > Greg, > > BTW, I think this is still in enough flux that we really ought to make > it a pgfoundry project. I don't think we'll have anything ready for 8.4 > contrib. [Been trying to find the right point to post this reply.] Is it only me that thinks this should be a service on the website too (or even first)? Fill in web form, click button, get sample postgresql.conf (with comments) back. Add a tick-box asking if we can keep a copy of their answers and you might get some useful usage info too. -- 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] Walsender process patch v1 for Synch Rep
On Tue, Nov 11, 2008 at 9:12 AM, Simon Riggs <[EMAIL PROTECTED]> wrote: > > On Mon, 2008-11-10 at 18:22 +0900, Fujii Masao wrote: > >> Yeah, I also add walsender as new auxiliary process at first. But, >> during coding, >> I made out that this is more complicated for code and user. >> >> First problem : Which process accepts the connection from standby? >> IMO, *postmaster* should accept it like normal database access. Since >> we >> can use the existing connection establishment logic, the change of the >> code >> is smaller and it's easier to use. So, I added walsender as a special >> backend >> which is forked when standby connects to postmaster. Is there any >> advantage >> which walsender or other processes accept the connection from standby? > >> Second problem : What should walsender do after the termination of the >> connection from standby? should die?, or remain alive and wait for >> next >> connection? IMO, we should handle it like normal database access, i.e. >> exit walsender. This and adding walsender as an auxiliary process >> seldom >> meet, I think. >> >> Does that answer you? Am I missing something? > > It's good to see your reasons written down. > > OK, I think I could like this way around. The "walsender" database > allows us to enforce restrictions in pg_hba.conf. Also avoids needing to > run a long running transaction to initiate wal sending feature, if we do > it just with user function. I'd like to see a longer README explaining > these design aspects though. OK, thanks. I'll try to write them. -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] Block-level CRC checks
On Thu, Nov 13, 2008 at 09:03:41PM -0300, Alvaro Herrera wrote: > The first idea that comes to mind is skipping hint bits in the CRC too. > That does away with a lot of the trouble (PD_UNLOGGED_CHANGE, the > necessity of WAL-logging hint bits, etc). The problem, again, is that > the checksumming process becomes page type-specific; but then maybe > that's the only workable approach. Which brings back the problem of having to decode the page to checksum it, so your checksumming code needs to have all sorts of failsafes in it to stop it going crazy on bad data. But I understand the problem is that you want to continue in the face of torn pages, something which is AFAICS ambitious. At least MS-SQL just blows up on a torn page, havn't found results for other databases... Have a nice day, -- Martijn van Oosterhout <[EMAIL PROTECTED]> 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] gram.y => preproc.y
On Thu, Nov 13, 2008 at 03:10:04PM -0500, Tom Lane wrote:
> VPATH build. (Parts of this patch have that right and part don't.
> You might want to test in a VPATH build before committing.)
Did that and fixed the remaining problems. Attached you'll find the latest
version. I already committed the new files, so the patch is way smaller.
However, the new files are not used without the attached changes to the build
system.
Does anyone see a problem with these changes? Or else I will commit.
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: michaelmeskes, Jabber: [EMAIL PROTECTED]
Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL!
diff --exclude CVS -ruN pgsql/src/interfaces/ecpg/preproc/Makefile pgsql/src.mm/interfaces/ecpg/preproc/Makefile
--- pgsql/src/interfaces/ecpg/preproc/Makefile 2008-11-14 11:01:55.0 +0100
+++ pgsql/src.mm/interfaces/ecpg/preproc/Makefile 2008-11-14 10:59:44.0 +0100
@@ -52,6 +52,9 @@
@$(missing) flex $< $@
endif
+$(srcdir)/preproc.y: $(top_srcdir)/src/backend/parser/gram.y
+ $(PERL) $(srcdir)/parse.pl $(srcdir) < $< > $@
+
ecpg_keywords.o c_keywords.o keywords.o preproc.o parser.o: preproc.h
# instead of maintaining our own list, take the one from the backend
diff --exclude CVS -ruN pgsql/src/tools/msvc/Solution.pm pgsql/src.mm/tools/msvc/Solution.pm
--- pgsql/src/tools/msvc/Solution.pm 2008-08-17 09:09:16.0 +0200
+++ pgsql/src.mm/tools/msvc/Solution.pm 2008-11-13 15:15:50.0 +0100
@@ -239,6 +239,19 @@
if (
IsNewer(
+'src\interfaces\ecpg\preproc\preproc.y',
+'src\backend\parser\gram.y'
+)
+ )
+{
+print "Generating preproc.y...\n";
+chdir('src\interfaces\ecpg\preproc');
+system('perl parse.pl . < ..\..\..\backend\parser\gram.y > preproc.y');
+chdir('..\..\..\..');
+}
+
+if (
+IsNewer(
'src\interfaces\ecpg\include\ecpg_config.h',
'src\interfaces\ecpg\include\ecpg_config.h.in'
)
--
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] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 9:48 AM, Simon Riggs <[EMAIL PROTECTED]> wrote: > > On Fri, 2008-11-14 at 08:57 +, Dave Page wrote: >> On Fri, Nov 14, 2008 at 8:10 AM, Simon Riggs <[EMAIL PROTECTED]> wrote: >> > >> > On Fri, 2008-11-14 at 02:21 +, Gregory Stark wrote: >> > >> >> On the other hand what does occur to me in retrospect is that I regret >> >> that I didn't think about how I was disparaging the importance of >> >> mental illness and hope nobody took offense for that reason. >> > >> > Your comments surprise me because you mentioned to me privately that you >> > disliked on-list bullies. >> >> It hardly seems like bullying to me - a tongue-in-cheek humorous >> remark to someone many of us, including Greg, have known and worked >> with for years. If it were made to a newbie to the community, that >> would be another matter. > > I was not the one complaining about bullying comments. No, but you drew the comparison. > It seems a strange moderation policy that allows bald insults from some > people but not others. And stranger still that you think you should leap > to the defence of any person making them. If the comments were meant so > lightly, it would seem easy to withdraw them also, with a smile. No stranger than you leaping to demand an apology. My point was, that I didn't see an deliberate insult, and by the sound of it neither did Josh. I saw a humorous response that was unfortunately missing a smiley to make the intent clear for those that haven't been in these parts for years. I would have thought that you too would know Greg well enough to know that he wasn't trying to be rude but was making a joke. Anyhoo, I don't think this is worth wasting any more bandwidth so I'll shut up now. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.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] Simple postgresql.conf wizard
On Fri, 2008-11-14 at 08:57 +, Dave Page wrote: > On Fri, Nov 14, 2008 at 8:10 AM, Simon Riggs <[EMAIL PROTECTED]> wrote: > > > > On Fri, 2008-11-14 at 02:21 +, Gregory Stark wrote: > > > >> On the other hand what does occur to me in retrospect is that I regret > >> that I didn't think about how I was disparaging the importance of > >> mental illness and hope nobody took offense for that reason. > > > > Your comments surprise me because you mentioned to me privately that you > > disliked on-list bullies. > > It hardly seems like bullying to me - a tongue-in-cheek humorous > remark to someone many of us, including Greg, have known and worked > with for years. If it were made to a newbie to the community, that > would be another matter. I was not the one complaining about bullying comments. It seems a strange moderation policy that allows bald insults from some people but not others. And stranger still that you think you should leap to the defence of any person making them. If the comments were meant so lightly, it would seem easy to withdraw them also, with a smile. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Visibility map, partial vacuums
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > The next question is whether the "pending rel deletion" stuff in smgr.c should > be moved to the new file too. It seems like it would belong there better. That > would leave smgr.c as a very thin wrapper around md.c Well it's just a switch, albeit with only one case, so I wouldn't expect it to be much more than a thin wrapper. If we had more storage systems it might be clearer what features were common to all of them and could be hoisted up from md.c. I'm not clear there are any though. Actually I wonder if an entirely in-memory storage system would help with the "temporary table" problem on systems where the kernel is too aggressive about flushing file buffers or metadata. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- 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] Simple postgresql.conf wizard
On Fri, Nov 14, 2008 at 8:10 AM, Simon Riggs <[EMAIL PROTECTED]> wrote: > > On Fri, 2008-11-14 at 02:21 +, Gregory Stark wrote: > >> On the other hand what does occur to me in retrospect is that I regret >> that I didn't think about how I was disparaging the importance of >> mental illness and hope nobody took offense for that reason. > > Your comments surprise me because you mentioned to me privately that you > disliked on-list bullies. It hardly seems like bullying to me - a tongue-in-cheek humorous remark to someone many of us, including Greg, have known and worked with for years. If it were made to a newbie to the community, that would be another matter. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.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] Visibility map, partial vacuums
Heikki Linnakangas wrote: Another thing that does need to be fixed, is the way that the extension and truncation of the visibility map is handled; that's broken in the current patch. I started working on the patch a long time ago, before the FSM rewrite was finished, and haven't gotten around fixing that part yet. We already solved it for the FSM, so we could just follow that pattern. The way we solved truncation in the FSM was to write a separate WAL record with the new heap size, but perhaps we want to revisit that decision, instead of adding again new code to write a third WAL record, for truncation of the visibility map. smgrtruncate() writes a WAL record of its own, if any full blocks are truncated away of the FSM, but we needed a WAL record even if no full blocks are truncated from the FSM file, because the "tail" of the last remaining FSM page, representing the truncated away heap pages, still needs to cleared. Visibility map has the same problem. One proposal was to piggyback on the smgrtruncate() WAL-record, and call FreeSpaceMapTruncateRel from smgr_redo(). I considered that ugly from a modularity point of view; smgr.c shouldn't be calling higher-level functions. But maybe it wouldn't be that bad, after all. Or, we could remove WAL-logging from smgrtruncate() altogether, and move it to RelationTruncate() or another higher-level function, and handle the WAL-logging and replay there. In preparation for the visibility map patch, I revisited the truncation issue, and hacked together a patch to piggyback the FSM truncation to the main fork smgr truncation WAL record. I moved the WAL-logging from smgrtruncate() to RelationTruncate(). There's a new flag to RelationTruncate indicating whether the FSM should be truncated too, and only one truncation WAL record is written for the operation. That does seem cleaner than the current approach where the FSM writes a separate WAL record just to clear the bits of the last remaining FSM page. I had to move RelationTruncate() to smgr.c, because I don't think a function in bufmgr.c should be doing WAL-logging. However, RelationTruncate really doesn't belong in smgr.c either. Also, now that smgrtruncate doesn't write its own WAL record, it doesn't seem right for smgrcreate to be doing that either. So, I think I'll take this one step forward, and move RelationTruncate() to a new higher level file, e.g. src/backend/catalog/storage.c, and also create a new RelationCreateStorage() function that calls smgrcreate(), and move the WAL-logging from smgrcreate() to RelationCreateStorage(). So, we'll have two functions in a new file: /* Create physical storage for a relation. If 'fsm' is true, an FSM fork is also created */ RelationCreateStorage(Relation rel, bool fsm) /* Truncate the relation to 'nblocks' blocks. If 'fsm' is true, the FSM is also truncated */ RelationTruncate(Relation rel, BlockNumber nblocks, bool fsm) The next question is whether the "pending rel deletion" stuff in smgr.c should be moved to the new file too. It seems like it would belong there better. That would leave smgr.c as a very thin wrapper around md.c -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] gram.y => preproc.y
On Thu, Nov 13, 2008 at 03:57:49PM -0500, Tom Lane wrote: > Michael Meskes <[EMAIL PROTECTED]> writes: > > That's what I did first, but Magnus had a good reasoning to not keep > > preproc.y > > if we keep preproc.c in our tarball. And I agreed that there doesn't seem > > to be > > an advantage. > > Other than whether it *works*, you mean? > > make will not be happy if it has a dependency from preproc.c to preproc.y > and preproc.y is not there. Please don't mess with this. It doesn't complain but rebuild preproc.y which makes no sense at all. Removed preproc.y from make clean. 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: michaelmeskes, Jabber: [EMAIL PROTECTED] 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] Simple postgresql.conf wizard
On Fri, 2008-11-14 at 02:21 +, Gregory Stark wrote: > On the other hand what does occur to me in retrospect is that I regret > that I didn't think about how I was disparaging the importance of > mental illness and hope nobody took offense for that reason. Your comments surprise me because you mentioned to me privately that you disliked on-list bullies. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
