Re: [HACKERS] Fixing GIN for empty/null/full-scan cases
On Jan 14, 2011, at 5:54 PM, Tom Lane wrote: > "David E. Wheeler" writes: >> So some questions: > >> * Is something seriously wrong with GiST index creation on integer[] columns? > >> * Why does GIN performance appear to be no better than table scans on >> integer[] columns? > >> * Why does it take 3-4x longer to create the GIN than the GiST index on >> tsvector? I thought that GIN was supposed to be faster to update > > Hard to comment on any of this without a concrete example (including > data) to look at. Given the bugs we've recently found in the picksplit > algorithms for other contrib modules, I wouldn't be too surprised if the > sucky GiST performance traced to a similar bug in intarray. But I'm not > excited about devising my own test case. I could give you access to the box in question if you'd like to poke at it. Send me a public key. > One other point here is that GIN index build time is quite sensitive to > maintenance_work_mem --- what did you have that set to? 64MB 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] auto-sizing wal_buffers
Kevin Grittner wrote: I guess a manual override doesn't bother me too much, but I am a bit dubious of its value, and there is value in keeping the GUC count down... It's a risk-reward thing really. The reward for removing it is that a few lines of code and a small section of the documentation go away. It's not very big. The risk seems low, but it's not zero. Let's say this goes in, we get to 9.2 or later, and a survey suggests that no one has needed to ever set wal_buffers when deploying 9.1. At that point I think everyone would feel much better considering to nuke it altogether. I just looked at the code again when developing the patch, and there's really not enough benefit to removing it to worry about taking any risk right now. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us "PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] auto-sizing wal_buffers
Tom Lane wrote:
I think we need to keep the override capability until the autotune
algorithm has proven itself in the field for a couple of years.
I agree with Josh that a negative value should be used to select the
autotune method.
Agreed on both fronts. Attached patch does the magic. Also available
in branch "walbuffers" from git://github.com/greg2ndQuadrant/postgres.git
By changing only shared_buffers I get the following quite reasonable
automatic behavior:
$ psql -c "SELECT name,unit,boot_val,setting,current_setting(name) FROM
pg_settings WHERE name IN ('wal_buffers','shared_buffers')"
name | unit | boot_val | setting | current_setting
+--+--+-+-
shared_buffers | 8kB | 1024 | 3072| 24MB
wal_buffers| 8kB | -1 | 96 | 768kB
shared_buffers | 8kB | 1024 | 4096| 32MB
wal_buffers| 8kB | -1 | 128 | 1MB
shared_buffers | 8kB | 1024 | 16384 | 128MB
wal_buffers| 8kB | -1 | 512 | 4MB
shared_buffers | 8kB | 1024 | 131072 | 1GB
wal_buffers| 8kB | -1 | 2048| 16MB
shared_buffers | 8kB | 1024 | 262144 | 2GB
wal_buffers| 8kB | -1 | 2048| 16MB
If you've set it to the auto-tuning behavior, you don't see that setting
of -1 in the SHOW output; you see the value it's actually been set to.
The only way to know that was set automatically is to look at boot_val
as I've shown here. I consider this what admins would prefer, as the
easy way to expose the value that was used. I would understand if
people considered it a little odd though. Since you can't change it
without a postgresql.conf edit and a server start anyway, and it's
tersely documented in the sample postgresql.conf what -1 does, I don't
see this being a problem for anyone in the field.
To try and clear up some of the confusion around how the earlier
documentation suggests larger values of this aren't needed, I added the
following updated description of how this has been observed to work for
admins in practice:
! Since the data is written out to disk at every transaction commit,
! the setting many only need to be be large enough to hold the
amount
! of WAL data generated by one typical transaction. Larger values,
! typically at least a few megabytes, can improve write performance
! on a busy server where many clients are committing at once.
! Extremely large settings are unlikely to provide additional
benefit.
And to make this easy as possible to apply if I got this right, here's
some proposed commit text:
Automatically set wal_buffers to be proportional
to the size of shared_buffers. Make it 1/32
as large when the auto-tuned behavior, which
is the default and set with a value of -1,
is used. The previous default of 64kB is still
enforced as a minimum value. The maximum
automatic value is limited to 16MB.
(Note that this not exactly what I put in my own commit message if you
grab from my repo, that had a typo)
--
Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8e2a2c5..c3f5632 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** SET ENABLE_SEQSCAN TO OFF;
*** 1638,1649
! The amount of memory used in shared memory for WAL data. The
! default is 64 kilobytes (64kB). The setting need only
! be large enough to hold the amount of WAL data generated by one
! typical transaction, since the data is written out to disk at
! every transaction commit. This parameter can only be set at server
! start.
--- 1638,1659
! The amount of shared memory used for storing WAL data. The
! default setting of -1 adjusts this automatically based on the size
! of shared_buffers, making it 1/32 (about 3%) of
! the size of that normally larger shared memory block. Automatically
! set values are limited to a maximum of 16 megabytes
! (16MB), sufficient to hold one WAL segment worth of data.
! The smallest allowable setting is 64 kilobytes (64kB).
!
!
!
! Since the data is written out to disk at every transaction commit,
! the setting many only need to be be large enough to hold the amount
! of WAL data generated by one typical transaction. Larger values,
! typically at least a few megabytes, can improve write performance
! on a busy server where many clients are committing at once.
! Extremely large settings are unlikely to provide additional benefit.
diff --git a/src/backend/access/transa
Re: [HACKERS] ALTER TYPE 0: Introduction; test cases
Here's v2 based on your feedback.
I pruned test coverage down to just the highlights. By the end of patch series,
the net change becomes +67 to alter_table.sql and +111 to alter_table.out. The
alter_table.out delta is larger in this patch (+150), because the optimizations
don't yet apply and more objects are reported as rebuilt.
If this looks sane, I'll rebase the rest of the patches accordingly.
On Tue, Jan 11, 2011 at 09:41:35PM -0500, Robert Haas wrote:
> On Tue, Jan 11, 2011 at 7:25 AM, Noah Misch wrote:
> I'm wondering if we should consider moving this call to index_build()
> so that it appears everywhere that we build an index rather than just
> in ALTER TABLE, and saying something like:
>
> building index "%s" on table "%s"
Done.
I also fixed the leading capital letter on the other new messages.
> > The theoretical basis is that users can do little to directly change the
> > need to
> > rebuild a TOAST index. ?We'll rebuild the TOAST index if and only if we
> > rewrote
> > the table. ?The practical basis is that the TOAST relation names contain
> > parent
> > relation OIDs, so we don't want them mentioned in regression test output.
>
> Perhaps in this case we could write:
>
> building TOAST index for table "%s"
>
> There's limited need for users to know the actual name of the TOAST
> table, but I think it's better to log a line for all the actions we're
> performing or none of them, rather than some but not others.
That was a good idea, but the implementation is awkward. index_build has the
TOAST table and index relations, and there's no good way to find the parent
table from either. No index covers pg_class.reltoastrelid, so scanning by that
would be a bad idea. Autovacuum solves this by building its own hash table with
the mapping; that wouldn't fit well here. We could parse the parent OID out of
the TOAST name (heh, heh). I suppose we could pass the parent relation from
create_toast_table down through index_create to index_build. Currently,
index_create knows nothing of the fact that it's making a TOAST index, and
changing that to improve this messages seems a bit excessive. Thoughts?
For this version, I've kept the DEBUG1/DEBUG2 split by TOAST.
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 5790f87..8ff6927 100644
*** a/src/backend/catalog/index.c
--- b/src/backend/catalog/index.c
***
*** 1420,1425 index_build(Relation heapRelation,
--- 1420,1430
procedure = indexRelation->rd_am->ambuild;
Assert(RegProcedureIsValid(procedure));
+ ereport(IsToastRelation(indexRelation) ? DEBUG2 : DEBUG1,
+ (errmsg("building index \"%s\" on table \"%s\"",
+ RelationGetRelationName(indexRelation),
+
RelationGetRelationName(heapRelation;
+
/*
* Switch to the table owner's userid, so that any index functions are
run
* as that user. Also lock down security-restricted operations and
diff --git a/src/backend/commands/index f3bd565..e3921e4 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***
*** 3443,3448 ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap,
LOCKMODE lockmode)
--- 3443,3457
List *dropped_attrs = NIL;
ListCell *lc;
+ if (newrel)
+ ereport(DEBUG1,
+ (errmsg("rewriting table \"%s\"",
+
RelationGetRelationName(oldrel;
+ else
+ ereport(DEBUG1,
+ (errmsg("verifying table \"%s\"",
+
RelationGetRelationName(oldrel;
+
econtext = GetPerTupleExprContext(estate);
/*
***
*** 3836,3841 ATTypedTableRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd,
--- 3845,3854
* Eventually, we'd like to propagate the check or rewrite operation
* into other such tables, but for now, just error out if we find any.
*
+ * Table, NOT NULL and DEFAULT constraints and the "oid" system column do
+ * not (currently) follow the row type, so they require no attention here.
+ * The non-propagation of DEFAULT and NOT NULL make ADD COLUMN safe, too.
+ *
* Caller should provide either a table name or a type name (not both) to
* report in the error message, if any.
*
***
*** 5789,5794 validateForeignKeyConstraint(Constraint *fkconstraint,
--- 5802,5811
HeapTuple tuple;
Trigger trig;
+ ereport(DEBUG1,
+ (errmsg("validating foreign key constraint \"%s\"",
+ fkconstraint->conname)));
+
/*
* Build a trigger call structure; we'll need it e
Re: [HACKERS] Optimize PL/Perl function argument passing [PATCH]
On Tue, Dec 7, 2010 at 07:24, Tim Bunce wrote:
> Changes:
>
> Sets the local $_TD via C instead of passing an extra argument.
> So functions no longer start with "our $_TD; local $_TD = shift;"
>
> Pre-extend stack for trigger arguments for slight performance gain.
>
> Passes installcheck.
Cool, surprisingly in the non trigger case I saw up to an 18% speedup.
The trigger case remained about the same, I suppose im I/O bound.
Find attached a v2 with some minor fixes, If it looks good to you Ill
mark this as "Ready for Commit".
Changes:
- move up a declaration to make it c90 safe
- avoid using tg_trigger before it was initialized
- only extend the stack to the size we need (there was + 1 which
unless I am missing something was needed because we used to push $_TD
on the stack, but we dont any more)
Benchmarks:
---
create table t (a int);
create or replace function func(int) returns int as $$ return $_[0];
$$ language plperl;
create or replace function trig() returns trigger as $$
$_TD->{'new'}{'a'}++; return 'MODIFY'; $$ language plperl;
-- pre patch, simple function call, 3 runs
SELECT sum(func(1)) from generate_series(1, 1000);
Time: 30908.675 ms
Time: 30916.995 ms
Time: 31173.122 ms
-- post patch
SELECT sum(func(1)) from generate_series(1, 1000);
Time: 26460.987 ms
Time: 26465.480 ms
Time: 25958.016 ms
-- pre patch, trigger
insert into t (a) select generate_series(1, 100);
Time: 18186.390 ms
Time: 21291.721 ms
Time: 20782.238 ms
-- post
insert into t (a) select generate_series(1, 100);
Time: 19136.633 ms
Time: 21140.095 ms
Time: 22062.578 ms
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 5595baa..7fb69df 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -1421,7 +1421,7 @@ plperl_create_sub(plperl_proc_desc *prodesc, char *s, Oid fn_oid)
EXTEND(SP, 4);
PUSHs(sv_2mortal(newSVstring(subname)));
PUSHs(sv_2mortal(newRV_noinc((SV *) pragma_hv)));
- PUSHs(sv_2mortal(newSVstring("our $_TD; local $_TD=shift;")));
+ PUSHs(&PL_sv_no); /* unused */
PUSHs(sv_2mortal(newSVstring(s)));
PUTBACK;
@@ -1493,9 +1493,7 @@ plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
SAVETMPS;
PUSHMARK(SP);
- EXTEND(sp, 1 + desc->nargs);
-
- PUSHs(&PL_sv_undef); /* no trigger data */
+ EXTEND(sp, desc->nargs);
for (i = 0; i < desc->nargs; i++)
{
@@ -1575,21 +1573,22 @@ plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
SV *td)
{
dSP;
- SV *retval;
- Trigger*tg_trigger;
- int i;
- int count;
+ SV *retval, *TDsv;
+ int i, count;
+ Trigger*tg_trigger = ((TriggerData *) fcinfo->context)->tg_trigger;
ENTER;
SAVETMPS;
- PUSHMARK(sp);
+ TDsv = get_sv("_TD", GV_ADD);
+ SAVESPTR(TDsv); /* local $_TD */
+ sv_setsv(TDsv, td);
- XPUSHs(td);
+ PUSHMARK(sp);
+ EXTEND(sp, tg_trigger->tgnargs);
- tg_trigger = ((TriggerData *) fcinfo->context)->tg_trigger;
for (i = 0; i < tg_trigger->tgnargs; i++)
- XPUSHs(sv_2mortal(newSVstring(tg_trigger->tgargs[i])));
+ PUSHs(sv_2mortal(newSVstring(tg_trigger->tgargs[i])));
PUTBACK;
/* Do NOT use G_KEEPERR here */
--
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 support for logging the current role
Stephen Frost writes: > Okay, next user-interface question- thoughts on handling SIGHUP? My > first reaction is that we should create a new log file on SIGHUP (if we > don't already, havn't checked), or maybe just on SIGHUP if this variable > changes. > Point being, until we get Andrew's jagged-csv-import magic committed to > core, we won't be able to import a log file that a user has changed the > field list for mid-stream (following the add-a-new-column use-case we've > been discussing). Now I think you're reaching the mission-to-mars stage that Robert was complaining about. Solving that sort of problem is well outside the scope of this patch. I don't care if people have to shut down and restart their servers in order to make a change to the log format. Even if I did, the other patch sounds like a better approach. 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 support for logging the current role
Stephen Frost writes: > I'd been puzzling over how to deal with this big list of fields in > postgresql.conf and I do like the idea of some kind of short-cut being > provided to ease the pain for users. Yeah, I agree with the worry that a default value that's a mile long is going to be a bit of a PITA. But I don't think we're there yet on having a better solution. > What about something other than > version_x_y? I could maybe see having a 'default' and an 'all' > instead.. Then have the default be what we have currently and 'all' be > the full list I'm thinking about. If "default" always means what it means today, I can live with that. But if the meaning of "all" changes from version to version, that seems like a royal mess. Again, I'm concerned that an external tool like pgfouine be able to make sense of the value without too much context. If it doesn't know what some of the columns are, it can just ignore them --- but a magic summary identifier that it doesn't understand at all is a problem. 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 support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Yeah, an array or list of integer codes was what I was thinking too. Hm, yeah, a list of integer codes might be even better/simpler. Okay, next user-interface question- thoughts on handling SIGHUP? My first reaction is that we should create a new log file on SIGHUP (if we don't already, havn't checked), or maybe just on SIGHUP if this variable changes. Point being, until we get Andrew's jagged-csv-import magic committed to core, we won't be able to import a log file that a user has changed the field list for mid-stream (following the add-a-new-column use-case we've been discussing). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: > everything that looks at this GUC value > will know instantly what it is for each version. > The last bit is kind of a killer for tools like pgfouine, no? Ugh.. Could we just accept it as input but return the full list if asked for it> > In any case I thought the > expectation here was that the default column list would be frozen at > what it is now, and probably will never change. This I don't like.. When I install a new version fresh, I like to get all of the "bells & whistles" that go along with it, which, in my view, would include new fields that the smart PG folks have decided might be useful for me. I'd like to provide a way for users who are upgrading to be able to get the old behavior back, to minimize the trouble for them, and being able to say "just change the version_9_1 to version_9_0 in your log_csv_fields GUC" is a heck of a lot better than "well, rip out the default and replace it with this huge list of fields". I'd been puzzling over how to deal with this big list of fields in postgresql.conf and I do like the idea of some kind of short-cut being provided to ease the pain for users. What about something other than version_x_y? I could maybe see having a 'default' and an 'all' instead.. Then have the default be what we have currently and 'all' be the full list I'm thinking about. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> In any case, if the GUC representation is a list of field names, I think >> the POLA demands that the system honor the list order. > Agreed. That puts us back into the question of how to make it > efficient. My best thought at the moment, which doesn't strike me as > particularly efficient, is to build an array of the columns as enum's > and then have loop through the array and use a switch() on the enum. Yeah, an array or list of integer codes was what I was thinking too. > At least it's all integer-based there then and we're not calling > strcmp() for every field or strchr to find the next field, but > couldn't we do better? I really doubt that the cycles spent in the loop + switch are going to amount to anything at all, compared to the cycles involved in formatting each field and then pushing it through the CSV logic. Not to mention the I/O costs of sending the string somewhere afterwards. 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] Snapshot synchronization, again...
Hello Joachim, I'm reviewing this patch for CommitFest 2011-01. On Fri, Jan 07, 2011 at 06:41:38AM -0500, Joachim Wieland wrote: > On Thu, Dec 30, 2010 at 7:31 AM, Joachim Wieland wrote: > > What I am proposing now is the following: > > > > We return snapshot information as a chunk of data to the client. At > > the same time however, we set a checksum in shared memory to protect > > against modification of the snapshot. A publishing backend can revoke > > its snapshot by deleting the checksum and a backend that is asked to > > install a snapshot can verify that the snapshot is correct and current > > by calculating the checksum and comparing it with the one in shared > > memory. I like this design. It makes the backend's use of resources to track exported snapshots very simple. It also avoids pessimizing the chances that we can, at some point, synchronize snapshots across various standby clusters without changing the user-facing API. > > So here's the patch implementing this idea. > > I named the user visible functions pg_export_snapshot() and > pg_import_snapshot(). > > A user starts a transaction and calls pg_export_snapshot() to get a > chunk of bytea data. In a different connection he opens a transaction in > isolation level serializable and passes that chunk of data into > pg_import_snapshot(). Now subsequent queries of the second connection see the > snapshot that was current when pg_export_snapshot() has been executed. In case > both transactions are in isolation level serializable then both see the same > data from this moment on (this is the case of pg_dump). > > There are most probably a few loose ends and someone who is more familiar to > this area than me needs to look into it but at least everybody should be happy > now with the overall approach. > > These are the implementation details and restrictions of the patch: > > The exporting transaction > > - should be read-only (to discourage people from expecting that writes of > the exporting transaction can be seen by the importing transaction) > - must not be a subtransaction (we don't add subxips of our own > transaction > to the snapshot, so importing the snapshot later would result in missing > subxips) > - adds its own xid (if any) to the xip-array > > The importing transaction > > - will not import a snapshot of the same backend (even though it would > probably work) > - will not import a snapshot of a different database in the cluster > - should be isolation level serializable > - must not be a subtransaction (can we completely rollback on > subxact-rollback?) I don't know, but this restriction does not seem onerous. > - leaves curcid as is, otherwise effects of previous commands would get > lost > and magically appear later when curcid increases > - applies xmin, xmax, xip, subxip values of the exported snapshot to > GetActiveSnapshot() and GetTransactionSnapshot() > - takes itself out of the xip array > - updates MyProc->xmin but sets it only backwards but not forwards > > The snapshot is invalidated on transaction commit/rollback as well as when > doing > "prepare transaction". > > Comments welcome. General points: Basics: patch has the correct format and applies cleanly (offset in pg_proc.h). It includes no tests, but perhaps it should not. Really testing this requires concurrency, and our test framework is not set up for that. We could add some cheap tests to ensure the functions fail cleanly in some basic situations, but that would be about it. The patch does not, but should, document the new functions. The new functions also need comments preceding their definitions, similar to those found on all other functions in procarray.c. You add four elog() calls, all of which should be ereport() to facilitate translation. Also consider suitable error codes. The error messages do not follow style; in particular, they begin with capitals. See the style guide, http://www.postgresql.org/docs/current/interactive/error-style-guide.html. ImportSnapshot() has no paths that justify returning false. It should always return true or throw a suitable error. See below for various specific comments. > diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c > index 95beba8..c24150f 100644 > *** a/src/backend/storage/ipc/ipci.c > --- b/src/backend/storage/ipc/ipci.c > *** CreateSharedMemoryAndSemaphores(bool mak > *** 124,129 > --- 124,130 > size = add_size(size, BTreeShmemSize()); > size = add_size(size, SyncScanShmemSize()); > size = add_size(size, AsyncShmemSize()); > + size = add_size(size, SyncSnapshotShmemSize()); > #ifdef EXEC_BACKEND > size = add_size(size, ShmemBackendArraySize()); > #endif > *** CreateSharedMemoryAndSemaphores(bool mak > *** 228,233 > --- 229,235 > BTreeShmemInit(); > Sy
Re: [HACKERS] Add support for logging the current role
Andrew Dunstan writes: > The only thing I'd suggest extra is that we might allow "version_n_m" as > shorthand for the default table layout from the relevant version. Mmm ... seems like that just complicates matters. To make that useful, you have to assume that there *is* a default table layout, it's different across versions, and everything that looks at this GUC value will know instantly what it is for each version. The last bit is kind of a killer for tools like pgfouine, no? In any case I thought the expectation here was that the default column list would be frozen at what it is now, and probably will never change. 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 support for logging the current role
* Andrew Dunstan (and...@dunslane.net) wrote: > The only thing I'd suggest extra is that we might allow > "version_n_m" as shorthand for the default table layout from the > relevant version. I like that idea, makes the default a lot simpler to deal with too. :) Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: > In any case, if the GUC representation is a list of field names, I think > the POLA demands that the system honor the list order. Agreed. That puts us back into the question of how to make it efficient. My best thought at the moment, which doesn't strike me as particularly efficient, is to build an array of the columns as enum's and then have loop through the array and use a switch() on the enum. At least it's all integer-based there then and we're not calling strcmp() for every field or strchr to find the next field, but couldn't we do better? > BTW, in case you didn't know, there are some GUCs defined as lists of > identifiers already (look for GUC_LIST bits). Be sure to steal code. No, I didn't.. Excellent. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
On 01/14/2011 09:51 PM, Tom Lane wrote: Stephen Frost writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: A user-settable column list seems pretty on-target for solving those problems to me. I'm looking into implementing this. An interesting initial question is- should the users be able to control the *order* of the columns? My gut feeling, if we're giving them a GUC that's a list of fields, is 'yes', but I'm happy to listen to other thoughts. Yeah, I was just thinking about that in connection with the suggestion of using a bitmap as the pre-parsed representation (which would more or less force adoption of the fixed-column-order approach). I really think we can't get away with that. Remember what Andrew pointed out upthread: it's important to be able to load the csvlog output directly into a table without any extra processing. Suppose a DBA is logging columns A,B,D and he later realizes that logging C would be a good thing too. He's going to have to ALTER TABLE ADD COLUMN to add C to his logging table ... and now it's at the end. This is no problem if he can set the GUC to be "A,B,D,C" and have the field order be honored. Otherwise he's got a problem. Ok, you sold me. Until I read this I was inclined to say not, on KISS principles. The only thing I'd suggest extra is that we might allow "version_n_m" as shorthand for the default table layout from the relevant version. 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] Add support for logging the current role
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> A user-settable column list seems pretty on-target >> for solving those problems to me. > I'm looking into implementing this. > An interesting initial question is- should the users be able to control > the *order* of the columns? My gut feeling, if we're giving them a GUC > that's a list of fields, is 'yes', but I'm happy to listen to other > thoughts. Yeah, I was just thinking about that in connection with the suggestion of using a bitmap as the pre-parsed representation (which would more or less force adoption of the fixed-column-order approach). I really think we can't get away with that. Remember what Andrew pointed out upthread: it's important to be able to load the csvlog output directly into a table without any extra processing. Suppose a DBA is logging columns A,B,D and he later realizes that logging C would be a good thing too. He's going to have to ALTER TABLE ADD COLUMN to add C to his logging table ... and now it's at the end. This is no problem if he can set the GUC to be "A,B,D,C" and have the field order be honored. Otherwise he's got a problem. In any case, if the GUC representation is a list of field names, I think the POLA demands that the system honor the list order. You could escape that expectation by controlling the feature with a pile of booleans (csv_log_pid = on, csv_log_timestamp = off, etc) but I can't say that that sort of API appeals to me. BTW, in case you didn't know, there are some GUCs defined as lists of identifiers already (look for GUC_LIST bits). Be sure to steal code. 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 support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: > A user-settable column list seems pretty on-target > for solving those problems to me. I'm looking into implementing this. An interesting initial question is- should the users be able to control the *order* of the columns? My gut feeling, if we're giving them a GUC that's a list of fields, is 'yes', but I'm happy to listen to other thoughts. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
Robert Haas writes: > On Fri, Jan 14, 2011 at 8:00 PM, Tom Lane wrote: >> I was thinking of it as being strictly a field list. > I think we're in the process of designing a manned mission to Mars to > solve the problem that our shoelaces are untied. How so? ISTM the problems at hand are (a) we can't add a new CSV column without causing a flag day for users who may not even care about the information, and (b) we're worried that emitting all these columns may result in a performance hit, again for information that a particular user may not need. A user-settable column list seems pretty on-target for solving those problems to me. 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 support for logging the current role
On 01/14/2011 08:41 PM, Robert Haas wrote: I think we're in the process of designing a manned mission to Mars to solve the problem that our shoelaces are untied. What's your suggestion, then? cheers andre -- 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 support for logging the current role
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> I was thinking of it as being strictly a field list. I don't know >> whether it's really practical to borrow log_line_prefix's one-character >> names for the fields (for one thing, there would need to be names for >> all the existing CSV columns, not all of which equate to log_line_prefix >> escapes); > I'm not really happy about the idea that you can only get certain > information in a log file if you use CSV format. I said no such thing! The point here is that there is a great deal of stuff in the textual log format that is not governed by log_line_prefix, so log_line_prefix provides no precedent for naming it: the error level, the SQLSTATE, the primary message, the detail, the hint, etc, all come out without any connection to log_line_prefix. In CSV all of those already exist as columns. If we want users to be able to control which CSV columns get emitted, they'll need to be able to name those columns, and log_line_prefix doesn't provide any precedent for that. > I also don't know > that there's really any particular reason log_line_prefix's names > have to be one character. Well, it's pretty much conventional for %-escapes to be that way, though I agree we're kind of straining the system already. > I do like the idea of having just a field list though, to keep things > simple for the CSV users, and we could also pre-process the list into > flag variables or a bitmask or similar to be able to quickly check if a > certain field should be included or not. I'm not really keen about how > log_line_prefix currently parses the direct user-provided syntax every > time; strikes me as inefficient. For log_line_prefix I'm not sure you could do a lot better. I agree that a field name list for CSV would have to be preprocessed somehow for efficiency. 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] Fixing GIN for empty/null/full-scan cases
"David E. Wheeler" writes: > So some questions: > * Is something seriously wrong with GiST index creation on integer[] columns? > * Why does GIN performance appear to be no better than table scans on > integer[] columns? > * Why does it take 3-4x longer to create the GIN than the GiST index on > tsvector? I thought that GIN was supposed to be faster to update Hard to comment on any of this without a concrete example (including data) to look at. Given the bugs we've recently found in the picksplit algorithms for other contrib modules, I wouldn't be too surprised if the sucky GiST performance traced to a similar bug in intarray. But I'm not excited about devising my own test case. One other point here is that GIN index build time is quite sensitive to maintenance_work_mem --- what did you have that set to? 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] Named restore points
On Fri, Jan 14, 2011 at 8:33 PM, Euler Taveira de Oliveira wrote: > > OK. I will review your patch at the beginning of the week. > thanks -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Andrew Dunstan writes: > > If you have a format string, what do you want to do with the bits of the > > format that aren't field references? > > I was thinking of it as being strictly a field list. I don't know > whether it's really practical to borrow log_line_prefix's one-character > names for the fields (for one thing, there would need to be names for > all the existing CSV columns, not all of which equate to log_line_prefix > escapes); I'm not really happy about the idea that you can only get certain information in a log file if you use CSV format. I also don't know that there's really any particular reason log_line_prefix's names have to be one character. > but in any case anything other than field references would be > disallowed. If you prefer to use a name list as the syntax that's fine > with me. I do like the idea of having just a field list though, to keep things simple for the CSV users, and we could also pre-process the list into flag variables or a bitmask or similar to be able to quickly check if a certain field should be included or not. I'm not really keen about how log_line_prefix currently parses the direct user-provided syntax every time; strikes me as inefficient. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add support for logging the current role
On Fri, Jan 14, 2011 at 8:00 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 01/14/2011 05:08 PM, Tom Lane wrote: >>> It actually sounded like a pretty good idea to me. > >> If you have a format string, what do you want to do with the bits of the >> format that aren't field references? > > I was thinking of it as being strictly a field list. I don't know > whether it's really practical to borrow log_line_prefix's one-character > names for the fields (for one thing, there would need to be names for > all the existing CSV columns, not all of which equate to log_line_prefix > escapes); but in any case anything other than field references would be > disallowed. If you prefer to use a name list as the syntax that's fine > with me. I think we're in the process of designing a manned mission to Mars to solve the problem that our shoelaces are untied. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Named restore points
Em 14-01-2011 19:50, Jaime Casanova escreveu: On Fri, Jan 14, 2011 at 5:42 PM, Euler Taveira de Oliveira wrote: Em 14-01-2011 17:41, Jaime Casanova escreveu: Here is a patch that implements "named restore points". Nice feature. I only read the provided documentation and it seems inconsistent to allow name, time, and xid at recovery_target_name it only allow names, but those names could be anything OK. I will review your patch at the beginning of the week. -- Euler Taveira de Oliveira http://www.timbira.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] Fixing GIN for empty/null/full-scan cases
On Jan 8, 2011, at 9:41 PM, Tom Lane wrote:
> "David E. Wheeler" writes:
>> On Jan 7, 2011, at 4:19 PM, Tom Lane wrote:
>>> Well, actually, I just committed it. If you want to test, feel free.
>>> Note that right now only the anyarray && <@ @> operators are genuinely
>>> fixed ... I plan to hack on tsearch and contrib pretty soon though.
>
>> Hrm, the queries I wrote for this sort of thing use intarray:
>>WHERE blah @@ '(12|14)'::query_int
>> That's not done yet though, right?
>
> intarray is done now, feel free to test ...
Tom,
Well, I regret to say that what I found is…all over the place.
So I have a rather wide table with two GIST indexes, one on an integer[] column
and one on a tsvector column. I duped the table and replaced those indexes with
GIN indexes. And the results of my testing don't make much sense.
Well, first the good news: I got no NULL-related errors at all. There are a lot
of rows with an empty array in the integer[] column. And I got the same results
for my queries against the table with the GIN indexes as the one with the GiST
indexes. So all that's good.
One of the reasons our client wants GIN for the integer[] column so bad is
because recreating the GiST integer[] index is quite painful. Before I duped
the table, I was just dropping and recreating the index on the original table.
It was great to create the GIN index; for 400K rows, it took 1300 ms. After my
initial round of testing, I dropped it and created the GiST index. That ran
for…well, *hours*. Four at least, maybe six or seven (I forgot \timing and was
letting it run on screen while I did some iOS hacking). I think something might
be really wrong with GiST index creation for integer arrays, because the
difference was just appalling.
As a sanity check, I did the same thing today with the same table(s) on their
ts_vector columns. Creating the GiST index took just under 7 seconds; the GIN
index took 23.4 seconds. On a second attempt, GiST took 16371 ms and GIN 30452.
I had expected GIN to be faster here, but both are within the realms of the
acceptable, compared to the time it took to create the GiST index on the
integer[] column.
As for the queries, here too I was surprised. As I said, the integer[] column
had a lot of empty arrays. And the indexes look like so:
"idx_gist_features" gist (features) WHERE deleted_at IS NULL AND status = 1
"idx_gist_textsearch" gist (ts_index_col) WHERE deleted_at IS NULL AND status
= 1
Just s/gist/gin/ for the gin indexes. For the integer[] column, I ran a bunch
of queries like so (again, just s/gist/gin/ for the gin versions):
explain analyze SELECT count(*) FROM gist_listings
WHERE features @@ '(1369043)'::query_int
AND deleted_at IS NULL AND mls_status_id = 1;
This integer had pretty high selectvity, 86 out of 406K rows.
Gist: 117.444 on the first run, around 3.2 ms thereafter
GIN: Around 325 ms on all runs
explain analyze SELECT count(*) FROM gist_listings
WHERE features @@ '(1368798|1369043)'::query_int
AND deleted_at IS NULL AND mls_status_id = 1;
Rows selected: 91528.
Gist: 4030.282 ms on the first run, around 210 ms thereafter.
GIN: 4309.259 ms on the first run, around 400 ms thereafter
explain analyze SELECT count(*) FROM gist_listings
WHERE features @@ '(1368799&1368800&1369043)'::query_int
AND deleted_at IS NULL AND mls_status_id = 1;
Rows selected: 91528
Gist: 1738.568 ms on the first run, around 24 ms thereafter.
GIN: 4427.517 ms on the first run, around 340 ms thereafter
These numbers are a bit crazy-making, but the upshot is that Gist is slow out
of the gate, but with data cached, it's pretty speedy. With indexscan and
bitmapscan disabled, these queries all took 300-400 ms. So GIN was never better
performing than a table scan. So while GIN is a big win for re-indexing (this
database gets its intarray Gist indexes updated quite frequently, as they get
quited bloated in a hurry), it's not a win at all for querying.
So, thinking that there might be something funky with intarray GIN support, we
wanted to test performance of GIST vs. GIN on the tsquery column. Here GIN was
a much bigger win. With a query like this:
SELECT l.* FROM gist_listings
WHERE ts_index_col @@ to_tsquery(regexp_replace(
plainto_tsquery('english', '1 Infinite Loop')::text,
'''(?=[[:space:]]|$)', ''':B', 'g'
)) and deleted_at IS NULL AND status = 1;
With zero rows returned, GIN consistently executed in 20 ms. Gist took 838.274
for the first run and 25-30 ms on subsequent runs. So GIN is the clear winner
here, except for index creation as noted above.
And here's one that selects a single row:
SELECT l.* FROM gist_listings
WHERE ts_index_col @@ to_tsquery(regexp_replace(
plainto_tsquery('english', 'volkswagon')::text,
'''(?=[[:space:]]|$)', ''':B', 'g'
)) and deleted_at IS NULL AND status = 1;
GiST: 495.867 first run, 380 thereafter
GI
Re: [HACKERS] Per-column collation, the finale
Em 14-01-2011 20:47, Robert Haas escreveu: On Fri, Jan 14, 2011 at 4:41 PM, Peter Eisentraut wrote: I've been going over this patch with a fine-tooth comb for the last two weeks, fixed some small bugs, added comments, made initdb a little friendlier, but no substantial changes. I'm going to start working on SQL-level CREATE/DROP/ALTER COLLATION support and associated things now. Maybe I'm all wet here, but isn't it time to commit what you've got and punt the things that aren't done to 9.2? I think Peter want another person to take a look at his patch. I personally would like to eyeball his patch (but it will be during the week). -- Euler Taveira de Oliveira http://www.timbira.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] limiting hint bit I/O
On 1/14/11 11:51 AM, Tom Lane wrote: > The people whose tables are mostly insert-only complain about it, but > that's not the majority of our userbase IMO. We just happen to have a > couple of particularly vocal ones, like Berkus. It might or might not be the majority, but it's an extremely common case affecting a lot of users. Many, if not most, software applications have a "log" table (or two, or three) which just accumulates rows, and when that log table gets a vacuum freeze it pretty much halts the database in its tracks. Between my client practice and IRC, I run across complaints about this issue around 3 times a month. And data warehousing is a significant portion of our user base, and *all* DW users are affected by this. In some cases, vacuum issues are sufficient to prevent people from using PostgreSQL for data warehousing. I'd dare say that there are more users who would like autovacuum to handle big tables better than want synchronous replication, for example. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LOCK for non-tables
On Fri, 2011-01-14 at 17:34 -0500, Tom Lane wrote: > > > I'm not keen to explain to people how we broke their applications > just > > because we wanted to add new functionality AND avoid one > shift/reduce > > conflict in our SQL grammar. Avoiding changes to user code isn't > third > > on that list of three things I want, its first. > > I grow weary of discussions in which somebody argues that > consideration X always outweighs every other consideration. We're > doing engineering here, not theology, and there are always tradeoffs > to be made. In this case it's my opinion that a small syntax > adjustment is the best tradeoff. I didn't say avoiding changes to user code *always* outweighs other considerations, it just does so in this case, for me. I too am doing engineering, not theology; our opinions differ in one area, that's all. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Named restore points
On Fri, 2011-01-14 at 17:18 -0500, Tom Lane wrote: > Jaime Casanova writes: > > Here is a patch that implements "named restore points". > > > It allows DBAs to specify an exact point to which they can recover > > but that point will have a name, so they have a better control of when > > they want to stop recovery (ie: DBA's won't depend of remember > > specific times, dates and such). > > > This adds a new function: pg_create_restore_point(text) (i'm not > > wedded with the name so if someone wants to suggest something better, > > that's fine with me), a new xlog record and a new recovery_target > > parameter in recovery.conf > > This seems like it's a lot of mechanism for an awfully small use-case. > How often are people actually going to have the foresight to know that > "right now" is when they would want to restore to later? And is it > really any easier to use a label for that than a timestamp? You're > still going to need to keep track of which label means what. I think its the other way around. In order to know what time to restore to, you have to keep an external list of times when interesting things happened. This gives you a way of putting that metadata into the log stream so everything you need is in one place. You can put a restore point in before or after any major activity, so you can restore the database if that fails. e.g. 'daily backup 2001/1/11', 'reference data update 2011/2/5', 'pg_upgrade', etc.. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LOCK for non-tables
Robert Haas writes: > Me, too. But I don't agree with your particular choice of small > syntax adjustment. Maybe we should just let the issue drop for now. > Nobody's actually complained about this that I can recall; it's just a > comment that's been sitting there in pg_dump for ages, and I was > inspired to think of it again because of the SQL/MED work. I'm not > sufficiently in love with this idea to walk through fire for it. Agreed. Once there's some pressing need for it, it'll be easier to make the case that some amount of incompatibility is acceptable. 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 support for logging the current role
Andrew Dunstan writes: > On 01/14/2011 05:08 PM, Tom Lane wrote: >> It actually sounded like a pretty good idea to me. > If you have a format string, what do you want to do with the bits of the > format that aren't field references? I was thinking of it as being strictly a field list. I don't know whether it's really practical to borrow log_line_prefix's one-character names for the fields (for one thing, there would need to be names for all the existing CSV columns, not all of which equate to log_line_prefix escapes); but in any case anything other than field references would be disallowed. If you prefer to use a name list as the syntax that's fine with me. 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] SQL/MED - FDW API
On Fri, 14 Jan 2011 14:59:00 -0500 Andrew Dunstan wrote: > Have you actually posted this version of file_fdw? I haven't seen it. Sorry, now it's been posted. http://archives.postgresql.org/pgsql-hackers/2011-01/msg01205.php Regarads, -- Shigeru Hanada -- 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 support for logging the current role
On 01/14/2011 05:08 PM, Tom Lane wrote: Andrew Dunstan writes: On 01/14/2011 11:48 AM, Stephen Frost wrote: My first thought would be to have a 'log_csv_format' GUC that's very similar to 'log_line_prefix' (and uses the same variables if possible..). We could then ship a default in postgresql.conf that matches what the current format is while adding the other options if people want to use them. I'm not sure I really want to make it that flexible :-) It actually sounded like a pretty good idea to me. The current CSV format is already overly bulky/verbose, because it includes absolutely everything anybody ever wanted before now. Allowing people to select what they actually need, and thereby get rid of some of the overhead they're currently paying, would be a good thing. If you have a format string, what do you want to do with the bits of the format that aren't field references? What about delimiters? A format string makes it too easy to muck up and too hard to get right, IMNSHO. History has shown how easy it is to muck up CSVs. The suggestion I made of allowing people to suppress production of certain columns would take care of the bulk problem much more safely, I think. We've actually had remarkably few issues with CSV logs not being loadable, that I know of anyway. When we implemented it, I expected many more issues with it than we've had. I'd like to keep it that way. 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] Per-column collation, the finale
On Fri, Jan 14, 2011 at 4:41 PM, Peter Eisentraut wrote: > I've been going over this patch with a fine-tooth comb for the last two > weeks, fixed some small bugs, added comments, made initdb a little > friendlier, but no substantial changes. > > I'm going to start working on SQL-level CREATE/DROP/ALTER COLLATION > support and associated things now. Maybe I'm all wet here, but isn't it time to commit what you've got and punt the things that aren't done to 9.2? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp wrote: > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this is > worth covering in an updated patch too? > And if I change that, people might expect the same from DROP X IF EXISTS too? It's far less clear what you'd change those cases to say, and they already emit a NOTICE, so it seems unnecessary. >> Also, I don't really like the way this spreads knowledge of the >> completionTag out all over the backend. I think it would be better to >> follow the existing model used by the COPY and COMMIT commands, >> whereby the return value indicates what happened and >> standard_ProcessUtility() uses that to set the command tag. > > Right. I created this pattern after PerformPortalFetch() which already > took a completionTag argument. But your approach seems more > reasonable. OK. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Named restore points
On Fri, Jan 14, 2011 at 3:41 PM, Jaime Casanova wrote: > Here is a patch that implements "named restore points". > > It allows DBAs to specify an exact point to which they can recover > but that point will have a name, so they have a better control of when > they want to stop recovery (ie: DBA's won't depend of remember > specific times, dates and such). > > This adds a new function: pg_create_restore_point(text) (i'm not > wedded with the name so if someone wants to suggest something better, > that's fine with me), a new xlog record and a new recovery_target > parameter in recovery.conf Neat. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] LOCK for non-tables
On Fri, Jan 14, 2011 at 5:34 PM, Tom Lane wrote: >> I'm not keen to explain to people how we broke their applications just >> because we wanted to add new functionality AND avoid one shift/reduce >> conflict in our SQL grammar. Avoiding changes to user code isn't third >> on that list of three things I want, its first. > > I grow weary of discussions in which somebody argues that consideration > X always outweighs every other consideration. We're doing engineering > here, not theology, and there are always tradeoffs to be made. In this > case it's my opinion that a small syntax adjustment is the best > tradeoff. Me, too. But I don't agree with your particular choice of small syntax adjustment. Maybe we should just let the issue drop for now. Nobody's actually complained about this that I can recall; it's just a comment that's been sitting there in pg_dump for ages, and I was inspired to think of it again because of the SQL/MED work. I'm not sufficiently in love with this idea to walk through fire for it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] SQL/MED - file_fdw
On Fri, 14 Jan 2011 14:20:20 +0900 Shigeru HANADA wrote: > On Fri, 14 Jan 2011 13:03:27 +0900 > Itagaki Takahiro wrote: > > > Good catch. I merged your fix into the attached patch. > > Thanks, I'll rebase my patches. I've rebased WIP patches for file_fdw onto Itagaki-san's recent copy_export patch. Interface of NextCopyFrom() is fixed to return HeapTuple, to support tableoid without any change to TupleTableSlot. Please apply patches in this order: 1) patches for FDW API (NOT attached to this message) These patches are attached and described in this message. http://archives.postgresql.org/pgsql-hackers/2011-01/msg01096.php 2) copy_export-20110114.patch (NOT attached to this message) This patch is attached and described in this message. http://archives.postgresql.org/pgsql-hackers/2011-01/msg01066.php 3) 20110114-copy_export_HeapTupe.patch This patch fixes interface of NextCopyFrom() to return results as HeapTuple. 4) 20110114-foreign_scan.patch This patch adds HANDLER option of FOREIGN DATA WRAPPER, and ForeignScan executor node. Note that no wrapper is available in this patch. 5) 20110114-file_fdw.patch This patch adds contrib/file_fdw, foreign-data wrapper for server-side files. Supported file formats are similar to COPY command, and COPY options except "force_not_null" are accepted as generic options of foreign table. Regards, -- Shigeru Hanada 20110114-copy_export_HeapTuple.patch.gz Description: Binary data 20110114-foreign_scan.patch.gz Description: Binary data 20110114-file_fdw.patch.gz Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Named restore points
On Fri, Jan 14, 2011 at 5:42 PM, Euler Taveira de Oliveira wrote: > Em 14-01-2011 17:41, Jaime Casanova escreveu: >> >> Here is a patch that implements "named restore points". >> > Nice feature. I only read the provided documentation and it seems > inconsistent to allow name, time, and xid at recovery_target_name it only allow names, but those names could be anything -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte y capacitación de PostgreSQL -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
Aidan Van Dyk writes: > If there is going to be any change, how about using fixed columns (an > possibly allowing them to be empty for stuff that's expensive to > create/write), but adding a 1st column that contains a "version" > identifyer. And to make it easy, maybe the PG major version as the > version value. Seems like that just adds even more overhead, without really solving any of the problems we're concerned about. Code consuming the CSV log would still need a-priori knowledge of what columns to expect. 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] Named restore points
Em 14-01-2011 17:41, Jaime Casanova escreveu: Here is a patch that implements "named restore points". Nice feature. I only read the provided documentation and it seems inconsistent to allow name, time, and xid at recovery_target_name because (i) someone could name the recovery point as '1234567' (xid) or '2011-01-14' (I use this format a lot) and (ii) if the suffix name is *_name* it shouldn't allow xid and time. IMHO, recovery_target_name should allow only names. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Reduce the amount of data loss on the standby
Hi, I propose the patch which reduces the amount of data loss on the standby. If you create the trigger file after replication is terminated, the standby tries to replay all the WAL available in pg_xlog and the archive. This behavior is OK, and it's helpful to reduce the amount of data loss. But, if you create the trigger file while walreceiver is in progress, the standby replays only the WAL streamed from the master. Even if there are many outstanding WAL files in the archive or pg_xlog, they are not replayed. This might cause much data loss, so I think we should fix it. The attached patch changes the startup process so that it tries to replay all the WAL available in the archive and pg_xlog even if the trigger file is found while walreceiver is running. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center reduce_data_loss_v1.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] LOCK for non-tables
Simon Riggs writes: > On Fri, 2011-01-14 at 16:09 -0500, Tom Lane wrote: >> I think the realistic options are (1) change the syntax >> non-backward-compatibly or (2) don't add any functionality here. > (3) think of another way. The only third way that I can see is to invent some new, unrelated syntax for the new facilities. For example (just a straw man) ACQUIRE LOCK FOR object-type object-name opt-lock-type opt-no-wait The objection to this approach (which can also be lodged against your function proposal) is that it's a larger burden on users: now they have two syntaxes to remember, more complicated code to write if it needs to use both syntaxes, etc. In the long run this is more trouble than a one-time adjustment, especially if that adjustment can be limited to a small number of uncommon cases. > I'm not keen to explain to people how we broke their applications just > because we wanted to add new functionality AND avoid one shift/reduce > conflict in our SQL grammar. Avoiding changes to user code isn't third > on that list of three things I want, its first. I grow weary of discussions in which somebody argues that consideration X always outweighs every other consideration. We're doing engineering here, not theology, and there are always tradeoffs to be made. In this case it's my opinion that a small syntax adjustment is the best tradeoff. 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] Named restore points
Jaime Casanova writes: > Here is a patch that implements "named restore points". > It allows DBAs to specify an exact point to which they can recover > but that point will have a name, so they have a better control of when > they want to stop recovery (ie: DBA's won't depend of remember > specific times, dates and such). > This adds a new function: pg_create_restore_point(text) (i'm not > wedded with the name so if someone wants to suggest something better, > that's fine with me), a new xlog record and a new recovery_target > parameter in recovery.conf This seems like it's a lot of mechanism for an awfully small use-case. How often are people actually going to have the foresight to know that "right now" is when they would want to restore to later? And is it really any easier to use a label for that than a timestamp? You're still going to need to keep track of which label means what. 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] LOCK for non-tables
Simon Riggs writes: > It's a major undertaking trying to write software that runs against > PostgreSQL for more than one release. We should be making that easier, > not harder. None of the proposals would make it impossible to write a LOCK statement that works on all available releases, so I think the above argument is a tad off-topic. 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 support for logging the current role
On 01/14/2011 05:04 PM, Aidan Van Dyk wrote: If there is going to be any change, how about using fixed columns (an possibly allowing them to be empty for stuff that's expensive to create/write), but adding a 1st column that contains a "version" identifyer. And to make it easy, maybe the PG major version as the version value. If the 1st column is always the version, tools can easily know if they understand all the columns (and what order they are in) and it' easy to write a "conversion" that strips/re-aranges columns from a newer CVS dump to match an older one if you have tools that don't know about newer column layouts.. The whole point of having CSV logs is so you can load them into a database table without needing preprocessing tools. So I'm not going to be very receptive to changes that are predicated on using such tools. 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] Add support for logging the current role
Andrew Dunstan writes: > On 01/14/2011 11:48 AM, Stephen Frost wrote: >> My first thought would be to have a 'log_csv_format' GUC that's very >> similar to 'log_line_prefix' (and uses the same variables if >> possible..). We could then ship a default in postgresql.conf that >> matches what the current format is while adding the other options if >> people want to use them. > I'm not sure I really want to make it that flexible :-) It actually sounded like a pretty good idea to me. The current CSV format is already overly bulky/verbose, because it includes absolutely everything anybody ever wanted before now. Allowing people to select what they actually need, and thereby get rid of some of the overhead they're currently paying, would be a good thing. 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 support for logging the current role
On Fri, Jan 14, 2011 at 4:56 PM, Andrew Dunstan wrote: > > I'm not sure I really want to make it that flexible :-) > > To deal with the issue Tom's referring to, I think it would be sufficient if > we just allowed users to suppress production of certain columns (as long as > we never do anything so evil as to add a new column in the middle). > > There are some other issues with the format. I know Josh has bitched about > the presence of command tags in certain fields, for example. If there is going to be any change, how about using fixed columns (an possibly allowing them to be empty for stuff that's expensive to create/write), but adding a 1st column that contains a "version" identifyer. And to make it easy, maybe the PG major version as the version value. If the 1st column is always the version, tools can easily know if they understand all the columns (and what order they are in) and it' easy to write a "conversion" that strips/re-aranges columns from a newer CVS dump to match an older one if you have tools that don't know about newer column layouts.. Personally, I'm not worried about the CSV logs being backwards compatible as long as there's a very easy way to know what I might be looking at, so conversion is easy... But then again, I don't have multiple gigabytes of logs to process either. a. -- Aidan Van Dyk Create like a god, ai...@highrise.ca command like a king, http://www.highrise.ca/ work like a slave. -- 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 support for logging the current role
On 01/14/2011 11:48 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan writes: I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement. It would definitely be nice to support optional columns a little better. I'm not even sure whether the runtime overhead is worth worrying about (maybe it is, maybe it isn't, I have no data). But I do know that adding a column to the CSV output format spec causes a flag day for users. How can we avoid that? My first thought would be to have a 'log_csv_format' GUC that's very similar to 'log_line_prefix' (and uses the same variables if possible..). We could then ship a default in postgresql.conf that matches what the current format is while adding the other options if people want to use them. If we could have all the processing to generate that line go through the same function for log_line_prefix and log_csv_format, that'd be even better. Makes me tempted to throw out the current notion of 'log_line_*prefix*' and replace it with 'log_line_*format*' to match exactly the 'log_csv_format' that I'm proposing. That'd undoubtably cause more user headaches tho... :( I'm not sure I really want to make it that flexible :-) To deal with the issue Tom's referring to, I think it would be sufficient if we just allowed users to suppress production of certain columns (as long as we never do anything so evil as to add a new column in the middle). There are some other issues with the format. I know Josh has bitched about the presence of command tags in certain fields, for example. 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] limiting hint bit I/O
Tom Lane wrote: > "Kevin Grittner" writes: >> Robert Haas wrote: >>> The critical issue is whether the tuples get frozen while >>> they're still invisible to some transactions on the standby >>> server. That's when you get query cancellations. > >> Oh, OK; I get that. That seems easy enough to at least mitigate >> to a large degree by some threshold GUC. But of course, the >> longer you wait to freeze so that you don't cancel queries on the >> standby, the more you pay to recalculate visibility, so it'd be a >> fussy thing to tune. > > Yeah. Also, most of the argument for early freezing hinges on the > hope that it could happen before the tuples go to disk the first > time, which makes the window even narrower. Is there any merit to the idea that the hot standbys could be enhanced (in some post-9.1 version) to stash a list of tuples to freeze in a persistent SLRU, applying them when GLobalXmin passes the associated xid? It seems as though this would eliminate the need to roll back transactions based on freezing without slowing down the master or compromising the usability of the standby (assuming that any pending ones get applied as part of promotion, although I suppose if that time could be non-negligible, that might be the fatal flaw). This is more of a brainstorming thought than a well-researched proposal, so I won't be too surprised if there's a hole in the idea big enough to drive a truck through -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] limiting hint bit I/O
"Kevin Grittner" writes: > Robert Haas wrote: >> The critical issue is whether the tuples get frozen while they're >> still invisible to some transactions on the standby server. >> That's when you get query cancellations. > Oh, OK; I get that. That seems easy enough to at least mitigate to > a large degree by some threshold GUC. But of course, the longer you > wait to freeze so that you don't cancel queries on the standby, the > more you pay to recalculate visibility, so it'd be a fussy thing to > tune. Yeah. Also, most of the argument for early freezing hinges on the hope that it could happen before the tuples go to disk the first time, which makes the window even narrower. 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] LOCK for non-tables
On Fri, 2011-01-14 at 16:09 -0500, Tom Lane wrote: > I think the realistic options are (1) change the syntax > non-backward-compatibly or (2) don't add any functionality here. (3) think of another way. I'm not keen to explain to people how we broke their applications just because we wanted to add new functionality AND avoid one shift/reduce conflict in our SQL grammar. Avoiding changes to user code isn't third on that list of three things I want, its first. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pov 1.0 is released, testers with huge schemas needed
Dear pghackers, During the last two weeks, I have enjoyed a total of 113 hours of development of pov - PostgreSQL Object Version control system. Version 1.0 is now finally released! POV will not touch any of your "data" (tables, sequences, indexes) - it will only keep track of your "logics" (functions, constraints, defaults, triggers, aggregates and views). The possibilities are hopefully end-less, imagine a world where you never have to worrie about cumbersome reverting. If you screw-up your logics, you can simply travel back in time without losing any of your data. I just managed to start out with a clean database, load pov, take a snapshot: select pov(), import my company's entire production database (consisting of 2159 objects), take a new snapshot: select pov(), then restore to the original state: select pov(1), i.e. removing all the functions/constraints/defaults/triggers/aggregates/views back to the orignal state, then restoring the database again: select pov(2), without losing any of the data nor touching any of the "data containers" (i.e. tables, sequences, indexes). 5 minute teaser demo video: http://screencast.com/t/mYFlvcI5H38X To install: git clone g...@github.com:gluefinance/pov.git cd pov psql -f $PGSRC/postgresql-9.1alpha3/contrib/pgcrypto/pgcrypto.sql psql -f sql/install.sql To take a snapshot: postgres=# select pov(); To restore a snapshot: postgres=# select pov(1); -- replace "1" with the snapshot # Potential future features: merging schemas, diffing, etc... I would highly appreciate if you hackers could test to play around with this, using real-life database schemas, to detect any bugs. To all of you, have a great weekend! Happy hacking! -- Best regards, Joel Jacobson Glue Finance -- 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] LOCK for non-tables
I wrote: > It looks to me like the reason why there's a shift/reduce conflict is > not so much that TABLE is optional as that we allow the syntax > LOCK tablename NOWAIT BTW, I did confirm this to the extent of showing that the shift-reduce conflict could be eliminated by attaching precedences to SEQUENCE and NOWAIT, a la %nonassoc NOWAIT %nonassoc SEQUENCE This causes the ambiguous case LOCK SEQUENCE NOWAIT to be resolved by reducing SEQUENCE to unreserved_keyword, ie it's a NOWAIT request for a table named "sequence", which is backwards compatible. However, I'm not seriously proposing this as a usable fix. I think there's far too much risk of unforeseen side-effects on the behavor of other productions. We'd have to similarly attach a precedence to every object-type keyword that we cared to use in LOCK, and that would mean the potential for bollixing the behavior of an awful lot of cases. I think the realistic options are (1) change the syntax non-backward-compatibly or (2) don't add any functionality here. 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] LOCK for non-tables
On Fri, 2011-01-14 at 15:46 -0500, Tom Lane wrote: > Simon Riggs writes: > > On Fri, 2011-01-14 at 15:05 -0500, Tom Lane wrote: > >> No, that will not work at all. LOCK has to be a utility command. > > > But it doesn't break the use case for locking sequences, ISTM. > > You haven't stated what you think that use case is, but in any case > I'm sure someone can come up with another one where not freezing > the transaction snapshot *is* a consideration. > > Anyway, any suggestion that randomly breaks user applications is not > > good. If there is a good reason to do that, OK, but I don't see that > > here. > > The good reason is adding functionality. Or is it your position that > the functionality under discussion is not worth any syntax breakage, > no matter how narrowly circumscribed? I'm willing to listen to a clear restatement of the functionality being added, so we can compare that to what we will lose. Currently, IMHO, the importance of the new functionality seems low and the effect of breakage is high for our users. And I'm also interested in exploring other ways that give us the functionality requested without the breakage. Evolution, not revolution. > If we take that position then > we can drop this whole thread, because nothing's going to happen. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Database file copy
Thanks for considering our special scenario. I did not use the vacuum freeze option because the documentation said it is going to be deprecrated. Based on the positive votes so far, I gather that a vacuum (freeze) syntax will be supported in some version in the future, until then, I can continue to use the existing vacuum freeze syntax? I did try it and it works. Thank you, Srini From: Robert Haas To: Tom Lane Cc: Kevin Grittner ; Alvaro Herrera ; Bruce Momjian ; pgsql-hackers ; Srini Raghavan Sent: Fri, January 14, 2011 3:36:02 PM Subject: Re: [HACKERS] Database file copy On Fri, Jan 14, 2011 at 2:15 PM, Tom Lane wrote: > "Kevin Grittner" writes: >> If we're going to be supporting that long term, we should probably >> change the note about FREEZE being deprecated, though. > >> So, still +1 on removing the wording about FREEZE being deprecated, >> but instead we should mention what actually *is* deprecated (the >> omission of the parentheses). > > If we're going to do that, we should deprecate the unparenthesized > syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE > as well. I'm not wildly enthusiastic about breaking this with only one intervening release. We normally support deprecated syntax for quite a bit longer than that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Thanks for reviewing! On Fri, Jan 14, 2011 at 13:40, Robert Haas wrote: > Did you check whether this updated the code for 100% of the object > types where this could apply? I walked through all the CREATE statements in the documentation and these four seem to be the only ones that accept FOR REPLACE. There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this is worth covering in an updated patch too? And if I change that, people might expect the same from DROP X IF EXISTS too? > Also, I don't really like the way this spreads knowledge of the > completionTag out all over the backend. I think it would be better to > follow the existing model used by the COPY and COMMIT commands, > whereby the return value indicates what happened and > standard_ProcessUtility() uses that to set the command tag. Right. I created this pattern after PerformPortalFetch() which already took a completionTag argument. But your approach seems more reasonable. Regards, Marti -- 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] LOCK for non-tables
Simon Riggs writes: > On Fri, 2011-01-14 at 15:05 -0500, Tom Lane wrote: >> No, that will not work at all. LOCK has to be a utility command. > But it doesn't break the use case for locking sequences, ISTM. You haven't stated what you think that use case is, but in any case I'm sure someone can come up with another one where not freezing the transaction snapshot *is* a consideration. > Anyway, any suggestion that randomly breaks user applications is not > good. If there is a good reason to do that, OK, but I don't see that > here. The good reason is adding functionality. Or is it your position that the functionality under discussion is not worth any syntax breakage, no matter how narrowly circumscribed? If we take that position then we can drop this whole thread, because nothing's going to happen. 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] Database file copy
On Fri, Jan 14, 2011 at 3:41 PM, Kevin Grittner wrote: > Robert Haas wrote: > >> I'm not wildly enthusiastic about breaking this with only one >> intervening release. We normally support deprecated syntax for >> quite a bit longer than that. > > "one intervening release"? Where did you see that? > > I thought we were just talking about deprecating the old syntax, not > breaking it. If history is any guide, getting the deprecation > mentioned in the docs now would lead to actual removal somewhere > around 10.0. Oh, I guess I'm confused then... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Named restore points
Hi,
Here is a patch that implements "named restore points".
It allows DBAs to specify an exact point to which they can recover
but that point will have a name, so they have a better control of when
they want to stop recovery (ie: DBA's won't depend of remember
specific times, dates and such).
This adds a new function: pg_create_restore_point(text) (i'm not
wedded with the name so if someone wants to suggest something better,
that's fine with me), a new xlog record and a new recovery_target
parameter in recovery.conf
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index db7c834..e12720c 100644
*** a/doc/src/sgml/backup.sgml
--- b/doc/src/sgml/backup.sgml
*** restore_command = 'cp /mnt/server/archiv
*** 1075,1083
the junior DBA dropped your main transaction table), just specify the
required stopping point in recovery.conf. You can specify
the stop point, known as the recovery target, either by
! date/time or by completion of a specific transaction ID. As of this
! writing only the date/time option is very usable, since there are no tools
! to help you identify with any accuracy which transaction ID to use.
--- 1075,1084
the junior DBA dropped your main transaction table), just specify the
required stopping point in recovery.conf. You can specify
the stop point, known as the recovery target, either by
! date/time, a named restore point or by completion of a specific transaction ID.
! As of this writing, there are no tools to help you identify with any accuracy
! which transaction ID to use so only date/time and named restore points are
! useful.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 04769f1..d335104 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SELECT set_config('log_statement_stats',
*** 13930,13935
--- 13930,13938
pg_switch_xlog
+ pg_create_restore_point
+
+
pg_xlogfile_name
*** SELECT set_config('log_statement_stats',
*** 13988,13993
--- 13991,14003
+ pg_create_restore_point(name text )
+
+text
+Establish a named point for restore (restricted to superusers)
+
+
+
pg_xlogfile_name(location text)
text
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index bd9dfd1..b3fc001 100644
*** a/doc/src/sgml/recovery-config.sgml
--- b/doc/src/sgml/recovery-config.sgml
*** restore_command = 'copy "C:\\server\\arc
*** 144,149
--- 144,171
Recovery target settings
+
+ recovery_target_name
+(string)
+
+
+ recovery_target_name recovery parameter
+
+
+
+ This parameter specifies a named restore point, created with
+ pg_create_restore_point(), to which recovery
+ will proceed.
+ At most one of recovery_target_name,
+ or
+ can be specified.
+ The default is to recover to the end of the WAL log.
+ The precise stopping point is also influenced by
+ .
+
+
+
+
recovery_target_time
(timestamp)
*** restore_command = 'copy "C:\\server\\arc
*** 155,161
This parameter specifies the time stamp up to which recovery
will proceed.
! At most one of recovery_target_time and
can be specified.
The default is to recover to the end of the WAL log.
The precise stopping point is also influenced by
--- 177,184
This parameter specifies the time stamp up to which recovery
will proceed.
! At most one of recovery_target_time,
! or
can be specified.
The default is to recover to the end of the WAL log.
The precise stopping point is also influenced by
*** restore_command = 'copy "C:\\server\\arc
*** 177,183
start, transactions can complete in a different numeric order.
The transactions that will be recovered are those that committed
before (and optionally including) the specified one.
! At most one of recovery_target_xid and
can be specified.
The default is to recover to the end of the WAL log.
The precise stopping point is also influenced by
--- 200,207
start, transactions can complete in a different numeric order.
The transactions that will be recovered are those that committed
before (and optionally including) the specified one.
! At most one of recovery_target_xid,
! or
can be sp
Re: [HACKERS] Database file copy
Robert Haas wrote: > I'm not wildly enthusiastic about breaking this with only one > intervening release. We normally support deprecated syntax for > quite a bit longer than that. "one intervening release"? Where did you see that? I thought we were just talking about deprecating the old syntax, not breaking it. If history is any guide, getting the deprecation mentioned in the docs now would lead to actual removal somewhere around 10.0. -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] Database file copy
On Fri, Jan 14, 2011 at 2:15 PM, Tom Lane wrote: > "Kevin Grittner" writes: >> If we're going to be supporting that long term, we should probably >> change the note about FREEZE being deprecated, though. > >> So, still +1 on removing the wording about FREEZE being deprecated, >> but instead we should mention what actually *is* deprecated (the >> omission of the parentheses). > > If we're going to do that, we should deprecate the unparenthesized > syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE > as well. I'm not wildly enthusiastic about breaking this with only one intervening release. We normally support deprecated syntax for quite a bit longer than that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 2:51 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane wrote: >>> Freezing sooner isn't likely to reduce I/O compared to hint bits. What >>> that does is create I/O that you *have* to execute ... both in the pages >>> themselves, and in WAL. > >> It depends on which way you tilt your head - right now, we rewrite >> each table 3x - once to populate, once to hint, and once to freeze. >> If the table is doomed to survive long enough to go through all three >> of those, then freezing is better than hinting. Of course, that's not >> always the case, but people keep complaining about the way this shakes >> out. > > The people whose tables are mostly insert-only complain about it, but > that's not the majority of our userbase IMO. We just happen to have a > couple of particularly vocal ones, like Berkus. True. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Wildcard search support for pg_trgm
On 08/01/11 23:37, Alexander Korotkov wrote: > I updated my patch to make it use full index scan in GIN index which is > possible thanks to recent Tom Lane patch. Now wildcard, where no trigram can > be extracted from, invokes full index scan, which is slow but correct. Hi, unfortunately, this change made the patch not apply: http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=be0c3ea2d30ba225f0249ae88d6b0bdf3b753162 I'm getting rejects in trgm_gin.c. Could you update the patch please? Cheers, 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] LOCK for non-tables
On Fri, 2011-01-14 at 15:05 -0500, Tom Lane wrote: > Simon Riggs writes: > > On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote: > >> In any case I'd rather break apps using "LOCK foo NOWAIT" than break > >> every application using any form of LOCK at all, which is what I think > >> your proposal will amount to in practice. > > > Can I suggest that we don't break anything at all? > > > pg_lock_object(objectname, objecttype, mode); > > or > > pg_lock_sequence(name, mode); > > > is all we need... > > No, that will not work at all. LOCK has to be a utility command. > A function called by SELECT isn't a substitute, because SELECT > will acquire a transaction snapshot before executing the function, > and that breaks many use cases for locks. But it doesn't break the use case for locking sequences, ISTM. Anyway, any suggestion that randomly breaks user applications is not good. If there is a good reason to do that, OK, but I don't see that here. It's a major undertaking trying to write software that runs against PostgreSQL for more than one release. We should be making that easier, not harder. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Database file copy
Tom Lane wrote: > "Kevin Grittner" writes: > > Tom Lane wrote: > >> The reason for wanting to deprecate and ultimately remove that > >> syntax is so we can get rid of FREEZE as a reserved word. > > > Oh, OK. I can go along with that. If we're going that route, > > though, shouldn't we be getting support for the new syntax added, so > > there can be a release or two supporting both? > > You mean like 9.0? FYI, I just checked and pg_upgrade does not run the VACUUM command at all, but vacuumdb, and vacuumdb knows to use parentheses when connecting to a >= 9.0 server. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] LOCK for non-tables
Simon Riggs writes: > On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote: >> In any case I'd rather break apps using "LOCK foo NOWAIT" than break >> every application using any form of LOCK at all, which is what I think >> your proposal will amount to in practice. > Can I suggest that we don't break anything at all? > pg_lock_object(objectname, objecttype, mode); > or > pg_lock_sequence(name, mode); > is all we need... No, that will not work at all. LOCK has to be a utility command. A function called by SELECT isn't a substitute, because SELECT will acquire a transaction snapshot before executing the function, and that breaks many use cases for locks. 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] SQL/MED - FDW API
On 01/14/2011 07:23 AM, Shigeru HANADA wrote: You would be able to test these patches with "20110114" version of file_fdw wrapper patches which will be posted in another thread. Have you actually posted this version of file_fdw? I haven't seen it. 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] LOCK for non-tables
On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote: > In any case I'd rather break apps using "LOCK foo NOWAIT" than break > every application using any form of LOCK at all, which is what I think > your proposal will amount to in practice. Can I suggest that we don't break anything at all? pg_lock_object(objectname, objecttype, mode); or pg_lock_sequence(name, mode); is all we need... -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] LOCK for non-tables
Le 14 janv. 2011 à 20:08, Robert Haas a écrit : > On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane wrote: > >> So it looks to me like there are at least two fixes other than the ones >> you enumerated: >> >> 1. Make NOWAIT a reserved word. Not good, but perhaps better than >> reserving all the different object type names. >> >> 2. Disallow the above abbreviated syntax; allow NOWAIT only after an >> explicit IN ... MODE phrase. This would probably break a couple of >> applications, but I bet a lot fewer than changing the longer-established >> parts of the command syntax would break. >> >> I think #2 might be the best choice here. +1 > > That strikes me as pretty unintuitive. I'd rather take the hit of > forcing people to write "LOCK TABLE foo" instead of just "LOCK foo" > than try to explain why they have to include "IN ACCESS EXCLUSIVE > MODE" if they want to stick "NOWAIT" on the end. However, I guess > it's a matter of opinion so... anyone else have an opinion? Since you ask, see above. :) -- dim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] limiting hint bit I/O
Robert Haas writes: > On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane wrote: >> Freezing sooner isn't likely to reduce I/O compared to hint bits. What >> that does is create I/O that you *have* to execute ... both in the pages >> themselves, and in WAL. > It depends on which way you tilt your head - right now, we rewrite > each table 3x - once to populate, once to hint, and once to freeze. > If the table is doomed to survive long enough to go through all three > of those, then freezing is better than hinting. Of course, that's not > always the case, but people keep complaining about the way this shakes > out. The people whose tables are mostly insert-only complain about it, but that's not the majority of our userbase IMO. We just happen to have a couple of particularly vocal ones, like Berkus. 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] Database file copy
Tom Lane wrote: > "Kevin Grittner" writes: >> So, still +1 on removing the wording about FREEZE being >> deprecated, but instead we should mention what actually *is* >> deprecated (the omission of the parentheses). > > If we're going to do that, we should deprecate the unparenthesized > syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE > as well. +1 -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] LOCK for non-tables
Robert Haas writes: > On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane wrote: >> 2. Disallow the above abbreviated syntax; allow NOWAIT only after an >> explicit IN ... MODE phrase. This would probably break a couple of >> applications, but I bet a lot fewer than changing the longer-established >> parts of the command syntax would break. > That strikes me as pretty unintuitive. I'd rather take the hit of > forcing people to write "LOCK TABLE foo" instead of just "LOCK foo" > than try to explain why they have to include "IN ACCESS EXCLUSIVE > MODE" if they want to stick "NOWAIT" on the end. However, I guess > it's a matter of opinion so... anyone else have an opinion? It doesn't seem amazingly unintuitive to me; the syntax diagram just changes from LOCK ... [ IN lockmode MODE ] [ NOWAIT ] to LOCK ... [ IN lockmode MODE [ NOWAIT ] ] If it had been that way to start with, nobody would have questioned it. In any case I'd rather break apps using "LOCK foo NOWAIT" than break every application using any form of LOCK at all, which is what I think your proposal will amount to in practice. People aren't that eager to write useless noise words, which is what TABLE has been up to now in this 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 2:07 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane wrote: >>> That 9.0 change was far less invasive than this: it only added a count >>> field to SELECT and CTAS result tags. Quite aside from the fact that >>> the tag name stayed the same, in the SELECT case it's unlikely anyone >>> would have checked the tag at all rather than just testing for >>> PQresultStatus() == PGRES_TUPLES_OK. > >> Yeah, but that one command tag was SELECT. That's a pretty commonly >> used command. > > You're ignoring the point that people would probably use PQresultStatus > in preference to checking the tag at all, when dealing with SELECT. I would assume they would also use PQresultStatus() when checking whether CREATE OR REPLACE FUNCTION worked. Even if they were using PQcmdStatus() for some reason, which seems like an odd thing to do, there'd be no reason to check for anything beyond "is it empty?". The idea that there are massive amounts of code out there that are expecting the command tag to be *exactly* CREATE FUNCTION and will break if it differs by a byte seems quite improbable. Can you produce an example of any such code? > The other side of the argument that needs to be considered is what the > benefit is. There was a fairly clear functional gain from reporting > the rowcount for CTAS. I'm less convinced that sending back REPLACE > is a big benefit worth taking big compatibility risks for. Asserting that there are "big compatibility risks" doesn't make it so - you've offered no evidence of that. Even if a handful of people had complained about that one, I would still felt it was a good change, but it doesn't seem that there are any at all. I classify this as one of a dozen or two minor usability enhancements that we make in every release, and most people don't care, and those who do go "oh, that's handy". I think before we reject a patch for breaking things, we ought to be able to identify either some actual application that is broken by it, or at least some reasonably plausible coding pattern that would blow up. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] limiting hint bit I/O
Robert Haas wrote: > The critical issue is whether the tuples get frozen while they're > still invisible to some transactions on the standby server. > That's when you get query cancellations. Oh, OK; I get that. That seems easy enough to at least mitigate to a large degree by some threshold GUC. But of course, the longer you wait to freeze so that you don't cancel queries on the standby, the more you pay to recalculate visibility, so it'd be a fussy thing to tune. Perhaps such freeze information could be queued until a safe time on the standby. (Now that I've learned the joys of SLRU, I can see all sorts of possible uses for it) > Well, let me put together a quick patch that obliterates hint bits > entirely, and we can measure that. The background writer has > always pushed out hint bit pages; I think the reduced performance > was probably due to needing to reset hint bits on pages that we > threw away without pushing them out. It would be good to confirm and quantify. -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] limiting hint bit I/O
Tom Lane wrote: > Robert Haas writes: >> Those things are related, though. Freezing sooner could be >> viewed as an alternative to hint bits. > > Freezing sooner isn't likely to reduce I/O compared to hint bits. > What that does is create I/O that you *have* to execute ... both > in the pages themselves, and in WAL. In an environment where the vast majority of tuples live long enough to need to be frozen anyway, freezing sooner doesn't really do that to you. Granted, explicit freezing off-hours prevents autovacuum from doing that to you in large bursts at unexpected times, but if you're comparing background writer freezing to autovacuum freezing, I'm not clear on where the extra pain comes from. I am assuming that the background writer would be sane about how it did this, of course. We could all set up straw man implementations which would clobber performance. I suspect that you can envision a hueristic which would be no more bothersome than autovacuum. -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] Database file copy
"Kevin Grittner" writes: > If we're going to be supporting that long term, we should probably > change the note about FREEZE being deprecated, though. > So, still +1 on removing the wording about FREEZE being deprecated, > but instead we should mention what actually *is* deprecated (the > omission of the parentheses). If we're going to do that, we should deprecate the unparenthesized syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE as well. 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane wrote: >>> Um, yeah, I think you're having a problem keeping all the ideas straight >>> ;-). The argument about forensics has to do with how soon we're willing >>> to freeze tuples, ie replace the XID with a constant. Not about hint >>> bits. > >> Those things are related, though. Freezing sooner could be viewed as >> an alternative to hint bits. > > Freezing sooner isn't likely to reduce I/O compared to hint bits. What > that does is create I/O that you *have* to execute ... both in the pages > themselves, and in WAL. It depends on which way you tilt your head - right now, we rewrite each table 3x - once to populate, once to hint, and once to freeze. If the table is doomed to survive long enough to go through all three of those, then freezing is better than hinting. Of course, that's not always the case, but people keep complaining about the way this shakes out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:52 PM, Kevin Grittner wrote: > Robert Haas wrote: > >> Background freezing plays havoc with Hot Standby > > I must have missed or forgotten the issue of background vacuums > and hot standby. Can you summarize why that's worse than hitting > thresholds where autovacuum is freezing things? The critical issue is whether the tuples get frozen while they're still invisible to some transactions on the standby server. That's when you get query cancellations. >> this test is sufficient to show that eliminating hint bits >> altogether would a significant regression on some workloads. > > That wasn't clear to me from what you posted -- I thought that the > reduced performance might be partly (largely? mostly?) due to > competition with the background writer's work pushing the hinted > pages out. Maybe I'm missing something or you didn't post > everything you observed in this regard Well, let me put together a quick patch that obliterates hint bits entirely, and we can measure that. The background writer has always pushed out hint bit pages; I think the reduced performance was probably due to needing to reset hint bits on pages that we threw away without pushing them out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Determining client_encoding from client locale
On lör, 2009-07-25 at 01:41 -0500, Jaime Casanova wrote:
> On Fri, Jul 24, 2009 at 2:23 AM, Magnus Hagander wrote:
> >>
> >> 1) it introduces a dependency for -lpgport when compiling a client
> >> that uses libpq
> >>http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php
> >
> > For other parts of libpgport that are needed, we pull in the
> > individual source files. We specifically *don't* link libpq with
> > libpgport, for a reason. There's a comment in the Makefile that
> > explains why.
> >
>
> ok, attached a version that modifies src/interfaces/libpq/Makefile to
> push chklocale.o and eliminate the dependency on libpgport, this
> change also fixes the compile problem on windows
I have adjusted your old patch for the current tree, and it seems to
work. I think it was just forgotten last time because the move to
PQconnectdbParams had to happen first. But I'll throw it back into the
ring now.
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index fe661b8..cb3f6e3 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -245,6 +245,21 @@ PGconn *PQconnectdbParams(const char **keywords, const char **values, int expand
+
+ client_encoding
+
+
+ This sets the client_encoding
+ configuration parameter for this connection. In addition to
+ the values accepted by the corresponding server option, you
+ can use auto to determine the right
+ encoding from the current locale in the client
+ (LC_CTYPE environment variable on Unix
+ systems).
+
+
+
+
options
@@ -6341,6 +6356,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
linkend="libpq-connect-connect-timeout"> connection parameter.
+
+
+
+
+ PGCLIENTENCODING
+
+ PGCLIENTENCODING behaves the same as connection parameter.
+
+
@@ -6377,17 +6402,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
- PGCLIENTENCODING
-
- PGCLIENTENCODING sets the default client character
- set encoding. (Equivalent to SET client_encoding TO
- )
-
-
-
-
-
-
PGGEQO
PGGEQO sets the default mode for the genetic query
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..1716b3b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -593,6 +593,17 @@ $ psql "service=myservice sslmode=require"
privileges, server is not running on the targeted host, etc.),
psql will return an error and terminate.
+
+
+ If at least one of standard input or standard output are a
+ terminal, then psql sets the client
+ encoding to auto, which will detect the
+ appropriate client encoding from the locale settings
+ (LC_CTYPE environment variable on Unix systems).
+ If this doesn't work out as expected, the client encoding can be
+ overridden using the environment
+ variable PGCLIENTENCODING.
+
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..37f696a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1475,7 +1475,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
while (true)
{
-#define PARAMS_ARRAY_SIZE 7
+#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
@@ -1491,8 +1491,10 @@ do_connect(char *dbname, char *user, char *host, char *port)
values[4] = dbname;
keywords[5] = "fallback_application_name";
values[5] = pset.progname;
- keywords[6] = NULL;
- values[6] = NULL;
+ keywords[6] = "client_encoding";
+ values[6] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
+ keywords[7] = NULL;
+ values[7] = NULL;
n_conn = PQconnectdbParams(keywords, values, true);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 4f3815a..539f8be 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -171,7 +171,7 @@ main(int argc, char *argv[])
/* loop until we have a password if requested by backend */
do
{
-#define PARAMS_ARRAY_SIZE 7
+#define PARAMS_ARRAY_SIZE 8
const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
@@ -189,8 +189,10 @@ main(int argc, char *argv[])
"postgres" : options.dbname;
keywords[5] = "fallback_application_name";
values[5] = pset.progname;
- keywords[6] = NULL;
- values[6] = NULL;
+ keywords[6] = "client_encoding";
+ values[6] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
+ keywords[7] = NULL;
+ values[7] = NULL;
new_pass = false;
pset.db = PQconnectdbParams(keywo
Re: [HACKERS] limiting hint bit I/O
On Fri, Jan 14, 2011 at 2:01 PM, Kevin Grittner wrote: >> Trouble is, it breaks Hot Standby, badly. > > You're really starting to worry me here. Both for performance and > to reduce the WAN bandwidth demands of our backup strategy we are > very aggressive with our freezing. Do off-hours VACUUM (FREEZE) > runs break hot standby? Autovacuum freezing? What are the > symptoms? Freezing removes XIDs, so latestRemovedXid advances. VACUUM (FREEZE) is fine if you do it when there are no queries running on your Hot Standby server, but if there ARE queries running on the Hot Standby server, they'll be cancelled once max_standby_delay expires. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] limiting hint bit I/O
Robert Haas writes: > On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane wrote: >> Um, yeah, I think you're having a problem keeping all the ideas straight >> ;-). The argument about forensics has to do with how soon we're willing >> to freeze tuples, ie replace the XID with a constant. Not about hint >> bits. > Those things are related, though. Freezing sooner could be viewed as > an alternative to hint bits. Freezing sooner isn't likely to reduce I/O compared to hint bits. What that does is create I/O that you *have* to execute ... both in the pages themselves, and in WAL. 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] LOCK for non-tables
On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane wrote: > Robert Haas writes: >> Tom - I am willing to implement this if you think it's valuable, but >> I'd like your input on the syntax. >> http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php > > It looks to me like the reason why there's a shift/reduce conflict is > not so much that TABLE is optional as that we allow the syntax > > LOCK tablename NOWAIT > > If that weren't possible, then a table name would have to be followed by > EOL or IN (which is full-reserved), while an optional object type name > could not be followed by either, so there would be no shift/reduce > conflict. So we broke it when we added NOWAIT, not when TABLE was made > optional. Hmm, OK. > So it looks to me like there are at least two fixes other than the ones > you enumerated: > > 1. Make NOWAIT a reserved word. Not good, but perhaps better than > reserving all the different object type names. > > 2. Disallow the above abbreviated syntax; allow NOWAIT only after an > explicit IN ... MODE phrase. This would probably break a couple of > applications, but I bet a lot fewer than changing the longer-established > parts of the command syntax would break. > > I think #2 might be the best choice here. That strikes me as pretty unintuitive. I'd rather take the hit of forcing people to write "LOCK TABLE foo" instead of just "LOCK foo" than try to explain why they have to include "IN ACCESS EXCLUSIVE MODE" if they want to stick "NOWAIT" on the end. However, I guess it's a matter of opinion so... anyone else have an opinion? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Robert Haas writes: > On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane wrote: >> That 9.0 change was far less invasive than this: it only added a count >> field to SELECT and CTAS result tags. Quite aside from the fact that >> the tag name stayed the same, in the SELECT case it's unlikely anyone >> would have checked the tag at all rather than just testing for >> PQresultStatus() == PGRES_TUPLES_OK. > Yeah, but that one command tag was SELECT. That's a pretty commonly > used command. You're ignoring the point that people would probably use PQresultStatus in preference to checking the tag at all, when dealing with SELECT. psql itself is an example --- it never looks at the tag, nor shows it to the user, in the SELECT case. That patch only really changed the exposed behavior for CREATE TABLE AS SELECT / SELECT INTO, neither of which can be claimed to be hugely popular things for programs to issue. The other side of the argument that needs to be considered is what the benefit is. There was a fairly clear functional gain from reporting the rowcount for CTAS. I'm less convinced that sending back REPLACE is a big benefit worth taking big compatibility risks for. 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] LOCK for non-tables
On Fri, 2011-01-14 at 13:58 -0500, Tom Lane wrote: > Robert Haas writes: > > Tom - I am willing to implement this if you think it's valuable, but > > I'd like your input on the syntax. > > http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php > > It looks to me like the reason why there's a shift/reduce conflict is > not so much that TABLE is optional as that we allow the syntax > > LOCK tablename NOWAIT > > If that weren't possible, then a table name would have to be followed by > EOL or IN (which is full-reserved), while an optional object type name > could not be followed by either, so there would be no shift/reduce > conflict. So we broke it when we added NOWAIT, not when TABLE was made > optional. > > So it looks to me like there are at least two fixes other than the ones > you enumerated: > > 1. Make NOWAIT a reserved word. Not good, but perhaps better than > reserving all the different object type names. > > 2. Disallow the above abbreviated syntax; allow NOWAIT only after an > explicit IN ... MODE phrase. This would probably break a couple of > applications, but I bet a lot fewer than changing the longer-established > parts of the command syntax would break. > > I think #2 might be the best choice here. There's a similar issue on my new syntax for skipping the validation check on FKs. I'd appreciate it if you could propose a solution there also. I'm not sure whether I solved it, or am adding issues for the future. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] limiting hint bit I/O
Robert Haas wrote: > Freezing sooner could be viewed as an alternative to hint bits. Exactly. And as your test showed, things run faster frozen than unfrozen with hint bits set. > Trouble is, it breaks Hot Standby, badly. You're really starting to worry me here. Both for performance and to reduce the WAN bandwidth demands of our backup strategy we are very aggressive with our freezing. Do off-hours VACUUM (FREEZE) runs break hot standby? Autovacuum freezing? What are the symptoms? -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] LOCK for non-tables
Robert Haas writes: > Tom - I am willing to implement this if you think it's valuable, but > I'd like your input on the syntax. > http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php It looks to me like the reason why there's a shift/reduce conflict is not so much that TABLE is optional as that we allow the syntax LOCK tablename NOWAIT If that weren't possible, then a table name would have to be followed by EOL or IN (which is full-reserved), while an optional object type name could not be followed by either, so there would be no shift/reduce conflict. So we broke it when we added NOWAIT, not when TABLE was made optional. So it looks to me like there are at least two fixes other than the ones you enumerated: 1. Make NOWAIT a reserved word. Not good, but perhaps better than reserving all the different object type names. 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. I think #2 might be the best choice here. 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] limiting hint bit I/O
Robert Haas wrote: > Background freezing plays havoc with Hot Standby I must have missed or forgotten the issue of background vacuums and hot standby. Can you summarize why that's worse than hitting thresholds where autovacuum is freezing things? > this test is sufficient to show that eliminating hint bits > altogether would a significant regression on some workloads. That wasn't clear to me from what you posted -- I thought that the reduced performance might be partly (largely? mostly?) due to competition with the background writer's work pushing the hinted pages out. Maybe I'm missing something or you didn't post everything you observed in this regard -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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane wrote: > "Kevin Grittner" writes: >> Anyway, there are so many ideas in this area, it's hard to keep them >> all straight. Personally, if I was going to start with something, >> it would probably be to better establish what the impact is on >> various workloads of *eliminating* hint bits. > >> I know some people find them useful for forensics to a degree that >> they would prefer not to see this, > > Um, yeah, I think you're having a problem keeping all the ideas straight > ;-). The argument about forensics has to do with how soon we're willing > to freeze tuples, ie replace the XID with a constant. Not about hint > bits. Those things are related, though. Freezing sooner could be viewed as an alternative to hint bits. Trouble is, it breaks Hot Standby, badly. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Error code for "terminating connection due to conflict with recovery"
On Fri, Jan 14, 2011 at 12:29 PM, Simon Riggs wrote:
> This whole thing is confused. No change is appropriate here at all.
>
> We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for
> recovery conflicts.
>
> We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable,
> which occurs if someone drops the database that the user was connected
> to when they get kicked off. That code exists specifically to inform the
> user that they *cannot* reconnect. So pgpool should not be trying to
> trap that error and reconnect.
CheckRecoveryConflictDeadlock() uses ERRCODE_QUERY_CANCELLED.
ProcessInterrupts() sometimes uses ERRCODE_T_R_SERIALIZATION_FAILURE
and sometimes uses ERRCODE_ADMIN_SHUTDOWN. It seems to me that it
wouldn't be a bad thing to be a bit more consistent, and perhaps to
have dedicated error codes for recovery conflicts. This bit strikes
me as particularly strange:
else if (RecoveryConflictPending && RecoveryConflictRetryable)
{
pgstat_report_recovery_conflict(RecoveryConflictReason);
ereport(FATAL,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
errmsg("terminating connection due to
conflict with recovery"),
errdetail_recovery_conflict()));
}
else if (RecoveryConflictPending)
{
pgstat_report_recovery_conflict(RecoveryConflictReason);
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
errmsg("terminating connection due to
conflict with recovery"),
errdetail_recovery_conflict()));
}
That's the same error message at the same severity level with two
different SQLSTATEs depending on RecoveryConflictRetryable. Seems a
bit cryptic.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane wrote: > Robert Haas writes: >> If we're going to reject this patch on backwards-compatibility >> grounds, we need to make an argument that the backward-compatibility >> hazards are a real concern. So, again, has anyone complained about >> the changes we made in this area in 9.0? > > That 9.0 change was far less invasive than this: it only added a count > field to SELECT and CTAS result tags. Quite aside from the fact that > the tag name stayed the same, in the SELECT case it's unlikely anyone > would have checked the tag at all rather than just testing for > PQresultStatus() == PGRES_TUPLES_OK. So it was basically only changing > the result for *one* command type. I don't think it's a good basis for > arguing that this patch won't cause problems. Yeah, but that one command tag was SELECT. That's a pretty commonly used command. Most production environments probably use all of the commands affected by this patch together an order of magnitude less often than they use SELECT. Again, on what basis are we arguing that people are going to be looking at the command tag of a command that always returns the same tag? That seems pretty darn unlikely to me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] limiting hint bit I/O
"Kevin Grittner" writes: > Anyway, there are so many ideas in this area, it's hard to keep them > all straight. Personally, if I was going to start with something, > it would probably be to better establish what the impact is on > various workloads of *eliminating* hint bits. > I know some people find them useful for forensics to a degree that > they would prefer not to see this, Um, yeah, I think you're having a problem keeping all the ideas straight ;-). The argument about forensics has to do with how soon we're willing to freeze tuples, ie replace the XID with a constant. Not about hint bits. 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:34 PM, Kevin Grittner wrote: > Robert Haas wrote: > >> I'm hoping some will pick it up and play with it some more (hint, >> hint). > > That was a bit of a pun, eh? Unintentional... > Anyway, there are so many ideas in this area, it's hard to keep them > all straight. Personally, if I was going to start with something, > it would probably be to better establish what the impact is on > various workloads of *eliminating* hint bits. If the impact is > negative to a significant degree, my next step might be to try > background *freezing* of tuples (in a manner somewhat similar to > what you've done in this test) with the hint bits gone. Background freezing plays havoc with Hot Standby, and this test is sufficient to show that eliminating hint bits altogether would a significant regression on some workloads. I don't think either of those ideas can get off the ground. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Robert Haas writes: > If we're going to reject this patch on backwards-compatibility > grounds, we need to make an argument that the backward-compatibility > hazards are a real concern. So, again, has anyone complained about > the changes we made in this area in 9.0? That 9.0 change was far less invasive than this: it only added a count field to SELECT and CTAS result tags. Quite aside from the fact that the tag name stayed the same, in the SELECT case it's unlikely anyone would have checked the tag at all rather than just testing for PQresultStatus() == PGRES_TUPLES_OK. So it was basically only changing the result for *one* command type. I don't think it's a good basis for arguing that this patch won't cause 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] limiting hint bit I/O
Robert Haas wrote: > I'm hoping some will pick it up and play with it some more (hint, > hint). That was a bit of a pun, eh? Anyway, there are so many ideas in this area, it's hard to keep them all straight. Personally, if I was going to start with something, it would probably be to better establish what the impact is on various workloads of *eliminating* hint bits. If the impact is negative to a significant degree, my next step might be to try background *freezing* of tuples (in a manner somewhat similar to what you've done in this test) with the hint bits gone. I know some people find them useful for forensics to a degree that they would prefer not to see this, but I think it makes sense to establish what cost people are paying every day to maintain forensic information in this format. In previous discussions there has been some talk about being able to get better forensics from WAL files if certain barriers could be overcome -- having hard numbers on the performance benefits which might also accrue might put that work in a different perspective. -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] limiting hint bit I/O
Robert Haas writes: > On Fri, Jan 14, 2011 at 1:06 PM, Tom Lane wrote: >> Well, it reinforces my opinion that it's experimental ;-). But "first >> run" of what, exactly? > See the test case in my OP. The "runs" in question are "select sum(1) from > s". >> And are you sure you're taking a wholistic view >> of the costs/benefits? > No. Well, IMO it would be a catastrophic mistake to evaluate a patch like this on the basis of any single test case, let alone one as simplistic as that. I would observe in particular that your test case creates a table containing only one distinct value of xmin, which means that the single-transaction cache in transam.c is 100% effective, which doesn't seem to me to be a very realistic test condition. I think this is vastly understating the cost of missing hint bits. So what it needs now is a lot more testing. pg_bench might be worth trying if you want something with minimal development effort, though I'm not sure if its clog access pattern is particularly realistic either. 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] FOR KEY LOCK foreign keys
Excerpts from Robert Haas's message of vie ene 14 15:08:27 -0300 2011: > On Fri, Jan 14, 2011 at 1:00 PM, David E. Wheeler > wrote: > > On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: > > > >> Something else that might be of interest: the patch as presented here > >> does NOT solve the deadlock problem originally presented by Joel > >> Jacobson. It does solve the second, simpler example I presented in my > >> blog article referenced above, however. I need to have a closer look at > >> that problem to figure out if we could fix the deadlock too. > > > > Sounds like a big win already. Should this be considered a WIP patch, > > though, if you still plan to look at Joel's deadlock example? > > Alvaro, are you planning to add this to the CF? Eh, yes. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] FOR KEY LOCK foreign keys
Excerpts from David E. Wheeler's message of vie ene 14 15:00:48 -0300 2011: > On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: > > > Something else that might be of interest: the patch as presented here > > does NOT solve the deadlock problem originally presented by Joel > > Jacobson. It does solve the second, simpler example I presented in my > > blog article referenced above, however. I need to have a closer look at > > that problem to figure out if we could fix the deadlock too. > > Sounds like a big win already. Should this be considered a WIP patch, though, > if you still plan to look at Joel's deadlock example? Not necessarily -- we can implement that as a later refinement/improvement. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 12:07 PM, Tom Lane wrote: > Alvaro Herrera writes: >> Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011: >>> Also, I don't really like the way this spreads knowledge of the >>> completionTag out all over the backend. I think it would be better to >>> follow the existing model used by the COPY and COMMIT commands, >>> whereby the return value indicates what happened and >>> standard_ProcessUtility() uses that to set the command tag. > >> Yeah, that looks ugly. However it's already ugly elsewhere: for example >> see PerformPortalFetch. I am not sure if it should be this patch's >> responsability to clean that stuff up. (Maybe we should decree that at >> least this patch shouldn't make the situation worse.) > > I thought we were going to reject the patch outright anyway. The > compatibility consequences of changing command tags are not worth the > benefit, independently of how ugly the backend-side code may or may > not be. My previous response to this criticism was here: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01899.php Your response, which seemed at least partially in agreement, is here: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01901.php If we're going to reject this patch on backwards-compatibility grounds, we need to make an argument that the backward-compatibility hazards are a real concern. So, again, has anyone complained about the changes we made in this area in 9.0? And under what circumstances do we foresee someone relying on the command tag of a command that always returns the same tag? I'm as quick as anyone to bow before a compelling argument, but I don't think anyone's made such an argument. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
