Re: [HACKERS] plperl and inline functions -- first draft
2009/11/10 Joshua Tolley eggyk...@gmail.com: Ok, updated patch attached. As far as I know, this completes all outstanding issues: Hi Joshua, I'm taking a look at this patch for the commitfest. I see that Andrew has already taken an interest in the technical aspects of the patch, so I'll focus on submission/code style/documentation. I noticed that there was a fairly large amount of bogus/inconsistent whitespace in the patch, particularly in the body of plperl_inline_handler(). Some of the lines were indented with tabs, others with spaces. You should stick with tabs. There were also a lot of lines with a whole lot of trailing whitespace at the end. See attached patch which repairs the whitespace. I see you generated the patch with git, so I recommend `git diff --check`, it'll helpfully report about some types of whitespace error. In the documentation you refer to this feature as inline functions. I think this might be mixing up the terminology ... although the code refers to inline handlers internally, the word inline doesn't appear in the user-facing documentation for the DO command. Instead they are referred to as anonymous code blocks. I think it would improve consistency if the PL/Perl mention used the same term. Apart from those minor quibbles, the patch appears to apply, compile and test fine, and work as advertised. Cheers, BJ plperl-do-whitespace.patch 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] patch - per-tablespace random_page_cost/seq_page_cost
On Sat, Nov 14, 2009 at 6:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not really convinced of that, but even if we do, so what? It's not that much code to have an extra cache watching the syscache traffic. There's an example in parse_oper.c of a specialized cache that's about as complicated as this would be. It's about 150 lines including copious comments. We didn't even bother to split it out into its own source file. Well, if it's that simple maybe it's not too bad. I'll take a look at that one. With hardwired columns, a regular catcache is all we need. But the reloptions stuff is designed to populate a struct, and once we populate that struct we have to have someplace to hang it - or I guess maybe we could reparse it on every call to cost_seqscan(), cost_index(), genericcostestimate(), etc, but that doesn't seem like a great idea. Well, no, we would not do it that way. I would imagine instead that plancat.c would be responsible for attaching appropriate cost values to each RelOptInfo struct, so it'd be more like one lookup per referenced table per query. It's possible that a cache would be useful even at that load level, but I'm not convinced. I'm not sure exactly what you mean by the last sentence, but my current design attaches the tablespace OID to RelOptInfo (for baserels only, of course) and IndexOptInfo, and the costing functions trigger the actual lookup of the page costs. I guess that might be slightly inferior to actually attaching the actualized values to the RelOptInfo, since each possible index-path needs the values for both the index and the underlying table. I will take another crack at 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] New VACUUM FULL
On Sat, 2009-11-14 at 19:25 -0500, Tom Lane wrote: As a rule of thumb, I'd recommend keeping as much complexity as possible *out* of gram.y. It's best if that code just reports the facts, ie what options the user entered. Deriving conclusions from that (like what default behaviors should be used) is best done later. One example of why is that if you want the defaults to depend on GUC settings then that logic must *not* happen in gram.y, since the parse tree might outlive the current GUC settings. I was referring to (in vacuum()): + if (vacstmt-options (VACOPT_INPLACE | VACOPT_REPLACE | VACOPT_FREEZE)) + vacstmt-options |= VACOPT_VACUUM; + if (vacstmt-va_cols) + vacstmt-options |= VACOPT_ANALYZE; I think what you say applies to VACOPT_ANALYZE, which is implied when columns are supplied by the user but ANALYZE is not specified explicitly. In that case it should be set in vacuum() but not in gram.y (unless specified by the user). However, for VACOPT_VACUUM, I think that's still in the territory of gram.y. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TRIGGER with WHEN clause
KaiGai Kohei wrote:
Itagaki Takahiro wrote:
Here is a updated TRIGGER with WHEN cluase patch.
I adjusted it to work with recent parser cleanup (*OLD* and *NEW*).
I would like to volunteer for reviewing the patch in RR-stage.
It seems to me an interesting feature.
It seems to me you did an impressive work.
However, I could find a few matters in this patch, as follows:
* It does not prevent set up a conditional statement trigger.
I'm uncertain how Oracle handles the condition on the statement
triggers. But it seems to me WHEN clause on the statement triggers
are nonsense.
If you have any opposition, the CreateTrigger() should raise an error
when a valid stmt-whenClause is given for statement triggers.
In addition, it makes a server crash. :-)
postgres=# CREATE or replace function trig_func() returns trigger
language plpgsql as
'begin raise notice ''trigger invoked''; return null; end';
CREATE FUNCTION
postgres=# CREATE TRIGGER t1_trig BEFORE update ON t1
WHEN (true) EXECUTE PROCEDURE trig_func();
^^^
CREATE TRIGGER
postgres=# update t1 set b = b;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The core dump says:
(gdb) bt
#0 0x08195396 in TriggerEnabled (trigger=0x9c7a0c4, event=10,
modifiedCols=0x9c484bc, estate=0x0,
tupdesc=0x0, oldtup=0x0, newtup=0x0) at trigger.c:2376
#1 0x08196783 in ExecBSUpdateTriggers (estate=0x9c79f44, relinfo=0x9c79fec)
at trigger.c:2040
#2 0x081cd4e9 in fireBSTriggers (node=value optimized out) at
nodeModifyTable.c:593
#3 ExecModifyTable (node=value optimized out) at nodeModifyTable.c:657
#4 0x081b70b8 in ExecProcNode (node=0x9c7a190) at execProcnode.c:359
#5 0x081b5c0d in ExecutePlan (dest=value optimized out, direction=value
optimized out,
numberTuples=value optimized out, sendTuples=value optimized out,
operation=value optimized out, planstate=value optimized out,
estate=value optimized out)
at execMain.c:1189
#6 standard_ExecutorRun (dest=value optimized out, direction=value
optimized out,
numberTuples=value optimized out, sendTuples=value optimized out,
operation=value optimized out, planstate=value optimized out,
estate=value optimized out)
at execMain.c:278
#7 0x0827a016 in ProcessQuery (plan=value optimized out, sourceText=value
optimized out,
params=value optimized out, dest=0x9c72024, completionTag=0xbfb8a51e
) at pquery.c:196
#8 0x0827a24e in PortalRunMulti (portal=0x9beabec, isTopLevel=value
optimized out,
dest=value optimized out, altdest=0x9c72024, completionTag=0xbfb8a51e
) at pquery.c:1269
#9 0x0827a9d4 in PortalRun (portal=0x9beabec, count=2147483647,
isTopLevel=36 '$', dest=0x9c72024,
altdest=0x9c72024, completionTag=0xbfb8a51e ) at pquery.c:823
#10 0x08276cf0 in exec_simple_query (query_string=value optimized out) at
postgres.c:1046
#11 0x08277f43 in PostgresMain (argc=2, argv=0x9bcfb70, username=0x9bcfb40
kaigai) at postgres.c:3624
#12 0x08241198 in BackendRun (port=value optimized out) at postmaster.c:3366
#13 BackendStartup (port=value optimized out) at postmaster.c:3073
#14 ServerLoop (port=value optimized out) at postmaster.c:1399
#15 0x08243bd8 in PostmasterMain (argc=1, argv=0x9bcda18) at postmaster.c:1064
#16 0x081e3d5f in main (argc=1, argv=0x9bcda18) at main.c:188
2368 /* Check for WHEN clause */
2369 if (trigger-tgqual)
2370 {
2371 ExprContext*econtext;
2372 List *predicate;
2373 TupleTableSlot *oldslot = NULL;
2374 TupleTableSlot *newslot = NULL;
2375
2376 *econtext = GetPerTupleExprContext(estate);
2377
2378 predicate = list_make1(ExecPrepareExpr((Expr *)
trigger-tgqual, estate));
2379
* the documentation seems to me misleading.
varlistentry
+ termreplaceable class=parametercondition/replaceable/term
+ listitem
+ para
+ Any acronymSQL/acronym conditional expression (returning
+ typeboolean/type). Only literalFOR EACH ROW/literal triggers
+ can refer literalNEW/ and literalOLD/ tuples.
+ literalINSERT/literal trigger can refer literalNEW/,
+ literalDELETE/literal trigger can refer literalOLD/,
+ and literalUPDATE/literal trigger can refer both of them.
+ /para
+ /listitem
+/varlistentry
It saids, NEW and OLD are only available and ...
o INSERT can refer NEW
o UPDATE can refer NEW, OLD
o DELETE can refer OLD
But, it may actually incorrect, if user gives several events on a certain
trigger. For example, when a new trigger is invoked for each row on INSERT
or UPDATE statement, the function cannot refer the OLD.
+ if (TRIGGER_FOR_ROW(tgtype))
+ {
+ RangeTblEntry *rte;
+
+
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
2009/11/14 Merlin Moncure mmonc...@gmail.com: On Sat, Nov 14, 2009 at 3:24 PM, Tom Lane t...@sss.pgh.pa.us wrote: This might look neat but I don't think it's actually useful for any production application. We'd need to find some way of expressing it that allows caching of the expression plans. But really I think the entire approach is pretty much backwards from an efficiency standpoint. I would sooner have some sort of primitive changed_columns(NEW, OLD) that spits out a list of the names of changed columns (or maybe the not-changed ones, not sure). It would not require any fundamental restructuring and it would run several orders of magnitude faster than you could ever hope to do it at the plpgsql level. huge +1 to this. This problem comes up all the time...I was in fact this exact moment working on something just like Florian for table auditing purposes...comparing new/old but needing to filter out uninteresting columns. One of those things that should be a SMOP but isn't ;-). I worked out a plpgsql approach using dynamic sql...performance wasn't _that_ bad, but any speedup is definitely welcome. The way I did it was to pass both new and old to a function as text, and build an 'is distinct from' from with the interesting field list querying out fields from the expanded composite type...pretty dirty. isn't better job for TRIGGER WHEN clause Pavel merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] add more frame types in window functions (ROWS)
2009/11/15 Tom Lane t...@sss.pgh.pa.us: Andrew Gierth and...@tao11.riddles.org.uk writes: (A question here: the spec allows (by my reading) the use of parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat the values as Exprs, albeit very limited ones, and eval them at startup rather than assuming we know the node type and digging down into it all over the place?) Seems like you might as well allow any expression not containing local Vars. Compare the handling of LIMIT. Hmm, I've read it wrong, was assuming a constant for unsigned value specification which actually includes any expression. But it's a fixed value during execution, right? Otherwise, we cannot predicate frame boundary. Regards, -- Hitoshi Harada -- 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] add more frame types in window functions (ROWS)
Thanks for your review. 2009/11/15 Andrew Gierth and...@tao11.riddles.org.uk: Hi, I've started reviewing your patch. I've already found some things that need work: - missing _readWindowFrameDef function (all nodes that are output from parse analysis must have both _read and _out functions, otherwise views can't work) I added _outWindowFramedef() but seem to forget _read one. Will add it. - the A_Const nodes should probably be transformed to Const nodes in parse analysis, since A_Const has no _read/_out functions, which means changing the corresponding code in the executor. Thanks for this comment. I hadn't determined which node should be used as a value node passed to executor. Including Tom's comment, I must consider which should be again. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] NULL input for array_agg()?
Hi, During reviewing aggregates ORDER BY, I was reading spec and found description like: == snip == Of the rows in the aggregation, the following do not qualify: — If DISTINCT is specified, then redundant duplicates. — Every row in which the value expression evaluates to the null value. == /snip == ... and array_agg() is among the functions that description above refers to. So I wonder if array_agg doesn't accept NULL input (with STRICT trans function). Did we discussed about this issue when implementing it for 8.4? Regards, -- Hitoshi Harada -- 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] add more frame types in window functions (ROWS)
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes: (A question here: the spec allows (by my reading) the use of parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat the values as Exprs, albeit very limited ones, and eval them at startup rather than assuming we know the node type and digging down into it all over the place?) Seems like you might as well allow any expression not containing local Vars. Compare the handling of LIMIT. Hitoshi Hmm, I've read it wrong, was assuming a constant for unsigned value Hitoshi specification which actually includes any expression. But it's a Hitoshi fixed value during execution, right? Otherwise, we cannot predicate Hitoshi frame boundary. The spec doesn't allow arbitrary expressions there, only literals and parameters. Allowing more than that would be going beyond the spec, but as with LIMIT, could be useful nonetheless. For it to be a fixed value during execution, the same rules apply as for LIMIT; it can't contain Vars of the current query level. My thinking is that the executor definitely shouldn't be relying on it being a specific node type, but should just ExecEvalExpr it on the first call and store the result; then you don't have to know whether it's a Const or Param node or a more complex expression. -- 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] add more frame types in window functions (ROWS)
2009/11/15 Andrew Gierth and...@tao11.riddles.org.uk: My thinking is that the executor definitely shouldn't be relying on it being a specific node type, but should just ExecEvalExpr it on the first call and store the result; then you don't have to know whether it's a Const or Param node or a more complex expression. Yeah, so that we allow something like ROWS BETWEEN 1 + $1 PRECEDING AND ... And to support RANGE BETWEEN n PRECEDING ..., we should make datum to add or to subtract from current value on initial call anyway. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] patch - Report the schema along table name in a referential failure error message
I've put together a small patch to provide a schema name in an fk violation in deference to the todo item Report the schema along table name in a referential failure error message The error message only contains the schema if the table name being referenced is non-unique or not present in the search_path. It passes a make check, and I've added a couple of test cases which expect the schema's output in the cases mentioned above. Also, it looks like Rev 1.113 added spaces to the values specified in errdetail for failed FK violations, but the testoutput wasn't updated. I haven't included that in this patch for clarity, but it probably should be corrected. Have at it, -George fk_schemas.patch 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] NULL input for array_agg()?
Hitoshi == Hitoshi Harada umi.tan...@gmail.com writes: Hitoshi Hi, During reviewing aggregates ORDER BY, I was reading spec Hitoshi and found description like: Hitoshi == snip == Hitoshi Of the rows in the aggregation, the following do not qualify: Hitoshi — If DISTINCT is specified, then redundant duplicates. Hitoshi — Every row in which the value expression evaluates to the null value. Hitoshi == /snip == Where did you find that? The SQL2008 last-call draft says this: 4) If general set function is specified, then: a) Let TX be the single-column table that is the result of applying the value expression to each row of T1 and eliminating null values. If one or more null values are eliminated, then a completion condition is raised: warning -- null value eliminated in set function. b) Case: i) If DISTINCT is specified, then let TXA be the result of eliminating redundant duplicate values from TX, using the comparison rules specified in Subclause 8.2, comparison predicate, to identify the redundant duplicate values. ii) Otherwise, let TXA be TX. [more subclauses of rule (4) snipped as irrelevant] 8) If array aggregate function is specified, then: a) If sort specification list is specified, then let K be the number of sort keys; otherwise, let K be 0 (zero). b) Let TXA be the table of K+1 columns obtained by applying the value expression immediately contained in the array aggregate function to each row of T1 to obtain the first column of TXA, and, for all i, 1 (one) i K, applying the value expression simply contained in the i-th sort key to each row of T1 to obtain the (i+1)-th column of TXA. c) Let TXA be ordered according to the values of the sort keys found in the second through (K+1)-th columns of TXA. If K is 0 (zero), then the ordering of TXA is implementation-dependent. d) Let N be the number of rows in TXA. e) If N is greater than IDMC, then an exception condition is raised: data exception -- array data, right truncation. f) Let Ri, 1 (one) i N, be the rows of TXA according to the ordering of TXA. g) Case: i) If TXA is empty, then the result of array aggregate function is the null value. ii) Otherwise, the result of array aggregate function is an array of N elements such that for all i, 1 (one) i N, the value of the i-th element is the value of the first column of Ri. NOTE 267 -- Null values are not eliminated when computing array aggregate function. This, plus the optional sort specification list, sets array aggregate function apart from general set functions. array_agg is an array aggregate function (in fact the only such), whereas general set function includes almost all the other single-arg aggregates (avg, min, max, etc.) -- Andrew (irc:RhodiumToad) -- 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] cvs head doesn't pass make check on one of the machines here
On fre, 2009-11-13 at 14:39 -0500, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: On fre, 2009-11-13 at 15:05 +0100, Grzegorz Jaśkiewicz wrote: As per Tom's - yes, this laptop has LANG set to UTF8 Polish. Setting it back to EN actually makes this error go away. The Polish locale isn't supported by the regression tests. With only one result-ordering difference, it seems like we could easily support that if there were enough demand. I'd want somebody running a buildfarm machine in Polish locale, though, to catch future breakages. Yeah, I don't mind, as long as someone can personally verify that the current results are actually correct. When I fixed most of the other locales a while back, most of the differences where like sorts q like x, which could easily be verified by, say, Wikipedia. The results of the Polish locale, however, didn't make sense to me, and the glibc locale sources are also not in line with most of the other locales (which are just standard UTF-8 locale + language differences). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] pg_ctl init extension
On tor, 2009-09-17 at 21:43 +0200, Zdenek Kotala wrote: Attached patch extends pg_ctl command with init option. pg_ctl -D /var/lib/postgres [-s] init This should replace usage of initdb command which has problematic name as we already discussed several times. Initdb binary will be still there, but it can be renamed and move into execlib dir in the future. Patch does not contains documentation changes. They will depends on decision which database initialization method will be preferred. OK, let's see. The patch is pretty straightforward, but does anyone else actually want this? Comments? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch committers
On Sat, Nov 14, 2009 at 04:55, Craig Ringer cr...@postnewspapers.com.au wrote: (I'm not sure I should piping up here, so feel free to ignore, but perhaps I have something small to offer. I've been following the list for a while, but try to keep my mouth shut.) Meh. All constructive input is welcome! On 13/11/2009 3:07 AM, Selena Deckelmann wrote: * Distributed revision control as standard for the project This would also make it a lot easier to track in-progress work on particular features of interest, allowing interested users to help with advance testing of early versions of major feature work. Chasing patches on a mailing list is not an attractive way to try to keep up with someone's in-progress work, and is demotivating to people interested in testing that work. Think: HOT, warm standby, etc. It also helps with the issue where a patch is posted, followed by short thread of corrections and changes you have to manually apply to reach (you hope) the same codebase others are testing. Sure, sometimes a follow-up patch is posted with the changes, but often not. This is probably most important for large patches, but the line where it becomes useful is very fuzzy. I think it's helpful at a much lower complexity level than most people realize. I think we should encourage more poeple to use this. How can we do that? Perhaps we can put an encouragement in the description how to submit a patch? How about we add specific feature(s) about tihs to the commitfest management tool? Like the possibility to directly link a git repo/branch with the patch? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] UTF8 with BOM support in psql
On ons, 2009-10-21 at 13:11 +0900, Itagaki Takahiro wrote: Client encoding is declared in body of a file, but BOM is in head of the file. So, we should always ignore BOM sequence at the file head no matter what client encoding is used. The attached patch replace BOM with while spaces, but it does not change client encoding automatically. I think we can always ignore client encoding at the replacement because SQL command cannot start with BOM sequence. If we don't ignore the sequence, execution of the script must fail with syntax error. I don't know what the best solution is here. The BOM encoded as UTF-8 is valid data in other encodings. Of course, there is your point that such data cannot be at the start of an SQL command. There is also the notion of how files are handled on Unix. Normally, you'd assume that all of psql -f file.sql psql file.sql cat file.sql | psql cat file1.sql file2.sql | psql behave consistently. That would require that the BOM is ignored in the middle of the data stream (which is legal and required per Unicode standard) and that this only happens if the character set is actually Unicode. Any ideas? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch committers
On Sat, Nov 14, 2009 at 4:11 AM, Magnus Hagander mag...@hagander.net wrote: How about we add specific feature(s) about tihs to the commitfest management tool? Like the possibility to directly link a git repo/branch with the patch? So two fields, one for the repo URL and one for the branch name? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Hot standby, race condition between recovery snapshot and commit
There's a race condition between transaction commit and GetRunningTransactionData(). If GetRunningTransactionData() runs between the RecordTransactionCommit() and ProcArrayEndTransaction() calls in CommitTransaction(): /* * Here is where we really truly commit. */ latestXid = RecordTransactionCommit(false); TRACE_POSTGRESQL_TRANSACTION_COMMIT(MyProc-lxid); /* * Let others know about no transaction in progress by me. Note that this * must be done _before_ releasing locks we hold and _after_ * RecordTransactionCommit. */ ProcArrayEndTransaction(MyProc, latestXid); The running-xacts snapshot will include the transaction that's just committing, but the commit record will be before the running-xacts WAL record. If standby initializes transaction tracking from that running-xacts record, it will consider the just-committed transactions as still in-progress until the next running-xact record (at next checkpoint). I can't see any obvious way around that. We could have transaction commit acquire the new RecoveryInfoLock across those two calls, but I'd like to avoid putting any extra overhead into such a critical path. Hmm, actually ProcArrayApplyRecoveryInfo() could check every xid in the running-xacts record against clog. If it's marked as finished in clog already (because we already saw the commit/abort record before the running-xacts record), we know it's not running after all. Because of the sequence that commit removes entry from procarray and releases locks, it also seems possible for GetRunningTransactionsData() to acquire a snapshot that contains an AccessExclusiveLock for a transaction, but that XID is not listed as running in the XID list. That sounds like trouble too. -- 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] UTF8 with BOM support in psql
Peter Eisentraut wrote: On ons, 2009-10-21 at 13:11 +0900, Itagaki Takahiro wrote: Client encoding is declared in body of a file, but BOM is in head of the file. So, we should always ignore BOM sequence at the file head no matter what client encoding is used. The attached patch replace BOM with while spaces, but it does not change client encoding automatically. I think we can always ignore client encoding at the replacement because SQL command cannot start with BOM sequence. If we don't ignore the sequence, execution of the script must fail with syntax error. I don't know what the best solution is here. The BOM encoded as UTF-8 is valid data in other encodings. Of course, there is your point that such data cannot be at the start of an SQL command. There is also the notion of how files are handled on Unix. Normally, you'd assume that all of psql -f file.sql psql file.sql cat file.sql | psql cat file1.sql file2.sql | psql behave consistently. That would require that the BOM is ignored in the middle of the data stream (which is legal and required per Unicode standard) and that this only happens if the character set is actually Unicode. Cases 2 and 3 should be indistinguishable from psql's POV, although case 3 wins a Useless Use of cat award. If we are only eating a BOM at the start of a file, which was the consensus IIRC, and we treat STDIN as a file for this purpose, then we would eat the leading BOM on file.sql and file1.sql in all the cases above but not on file2.sql since we would not have any idea where the file boundary was. That last case strikes me as a not very likely usage (I'm pretty sure I've never used it, at least). A file containing: \i file1.sql \i file2.sql would be the workaround if needed. As for handling the fact that client encoding can't be set in a script until after the leading BOM, there is always PGOPTIONS=-c client_encoding=UTF8 or similar. 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] [patch] pg_ctl init extension
Peter Eisentraut píše v so 14. 11. 2009 v 10:41 +0200: On tor, 2009-09-17 at 21:43 +0200, Zdenek Kotala wrote: Attached patch extends pg_ctl command with init option. pg_ctl -D /var/lib/postgres [-s] init This should replace usage of initdb command which has problematic name as we already discussed several times. Initdb binary will be still there, but it can be renamed and move into execlib dir in the future. Patch does not contains documentation changes. They will depends on decision which database initialization method will be preferred. OK, let's see. The patch is pretty straightforward, but does anyone else actually want this? Comments? Maybe we could ask on general where is more admins. I will send voting email. thanks for looking on this Zdenek -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] DTrace compiler warnings
I'm just seeing these kind of compiler warnings in current HEAD: pgstat.c: In function ‘pgstat_report_activity’: pgstat.c:2276: warning: passing argument 1 of ‘__dtrace_probe$postgresql$statement__status$v1$63686172202a’ discards qualifiers from pointer target type There are more of them (e.g. postgres.c), all passing a const char pointer. Platform is Snow Leopard, 10.6.2, gcc 4.2.1 -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DTrace compiler warnings
Hmm, const is also problem on solaris. dtrace generates probe.h file and eats const. It generates following noise on solaris build: postgres.c, line 554: warning: argument #1 is incompatible with prototype: prototype: pointer to char : ../../../src/include/utils/probes.h, line 582 argument : pointer to const char IIRC, it was discussed. I cc Robert. He should know answer why const is ignored. Zdenek Bernd Helmle píše v so 14. 11. 2009 v 14:54 +0100: I'm just seeing these kind of compiler warnings in current HEAD: pgstat.c: In function ‘pgstat_report_activity’: pgstat.c:2276: warning: passing argument 1 of ‘__dtrace_probe$postgresql$statement__status$v1$63686172202a’ discards qualifiers from pointer target type There are more of them (e.g. postgres.c), all passing a const char pointer. Platform is Snow Leopard, 10.6.2, gcc 4.2.1 -- Thanks Bernd -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch committers
On Sat, Nov 14, 2009 at 13:35, Robert Haas robertmh...@gmail.com wrote: On Sat, Nov 14, 2009 at 4:11 AM, Magnus Hagander mag...@hagander.net wrote: How about we add specific feature(s) about tihs to the commitfest management tool? Like the possibility to directly link a git repo/branch with the patch? So two fields, one for the repo URL and one for the branch name? Yeah, I think that's it. It might actually be interesting to pull the latest version date and make a note in the cf management stuff automagically in case there the git repo has a more updated version than the one that was submitted. I think that could be quite useful - shouldn't be too hard to do, I think. Probably just a cron job that updates a third col in the db? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] tsearch parser inefficiency if text includes urls or emails - new version
Andres Freund wrote: On Saturday 14 November 2009 01:03:33 Kevin Grittner wrote: It is in context format, applies cleanly, and passes make check. Unfortunately the latter is not saying much - I had a bug there and it was not found by the regression tests. Perhaps I should take a stab and add at least some more... Sounds like a good idea. The one thing to avoid is anything with a long enough run time to annoy those that run it many times a day. It is in context format, applies cleanly, and passes make check. Next I read through the code, and have the same question that Andres posed 12 days ago. His patch massively reduces the cost of the parser recursively calling itself for some cases, and it seems like the least invasive way to modify the parser to solve this performance problem; but it does beg the question of why a state machine like this should recursively call itself when it hits certain states. I was wondering about that as well. I am not completely sure but to me it looks like its just done to reduce the amount of rules and states. I'm assuming that's the reason, but didn't dig deep enough to be sure. I suspect to be really sure, I'd have to set it up without the recursion and see what breaks. I can't imagine it would be anything which couldn't be fixed by adding enough states; but perhaps they ran into something where these types would require so many new states that the recursion seemed like the lesser of evils. I have to say that that code is not exactly clear and well documented... Yeah. I was happy with the level of documentation that you added with your new code, but what was there before is mighty thin. If you gleaned enough information while working on it to feel comfortable adding documentation anywhere else, that would be a good thing. So far the only vote is to proceed with the mitigation, which was my preference, and apparently yours -- so I guess we're at 3 to 0 in favor of that. I'll mark the patch as Waiting on Author so you can add any comments and regression tests you feel are appropriate. By the way, I found one typo in the comments -- it should by useful, not usefull. I liked what I saw so far, but need to spend more time desk-checking for correctness, testing to confirm that it doesn't change results, and confirming the performance improvement. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] pg_ctl init extension
Peter Eisentraut wrote: The patch is pretty straightforward, but does anyone else actually want this? Comments? I agree that the initdb name seems odd next to the other executable names, but the functionality seems a little out of place to me in pg_ctl. The other options all correspond (more or less) to LSB init script actions (and we've been talking about the desirability of making that a closer fit); while this is something which would *not be appropriate* in an init script. We could filter this option out in the script, but it seemed like you wanted to keep the script as short and simple as possible -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [patch] pg_ctl init extension
Kevin Grittner kevin.gritt...@wicourts.gov writes: Peter Eisentraut wrote: The patch is pretty straightforward, but does anyone else actually want this? Comments? I agree that the initdb name seems odd next to the other executable names, but the functionality seems a little out of place to me in pg_ctl. The other options all correspond (more or less) to LSB init script actions (and we've been talking about the desirability of making that a closer fit); while this is something which would *not be appropriate* in an init script. Well, it's not appropriate or safe as a default action, but there already is a nonstandard service postgresql init action in at least the PGDG and Red Hat init scripts. In fact, I believe that Zdenek's entire rationale for this is predicated on the assumption that he can eventually make initdb's disappearance transparent, if he can get people used to using such a thing instead of initdb'ing by hand. 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] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote:
Florian G. Pflug f...@phlo.org writes:
Tom Lane wrote:
Trying to do this in plpgsql is doomed to failure and heartache,
Well, the proposed functions at least allow for some more
flexibility in working with row types, given that you know in
advance which types you will be dealing with (but not necessarily
the precise ordering and number of the record's fields). They might
feel a bit kludgy because of the anyelement dummy argument that
bridges the gap between the statically typed nature of SQL and the
rather dynamic RECORDs, but the kludgy-ness factor is still within
reasonable limits I think.
It sounds pretty d*mn klugy to me, and I stand by my comment that it
isn't going to work anyway. When you try it you are going to run
into parameter type doesn't match that while preparing the plan
errors.
Ok, I must be missing something. I currently fail to see how
my proposed
record_value(record, name, anyelement) returns anyelement
function differs (from the type system's point of view) from
value_from_string(text, anyelement) returns anyelement
which simply casts the text value to the given type and can easily be
implemented in plpgsq.
create or replace function
value_from_string(v_value text, v_type_dummy anyelement)
returns anyelement as
$body$
declare
v_result v_type_dummy%type;
begin
if v_value is null then
return null;
end if;
v_result := v_value;
return v_result;
end;
$body$ language plpgsql immutable;
-- Returns 124
select value_from_string('123', NULL::int) + 1;
-- returns {1,2,3,4}
select value_from_string('{1,2,3}', NULL::int[]) || array[4];
best regards,
Florian Pflug
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Florian G. Pflug f...@phlo.org writes: Ok, I must be missing something. I currently fail to see how my proposed record_value(record, name, anyelement) returns anyelement function differs (from the type system's point of view) from value_from_string(text, anyelement) returns anyelement which simply casts the text value to the given type and can easily be implemented in plpgsq. The problem is at the call site --- if you try to call it with different record types on different calls you're going to get a failure. Or so I expect 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] Hot standby, overflowed snapshots, testing
Hi Simon and Heikki, I can help set up automated basic tests for hot standby using 1+1 setups on Amazon. I¹m already working on tests for warm standby for our commercial Tungsten implementation and need to solve the problem of creating tests that adapt flexibly across different replication mechanisms. It would be nice to add a list of test cases to the write-up on the Hot Standby wiki (http://wiki.postgresql.org/wiki/Hot_Standby). I would be happy to help with that effort. Cheers, Robert On 11/13/09 1:43 PM PST, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2009-11-13 at 22:19 +0200, Heikki Linnakangas wrote: I got the impression earlier that you had some test environment set up to test hot standby. Can you share any details of what test cases you've run? Fair question. The Sep 15 submission happened too quickly for us to mobilise testers, so the final submission was submitted with only manual testing by me. Many last minute major bug fixes meant that the code was much less tested than I would have hoped - you found some of those while I lay exhausted from the efforts to hit a superimposed and unrealistic deadline. I expected us to kick in to fix those but it never happened and that was why I was keen to withdraw the patch about a week later. You've been kicking hell out of it for a while now, rightly so, so I've left it a while before commencing another set of changes and more testing to follow. It takes time, and money, to mobilise qualified testers, so that should begin again shortly. I agreed with you at PGday that we shouldn't expect a quick commit. There are good reasons for that, but still no panic in my mind about skipping this release. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] operator exclusion constraints
On Nov 13, 2009, at 8:39 PM, Robert Haas wrote: alter table foo add constraint bar exclude (a check with =, b check with =); I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] operator exclusion constraints
David E. Wheeler da...@kineticode.com writes: On Nov 13, 2009, at 8:39 PM, Robert Haas wrote: alter table foo add constraint bar exclude (a check with =, b check with =); I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. 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] Inspection of row types in pl/pgsql and pl/sql
Hello new hstore has a very nice interface for record field iteration http://okbob.blogspot.com/2009/10/dynamic-access-to-record-fields-in.html Regards Pavel Stehule 2009/11/13 Florian G. Pflug f...@phlo.org: Hi I'm currently working on a project where we need to build a global cache table containing all values of certain types found in any of the other tables. Currently, a seperate insert, update and delete (plpgsql) trigger function exists for each table in the database which is auto-generated by a (plpgsql) function which queries the system catalogs to find all fields with a certain type, and then generates the appropriate plpgsql function using EXECUTE '...'. I'd like to replace this function-generating function by a generic trigger function that works for all tables. Due to the lack of any way to inspect the *structure* of a record type, however, I'd have to use a C language function for that, which induces quite some maintenance headaches (especially if deployed on windows). I'm therefore thinking about implementing the following generate-purpose inspection functions for row types record_length(record) returns smallint Returns the number of fields in the given record. record_names(record) returns name[] Returns the names of the record's fields. Array will contain NULLs if one or more fields are unnamed. record_types(record) returns regtype[]; Returns the OIDs of the record's types. Array won't contain NULLs record_value(record, name, anyelement) returns anyelement Returns the value of a certain (named) field. The type of the third argument defines the return type (its value is ignored). The field's value is cast to that type if possible, otherwise an error is raised. record_value(record, smallint, anyelement) returns anyelement Returns the value of the field at the given position. record_values(record, regtype, anyelement) returns anyarray Returns an array of all values of all fields with the given type or whose type is a domain over the given type. No casting is done. Any comment/critique is appreciated. Would anyone else find those functions useful? best regards, Florian Pflug -- 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] Inspection of row types in pl/pgsql and pl/sql
Pavel Stehule wrote: Hello new hstore has a very nice interface for record field iteration Yes, and I have used it, but it really would be nicer to have some introspection facilities built in, especially for use in triggers. 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] operator exclusion constraints
On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
2009/11/14 Andrew Dunstan and...@dunslane.net: Pavel Stehule wrote: Hello new hstore has a very nice interface for record field iteration Yes, and I have used it, but it really would be nicer to have some introspection facilities built in, especially for use in triggers. I am not sure. PL/pgSQL is really bad language for this task. Any procedure developed in plpgsql should be pretty slow. Personally I am happy with current 8.5. If some need it, the he could to use hstore - contrib modules are not problem on every platform, but cannot generate too much general triggers simply (what is good). I understand well to motivation. But now I thinking, so general triggers are very bad and risk technique and is better do things else. If someone really need it, then he could to write C procedure. Regards Pavel Stehule 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
[HACKERS] Writeable CTE patch
Hi,
Attached is the latest version of this patch.
I altered rewriting a bit (I've brought the problems with the previous
approach up a couple of times before) and this version should have the
expected output in all situations. This patch doesn't allow you to use
INSERT/UPDATE/DELETE as the top level statement, but you can get around
that by putting the desired top-level statement in a new CTE.
Since the last patch I also moved ExecOpenIndices to nodeModifyTable.c
because the top-level executor doesn't know which result relations are
opened for which operations.
One thing which has bothered me a while is that there is no clear option
for commandType when you have a multiple types of statements in a single
Query. In some places it'd help to know that there are multiple
different statements. This is now achieved by having hasWritableCtes
variable in PlannedStmt, but that doesn't help in places where you don't
have access to (or there isn't yet one) PlannedStmt, which has lead me
to think that we could have a CMD_MULTI or a similar value to mark these
Queries. I haven't taken the time to look at this in detail, but it's
something to think about.
Regards,
Marko Tiikkaja
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index b2741bc..3aa7da5 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -1499,7 +1499,7 @@ SELECT 3, 'three';
synopsis
SELECT replaceableselect_list/replaceable FROM
replaceabletable_expression/replaceable
/synopsis
- and can appear anywhere a literalSELECT/ can. For example, you can
+ and can appear anywhere a literalSELECT/literal can. For example, you
can
use it as part of a literalUNION/, or attach a
replaceablesort_specification/replaceable (literalORDER BY/,
literalLIMIT/, and/or literalOFFSET/) to it. literalVALUES/
@@ -1529,10 +1529,11 @@ SELECT replaceableselect_list/replaceable FROM
replaceabletable_expression
/indexterm
para
- literalWITH/ provides a way to write subqueries for use in a larger
- literalSELECT/ query. The subqueries can be thought of as defining
- temporary tables that exist just for this query. One use of this feature
- is to break down complicated queries into simpler parts. An example is:
+ literalWITH/ provides a way to write subqueries for use in a
+ larger query. The subqueries can be thought of as defining
+ temporary tables that exist just for this query. One use of this
+ feature is to break down complicated queries into simpler parts.
+ An example is:
programlisting
WITH regional_sales AS (
@@ -1560,6 +1561,30 @@ GROUP BY region, product;
/para
para
+ A literalWITH/literal clause can also have an
+ literalINSERT/literal, literalUPDATE/literal or
+ literalDELETE/literal (each optionally with a
+ literalRETURNING/literal clause) statement in it. The example below
+ moves rows from the main table, foo_log into a partition,
+ foo_log_200910.
+
+programlisting
+WITH rows AS (
+DELETE FROM ONLY foo_log
+WHERE
+ foo_date gt;= '2009-10-01' AND
+ foo_date lt; '2009-11-01'
+ RETURNING *
+ ), t AS (
+ INSERT INTO foo_log_200910
+ SELECT * FROM rows
+ )
+VALUES(true);
+/programlisting
+
+ /para
+
+ para
The optional literalRECURSIVE/ modifier changes literalWITH/
from a mere syntactic convenience into a feature that accomplishes
things not otherwise possible in standard SQL. Using
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 8954693..3634d43 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -58,7 +58,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable
class=parameterexpression/replac
phraseand replaceable class=parameterwith_query/replaceable
is:/phrase
-replaceable class=parameterwith_query_name/replaceable [ (
replaceable class=parametercolumn_name/replaceable [, ...] ) ] AS (
replaceable class=parameterselect/replaceable )
+replaceable class=parameterwith_query_name/replaceable [ (
replaceable class=parametercolumn_name/replaceable [, ...] ) ] AS (
replaceable class=parameterselect/replaceable | (replaceable
class=parameterinsert/replaceable | replaceable
class=parameterupdate/replaceable | replaceable
class=parameterdelete/replaceable [ RETURNING...]))
TABLE { [ ONLY ] replaceable class=parametertable_name/replaceable [ * ]
| replaceable class=parameterwith_query_name/replaceable }
/synopsis
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 9100dd9..78d2344 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2160,7 +2160,8 @@ CopyFrom(CopyState cstate)
heap_insert(cstate-rel, tuple, mycid, hi_options,
bistate);
if (resultRelInfo-ri_NumIndices 0)
- recheckIndexes = ExecInsertIndexTuples(slot,
(tuple-t_self),
+
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Andrew Dunstan and...@dunslane.net writes: Yes, and I have used it, but it really would be nicer to have some introspection facilities built in, especially for use in triggers. Maybe, but the proposal at hand is spectacularly ugly --- in particular it seems designed around the assumption that a given trigger will only care about handling a predetermined set of datatypes, which hardly fits with PG's normal goals for datatype extensibility. If the argument is that you don't like hstore or other PLs because they'll smash everything to text, then I think you have to do better than this. 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] Unicode UTF-8 table formatting for psql text output
On Mon, Nov 09, 2009 at 05:40:54PM -0500, Bruce Momjian wrote: Tom Lane wrote: Greg Stark gsst...@mit.edu writes: While i agree this looks nicer I wonder what it does to things like excel/gnumeric/ooffice auto-recognizing table layouts and importing files. I'm not sure our old format was so great for this so maybe this is actually an improvement I'm asking for. Yeah. We can do what we like with the UTF8 format but I'm considerably more worried about the aspect of making random changes to the plain-ASCII output. On the other hand, we changed that just a release or so ago (to put in the multiline output in the first place) and I didn't hear complaints about it that time. Sorry for the delayed reply: The line continuation characters were chosen in 8.4 for clarity --- if you have found something clearer for 8.5, let's make the improvement. I think clarity is more important in this area than consistency with the previous psql output format. The attached patch is proposed for the upcoming commitfest, and hopefully takes into account the comments made in this thread. To summarise the changes: - it makes the handling of newlines and wrapped lines consistent between column header and data lines. - it includes additional logic such that both the old and new styles are representable using the format structures, so we can retain backward compatibility if so desired (it's easy to remove if not). - an ascii-old linestyle is added which is identical to the old style for those who need guaranteed reproducibility of output, but this is not the default. - The Unicode format uses ↵ in the right-hand margin to indicate newlines. Wrapped lines use … in the right-hand margin before, and left-hand margin after, a break (so you can visually follow the wrap). - The ASCII format is the same but uses + and . in place of carriage return and ellipsis symbols. - All the above is documented in the SGML documentation, including the old style, which I always found confusing. For comparison, I've included a transcript of all three linestyles with a test script (also attached). Any changes to the symbols used and/or their placement are trivially made by just altering the format in print.c. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 7f03802..4600303 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1765,18 +1765,40 @@ lo_import 152801 listitem para Sets the border line drawing style to one - of literalascii/literal or literalunicode/literal. - Unique abbreviations are allowed. (That would mean one - letter is enough.) + of literalascii/literal, literalascii-old/literal + or literalunicode/literal. Unique abbreviations are + allowed. (That would mean one letter is enough.) /para para - quoteASCII/quote uses plain acronymASCII/acronym characters. + quoteASCII/quote uses plain acronymASCII/acronym + characters. Newlines in data are shown using + a literal+/literal symbol in the right-hand margin. + Wrapped data uses a literal./literal symbol in the + right-hand margin of a wrapped line, and in the left-hand + margin of the following continuation line. /para para + quoteASCII-old/quote uses plain acronymASCII/acronym + characters, using the formatting style used + for productnamePostgreSQL/productname 8.4 and earlier. + Newlines in data are shown using a literal:/literal + symbol in place of the left-hand column separator, while + wrapped data uses a literal;/literal symbol. Newlines + in column headings are indicated by a literal+/literal + symbol in the left-hand margin of additional lines. + /para + + para quoteUnicode/quote uses Unicode box-drawing characters. - /para + Newlines in data are shown using a carriage return symbol + (literal#8629;/literal) in the right-hand margin. + Wrapped data uses an ellipsis symbol + (literal#8230;/literal) in the right-hand margin of a + wrapped line, and in the left-hand margin of the following + continuation line. + /para para When the selected output format is one that draws lines or boxes diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 190a8d3..544a677 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1795,11 +1795,13 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) ; else if (pg_strncasecmp(ascii,
Re: [HACKERS] operator exclusion constraints
On Sat, Nov 14, 2009 at 12:11 PM, David E. Wheeler da...@kineticode.com wrote: On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. I haven't thought about this too deeply, but could we allow the with = part to be optional? And would it be a good idea? Seems like you would commonly have one or more keys that exclude on equality and then the last one would use an overlap-type operator. ...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] operator exclusion constraints
Robert Haas robertmh...@gmail.com writes: I haven't thought about this too deeply, but could we allow the with = part to be optional? And would it be a good idea? I don't think so. We generally do not believe in defaulting operators based on name. If there were a way to select the standard exclusion operator based on opclass membership it might make sense, but almost by definition this facility is going to be working with unusual opclasses that might not even have an equality slot. 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] operator exclusion constraints
On Fri, 2009-11-13 at 23:39 -0500, Robert Haas wrote: [ reviewing ] Thank you for the comments so far. In index_create(), the elog() where relopxconstraints 0 should just complain about the value being negative, I think, rather than listing the value. If you just say the value is -3, it doesn't give the user a clue why that's bad. Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. I tried to make all user-visible errors into ereport()s. In ATAddOperatorExclusionConstraint(), the message method %s does not support gettuple seems a bit user-unfriendly. Can we explain the problem by referring to the functionality of getttuple(), rather than the name of it? How about operator exclusion constraints don't support method X? Then perhaps have a detail-level message to explain that the access method needs to support the gettuple interface. Trying to describe what gettuple does doesn't help a humble user much. All they care about is can't use gin. I don't really like this message, for a number of reasons. alter table foo add constraint bar exclude (a check with =, b check with =); ERROR: operator exclusion constraint violation detected: foo_a_exclusion DETAIL: Tuple (1, 1, 2) conflicts with existing tuple (1, 1, 3). The corresponding error for a UNIQUE index is: could not create unique index bar, which I like better. Only the relevant columns from the tuples are dumped, and the tuple is not surrounded by double quotes; any reason not to parallel that here? By relevant columns I assume you mean the entire index tuple. That means we need to have column names represented somewhere, because we don't want the user to have to match up ordinal index columns. Also, with exclusion constraints, both values are always relevant, not just the one being inserted. What if the user just wants to adjust their request slightly to avoid an overlap -- how do they know how far to go? I know this could be accomplished with extra queries, as well, but that doesn't always work for someone looking through the logs after the fact, when the original values may be gone. So, the kind of error message you're suggesting starts to get awkward: (a: 1 = 1, b: 1 = 1) or something? And then with more complex type output functions, and expression indexes, it starts to look very complex. What do you think is the cleanest approach? Also, the message is all lower-case. I know the error conventions are documented somewhere, but I completely forgot where. Can you please point me to the right place? I thought most error messages were supposed to be lower case, and detail messages were supposed to read like sentences. Similarly, for an insert/update situation, it seems that a message like key value violates exclusion constraint \%s\ would be better than the existing message. I can certainly simplify it, but I was trying to match the usefulness of UNIQUE constraint error messages. As a quick performance test, I inserted a million 3-integer tuples into a 3-column table with a unique constraint or an operator exclusion constraint over all three columns. The former took ~ 15 s, the latter ~ 21.5 s. That seems acceptable. Great news. I had similar results, though they depend on the conflict percentage as well (I assume you had zero conflicts). Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] operator exclusion constraints
2009/11/15 Jeff Davis pg...@j-davis.com: I know the error conventions are documented somewhere, but I completely forgot where. Can you please point me to the right place? I thought most error messages were supposed to be lower case, and detail messages were supposed to read like sentences. http://www.postgresql.org/docs/current/static/error-style-guide.html And you are correct. Cheers, BJ -- 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] Unicode UTF-8 table formatting for psql text output
On Sat, Nov 14, 2009 at 05:40:24PM +, Roger Leigh wrote: On Mon, Nov 09, 2009 at 05:40:54PM -0500, Bruce Momjian wrote: Tom Lane wrote: Greg Stark gsst...@mit.edu writes: While i agree this looks nicer I wonder what it does to things like excel/gnumeric/ooffice auto-recognizing table layouts and importing files. I'm not sure our old format was so great for this so maybe this is actually an improvement I'm asking for. Yeah. We can do what we like with the UTF8 format but I'm considerably more worried about the aspect of making random changes to the plain-ASCII output. On the other hand, we changed that just a release or so ago (to put in the multiline output in the first place) and I didn't hear complaints about it that time. Sorry for the delayed reply: The line continuation characters were chosen in 8.4 for clarity --- if you have found something clearer for 8.5, let's make the improvement. I think clarity is more important in this area than consistency with the previous psql output format. The attached patch is proposed for the upcoming commitfest, and hopefully takes into account the comments made in this thread. One note: because it's not possible to know in advance (without making things much more complex) whether or not a line will wrap or continue, the code currently makes sure we fully pad output up to the right margin of the table (finalspaces). This is in some cases unnecessary, but is needed when border=1 or else the continuation/wrap symbols don't end up in the margin and will look like they are part of the data. The side effect from this change is that some of the testsuite expected data will need updating due to the extra pad spaces now added to the output (triggers, dependency, tsearch, foreign_data, prepare, with). If the actual output format is OK with people then I'll update the patch to include the test data updates as well. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. signature.asc Description: Digital signature
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote: Florian G. Pflug f...@phlo.org writes: Ok, I must be missing something. I currently fail to see how my proposed record_value(record, name, anyelement) returns anyelement function differs (from the type system's point of view) from value_from_string(text, anyelement) returns anyelement which simply casts the text value to the given type and can easily be implemented in plpgsq. The problem is at the call site --- if you try to call it with different record types on different calls you're going to get a failure. Or so I expect anyway. Ah, OK - so it's really the record type, and not my anyelement kludge that might cause problems. Actually, I do now realize that record is a way more special case than I'd have initially thought. For example, I could have sworn that it's possible to pass record values to pl/pgsql functions, but just found out the hard way that it isn't. Seems that the possibility of declaring record variables lulled me into thinking it's pretty standard type when it actually isn't. best regards, Florian Pflug smime.p7s Description: S/MIME Cryptographic Signature
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Yes, and I have used it, but it really would be nicer to have some introspection facilities built in, especially for use in triggers. Maybe, but the proposal at hand is spectacularly ugly --- in particular it seems designed around the assumption that a given trigger will only care about handling a predetermined set of datatypes, which hardly fits with PG's normal goals for datatype extensibility. If the argument is that you don't like hstore or other PLs because they'll smash everything to text, then I think you have to do better than this. While I agree that handling arbitrary datatypes at runtime would be nice, I really don't see how that could ever be done from within a plpgsql procedure, unless plpgsql somehow morphs into a dynamically typed language. Plus, the set of datatypes an application deals with is usually much smaller than the set of tables, and less likely to change over time. I'd also argue that this restriction does not conflict with PG's goal of datatype extensibility at all. Datatype extensibility in PG's boils down to being able to create new datatypes without modifying postgres itself - but it still expects that you do so while designing your application. Which also is when trigger functions that use record_value() or a similar function would be written. Plus, fully generic handling of data of arbitrary type is a somewhat strange notion anyway, because it leaves you with very few operations guaranteed to be defined for those values. In the case of PG, you'd be pretty much limited to casting those values from and to text. best regards, Florian Pflug smime.p7s Description: S/MIME Cryptographic Signature
Re: [HACKERS] Unicode UTF-8 table formatting for psql text output
Roger Leigh rle...@codelibre.net writes: The side effect from this change is that some of the testsuite expected data will need updating due to the extra pad spaces No, we are *not* doing that. Somebody made a change to the print.c logic last year that started adding harmless white space to the last column, and it was a complete disaster for tracking whether anything important had changed in regression test output. Please undo that part of your patch. 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] Inspection of row types in pl/pgsql and pl/sql
Florian G. Pflug f...@phlo.org writes: While I agree that handling arbitrary datatypes at runtime would be nice, I really don't see how that could ever be done from within a plpgsql procedure, unless plpgsql somehow morphs into a dynamically typed language. Which is not likely to happen, which is why this is fundamentally a dead end. I don't think it's appropriate to put ugly, hard to use band-aids over the fact that plpgsql isn't designed to do this. One of the principal reasons why we work so hard to support multiple PLs is that they have different strengths. If you need something that's more dynamically typed than plpgsql, you should go use something else. Plus, fully generic handling of data of arbitrary type is a somewhat strange notion anyway, because it leaves you with very few operations guaranteed to be defined for those values. In the case of PG, you'd be pretty much limited to casting those values from and to text. Well, that's the wrong way to look at it. To me, the right design would involve saying that my trigger needs to do operation X on the data, and therefore it should support all datatypes that can do X. It should not need a hard-wired list of which types those are. Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 6:00 PM, Jeff Davis pg...@j-davis.com wrote: Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. Yeah, I think that's right. I think part of the rationale is that if the admin mucks around with catalog tables or does some DDL with inconsistent definitions (like an operator class that isn't internally consistent for example) then we don't treat those errors as user-visible errors that need to be translated but they shouldn't cause a database crash either. If it's possible for the case to arrive through users doing things through entirely supported means then they might need to be real ereports(). But I guess there are grey areas. -- greg -- 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] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote: Florian G. Pflug f...@phlo.org writes: While I agree that handling arbitrary datatypes at runtime would be nice, I really don't see how that could ever be done from within a plpgsql procedure, unless plpgsql somehow morphs into a dynamically typed language. Which is not likely to happen, which is why this is fundamentally a dead end. I don't think it's appropriate to put ugly, hard to use band-aids over the fact that plpgsql isn't designed to do this. One of the principal reasons why we work so hard to support multiple PLs is that they have different strengths. If you need something that's more dynamically typed than plpgsql, you should go use something else. Plus, fully generic handling of data of arbitrary type is a somewhat strange notion anyway, because it leaves you with very few operations guaranteed to be defined for those values. In the case of PG, you'd be pretty much limited to casting those values from and to text. Well, that's the wrong way to look at it. To me, the right design would involve saying that my trigger needs to do operation X on the data, and therefore it should support all datatypes that can do X. It should not need a hard-wired list of which types those are. Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? The two things I have wanted most badly in the past are a) to be able to address a field in NEW and OLD by a string name (usually passed in via a trigger argument) and b) to be able to discover the names if the fields in NEW and OLD 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] Inspection of row types in pl/pgsql and pl/sql
Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? The two things I have wanted most badly in the past are a) to be able to address a field in NEW and OLD by a string name (usually passed in via a trigger argument) and But what are you then going to do with that field? Are you just assuming that it will be of a pre-agreed datatype? Or that you can perform some specific operation on it? What are you expecting will happen if it isn't or can't? b) to be able to discover the names if the fields in NEW and OLD It doesn't seem hard or ugly to provide an API for that, but again I'm wondering what happens next. 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] patch - per-tablespace random_page_cost/seq_page_cost
Well, I was regretting missing the deadline for this CommitFest and then realized today was only the 14th, so I finished this up while the kids were napping. I ended up not reusing the reloptions.c code. It looks like a lot of extra complexity for no obvious benefit, considering that there is no equivalent of AMs for tablespaces and therefore no need to support AM-specific options. I did reuse the reloptions syntax, and I think the internal representation could always be redone later, if we find that there's a use case for something more complicated. ...Robert *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 1935,1940 archive_command = 'copy %p C:\\server\\archivedir\\%f' # Windows --- 1935,1943 para Sets the planner's estimate of the cost of a disk page fetch that is part of a series of sequential fetches. The default is 1.0. + This value can be overriden for a particular tablespace by setting + the tablespace parameter of the same name + (see xref linkend=sql-altertablespace). /para /listitem /varlistentry *** *** 1948,1953 archive_command = 'copy %p C:\\server\\archivedir\\%f' # Windows --- 1951,1962 para Sets the planner's estimate of the cost of a non-sequentially-fetched disk page. The default is 4.0. + This value can be overriden for a particular tablespace by setting + the tablespace parameter of the same name + (see xref linkend=sql-altertablespace). +/para + + para Reducing this value relative to varnameseq_page_cost/ will cause the system to prefer index scans; raising it will make index scans look relatively more expensive. You can raise *** a/doc/src/sgml/ref/alter_tablespace.sgml --- b/doc/src/sgml/ref/alter_tablespace.sgml *** *** 23,28 PostgreSQL documentation --- 23,30 synopsis ALTER TABLESPACE replaceablename/replaceable RENAME TO replaceablenew_name/replaceable ALTER TABLESPACE replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable + ALTER TABLESPACE replaceablename/replaceable SET ( replaceable class=PARAMETERtablespace_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) + ALTER TABLESPACE replaceablename/replaceable RESET ( replaceable class=PARAMETERtablespace_option/replaceable [, ... ] ) /synopsis /refsynopsisdiv *** *** 74,79 ALTER TABLESPACE replaceablename/replaceable OWNER TO replaceablenew_owner --- 76,99 /para /listitem /varlistentry + +varlistentry + termreplaceable class=parametertablespace_parameter/replaceable/term + listitem + para + A tablespace parameter to be set or reset. Currently, the only + available parameters are varnameseq_page_cost/ and + varnamerandom_page_cost/. Setting either value for a particular + tablespace will override the planner's usual estimate of the cost of + reading pages from tables in that tablespace, as established by + the configuration parameters of the same name (see + xref linkend=guc-seq-page-cost, + xref linkend=guc-random-page-cost). This may be useful if one + tablespace is located on a disk which is faster or slower than the + remainder of the I/O subsystem. + /para + /listitem +/varlistentry /variablelist /refsect1 *** a/src/backend/catalog/aclchk.c --- b/src/backend/catalog/aclchk.c *** *** 2621,2638 ExecGrant_Tablespace(InternalGrant *istmt) int nnewmembers; Oid *oldmembers; Oid *newmembers; - ScanKeyData entry[1]; - SysScanDesc scan; HeapTuple tuple; ! /* There's no syscache for pg_tablespace, so must look the hard way */ ! ScanKeyInit(entry[0], ! ObjectIdAttributeNumber, ! BTEqualStrategyNumber, F_OIDEQ, ! ObjectIdGetDatum(tblId)); ! scan = systable_beginscan(relation, TablespaceOidIndexId, true, ! SnapshotNow, 1, entry); ! tuple = systable_getnext(scan); if (!HeapTupleIsValid(tuple)) elog(ERROR, cache lookup failed for tablespace %u, tblId); --- 2621,2631 int nnewmembers; Oid *oldmembers; Oid *newmembers; HeapTuple tuple; ! /* Search syscache for pg_tablespace */ ! tuple = SearchSysCache(TABLESPACEOID, ObjectIdGetDatum(tblId), ! 0, 0, 0); if (!HeapTupleIsValid(tuple)) elog(ERROR, cache lookup failed for tablespace %u, tblId); *** *** 2703,2709 ExecGrant_Tablespace(InternalGrant *istmt) noldmembers, oldmembers, nnewmembers, newmembers); ! systable_endscan(scan); pfree(new_acl); --- 2696,2702 noldmembers, oldmembers, nnewmembers, newmembers); ! ReleaseSysCache(tuple); pfree(new_acl); *** *** 3443,3451
Re: [HACKERS] operator exclusion constraints
On Sat, Nov 14, 2009 at 1:58 PM, Greg Stark gsst...@mit.edu wrote: On Sat, Nov 14, 2009 at 6:00 PM, Jeff Davis pg...@j-davis.com wrote: Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. Yeah, I think that's right. I think part of the rationale is that if the admin mucks around with catalog tables or does some DDL with inconsistent definitions (like an operator class that isn't internally consistent for example) then we don't treat those errors as user-visible errors that need to be translated but they shouldn't cause a database crash either. If it's possible for the case to arrive through users doing things through entirely supported means then they might need to be real ereports(). But I guess there are grey areas. I guess my point wasn't that the message was likely to be exercised, but rather that it isn't obvious that it's describing an error condition at all. If you get this message: relation whatever has relopxconstraints = -3 ...you can't even tell that it's an error message unless it happens to be preceded by ERROR: . If you get something like: relation whatever has invalid relopxconstraints value (-3) ...it's much more clear that this is not just informational. ...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] operator exclusion constraints
On Sat, 2009-11-14 at 09:11 -0800, David E. Wheeler wrote: On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. Changed in new patch here: http://archives.postgresql.org/message-id/1258226849.708.97.ca...@jdavis Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote: Florian G. Pflug f...@phlo.org writes: While I agree that handling arbitrary datatypes at runtime would be nice, I really don't see how that could ever be done from within a plpgsql procedure, unless plpgsql somehow morphs into a dynamically typed language. Which is not likely to happen, which is why this is fundamentally a dead end. I don't think it's appropriate to put ugly, hard to use band-aids over the fact that plpgsql isn't designed to do this. One of the principal reasons why we work so hard to support multiple PLs is that they have different strengths. If you need something that's more dynamically typed than plpgsql, you should go use something else. In principle, I agree. In pratice, however, the company who I do my current project for has settled on plpgsql and isn't willing to use other PLs in their software because they lack the skill to maintain code written in other PLs. Therefore I'm trying to find an at least somewhat acceptable solution using plpgsql. Plus, fully generic handling of data of arbitrary type is a somewhat strange notion anyway, because it leaves you with very few operations guaranteed to be defined for those values. In the case of PG, you'd be pretty much limited to casting those values from and to text. Well, that's the wrong way to look at it. To me, the right design would involve saying that my trigger needs to do operation X on the data, and therefore it should support all datatypes that can do X. It should not need a hard-wired list of which types those are. True, but that'd require fairly large changes to plpgsql AFAICS. Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? I need to build a global index table of all values of a certain type together with a pointer to the row and table that contains them. Since all involved tables have an id column, storing that pointer is the easy part. The hard part is collecting all those values in an insert/update/delete trigger so that I can update the global index accordingly. Currently, a set of plpgsql functions generate a seperate trigger function for each table. Yuck! Instead of this nearly-impossible to read code-generating function I want to create a generic trigger function that works for any of the involved tables. Preferrably in plpgsql because of the skill issue mentioned above. best regards, Florian Pflug smime.p7s Description: S/MIME Cryptographic Signature
Re: [HACKERS] operator exclusion constraints
On Sat, 2009-11-14 at 14:36 -0500, Robert Haas wrote: I guess my point wasn't that the message was likely to be exercised, but rather that it isn't obvious that it's describing an error condition at all. If you get this message: relation whatever has relopxconstraints = -3 I pretty much copied similar code for relchecks, see pg_constraint.c:570. If you have a suggestion, I'll make the change. It doesn't sound too urgent though, to me. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] operator exclusion constraints
On Sat, Nov 14, 2009 at 2:39 PM, Jeff Davis pg...@j-davis.com wrote: If you have a suggestion, I'll make the change. It doesn't sound too urgent though, to me. Yeah, probably not. ...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] Inspection of row types in pl/pgsql and pl/sql
Florian G. Pflug f...@phlo.org writes: Tom Lane wrote: Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? I need to build a global index table of all values of a certain type together with a pointer to the row and table that contains them. Since all involved tables have an id column, storing that pointer is the easy part. The hard part is collecting all those values in an insert/update/delete trigger so that I can update the global index accordingly. So in this case it seems like you don't actually need any polymorphism at all; the target columns are always of a known datatype. You just don't want to commit to their names. I wonder though why you're willing to pin down the name of the id column but not the name of the data column. Currently, a set of plpgsql functions generate a seperate trigger function for each table. Yuck! Would you be happy with an approach similar to what Andrew mentioned, ie, you generate CREATE TRIGGER commands that list the names of the target column(s) as TG_ARGV arguments? The alternative to that seems to be that you iterate at runtime through all the table columns to see which ones are of the desired type. Which might be less trouble to set up, but the performance penalty of figuring out basically-unchanging information again on every single tuple update seems awful high. 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] patch - per-tablespace random_page_cost/seq_page_cost
On Sat, Nov 14, 2009 at 7:28 PM, Robert Haas robertmh...@gmail.com wrote: I ended up not reusing the reloptions.c code. It looks like a lot of extra complexity for no obvious benefit, considering that there is no equivalent of AMs for tablespaces and therefore no need to support AM-specific options. I did reuse the reloptions syntax, and I think the internal representation could always be redone later, if we find that there's a use case for something more complicated. a) effective_io_concurrency really deserves to be in the list as well. b) I thought Tom came down pretty stridently against any data model which hard codes a specific list of supported options. I can't remember exactly what level of flexibility he wanted but I think doesn't require catalog changes to add a new option might have been it. I agree that having everything smashed to text is a bit kludgy though. I'm not sure we have the tools to do much better though. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Postgres and likewise authentication
I'm curious if anyone has tried to link postgres authentication with a product called likewise. Likesoft software will allow a linux/unix system to authenticate against a windows domain. I have it working on several flavors of linux and working on getting it tied into several samba shares. I've heard there is a way to make it work with postgres but couldn't find any details. I'm curious if anyone has tried to do this and would love any tips :D Thanks! -- 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] operator exclusion constraints
On Mon, 2009-11-09 at 09:12 -0800, David E. Wheeler wrote: On Nov 8, 2009, at 7:43 PM, Jeff Davis wrote: Either of those names are fine with me, too. The current name is a somewhat shortened version of the name operator-based exclusion constraints, so we can also just use that name. Or, just exclusion constraints. (exclusion constraints)++ Ok, I guess this is another issue that requires consensus. Note: this is purely for documentation, release notes, and user-visible error messages. This does not have any impact on the syntax, I think we've got a strong consensus on that already and I would prefer not to break that discussion open again. 1. Operator Exclusion Constraints (current) 2. Generic/Generalized/General Exclusion Constraints 3. Exclusion Constraints (has the potential to cause confusion with constraint_exclusion) Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Tom Lane wrote: Perhaps it would help if we looked at some specific use-cases that people need, rather than debating abstractly. What do you need your generic trigger to *do*? The two things I have wanted most badly in the past are a) to be able to address a field in NEW and OLD by a string name (usually passed in via a trigger argument) and But what are you then going to do with that field? Are you just assuming that it will be of a pre-agreed datatype? Or that you can perform some specific operation on it? What are you expecting will happen if it isn't or can't? Yes, in many cases I'll assume it's a given datatype. A good example is an auto-update-timestamp trigger where the name of the timestamp field is passed in as a trigger argument. b) to be able to discover the names if the fields in NEW and OLD It doesn't seem hard or ugly to provide an API for that, but again I'm wondering what happens next. One case I have is a custom audit package that ignores certain fields when logging changes. So it would be nice to be able to iterate over the field names and check if NEW.foo is distinct from OLD.foo, skipping the field names we don't care about to decide if the change needs to be logged. 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] Inspection of row types in pl/pgsql and pl/sql
Tom Lane wrote:
Florian G. Pflug f...@phlo.org writes:
Tom Lane wrote:
Perhaps it would help if we looked at some specific use-cases
that people need, rather than debating abstractly. What do you
need your generic trigger to *do*?
I need to build a global index table of all values of a certain
type together with a pointer to the row and table that contains
them. Since all involved tables have an id column, storing that
pointer is the easy part. The hard part is collecting all those
values in an insert/update/delete trigger so that I can update the
global index accordingly.
So in this case it seems like you don't actually need any
polymorphism at all; the target columns are always of a known
datatype. You just don't want to commit to their names. I wonder
though why you're willing to pin down the name of the id column but
not the name of the data column.
There might be more than one (or none at all) columns of the type to be
indexed. I need to process all such columns (each of them produces a
seperate record in the index table). Plus, this schema is relatively
volatile - new fields are added about once a month or so.
Currently, a set of plpgsql functions generate a seperate trigger
function for each table. Yuck!
Would you be happy with an approach similar to what Andrew mentioned,
ie, you generate CREATE TRIGGER commands that list the names of the
target column(s) as TG_ARGV arguments? The alternative to that seems
to be that you iterate at runtime through all the table columns to
see which ones are of the desired type. Which might be less trouble
to set up, but the performance penalty of figuring out
basically-unchanging information again on every single tuple update
seems awful high.
Hm.. I had hoped to get away without any need to modify the trigger
definitions if the schema changes. But having a function that does DROP
TRIGGER; CREATE TRIGGER... is already a huge improvement over having
one that does CREATE FUNCTION
I've now played around with the
EXECUTE 'select $1.' || quote_ident(fieldname)' USING NEW/OLD
trick, and simply look up the existing field with
SELECT attname
FROM pg_attribute
WHERE
attrelid = TG_RELID AND
atttypeid IN (...) AND
attname NOT IN ('referenced_by', 'self') AND
attnum 0 AND NOT attisdropped
This at least gives me a working proof-of-concept implementation of the
trigger.
Still, doing that SELECT seems rather silly since NEW and OLD already
contain the required information. So I still believe that having
something like record_name() and record_types() would be useful. And at
least these functions have less of an issue with the type system...
best regards,
Florian Pflug
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [HACKERS] next CommitFest
Bruce Momjian wrote: I am probably more able than most to adjust my schedule to devote time to committing things. Yes, time is what has restricted what I can do. I'll try to do a bit more for this coming commitfest, but I'm rather sad that I haven't made a more substantial contribution to this release. 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] Inspection of row types in pl/pgsql and pl/sql
Andrew Dunstan and...@dunslane.net writes:
Tom Lane wrote:
Perhaps it would help if we looked at some specific use-cases that
people need, rather than debating abstractly. What do you need your
generic trigger to *do*?
One case I have is a custom audit package that ignores certain fields
when logging changes. So it would be nice to be able to iterate over the
field names and check if NEW.foo is distinct from OLD.foo, skipping the
field names we don't care about to decide if the change needs to be logged.
So, inventing syntax at will, what you're imagining is something like
modified := false;
for name in names(NEW) loop
-- ignore modified_timestamp
continue if name = 'modified_timestamp';
-- check all other columns
if NEW.{name} is distinct from OLD.{name} then
modified := true;
exit;
end if;
end loop;
if modified then ...
While this is perhaps doable, the performance would take your breath
away ... and I don't mean that in a positive sense. The only way we
could implement that in plpgsql as it stands would be that every
single execution of the IF would invole a parse/plan cycle for the
$1 IS DISTINCT FROM $2 expression. At best we would avoid a replan
when successive executions had the same datatypes for the tested
columns (ie, adjacent columns in the table have the same types).
Which would happen some of the time, but the cost of the replans would
still be enough to sink you.
This might look neat but I don't think it's actually useful for any
production application. We'd need to find some way of expressing it
that allows caching of the expression plans. But really I think the
entire approach is pretty much backwards from an efficiency standpoint.
I would sooner have some sort of primitive changed_columns(NEW, OLD)
that spits out a list of the names of changed columns (or maybe the
not-changed ones, not sure). It would not require any fundamental
restructuring and it would run several orders of magnitude faster
than you could ever hope to do it at the plpgsql level.
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] Inspection of row types in pl/pgsql and pl/sql
Florian G. Pflug f...@phlo.org writes: Still, doing that SELECT seems rather silly since NEW and OLD already contain the required information. So I still believe that having something like record_name() and record_types() would be useful. And at least these functions have less of an issue with the type system... Yeah. I don't have any objection in principle to providing such functions; I'm just wondering how far that really goes towards solving real-world 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] add more frame types in window functions (ROWS)
Hi, I've started reviewing your patch. I've already found some things that need work: - missing _readWindowFrameDef function (all nodes that are output from parse analysis must have both _read and _out functions, otherwise views can't work) - the A_Const nodes should probably be transformed to Const nodes in parse analysis, since A_Const has no _read/_out functions, which means changing the corresponding code in the executor. (A question here: the spec allows (by my reading) the use of parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat the values as Exprs, albeit very limited ones, and eval them at startup rather than assuming we know the node type and digging down into it all over the place?) You can see the problem here if you do this: create view v as select i,sum(i) over (order by i rows between 1 preceding and 1 following) from generate_series(1,10) i; select * from v; (A regression test for this would probably be good, which reminds me that I need to add one of those myself in the aggregate order by stuff.) Also, you're going to need to do some work in ruleutils.c (get_rule_windowspec) in order to be able to deparse your new frame definitions. I'll continue reviewing the stuff that does work, so I'm not marking this as waiting for author yet. -- Andrew (irc:RhodiumToad) -- 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] add more frame types in window functions (ROWS)
Andrew Gierth and...@tao11.riddles.org.uk writes: (A question here: the spec allows (by my reading) the use of parameters in the window frame clause, i.e. BETWEEN $1 PRECEDING AND $2 FOLLOWING. Wouldn't it therefore make more sense to treat the values as Exprs, albeit very limited ones, and eval them at startup rather than assuming we know the node type and digging down into it all over the place?) Seems like you might as well allow any expression not containing local Vars. Compare the handling of LIMIT. 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] patch - per-tablespace random_page_cost/seq_page_cost
On Sat, Nov 14, 2009 at 3:06 PM, Greg Stark gsst...@mit.edu wrote: On Sat, Nov 14, 2009 at 7:28 PM, Robert Haas robertmh...@gmail.com wrote: I ended up not reusing the reloptions.c code. It looks like a lot of extra complexity for no obvious benefit, considering that there is no equivalent of AMs for tablespaces and therefore no need to support AM-specific options. I did reuse the reloptions syntax, and I think the internal representation could always be redone later, if we find that there's a use case for something more complicated. a) effective_io_concurrency really deserves to be in the list as well. b) I thought Tom came down pretty stridently against any data model which hard codes a specific list of supported options. I can't remember exactly what level of flexibility he wanted but I think doesn't require catalog changes to add a new option might have been it. I agree that having everything smashed to text is a bit kludgy though. I'm not sure we have the tools to do much better though. I'm hoping Tom will reconsider, or at least flesh out his thinking. What the reloptions code does is create a general framework for handling options. Everything gets smashed down to text[], and then when we actually need to use the reloptions we parse them into a C struct appropriate to the underlying object type. This is really the only feasible design, because pg_class contains multiple different types of objects - in particular, tables and indices - and indices in turn come in multiple types, depending on the AM. So the exact options that are legal depend on the the type of object, and for indices the AM, and we populate a *different* C struct depending on the situation. pg_tablespace, on the other hand, only contains one type of object: a tablespace. So, if we stored the options as text[], we'd parse them out into a C struct just as we do for pg_class, but unlike the pg_class case, it would always be the *same* C struct. In other words, we CAN'T use dedicated columns for pg_class because we can't know in advance precisely what columns will be needed - it depends on what AMs someone chooses to load up. For pg_tablespace, we know exactly what columns will be needed, and the answer is exactly those options that we choose to support, because tablespaces are not extensible. That's my thinking, anyway... YMMV. ...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] New VACUUM FULL
On Fri, 2009-11-13 at 10:47 +0900, Itagaki Takahiro wrote: Alvaro Herrera alvhe...@commandprompt.com wrote: BTW I think the vacstmt.options change, which seems a reasonable idea to me, could be extracted from the patch and applied separately. That'd reduce the size of the patch somewhat. It's a good idea. I separated the part into the attached patch. It replaces VacuumStmt's vacuum, full, analyze, and verbose fields into one options flag field. The only user-visible change is to support VACUUM (options) syntax: VACUUM ( FULL, FREEZE, VERBOSE, ANALYZE ) table (columns) We don't bother with the order of options in this form :) There is a debatable issue that we can use VACUUM (VERBOSE) table (col) in the abobe syntax. Columns are specified but no ANALYZE option there. An ANALYZE option is added automatically in the current implementation, but we should have thrown an syntax error in such case. I have just begun review by reading some of the previous threads. I'd like to try to summarize the goals we have for VACUUM to put these patches in perspective: 1. Implement a rewrite version of VACUUM FULL, which is closer to CLUSTER. 2. Use the new options syntax, to make the order of vacuum options irrelevant. 3. Implement several flatfile maps to allow the rewrite version of VACUUM FULL to work on catalogs: http://archives.postgresql.org/message-id/19750.1252094...@sss.pgh.pa.us 4. Implement some kind of concurrent tuple mover that can slowly update tuples and lazily VACUUM in such a way that they migrate to lower heap pages (assuming no long-running transactions). This would be done with normal updates (new xmin) and would not remove the old tuple until it was really dead; otherwise there are concurrency problems. Such a utility would be useful in cases where a very small fraction of tuples need to be moved, or you don't have enough space for a rewrite. 5. Remove VACUUM FULL (in it's current form) completely. Some other ideas also came out of the thread, like: * Provide a way to truncate the dead space from the end of a relation in a blocking manner. Right now, lazy vacuum won't shrink the file unless it can acquire an exclusive lock without waiting, and there's no way to actually make it wait. This can be a separate command, not necessarily a part of VACUUM. * Josh Berkus suggested allowing the user to specify a different tablespace in which to rebuild the tablespace. I'll begin looking at the patches themselves now, which implement #1 and #2. If we can implement enough of these features (say, #3 also) to remove the current form of VACUUM FULL, then we can just call the new feature VACUUM FULL, and save ourselves from syntactical headaches. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Inspection of row types in pl/pgsql and pl/sql
On Sat, Nov 14, 2009 at 3:24 PM, Tom Lane t...@sss.pgh.pa.us wrote: This might look neat but I don't think it's actually useful for any production application. We'd need to find some way of expressing it that allows caching of the expression plans. But really I think the entire approach is pretty much backwards from an efficiency standpoint. I would sooner have some sort of primitive changed_columns(NEW, OLD) that spits out a list of the names of changed columns (or maybe the not-changed ones, not sure). It would not require any fundamental restructuring and it would run several orders of magnitude faster than you could ever hope to do it at the plpgsql level. huge +1 to this. This problem comes up all the time...I was in fact this exact moment working on something just like Florian for table auditing purposes...comparing new/old but needing to filter out uninteresting columns. One of those things that should be a SMOP but isn't ;-). I worked out a plpgsql approach using dynamic sql...performance wasn't _that_ bad, but any speedup is definitely welcome. The way I did it was to pass both new and old to a function as text, and build an 'is distinct from' from with the interesting field list querying out fields from the expanded composite type...pretty dirty. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Hot standby, race condition between recovery snapshot and commit
On Sat, 2009-11-14 at 14:59 +0200, Heikki Linnakangas wrote: There's a race condition Yes, I believe this is a major showstopper for the current approach/attemptbut... I can't see any obvious way around that. Huh? We're only doing this strict locking approach because you insisted that the looser approach was not acceptable. Have you forgotten that discussion so completely that you can't even remember the existence of other options? It amazes me that you should then use locking overhead as the reason to not pursue the current approach further, which was exactly my argument for not pursuing it in the first place. You're leading me a merry dance. -- Simon Riggs www.2ndQuadrant.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] Inspection of row types in pl/pgsql and pl/sql
2009/11/14 Merlin Moncure mmonc...@gmail.com: On Sat, Nov 14, 2009 at 3:24 PM, Tom Lane t...@sss.pgh.pa.us wrote: This might look neat but I don't think it's actually useful for any production application. We'd need to find some way of expressing it that allows caching of the expression plans. But really I think the entire approach is pretty much backwards from an efficiency standpoint. I would sooner have some sort of primitive changed_columns(NEW, OLD) that spits out a list of the names of changed columns (or maybe the not-changed ones, not sure). It would not require any fundamental restructuring and it would run several orders of magnitude faster than you could ever hope to do it at the plpgsql level. huge +1 to this. This problem comes up all the time...I was in fact this exact moment working on something just like Florian for table auditing purposes...comparing new/old but needing to filter out uninteresting columns. One of those things that should be a SMOP but isn't ;-). I worked out a plpgsql approach using dynamic sql...performance wasn't _that_ bad, but any speedup is definitely welcome. C function is_not_distinct(RECORD, RECORD, [variadic columnnames]) should not be a problem (I thing). Pavel The way I did it was to pass both new and old to a function as text, and build an 'is distinct from' from with the interesting field list querying out fields from the expanded composite type...pretty dirty. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch - per-tablespace random_page_cost/seq_page_cost
Robert Haas robertmh...@gmail.com writes: pg_tablespace, on the other hand, only contains one type of object: a tablespace. So, if we stored the options as text[], we'd parse them out into a C struct just as we do for pg_class, but unlike the pg_class case, it would always be the *same* C struct. The same, until it's different. There is no reason at all to suppose that the set of options people will want to apply to a tablespace will remain constant over time --- in fact, I don't think there's even a solid consensus right now on which GUCs people would want to set at the tablespace level. I don't believe it is wise to hardwire this into the catalog schema. Yes, it would look marginally nicer from a theoretical standpoint, but we'd be forever having to revise the schema, plus a lot of downstream code (pg_dump for example); which is not only significant work but absolutely prevents making any adjustments except at major version boundaries. And I don't see any concrete benefit that we get out of a hardwired schema for these things. It's not like we care about optimizing searches for tablespaces having a particular option setting, for example. 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] Listen / Notify rewrite
On Fri, Nov 13, 2009 at 11:08 AM, Tom Lane t...@sss.pgh.pa.us wrote: (By the way, has anyone yet tried to compare the speed of this implementation to the old code?) I quickly hacked pgbench to take a custom script on connection (for listen), and make pgbench'd 'notify x'; with all clients doing 'listen x'. The old method (measured on a 4 core high performance server) has severe scaling issues due to table bloat (we knew that): ./pgbench -c 10 -t 1000 -n -b listen.sql -f notify.sql run #1 tps = 1364.948079 (including connections establishing) run #2 tps = 573.988495 (including connections establishing) vac full pg_listener ./pgbench -c 50 -t 200 -n -b listen.sql -f notify.sql tps = 844.033498 (including connections establishing) new method on my dual core workstation (max payload 128): ./pgbench -c 10 -t 1 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 16343.012373 (including connections establishing) ./pgbench -c 20 -t 5000 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 7642.104941 (including connections establishing) ./pgbench -c 50 -t 5000 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 3184.049268 (including connections establishing) max payload 2048: ./pgbench -c 10 -t 1 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 12062.906610 (including connections establishing) ./pgbench -c 20 -t 5000 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 7229.505869 (including connections establishing) ./pgbench -c 50 -t 5000 -n -b listen.sql -f notify.sql -hlocalhost postgres tps = 3219.511372 (including connections establishing) getting sporadic 'LOG: could not send data to client: Broken pipe' throughout the test. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New VACUUM FULL
Jeff Davis pg...@j-davis.com writes: I'd like to try to summarize the goals we have for VACUUM to put these patches in perspective: Good summary, but ... Some other ideas also came out of the thread, like: * Provide a way to truncate the dead space from the end of a relation in a blocking manner. Right now, lazy vacuum won't shrink the file unless it can acquire an exclusive lock without waiting, and there's no way to actually make it wait. This can be a separate command, not necessarily a part of VACUUM. What I remembered was actually closer to the opposite: people are concerned about lazy vac holding the exclusive lock too long once it does acquire it. In a manual vacuum this leads to unexpected blockage of concurrent queries, and in an autovac this leads to a forced cancel preventing autovac from completing the operation (which means no space removal at all, and no stats update either). The code is designed on the assumption that it won't spend very long holding the ex lock, and so I think a reasonable TODO item is to ensure that by having it limit how many blocks it will scan during the shrinkage attempt. FWIW, once we provide the extensible option syntax, it doesn't seem like it'd be hard to have an option to make it wait for the lock, so I'd recommend that approach over anything with a separate command. 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] Partitioning option for COPY
Emmanuel Cecchet wrote: Hi all, Hi!, partitioning option for COPY Here's the review: == Submission == The patch is contextual, applies cleanly to current HEAD, compiles fine. The docs build cleanly. == Docs == They're reasonably clear, although they still mention ERROR_LOGGING, which was taken out of this patch. They could use some wordsmithing, but I didn't go into details, as there were more severe issues with the patch. One thing that made me cautious was the mention that triggers modifying tuples will make random errors appear. As is demonstrated later, triggers are a big issue. == Regression tests == They ran fine, there's one additional regression test that exercises the new option. == Style/nitpicks == Minor gripes include: o instead of using an ad-hoc data structure for the LRU cache list, I'd suggest an OidList from pg_list.h. o some mentions of method in comments should be changed to function o trailing whitespace in the patch (it's not the end of the world, of course) == Issues == Attached are 3 files that demonstrate problems the patch has. o test1.sql always segfaults for me, poking around with gdb suggests it's a case of an uninitialised cache list (another reason to use the builtin one). o test2.sql demonstrates, that indices on child tables are not being updated, probably because after resultRelInfo in check_tuple_constraints() gets created is never has ri_NumIndices set, and so the code that was supposed to take care of indices is never called. Looks like a copy-paste error. o test3.sql demonstrates, that some triggers that I would expect to be fired are in fact not fired. I guess it's the same reason as mentioned: ri_TrigDesc never gets set, so the code that calls triggers is dead. I stopped there, because unfortunately, apart from all that there's one fundamental problem with this patch, namely we probably don't want it. As it stands it's more of a proof of concept than a really usable solution, it feels like built from spare (copied from around copy.c) parts. IMHO it's much too narrow for a general partitioning solution, even if the design it's based upon would be accepted. It's assuming a lot of things about the presence of child tables (with proper constraints), the absence of triggers, and so on. Granted, it solves a particular problem (bulk loading into a partitioned table, with not extra features like triggers and with standard inheritance/exclusive check constraints setup), but that's not good enough in my opinion, even if all other issues would be addressed. Now I'm not a real Postgres user, it's been a while since I worked in a PG shop (or a DB shop for that matter), but from what I understand from following this community for a while, a patch like that doesn't have a lot of chances to be committed. That said, my puny experience with real PG installations and their needs must be taken into account here. I'll mark this patch as Waiting on Author, but I have little doubt that even after fixing those probably trivial segfaults etc. the patch would be promptly rejected by a committer. I suggest withdrawing it from this commitfest and trying to work out a more complete design first that would address the needs of a bigger variety of users, or joining some of the already underway efforts to bring full-featured partitioning into Postgres. Best, Jan -- 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] Partitioning option for COPY
Jan Urbański wrote: Emmanuel Cecchet wrote: Hi all, Hi!, partitioning option for COPY Attached are 3 files that demonstrate problems the patch has. And the click-before-you-think prize winner is... me. Test cases attached, see the comments for expected/actual results. Jan -- segfaults, probably uninitialised cache oid list -- disabling cache fixes it -- set copy_partitioning_cache_size = 0; drop table parent cascade; create table parent(i int); create table c1 (check (i 0 and i = 1)) inherits (parent); copy parent from stdin with (partitioning); 1 \. drop table parent cascade; create table parent(i int); create table c1 (check (i 0 and i = 1)) inherits (parent); copy parent from stdin with (partitioning); 1 \. set copy_partitioning_cache_size = 0; drop table parent cascade; create table parent(i int, j int); create table c1 (check (i 0 and i = 1)) inherits (parent); create table c2 (check (i 1 and i = 2)) inherits (parent); create table c3 (check (i 2 and i = 3)) inherits (parent); create index c1_idx on c1(j); copy (select i % 3 + 1, i from generate_series(1, 1000) s(i)) to '/tmp/parent'; copy parent from '/tmp/parent' with (partitioning); analyse; set enable_seqscan to false; -- no rows, index was not updated select * from c1 where j = 3; set enable_seqscan to true; set enable_indexscan to false; -- some rows select * from c1 where j = 3; set copy_partitioning_cache_size = 0; drop table parent cascade; drop table audit cascade; drop function audit(); create table parent(i int); create table c1 (check (i 0 and i = 1)) inherits (parent); create table c2 (check (i 1 and i = 2)) inherits (parent); create table c3 (check (i 2 and i = 3)) inherits (parent); create table audit(i int); create function audit() returns trigger as $$ begin insert into audit(i) values (new.i); return new; end; $$ language plpgsql; create trigger parent_a after insert on parent for each row execute procedure audit(); -- the before trigger on the parent would get fired -- create trigger parent_a2 before insert on parent for each row execute procedure audit(); create trigger c1_a before insert on c1 for each row execute procedure audit(); create trigger c1_a2 after insert on c1 for each row execute procedure audit(); copy parent from stdin with (partitioning); 1 2 3 \. -- no rows select * from audit; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch - per-tablespace random_page_cost/seq_page_cost
On Sat, Nov 14, 2009 at 4:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: pg_tablespace, on the other hand, only contains one type of object: a tablespace. So, if we stored the options as text[], we'd parse them out into a C struct just as we do for pg_class, but unlike the pg_class case, it would always be the *same* C struct. The same, until it's different. There is no reason at all to suppose that the set of options people will want to apply to a tablespace will remain constant over time --- in fact, I don't think there's even a solid consensus right now on which GUCs people would want to set at the tablespace level. I don't believe it is wise to hardwire this into the catalog schema. Yes, it would look marginally nicer from a theoretical standpoint, but we'd be forever having to revise the schema, plus a lot of downstream code (pg_dump for example); which is not only significant work but absolutely prevents making any adjustments except at major version boundaries. And I don't see any concrete benefit that we get out of a hardwired schema for these things. It's not like we care about optimizing searches for tablespaces having a particular option setting, for example. I can tell I've lost this argument, but I still don't get it. Why do we care if we have to change the schema? It's not a lot of work, and the number of times we would likely bump catversion for new pg_tablespace options seems unlikely to be significant in the grand scheme of things. I don't think there are very many parameters that make sense to set per-tablespace. As for major version boundaries, it seems almost unimaginable that we would backpatch code to add a new tablespace option whether the schema permits it or not. Can you clarify the nature of your concern here? What I'm concerned about with text[] is that I *think* it's going to force us to invent an analog of the relcache for tablespaces. With hardwired columns, a regular catcache is all we need. But the reloptions stuff is designed to populate a struct, and once we populate that struct we have to have someplace to hang it - or I guess maybe we could reparse it on every call to cost_seqscan(), cost_index(), genericcostestimate(), etc, but that doesn't seem like a great idea. So it seems like we'll need another caching layer sitting over the catcache. If we already had such a beast it would be reasonable to add this in, but I would assume that we wouldn't want to add such a thing without a fairly clear use case that I'm not sure we have. Maybe you see it differently? Or do you have some idea for a simpler way to set this up? ...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] New VACUUM FULL
On Fri, 2009-11-13 at 10:47 +0900, Itagaki Takahiro wrote: Alvaro Herrera alvhe...@commandprompt.com wrote: BTW I think the vacstmt.options change, which seems a reasonable idea to me, could be extracted from the patch and applied separately. That'd reduce the size of the patch somewhat. It's a good idea. I separated the part into the attached patch. It replaces VacuumStmt's vacuum, full, analyze, and verbose fields into one options flag field. I added a separate commitfest item for this patch to track it separately from the rewrite version of VACUUM: https://commitfest.postgresql.org/action/patch_view?id=222 The only user-visible change is to support VACUUM (options) syntax: VACUUM ( FULL, FREEZE, VERBOSE, ANALYZE ) table (columns) We don't bother with the order of options in this form :) I looked at this patch. You left INPLACE in the patch, which looks like an oversight when you were separating the two. Please remove that from this part. There is a debatable issue that we can use VACUUM (VERBOSE) table (col) in the abobe syntax. Columns are specified but no ANALYZE option there. An ANALYZE option is added automatically in the current implementation, but we should have thrown an syntax error in such case. Sounds fine, but worth a mention in the documentation. Just add to the columns part of the VACUUM page something like: If specified, implies ANALYZE. Other than these two minor issues, I don't see any problems with the patch. Please post an updated version to the new commitfest entry. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch - per-tablespace random_page_cost/seq_page_cost
Robert Haas robertmh...@gmail.com writes: I can tell I've lost this argument, but I still don't get it. Why do we care if we have to change the schema? It's not a lot of work, Try doing it a few times. Don't forget to keep psql and pg_dump apprised of which PG versions contain which columns. Not to mention other tools such as pgAdmin that might like to show these settings. It gets old pretty fast. What I'm concerned about with text[] is that I *think* it's going to force us to invent an analog of the relcache for tablespaces. I'm not really convinced of that, but even if we do, so what? It's not that much code to have an extra cache watching the syscache traffic. There's an example in parse_oper.c of a specialized cache that's about as complicated as this would be. It's about 150 lines including copious comments. We didn't even bother to split it out into its own source file. With hardwired columns, a regular catcache is all we need. But the reloptions stuff is designed to populate a struct, and once we populate that struct we have to have someplace to hang it - or I guess maybe we could reparse it on every call to cost_seqscan(), cost_index(), genericcostestimate(), etc, but that doesn't seem like a great idea. Well, no, we would not do it that way. I would imagine instead that plancat.c would be responsible for attaching appropriate cost values to each RelOptInfo struct, so it'd be more like one lookup per referenced table per query. It's possible that a cache would be useful even at that load level, but I'm not convinced. 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] New VACUUM FULL
On Tue, 2009-10-27 at 13:55 +0900, Itagaki Takahiro wrote: Here is a patch to support rewrite version of VACUUM FULL. Can you please provide a patch that applies cleanly on top of the vacuum options patch and on top of CVS HEAD (there are a couple minor conflicts with this version). That would make review easier. Initial comments: 1. Do we want to introduce syntax for INPLACE at all, if we are eventually going to remove the current mechanism? If not, then we should probably use REWRITE, because that's a better word than REPLACE, I think. My opinion is that if we really still need the current in-place mechanism, then VACUUM (FULL) should use the current in-place mechanism; and VACUUM (FULL REWRITE) should use your new rewrite mechanism. That gives us a nice migration path toward always using the rewrite mechanism. 2. Why do all of the following exist: VACOPT_FULL, VACOPT_REPLACE, and VACOPT_INPLACE? Shouldn't VACOPT_FULL be equivalent to one of the other two? This is essentially what Simon was getting at, I think. 3. Some options are being set in vacuum() itself. It looks like the options should already be set in gram.y, so should that be an Assert instead? I think it's cleaner to set all of the options properly early on, rather than waiting until vacuum() to interpret the combinations. I haven't looked at the implementation in detail yet, but at a glance, it seems to be a good approach. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New VACUUM FULL
Jeff Davis pg...@j-davis.com writes: 3. Some options are being set in vacuum() itself. It looks like the options should already be set in gram.y, so should that be an Assert instead? I think it's cleaner to set all of the options properly early on, rather than waiting until vacuum() to interpret the combinations. As a rule of thumb, I'd recommend keeping as much complexity as possible *out* of gram.y. It's best if that code just reports the facts, ie what options the user entered. Deriving conclusions from that (like what default behaviors should be used) is best done later. One example of why is that if you want the defaults to depend on GUC settings then that logic must *not* happen in gram.y, since the parse tree might outlive the current GUC settings. I haven't read the patch but it sounds like vacuum() is the right place for this type of activity. 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] Unicode UTF-8 table formatting for psql text output
On Sat, Nov 14, 2009 at 01:31:29PM -0500, Tom Lane wrote:
Roger Leigh rle...@codelibre.net writes:
The side effect from this change is that some of the testsuite
expected data will need updating due to the extra pad spaces
No, we are *not* doing that. Somebody made a change to the print.c
logic last year that started adding harmless white space to the
last column, and it was a complete disaster for tracking whether
anything important had changed in regression test output. Please
undo that part of your patch.
No problem, done as requested. I've attached an updated patch that
takes care to exactly match the trailing whitespace the existing
psql outputs. This fixes most of the changes between observed and
expected test results.
Due to the fact that this patch does alter the output for newlines
and wrapped lines (being its intent), the patch does alter expected
testcase output for these specific cases. Because the old
wrap/newline code put : and ; in place of | between columns,
this meant that it never worked for the first column of data, which
included single column result sets. This necessitated some changes
to the expected results to reflect this change (which now makes the
output uniform for all columns, irrespective of position).
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`-GPG Public Key: 0x25BFB848 Please GPG sign your mail.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7f03802..4b3fe71 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1765,18 +1765,40 @@ lo_import 152801
listitem
para
Sets the border line drawing style to one
- of literalascii/literal or literalunicode/literal.
- Unique abbreviations are allowed. (That would mean one
- letter is enough.)
+ of literalascii/literal, literalascii-old/literal
+ or literalunicode/literal. Unique abbreviations are
+ allowed. (That would mean one letter is enough.)
/para
para
- quoteASCII/quote uses plain acronymASCII/acronym characters.
+ quoteASCII/quote uses plain acronymASCII/acronym
+ characters. Newlines in data are shown using
+ a literal+/literal symbol in the right-hand margin,
+ while wrapped data uses a literal./literal symbol in the
+ right-hand margin of a wrapped line, and in the left-hand
+ margin of the following continuation line.
/para
para
+ quoteASCII-old/quote uses plain acronymASCII/acronym
+ characters, using the formatting style used
+ for productnamePostgreSQL/productname 8.4 and earlier.
+ Newlines in data are shown using a literal:/literal
+ symbol in place of the left-hand column separator, while
+ wrapped data uses a literal;/literal symbol. Newlines
+ in column headings are indicated by a literal+/literal
+ symbol in the left-hand margin of additional lines.
+ /para
+
+ para
quoteUnicode/quote uses Unicode box-drawing characters.
- /para
+ Newlines in data are shown using a carriage return symbol
+ (literal#8629;/literal) in the right-hand margin.
+ Wrapped data uses an ellipsis symbol
+ (literal#8230;/literal) in the right-hand margin of a
+ wrapped line, and in the left-hand margin of the following
+ continuation line.
+ /para
para
When the selected output format is one that draws lines or boxes
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 190a8d3..544a677 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1795,11 +1795,13 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
;
else if (pg_strncasecmp(ascii, value, vallen) == 0)
popt-topt.line_style = pg_asciiformat;
+ else if (pg_strncasecmp(ascii-old, value, vallen) == 0)
+ popt-topt.line_style = pg_asciiformat_old;
else if (pg_strncasecmp(unicode, value, vallen) == 0)
popt-topt.line_style = pg_utf8format;
else
{
- psql_error(\\pset: allowed line styles are ascii, unicode\n);
+ psql_error(\\pset: allowed line styles are ascii, ascii-old, unicode\n);
return false;
}
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 026e043..d382458 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -45,9 +45,9 @@ static char *grouping;
static char *thousands_sep;
/* Line style control structures */
-const printTextFormat pg_asciiformat =
+const printTextFormat pg_asciiformat_old =
{
- ascii,
+ ascii-old,
{
{ -, +, +, + },
{ -, +, +, + },
@@ -56,7 +56,36 @@ const printTextFormat pg_asciiformat =
},
:,
;,
-
+ ,
+ +,
+ ,
+ ,
+ ,
+ ,
+ ,
+ false
+};
+
+/* Line
