Re: [HACKERS] Odd behavior of updatable security barrier views on foreign tables
On 18 February 2015 at 16:22, Stephen Frost sfr...@snowman.net wrote: Here's the patch against master. I'm still fiddling with the comment wording and the commit message a bit, but barring objections these patches are what I'm planning to move forward with. Yes, that matches what I had in mind. While you're tweaking comments, you might want to look at the comment in the block above which also relates to this new code, and says that we will end up locking all rows which pass the securityQuals. That's not really accurate, I think it wants to say something like more like we won't necessarily be able to push user-defined quals down into the subquery since they may include untrusted functions, and that means that we may end up locking rows that don't pass the user-defined quals. In the worst case, we may end up locking all rows which pass the securityQuals Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
2015-02-19 16:06 GMT+01:00 Petr Jelinek p...@2ndquadrant.com: On 19/01/15 17:14, Pavel Stehule wrote: 2015-01-19 14:27 GMT+01:00 Robert Haas robertmh...@gmail.com mailto:robertmh...@gmail.com: On Mon, Jan 19, 2015 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote: I think you should just remove the WARNING, not change it to an error. If somebody wants to quote the operator name to be able to continue using it, I think that's OK. It looks so quoting doesn't help here + CREATE OPERATOR = ( +leftarg = int8,-- right unary +procedure = numeric_fac + ); + ERROR: syntax error at or near ( + LINE 1: CREATE OPERATOR = ( + ^ Well then the error check is just dead code. Either way, you don't need it. yes, I removed it I am marking this as Ready For Committer, the patch is trivial and works as expected, there is nothing to be added to it IMHO. The = operator was deprecated for several years so it should not be too controversial either. Thank you very much Pavel -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Turning recovery.conf into GUCs
On 1/6/15 4:22 PM, Peter Eisentraut wrote: That said, there is a much simpler way to achieve that specific functionality: Expose all the recovery settings as fake read-only GUC variables. See attached patch for an example. The commit fest app has this as the patch of record. I don't think there is a real updated patch under consideration here. This item should probably not be in the commit fest at all. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] How about to have relnamespace and relrole?
On 2/18/15 3:44 AM, Kyotaro HORIGUCHI wrote: Sorry, I sent the previous mail without patches by accident. The patches are attached to this mail. Hello, this is the patchset v2 of this feature. 0001-Add-regrole.patch Adding regrole as the name says. 0002-Add-regnamespace.patch Adding regnamespace. This depends on 0001 patch. 0003-Check-new-reg-types-are-not-used-as-default-values.patch Inhibiting the new OID aliss types from being used as constants in default values, which make dependencies on the other (existing) OID alias types. 0004-Documentation-for-new-OID-alias-types.patch Documentation patch for this new types. Somehow, these patches ended up in the commit fest without an author listed. That should probably not be possible. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
2015-02-19 19:51 GMT+01:00 David Fetter da...@fetter.org: On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote: Hi all I am sending a review of this patch: * What it does? - Allow to connect to other db by \connect uri connection format postgres=# \c postgresql://localhost?service=old psql (9.5devel, server 9.2.9) You are now connected to database postgres as user pavel. * Would we this feature? - yes, it eliminate inconsistency between cmd line connect and \connect. It is good idea without any objections. * This patch is cleanly applicable, later compilation without any issues * All regress tests passed * A psql documentation is updated -- this feature (and format) is not widely known, so maybe some more examples are welcome * When I tested this feature, it worked as expected * Code respects PostgreSQL coding rules. I prefer a little bit different test if keep password. Current code is little bit harder to understand. But I can live with David's code well too. if (!user) user = PQuser(o_conn); if (!host) host = PQhost(o_conn); if (!port) port = PQport(o_conn); if (dbname) has_connection_string = recognized_connection_string(dbname); /* we should not to keep password if some connection property is changed */ keep_password = strcmp(user, PQuser(o_conn)) == 0 strcmp(host, PQhost(o_conn)) == 0 strcmp(port, PQport(o_conn)) == 0 !has_connection_string; Changed. This is cleaner. I have not any other comments. Possible questions: 1. more examples in doc I'm not sure how best to illustrate those. Are you thinking of one example each for the URI and conninfo cases? some like most common form is: \c mydb but you can use any connection format described (libpq-connect.html#LIBPQ-PARAMKEYWORDS) like \c postgresql://tom@localhost/mydb?application_name=myapp 2. small change how to check keep_password Done. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
Ants Aasma a...@cybertec.at wrote: If I understood the issue correctly, you have long running snapshots accessing one part of the data, while you have high churn on a disjoint part of data. We would need to enable vacuum on the high churn data while still being able to run those long queries. The snapshot too old solution limits the maximum age of global xmin at the cost of having to abort transactions that are older than this and happen to touch a page that was modified after they were started. Pretty much. It's not quite as black and white regarding high churn and stable tables, though -- at least for the customer whose feedback drove my current work on containing bloat. They are willing to tolerate an occasional snapshot too old on a cursor, and they can automatically pick up again by restarting the cursor with a new lower limit and a new snapshot. What about having the long running snapshots declare their working set, and then only take them into account for global xmin for relations that are in the working set? Like a SET TRANSACTION WORKING SET command. This way the error is deterministic, vacuum on the high churn tables doesn't have to wait for the old transaction delay to expire and we avoid a hard to tune GUC (what do I need to set old_snapshot_threshold to, to reduce bloat while not having normal transactions abort). Let me make sure I understand your suggestion. You are suggesting that within a transaction you can declare a list of tables which should get the snapshot too old error (or something like it) if a page is read which was modified after the snapshot was taken? Snapshots within that transaction would not constrain the effective global xmin for purposes of vacuuming those particular tables? If I'm understanding your proposal, that would help some of the cases the snapshot too old case addresses, but it would not handle the accidental idle in transaction case (e.g., start a repeatable read transaction, run one query, then sit idle indefinitely). I don't think it would work quite as well for some of the other cases I've seen, but perhaps it could be made to fit if we could figure out the accidental idle in transaction case. I also think it might be hard to develop a correct global xmin for vacuuming a particular table with that solution. How do you see that working? -- Kevin Grittner EDB: 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] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0
On Thu, Feb 19, 2015 at 11:10 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: I fully agree with your summary here. However, why should we suppose that while we wait, the other backends don't both delete and then re-insert their tuple? They need the pre-check to know not to re-insert their tuple (seeing our tuple, immediately after we wake as the preferred backend with the older XID) in order to break the race. But today, exclusion constraints are optimistic in that the insert happens first, and only then the check. The pre-check turns that the other way around, in a limited though necessary sense. I'm not sure I understand exactly what you're saying, but AFAICS the pre-check doesn't completely solve that either. It's entirely possible that the other backend deletes its tuple, our backend then performs the pre-check, and the other backend re-inserts its tuple again. Sure, the pre-check reduces the chances, but we're talking about a rare condition to begin with, so I don't think it makes sense to add much code just to reduce the chances further. But super deletion occurs *before* releasing the token lock, which is the last thing we do before looping around and starting again. So iff we're the oldest XID, the one that gets to win by unexpectedly waiting on another's token in our second phase (second call to check_exclusion_or_unique_constraint()), we will not, in fact, see anyone else's tuple, because they'll all be forced to go through the first phase and find our pre-existing, never-deleted tuple, so we can't see any new tuple from them. And, because they super delete before releasing their token, they'll definitely have super deleted when we're woken up, so we can't see any old/existing tuple either. We have our tuple inserted this whole time - ergo, we do, in fact, win reliably. The fly in the ointment is regular inserters, perhaps, but we've agreed that they're not too important here, and even when that happens we're in deadlock land, not livelock land, which is obviously a nicer place to live. Does that make sense? -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Dead code in gin_private.h related to page split in WAL
On 02/19/2015 05:34 AM, Michael Paquier wrote: I noticed that the following structures are still defined in gin_private.h but they are used nowhere since 2c03216d that has reworked WAL format: - ginxlogSplitEntry - ginxlogSplitDataLeaf - ginxlogSplitDataInternal Attached is a trivial patch to remove them. Removed, thanks. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] deparsing utility commands
Stephen Frost wrote: Yes, I will push these unless somebody objects soon, as they seem perfectly reasonable to me. The only troubling thing is that there is no regression test for this kind of thing in event triggers (i.e. verify which command tags get support and which don't), which seems odd to me. Not these patches's fault, though, so I'm not considering adding any ATM. Ugh. I dislike that when we say an event trigger will fire before 'GRANT' what we really mean is GRANT when it's operating against a local object. At the minimum we absolutely need to be very clear in the documentation about that limitation. Perhaps there is something already which reflects that, but I don't see anything in the patch being added about that. Hmm, good point, will give this some thought. I'm thinking perhaps we can add a table of which object types are supported for generic commands such as GRANT, COMMENT and SECURITY LABEL. That sounds like a good idea. Here's a patch. I noticed that the introduction to event trigger already says they only act on local objects: The ddl_command_start event occurs just before the execution of a CREATE, ALTER, DROP, SECURITY LABEL, COMMENT, GRANT or REVOKE command. No check whether the affected object exists or doesn't exist is performed before the event trigger fires. As an exception, however, this event does not occur for DDL commands targeting shared objects — databases, roles, and tablespaces — or for commands targeting event triggers themselves. So adding more text to the same effect would be repetitive. I added a sixth column Notes to the table of supported command tags vs. events, with the text Only for local objects next to the four commands being added here. I think it's fair to push this patch as is. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services From b366da953a10dea2fc2ae3e172cf15ab10e1e7ad Mon Sep 17 00:00:00 2001 From: Alvaro Herrera alvhe...@alvh.no-ip.org Date: Thu, 19 Feb 2015 17:02:41 -0300 Subject: [PATCH] Support more commands in event triggers COMMENT, SECURITY LABEL, and GRANT/REVOKE now also fire ddl_command_start and ddl_command_end event triggers, when they operate on database-local objects. Reviewed-By: Michael Paquier, Andres Freund, Stephen Frost --- doc/src/sgml/event-trigger.sgml | 125 ++- src/backend/commands/event_trigger.c | 34 +- src/backend/tcop/utility.c | 68 +++ src/include/commands/event_trigger.h | 1 + 4 files changed, 211 insertions(+), 17 deletions(-) diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 156c463..04353ea 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -36,7 +36,9 @@ para The literalddl_command_start/ event occurs just before the - execution of a literalCREATE/, literalALTER/, or literalDROP/ + execution of a literalCREATE/, literalALTER/, literalDROP/, + literalSECURITY LABEL/, + literalCOMMENT/, literalGRANT/ or literalREVOKE/ command. No check whether the affected object exists or doesn't exist is performed before the event trigger fires. As an exception, however, this event does not occur for @@ -123,14 +125,15 @@ table id=event-trigger-by-command-tag titleEvent Trigger Support by Command Tag/title - tgroup cols=5 + tgroup cols=6 thead row -entrycommand tag/entry +entryCommand Tag/entry entryliteralddl_command_start/literal/entry entryliteralddl_command_end/literal/entry entryliteralsql_drop/literal/entry entryliteraltable_rewrite/literal/entry +entryNotes/entry /row /thead tbody @@ -140,6 +143,7 @@ entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry entry align=centerliteral-/literal/entry +entry align=center/entry /row row entry align=leftliteralALTER COLLATION/literal/entry @@ -147,6 +151,7 @@ entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry entry align=centerliteral-/literal/entry +entry align=center/entry /row row entry align=leftliteralALTER CONVERSION/literal/entry @@ -154,6 +159,7 @@ entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry entry align=centerliteral-/literal/entry +entry align=center/entry /row row entry align=leftliteralALTER DOMAIN/literal/entry @@ -161,6 +167,7 @@ entry align=centerliteralX/literal/entry entry align=centerliteral-/literal/entry entry align=centerliteral-/literal/entry +
Re: [HACKERS] pg_dump gets attributes from tables in extensions
On 2/16/15 2:45 AM, Michael Paquier wrote: While looking at the patch to fix pg_dump with extensions containing tables referencing each other, I got surprised by the fact that getTableAttrs tries to dump table attributes even for tables that are part of an extension. Is that normal? Attached is a patch that I think makes things right, but not dumping any tables that are part of ext_member. Can you provide an example/test case? (e.g., which publicly available extension contains tables with attributes?) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c
On 2/18/15 1:26 AM, Michael Paquier wrote: On Wed, Feb 18, 2015 at 10:06 AM, Michael Paquier wrote: Yes, the existing assertion is right. My point is that it is strange that we do not check the values of freeze parameters for an ANALYZE query, which should be set to -1 all the time. If this is thought as not worth checking, I'll drop this patch and my concerns. Perhaps this explains better what I got in mind, aka making the assertion stricter: Assert((vacstmt-options VACOPT_VACUUM) || - !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); + ((vacstmt-options (VACOPT_FULL | VACOPT_FREEZE)) == 0 + vacstmt-freeze_min_age 0 + vacstmt-freeze_table_age 0 + vacstmt-multixact_freeze_min_age 0 + vacstmt-multixact_freeze_table_age 0)); That's cool if you want to add those assertions, but please make them separate statements each, like Assert(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE) || vacstmt-freeze_min_age == -1); Assert(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE) || vacstmt-freeze_table_age == -1); ... Besides being more readable, this will give you more useful output if the assertion fails. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)
On 1/14/15 11:31 PM, Michael Paquier wrote: pg_regress will fail with test suites using only source files if the destination folders do not exist in the code tree. This is annoying because this forces to maintain empty folders sql/ and expected/ with a .gitignore ignoring everything. We'd still need the .gitignore files somewhere. Do you want to move them one directory up? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
On 02/19/2015 03:31 PM, Kevin Grittner wrote: What about having the long running snapshots declare their working set, and then only take them into account for global xmin for relations that are in the working set? Like a SET TRANSACTION WORKING SET command. This way the error is deterministic, vacuum on the high churn tables doesn't have to wait for the old transaction delay to expire and we avoid a hard to tune GUC (what do I need to set old_snapshot_threshold to, to reduce bloat while not having normal transactions abort). Let me make sure I understand your suggestion. You are suggesting that within a transaction you can declare a list of tables which should get the snapshot too old error (or something like it) if a page is read which was modified after the snapshot was taken? Snapshots within that transaction would not constrain the effective global xmin for purposes of vacuuming those particular tables? I thought it meant that the declared tables would only be vacuumed conservatively, so the transaction would expect not to see snapshot too old from them. 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] CATUPDATE confusion?
On 12/29/14 7:16 PM, Adam Brightwell wrote: Given this discussion, I have attached a patch that removes CATUPDATE for review/discussion. One of the interesting behaviors (or perhaps not) is how 'pg_class_aclmask' handles an invalid role id when checking permissions against 'rolsuper' instead of 'rolcatupdate'. I'd get rid of that whole check, not just replace rolcatupdate by rolsuper. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
On 2015-02-18 21:00:43 -0500, Tom Lane wrote:
Michael Paquier michael.paqu...@gmail.com writes:
3) heapam.c in three places with HeapTupleHeaderData:
struct
{
HeapTupleHeaderData hdr;
chardata[MaxHeapTupleSize];
} tbuf;
And this, though I'm not sure if we'd have to change the size of the
padding data[] member.
I don't think so.
/*
* MaxHeapTupleSize is the maximum allowed size of a heap tuple, including
* header and MAXALIGN alignment padding. Basically it's BLCKSZ minus the
* other stuff that has to be on a disk page. Since heap pages use no
* special space, there's no deduction for that.
...
#define MaxHeapTupleSize (BLCKSZ - MAXALIGN(SizeOfPageHeaderData +
sizeof(ItemIdData)))
5) reorderbuffer.h with its use of HeapTupleHeaderData:
Hmm. Andres will have to answer for that one ;-)
That should be fairly uncomplicated to replace.
...
/* tuple, stored sequentially */
HeapTupleData tuple;
HeapTupleHeaderData header;
chardata[MaxHeapTupleSize];
probably can just be replaced by a union of data and header part - as
quoted above MaxHeapTupleSize actually contains space for the
header. It's a bit annoying because potentially some output plugin might
reference .header - but they can just be changed to reference
tuple.t_data instead.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training 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] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
On 2015-02-18 17:29:27 -0500, Tom Lane wrote: Michael Paquier michael.paqu...@gmail.com writes: On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund and...@2ndquadrant.com wrote: The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the middle of a struct but not when when you embed a struct that uses it into the middle another struct. At least gcc doesn't and I think it'd be utterly broken if another compiler did that. If there's a compiler that does so, we need to make it define FLEXIBLE_ARRAY_MEMBER to 1. clang does complain on my OSX laptop regarding that ;) I'm a bit astonished that gcc doesn't consider this an error. Sure seems like it should. Why? The flexible arrary stuff tells the compiler that it doesn't have to worry about space for the array - it seems alright that it actually doesn't. There's pretty much no way you can do that sensibly if the variable length array itself is somewhere in the middle of a struct - but if you embed the whole struct somewhere you have to take care yourself. And e.g. the varlena cases Michael has shown do just that? (Has anyone tried it on recent gcc?) Yes. Moreover, if we have any code that is assuming such cases are okay, it probably needs a second look. Isn't this situation effectively assuming that a variable-length array is fixed-length? Not really. If you have struct varlena hdr; chardata[TOAST_MAX_CHUNK_SIZE]; /* make struct big enough */ the variable length part is preallocated in the data? You're right that many of these structs could just be replaced with a union though. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] INSERT ... ON CONFLICT UPDATE and logical decoding
Hi, On 2015-02-18 16:35:14 -0800, Peter Geoghegan wrote: Andres pointed out that the INSERT ... ON CONFLICT UPDATE patch doesn't work well with logical decoding. Just to make that clear: I didn't actually test it, but it ddidn't look good. I guess that the best way of fixing this is exposing to output plugins that a super deletion is a REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE. I'm pretty much dead set against exposing anything like this output plugins. The output plugin shouldn't care that a insertion was a upsert. For one they won't get it right since they will be really infrequent, for another it'll be hard to replay such an event on another node. This is kind of an internal ReorderBufferChangeType constant, because it means that the output plugin should probably just omit the tuple just inserted and now deleted. An output plugin can't just go back and call a tuple that was already sent to a client back. I tend to think that it would be best if the fact that a speculative insertion was a speculative insertion is not exposed. We just formalize that a REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE is a variant of REORDER_BUFFER_CHANGE_DELETE...it's implicit that things like triggers are not fired here (which may or may not matter, depending on the use case, I suppose). I can't see that working. How such a event - that obviously causes unique conflicts! - be replayed on another node? Would that be sufficiently flexible for the various logical replication use cases? I guess we are somewhat forced to push that kind of thing into output plugins, because doing so lets them decide how to handle this. I don't see what benefits that'd bring. It's a little unfortunate that this implementation detail is exposed to output plugins, though, which is why I'd be willing to believe that it'd be better to have transaction reassembly normalize the records such that a super deleted tuple was never exposed to output plugins in the first place... Yes. they'd only see a REORDER_BUFFER_CHANGE_INSERT when that was the definitive outcome of an UPSERT, or alternatively a REORDER_BUFFER_CHANGE_UPDATE when that was the definitive outcome. No need for output plugins to consider REORDER_BUFFER_CHANGE_INTERNAL_SUPERDELETE at all. Yes. It'd be easiest if the only the final insert/update were actually WAL logged as full actions. IIUC we can actually otherwise end with a, theoretically, arbitrarily large chain of insert/super deletions. Thoughts? Can't say that I've given conflict resolution for multi-master systems a great deal of thought before now, so I might be quite off the mark here. I don't think conflict resolution actually plays a role here. This is about consistency inside a single system, not consistency across systems. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] xpath changes in the recent back branches
Hi,
Commit 79af9a1d2668c9edc8171f03c39e7fed571eeb98 changed xpath handling
with regard to namespaces, and it seems to be fixing an actual issue.
However, it was also backpatched to all branches despite it breaking for
example code like this:
do $$
declare
_x xml;
begin
_x := (xpath('/x:Foo/x:Bar', xml 'Foo
xmlns=teh:urnBarBaz1/BazBat2/Bat/Bar/Foo',
array[['x','teh:urn']]))[1];
raise notice '%', xpath('/Bar/Baz/text()', _x);
raise notice '%', xpath('/Bar/Bat/text()', _x);
end
$$;
The problem is that there's no way to write the code like this in such a
way that it would work on both versions. If I add the namespace, it's
broken on 9.1.14. Without it it's broken on 9.1.15.
I'm now thinking of adding a workaround which strips namespaces, but
that doesn't seem to be easy to do, even with PL/Perl. Is there a
better workaround here that I'm not seeing?
I'm not sure how changing behavior like this in a minor release was
considered acceptable.
.m
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup may fail to send feedbacks.
On Wed, Feb 18, 2015 at 5:34 PM, Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp wrote: Hello, this is the last patch for pg_basebackup/pg_receivexlog on master (9.5). Preor versions don't have this issue. 4. basebackup_reply_fix_mst_v2.patch receivelog.c patch applyable on master. This is based on the same design with walrcv_reply_fix_91_v2.patch in the aspect of gettimeofday(). Thanks for updating the patches! But I'm still not sure if the idea depending on the frequent calls of gettimeofday() for each WAL receive is good or not. Some users may complain about the performance impact by such frequent calls and we may want to get rid of them from walreceiver loop in the future. If we adopt your idea now, I'm afraid that it would tie our hands in that case. How much impact can such frequent calls of gettimeofday() have on replication performance? If it's not negligible, probably we should remove them at first and find out another idea to fix the problem you pointed. ISTM that it's not so difficult to remove them. Thought? Do you have any numbers which can prove that such frequent gettimeofday() has only ignorable impact on the performance? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0
On 02/18/2015 11:43 PM, Peter Geoghegan wrote: Heikki seemed to think that the deadlock problems were not really worth fixing independently of ON CONFLICT UPDATE support, but rather represented a useful way of committing code incrementally. Do I have that right? Yes. The way I chose to break the livelock (what I call livelock insurance) does indeed involve comparing XIDs, which Heikki thought most promising. But it also involves having the oldest XID wait on another session's speculative token in the second phase, which ordinarily does not occur in the second phase/check_exclusion_or_unique_constraint() call. The idea is that one session is guaranteed to be the waiter that has a second iteration within its second-phase check_exclusion_or_unique_constraint() call, where (following the super deletion of conflict TIDs by the other conflicting session or sessions) reliably finds that it can proceed (those other sessions are denied the opportunity to reach their second phase by our second phase waiter's still-not-super-deleted tuple). However, it's difficult to see how to map this on to general exclusion constraint insertion + enforcement. In Heikki's recent sketch of this [1], there is no pre-check, since that is considered an UPSERT thing deferred until a later patch, and therefore my scheme here cannot work (recall that for plain inserts with exclusion constraints, we insert first and check last). I have a hard time justifying adding the pre-check for plain exclusion constraint inserters given the total lack of complaints from the field regarding unprincipled deadlocks, and given the fact that it would probably make the code more complicated than it needs to be. It is critical that there be a pre-check to prevent livelock, though, because otherwise the restarting sessions can go straight to their second phase (check_exclusion_or_unique_constraint() call), without ever realizing that they should not do so. Hmm. I haven't looked at your latest patch, but I don't think you need to pre-check for this to work. To recap, the situation is that two backends have already inserted the heap tuple, and then see that the other backend's tuple conflicts. To avoid a livelock, it's enough that one backend super-deletes its own tuple first, before waiting for the other to complete, while the other other backend waits without super-deleting. No? It seems like the livelock insurance is pretty close to or actually free, and doesn't imply that a second phase wait for token needs to happen too often. With heavy contention on a small number of possible tuples (100), and 8 clients deleting and inserting, I can see it happening only once every couple of hundred milliseconds on my laptop. It's not hard to imagine why the code didn't obviously appear to be broken before now, as the window for an actual livelock must have been small. Also, deadlocks bring about more deadlocks (since the deadlock detector kicks in), whereas livelocks do not bring about more livelocks. It might be easier to provoke the livelocks with a GiST opclass that's unusually slow. I wrote the attached opclass for the purpose of testing this a while ago, but I haven't actually gotten around to do much with it. It's called useless_gist, because it's a GiST opclass for integers, like btree_gist, but the penalty and picksplit functions are totally dumb. The result is that the tuples are put to the index in pretty much random order, and every scan has to scan the whole index. I'm posting it here, in the hope that it happens to be useful, but I don't really know if it is. - Heikki useless_gist.tar.gz Description: application/gzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Precedence of standard comparison operators
My Salesforce colleagues have been bugging me about this topic, and since I see in a nearby thread that we may be about to break backwards compatibility on =, maybe it's time to do something about this too. To wit, that the precedence of = = and is neither sane nor standards compliant. Up to now, Postgres has had special precedence rules for = and , but their multi-character brethren just get treated as general Op tokens. This has assorted surprising consequences; for example you can do this: regression=# select * from testjsonb where j-'space' j-'node'; but not this: regression=# select * from testjsonb where j-'space' = j-'node'; ERROR: operator does not exist: text = jsonb LINE 1: select * from testjsonb where j-'space' = j-'node'; ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. Of course the latter happens because - and = have identical precedence so the construct is parsed as ((j - 'space') = j) - 'node' whereas has lower precedence than user-defined operators so the first case parses in the expected fashion. I claim that this behavior is contrary to spec as well as being unintuitive. Following the grammar productions in SQL99: where clause ::= WHERE search condition search condition ::= boolean value expression boolean value expression ::= boolean term | boolean value expression OR boolean term boolean term ::= boolean factor | boolean term AND boolean factor boolean factor ::= [ NOT ] boolean test boolean test ::= boolean primary [ IS [ NOT ] truth value ] truth value ::= TRUE | FALSE | UNKNOWN boolean primary ::= predicate | parenthesized boolean value expression | nonparenthesized value expression primary parenthesized boolean value expression ::= left paren boolean value expression right paren predicate ::= comparison predicate | between predicate | in predicate | like predicate | null predicate | quantified comparison predicate | exists predicate | unique predicate | match predicate | overlaps predicate | similar predicate | distinct predicate | type predicate comparison predicate ::= row value expression comp op row value expression comp op ::= equals operator | not equals operator | less than operator | greater than operator | less than or equals operator | greater than or equals operator row value expression ::= row value special case | row value constructor contextually typed row value expression ::= row value special case | contextually typed row value constructor row value special case ::= value specification | value expression So both the examples I gave should be understood as row value special case value expressions related by comp ops. This line of reasoning says that any non-boolean operator should bind tighter than the six standard comparison operators, because it will necessarily be part of a value expression component of a boolean expression. We have that right for = but not for the other three standard-mandated comparison operators. I think we should change the grammar so that all six act like do now, that is, they should have %nonassoc precedence just above NOT. Another thought, looking at this closely, is that we have the precedence of IS tests (IS NOT NULL etc) wrong as well: they should bind less tightly than user-defined ops, not more so. I'm less excited about changing that, but there's certainly room to argue that it's wrong per spec. It's definitely weird that the IS tests bind more tightly than multicharacter Ops but less tightly than + - * /. I've not really experimented with this at all; it would be useful for example to see how many regression tests break as a gauge for how troublesome such changes would be. I thought I'd ask whether there's any chance at all of such a change getting accepted before doing any serious work on it. Thoughts? 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 min and max execute statement time in pg_stat_statement
On Wed, Feb 18, 2015 at 08:31:09PM -0700, David G. Johnston wrote: On Wed, Feb 18, 2015 at 6:50 PM, Andrew Dunstan and...@dunslane.net wrote: On 02/18/2015 08:34 PM, David Fetter wrote: On Tue, Feb 17, 2015 at 08:21:32PM -0500, Peter Eisentraut wrote: On 1/20/15 6:32 PM, David G Johnston wrote: In fact, as far as the database knows, the values provided to this function do represent an entire population and such a correction would be unnecessary. I guess it boils down to whether future queries are considered part of the population or whether the population changes upon each query being run and thus we are calculating the ever-changing population variance. I think we should be calculating the population variance. Why population variance and not sample variance? In distributions where the second moment about the mean exists, it's an unbiased estimator of the variance. In this, it's different from the population variance. Because we're actually measuring the whole population, and not a sample? We're not. We're taking a sample, which is to say past measurements, and using it to make inferences about the population, which includes all queries in the future. The key incorrect word in David Fetter's statement is estimator. We are not estimating anything but rather providing descriptive statistics for a defined population. See above. Users extrapolate that the next member added to the population would be expected to conform to this statistical description without bias (though see below). We can also then define the new population by including this new member and generating new descriptive statistics (which allows for evolution to be captured in the statistics). Currently (I think) we allow the end user to kill off the entire population and build up from scratch so that while, in the short term, the ability to predict the attributes of future members is limited once the population has reached a statistically significant level new predictions will no longer be skewed by population members who attributes were defined in a older and possibly significantly different environment. In theory it would be nice to be able to give the user the ability to specify - by time or percentage - a subset of the population to leave alive. I agree that stale numbers can fuzz things in a way we don't like, but let's not creep too much feature in here. Actual time-weighted sampling would be an alternative but likely one significantly more difficult to accomplish. Right. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
On Wed, Feb 18, 2015 at 4:57 PM, Kevin Grittner kgri...@ymail.com wrote: But max_standby_streaming_delay, max_standby_archive_delay and hot_standby_feedback are among the most frequent triggers for questions and complaints that I/we see. Agreed. And a really bad one used to be vacuum_defer_cleanup_age, because of confusing units amongst other things. Which in terms seems fairly close to Kevins suggestions, unfortunately. Particularly my initial suggestion, which was to base snapshot age it on the number of transaction IDs assigned. Does this look any better to you if it is something that can be set to '20min' or '1h'? Just to restate, that would not automatically cancel the snapshots past that age; it would allow vacuum of any tuples which became dead that long ago, and would cause a snapshot too old message for any read of a page modified more than that long ago using a snapshot which was older than that. I like this thought. One of the first things I do in a new Pg environment is setup a cronjob that watches pg_stat_activity and terminates most backends over N minutes in age (about 5x the length of normal work) with an exception for a handful of accounts doing backups and other maintenance operations. This prevents a stuck client from jamming up the database. Would pg_dump be able to opt-out of such a restriction? regards, Rod
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
Andrew Dunstan and...@dunslane.net wrote: On 02/19/2015 09:44 AM, Kevin Grittner wrote: I understand why this make people nervous. I wonder if it might be more palatable if there were a per-table setting that could enable it? If we could ensure that this was only applied to high churn queue tables, say, while tables touched by the report writer would not have it applied, that would calm a lot of my fears. That's an interesting idea. I think I should switch the unit of measure for too old to a time-based value first, since there seems to be universal agreement that it would be better than number of transactions. Once I've cleared that hurdle I'll see what this would take. I'm also interested in handling the case Stephen Frost described, where a tuple is effectively dead but we don't currently have the means of discovering the fact, because there is an older long running transaction which is never in fact going to be able to see the tuple. Absolutely. That's one of several other issues that I've been looking at over the last few weeks. It sounds like people are already working on that one, which is great. My personal priority list included that, but after the two I submitted here and a patch to allow combining near-empty btree pages so that btree bloat is constrained without having to reindex periodically for the cases where index tuples are added in index order (at one or a few insertion points) and most-but-not-all are deleted. You can currently wind up with a worst-case of one index tuple per page with no action to reduce that bloat by vacuum processes. -- Kevin Grittner EDB: 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] Allow snapshot too old error, to prevent bloat
Rod Taylor rod.tay...@gmail.com wrote: Would pg_dump be able to opt-out of such a restriction? I don't see how, since vacuum would be removing recently dead tuples that are still visible; the alternative to getting a snapshot too old error when reading a page which could be affected is to return incorrect results, and nobody wants that. The best you could do if you wanted to run pg_dump (or similar) and it might take more time than your old_snapshot_threshold would be to increase the threshold, reload, dump, set it back to the normal setting, and reload again. Andrew's suggestion of setting this per table would not help here. -- Kevin Grittner EDB: 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] Add min and max execute statement time in pg_stat_statement
On Thu, Feb 19, 2015 at 11:10 AM, David Fetter da...@fetter.org wrote: On Wed, Feb 18, 2015 at 08:31:09PM -0700, David G. Johnston wrote: On Wed, Feb 18, 2015 at 6:50 PM, Andrew Dunstan and...@dunslane.net wrote: On 02/18/2015 08:34 PM, David Fetter wrote: On Tue, Feb 17, 2015 at 08:21:32PM -0500, Peter Eisentraut wrote: On 1/20/15 6:32 PM, David G Johnston wrote: In fact, as far as the database knows, the values provided to this function do represent an entire population and such a correction would be unnecessary. I guess it boils down to whether future queries are considered part of the population or whether the population changes upon each query being run and thus we are calculating the ever-changing population variance. I think we should be calculating the population variance. Why population variance and not sample variance? In distributions where the second moment about the mean exists, it's an unbiased estimator of the variance. In this, it's different from the population variance. Because we're actually measuring the whole population, and not a sample? We're not. We're taking a sample, which is to say past measurements, and using it to make inferences about the population, which includes all queries in the future. All past measurements does not qualify as a random sample of a population made up of all past measurements and any potential members that may be added in the future. Without the random sample aspect you throw away all pretense of avoiding bias so you might as well just call the totality of the past measurements the population, describe them using population descriptive statistics, and call it a day. For large populations it isn't going to matter anyway but for small populations the difference of one in the divisor seems like it would make the past performance look worse than it actually was. David J.
Re: [HACKERS] POLA violation with \c service=
On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
Hi all
I am sending a review of this patch:
* What it does? - Allow to connect to other db by \connect uri connection
format
postgres=# \c postgresql://localhost?service=old
psql (9.5devel, server 9.2.9)
You are now connected to database postgres as user pavel.
* Would we this feature? - yes, it eliminate inconsistency between cmd line
connect and \connect. It is good idea without any objections.
* This patch is cleanly applicable, later compilation without any issues
* All regress tests passed
* A psql documentation is updated -- this feature (and format) is not
widely known, so maybe some more examples are welcome
* When I tested this feature, it worked as expected
* Code respects PostgreSQL coding rules. I prefer a little bit different
test if keep password. Current code is little bit harder to understand. But
I can live with David's code well too.
if
(!user)
user = PQuser(o_conn);
if
(!host)
host =
PQhost(o_conn);
if
(!port)
port =
PQport(o_conn);
if
(dbname)
has_connection_string =
recognized_connection_string(dbname);
/* we should not to keep password if some connection property is changed
*/
keep_password = strcmp(user, PQuser(o_conn)) == 0 strcmp(host,
PQhost(o_conn)) == 0
strcmp(port, PQport(o_conn)) == 0
!has_connection_string;
Changed. This is cleaner.
I have not any other comments.
Possible questions:
1. more examples in doc
I'm not sure how best to illustrate those. Are you thinking of one
example each for the URI and conninfo cases?
2. small change how to check keep_password
Done.
Cheers,
David.
--
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..cae6bf4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=gt;
/varlistentry
varlistentry
-termliteral\c/literal or literal\connect/literal literal[
replaceable class=parameterdbname/replaceable [ replaceable
class=parameterusername/replaceable ] [ replaceable
class=parameterhost/replaceable ] [ replaceable
class=parameterport/replaceable ] ]/literal/term
+termliteral\c/literal or literal\connect/literal literal [
{ [ replaceable class=parameterdbname/replaceable [ replaceable
class=parameterusername/replaceable ] [ replaceable
class=parameterhost/replaceable ] [ replaceable
class=parameterport/replaceable ] ] | replaceable
class=parameterconninfo/replaceable string | replaceable
class=parameterURI/replaceable } ] /literal/term
listitem
para
Establishes a new connection to a productnamePostgreSQL/
-server. If the new connection is successfully made, the
-previous connection is closed. If any of replaceable
+server using positional parameters as described below, a
+parameterconninfo/parameter string, or a acronymURI/acronym.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of replaceable
class=parameterdbname/replaceable, replaceable
class=parameterusername/replaceable, replaceable
class=parameterhost/replaceable or replaceable
class=parameterport/replaceable are omitted or specified
as literal-/literal, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the applicationlibpq/application default for
the parameter's value is used.
/para
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+
Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0
On Thu, Feb 19, 2015 at 5:21 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. I haven't looked at your latest patch, but I don't think you need to pre-check for this to work. To recap, the situation is that two backends have already inserted the heap tuple, and then see that the other backend's tuple conflicts. To avoid a livelock, it's enough that one backend super-deletes its own tuple first, before waiting for the other to complete, while the other other backend waits without super-deleting. No? I fully agree with your summary here. However, why should we suppose that while we wait, the other backends don't both delete and then re-insert their tuple? They need the pre-check to know not to re-insert their tuple (seeing our tuple, immediately after we wake as the preferred backend with the older XID) in order to break the race. But today, exclusion constraints are optimistic in that the insert happens first, and only then the check. The pre-check turns that the other way around, in a limited though necessary sense. Granted, it's unlikely that we'd livelock due to one session always deleting and then nullifying that by re-inserting in time, but the theoretical risk seems real. Therefore, I'm not inclined to bother committing something without a pre-check, particularly since we're not really interested in fixing unprincipled deadlocks for ordinary exclusion constraint inserters (useful to know that you also think that doesn't matter, BTW). Does that make sense? This is explained within livelock insurance new-to-V2.3 comments in check_exclusion_or_unique_constraint(). (Not that I think that explanation is easier to follow than this one). It might be easier to provoke the livelocks with a GiST opclass that's unusually slow. I wrote the attached opclass for the purpose of testing this a while ago, but I haven't actually gotten around to do much with it. It's called useless_gist, because it's a GiST opclass for integers, like btree_gist, but the penalty and picksplit functions are totally dumb. The result is that the tuples are put to the index in pretty much random order, and every scan has to scan the whole index. I'm posting it here, in the hope that it happens to be useful, but I don't really know if it is. Thanks. I'll try and use this for testing. Haven't been able to break exclusion constraints with the jjanes_upsert test suite in a long time, now. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
On Thu, Feb 19, 2015 at 6:01 PM, Kevin Grittner kgri...@ymail.com wrote: I can see how they would be, provided we can be confident that we're going to actually throw an error when the snapshot is out of date and not end up returning incorrect results. We need to be darn sure of that, both now and in a few years from now when many of us may have forgotten about this knob.. ;) I get that this is not zero-cost to maintain, and that it might not be of interest to the community largely for that reason. If you have any ideas about how to improve that, I'm all ears. But do consider the actual scope of what was needed for the btree coverage (above). (Note that the index-only scan issue doesn't look like anything AM-specific; if something is needed for that it would be in the pruning/vacuum area.) If I understood the issue correctly, you have long running snapshots accessing one part of the data, while you have high churn on a disjoint part of data. We would need to enable vacuum on the high churn data while still being able to run those long queries. The snapshot too old solution limits the maximum age of global xmin at the cost of having to abort transactions that are older than this and happen to touch a page that was modified after they were started. What about having the long running snapshots declare their working set, and then only take them into account for global xmin for relations that are in the working set? Like a SET TRANSACTION WORKING SET command. This way the error is deterministic, vacuum on the high churn tables doesn't have to wait for the old transaction delay to expire and we avoid a hard to tune GUC (what do I need to set old_snapshot_threshold to, to reduce bloat while not having normal transactions abort). Regards, Ants Aasma -- Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: http://www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
On Thu, Feb 19, 2015 at 3:44 PM, Kevin Grittner kgri...@ymail.com wrote: I'm also interested in handling the case Stephen Frost described, where a tuple is effectively dead but we don't currently have the means of discovering the fact, because there is an older long running transaction which is never in fact going to be able to see the tuple. Absolutely. That's one of several other issues that I've been looking at over the last few weeks. It sounds like people are already working on that one, which is great. My personal priority list included that, but after the two I submitted here and a patch to allow combining near-empty btree pages so that btree bloat is constrained without having to reindex periodically for the cases where index tuples are added in index order (at one or a few insertion points) and most-but-not-all are deleted. You can currently wind up with a worst-case of one index tuple per page with no action to reduce that bloat by vacuum processes. I'd be willing to test that patch. I have a big database that does that, and a script that fills the disk with said bloat. That's forced me to do periodic reindexing, which sucks. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0
On 02/19/2015 08:16 PM, Peter Geoghegan wrote: On Thu, Feb 19, 2015 at 5:21 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Hmm. I haven't looked at your latest patch, but I don't think you need to pre-check for this to work. To recap, the situation is that two backends have already inserted the heap tuple, and then see that the other backend's tuple conflicts. To avoid a livelock, it's enough that one backend super-deletes its own tuple first, before waiting for the other to complete, while the other other backend waits without super-deleting. No? I fully agree with your summary here. However, why should we suppose that while we wait, the other backends don't both delete and then re-insert their tuple? They need the pre-check to know not to re-insert their tuple (seeing our tuple, immediately after we wake as the preferred backend with the older XID) in order to break the race. But today, exclusion constraints are optimistic in that the insert happens first, and only then the check. The pre-check turns that the other way around, in a limited though necessary sense. I'm not sure I understand exactly what you're saying, but AFAICS the pre-check doesn't completely solve that either. It's entirely possible that the other backend deletes its tuple, our backend then performs the pre-check, and the other backend re-inserts its tuple again. Sure, the pre-check reduces the chances, but we're talking about a rare condition to begin with, so I don't think it makes sense to add much code just to reduce the chances further. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning recovery.conf into GUCs
On 02/19/2015 12:23 PM, Peter Eisentraut wrote: On 1/6/15 4:22 PM, Peter Eisentraut wrote: That said, there is a much simpler way to achieve that specific functionality: Expose all the recovery settings as fake read-only GUC variables. See attached patch for an example. The commit fest app has this as the patch of record. I don't think there is a real updated patch under consideration here. This item should probably not be in the commit fest at all. What happened to the original patch? Regardless of what we do, keeping recovery.conf the way it is can't be what we go forward with. -- Josh Berkus PostgreSQL Experts Inc. http://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
[HACKERS] dblink: add polymorphic functions.
In the course of writing a small side project which hopefully will make its
way onto pgxn soon, I was writing functions that had a polymorphic result
set.
create function foo( p_row_type anyelement, p_param1 ...) returns setof
anyelement
Inside that function would be multiple calls to dblink() in both
synchronous and async modes. It is a requirement of the function that each
query return a result set conforming to the structure passed into
p_row_type, but there was no way for me to express that.
Unfortunately, there's no way to say
select * from dblink_get_result('connname') as polymorphic record type;
Instead, I had to generate the query as a string like this.
with x as (
select a.attname || ' ' || pg_catalog.format_type(a.atttypid,
a.atttypmod) as sql_text
frompg_catalog.pg_attribute a
where a.attrelid = pg_typeof(p_row_type)::text::regclass
and a.attisdropped is false
and a.attnum 0
order by a.attnum )
select format('select * from dblink_get_result($1) as
t(%s)',string_agg(x.sql_text,','))
fromx;
Moreover, I'm now executing this string dynamically, incurring reparsing
and replanning each time (and if all goes well, this would be executed many
times). Granted, I could avoid that by rewriting the stored procedure in C
and using prepared statements (not available in PL/PGSQL), but it seemed a
shame that dblink couldn't itself handle this polymorphism.
So with a little help, we were able to come up with polymorphic set
returning dblink functions.
Below is the results of the patch applied to a stock 9.4 installation.
[local]:ubuntu@dblink_test# create extension dblink;
CREATE EXTENSION
Time: 12.778 ms
[local]:ubuntu@dblink_test# \df dblink
List of functions
Schema | Name | Result data type | Argument data types |
Type
++--+-+
public | dblink | SETOF record | text|
normal
public | dblink | SETOF anyelement | text, anyelement|
normal
public | dblink | SETOF record | text, boolean |
normal
public | dblink | SETOF anyelement | text, boolean, anyelement |
normal
public | dblink | SETOF record | text, text |
normal
public | dblink | SETOF anyelement | text, text, anyelement |
normal
public | dblink | SETOF record | text, text, boolean |
normal
public | dblink | SETOF anyelement | text, text, boolean, anyelement |
normal
(8 rows)
[local]:ubuntu@dblink_test# *select * from
dblink('dbname=dblink_test','select * from pg_tables order by tablename
limit 2',null::pg_tables);*
schemaname | tablename | tableowner | tablespace | hasindexes |
hasrules | hastriggers
+--++++--+-
pg_catalog | pg_aggregate | postgres || t | f
| f
pg_catalog | pg_am| postgres || t | f
| f
(2 rows)
Time: 6.813 ms
Obviously, this is a trivial case, but it shows that the polymorphic
function works as expected, and the code that uses it will be a lot more
straightforward.
Proposed patch attached.
diff --git a/contrib/dblink/dblink--1.1.sql b/contrib/dblink/dblink--1.1.sql
index 8733553..bf5ddaa 100644
--- a/contrib/dblink/dblink--1.1.sql
+++ b/contrib/dblink/dblink--1.1.sql
@@ -121,6 +121,26 @@ RETURNS setof record
AS 'MODULE_PATHNAME','dblink_record'
LANGUAGE C STRICT;
+CREATE FUNCTION dblink (text, text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
+CREATE FUNCTION dblink (text, boolean, anyelement)
+RETURNS setof anyelement
+AS 'MODULE_PATHNAME','dblink_record'
+LANGUAGE C;
+
CREATE FUNCTION dblink_exec (text, text)
RETURNS text
AS 'MODULE_PATHNAME','dblink_exec'
@@ -188,6 +208,16 @@ RETURNS SETOF record
AS 'MODULE_PATHNAME', 'dblink_get_result'
LANGUAGE C STRICT;
+CREATE FUNCTION dblink_get_result(text, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
+CREATE FUNCTION dblink_get_result(text, bool, anyelement)
+RETURNS SETOF anyelement
+AS 'MODULE_PATHNAME', 'dblink_get_result'
+LANGUAGE C;
+
CREATE FUNCTION dblink_get_connections()
RETURNS text[]
AS 'MODULE_PATHNAME', 'dblink_get_connections'
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 9fe750e..eb7f5f9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -680,27 +680,68 @@ dblink_record_internal(FunctionCallInfo fcinfo, bool
is_async)
if (!is_async)
{
- if (PG_NARGS() == 3)
+
[HACKERS] FDW for Oracle
Hello This is me problem: Saludos, Gilberto Castillo La Habana, Cuba --- This message was processed by Kaspersky Mail Gateway 5.6.28/RELEASE running at host imx3.etecsa.cu Visit our web-site: http://www.kaspersky.com, http://www.viruslist.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] FDW for Oracle
Hello This is me problem: fdw_test=# SELECT oracle_diag(); oracle_diag --- oracle_fdw 1.2.0, PostgreSQL 9.1.8, Oracle client 11.2.0.1.0, ORACLE_HOME=/opt/oracle/, TNS_ADMIN=/opt/oracle (1 row) In PostgreSQL: SELECT pg_backend_pid(); 5543 In a second session in the shell: ps -p5543 -oppid= 5290 As root or PostgreSQL OS user: cat /proc/5290/environ | xargs -0 -n1 LD_LIBRARY_PATH=/usr/lib:/opt/PostgreSQL/9.1/lib:/opt/oracle/instantclient:$LD_LIBRARY_PATH HOME=/opt/PostgreSQL/9.1 TNS_ADMIN=/opt/oracle PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/opt/PostgreSQL/9.1/bin:/usr/lib/postgresql/9.1/bin:/opt/PostgreSQL/9.1/bin/bin ORACLE_HOME=/opt/oracle DYLD_LIBRARY_PATH=/opt/oracle/instantclient:$ORACLE_HOME My error is: OCIEnvCreate ¿What is the solution? Please. Saludos, Gilberto Castillo La Habana, Cuba --- This message was processed by Kaspersky Mail Gateway 5.6.28/RELEASE running at host imx3.etecsa.cu Visit our web-site: http://www.kaspersky.com, http://www.viruslist.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] Allow snapshot too old error, to prevent bloat
On Feb 19, 2015 10:31 PM, Kevin Grittner kgri...@ymail.com wrote: What about having the long running snapshots declare their working set, and then only take them into account for global xmin for relations that are in the working set? Like a SET TRANSACTION WORKING SET command. This way the error is deterministic, vacuum on the high churn tables doesn't have to wait for the old transaction delay to expire and we avoid a hard to tune GUC (what do I need to set old_snapshot_threshold to, to reduce bloat while not having normal transactions abort). Let me make sure I understand your suggestion. You are suggesting that within a transaction you can declare a list of tables which should get the snapshot too old error (or something like it) if a page is read which was modified after the snapshot was taken? Snapshots within that transaction would not constrain the effective global xmin for purposes of vacuuming those particular tables? Sorry, I should have been clearer. I'm proposing that a transaction can declare what tables it will access. After that the transaction will constrain xmin for only those tables. Accessing any other table would give an error immediately. If I'm understanding your proposal, that would help some of the cases the snapshot too old case addresses, but it would not handle the accidental idle in transaction case (e.g., start a repeatable read transaction, run one query, then sit idle indefinitely). I don't think it would work quite as well for some of the other cases I've seen, but perhaps it could be made to fit if we could figure out the accidental idle in transaction case. Accidental idle in transaction seems better handled by just terminating transactions older than some timeout. This is already doable externally, but an integrated solution would be nice if we could figure out how the permissions for setting such a timeout work. I also think it might be hard to develop a correct global xmin for vacuuming a particular table with that solution. How do you see that working? I'm imagining the long running transaction would register it's xmin in a shared map keyed by relation for each referenced relation and remove the xmin from procarray. Vacuum would access this map by relation, determine the minimum and use that if it's earlier than the global xmin. I'm being deliberately vague here about the datastructure in shared memory as I don't have a great idea what to use there. It's somewhat similar to the lock table in that in theory the size is unbounded, but in practice it's expected to be relatively tiny. Regards, Ants Aasma
Re: [HACKERS] POLA violation with \c service=
On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
2015-02-19 19:51 GMT+01:00 David Fetter da...@fetter.org:
On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
I'm not sure how best to illustrate those. Are you thinking of one
example each for the URI and conninfo cases?
some like
most common form is:
\c mydb
but you can use any connection format described
(libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
\c postgresql://tom@localhost/mydb?application_name=myapp
Examples added.
Cheers,
David.
--
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=gt;
/varlistentry
varlistentry
-termliteral\c/literal or literal\connect/literal literal[
replaceable class=parameterdbname/replaceable [ replaceable
class=parameterusername/replaceable ] [ replaceable
class=parameterhost/replaceable ] [ replaceable
class=parameterport/replaceable ] ]/literal/term
+termliteral\c/literal or literal\connect/literal literal [
{ [ replaceable class=parameterdbname/replaceable [ replaceable
class=parameterusername/replaceable ] [ replaceable
class=parameterhost/replaceable ] [ replaceable
class=parameterport/replaceable ] ] | replaceable
class=parameterconninfo/replaceable string | replaceable
class=parameterURI/replaceable } ] /literal/term
listitem
para
Establishes a new connection to a productnamePostgreSQL/
-server. If the new connection is successfully made, the
-previous connection is closed. If any of replaceable
+server using positional parameters as described below, a
+parameterconninfo/parameter string, or a acronymURI/acronym.
If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if
any of replaceable
class=parameterdbname/replaceable, replaceable
class=parameterusername/replaceable, replaceable
class=parameterhost/replaceable or replaceable
class=parameterport/replaceable are omitted or specified
as literal-/literal, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there
is no previous
connection, the applicationlibpq/application default for
the parameter's value is used.
/para
@@ -822,6 +824,27 @@ testdb=gt;
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
/para
+
+para
+Positional syntax:
+programlisting
+=gt; \c mydb myuser host.dom 6432
+/programlisting
+/para
+
+para
+The conninfo form detailed in xref linkend=LIBPQ-CONNSTRING takes
two forms: keyword-value pairs
+/para
+programlisting
+=gt; \c service=foo
+=gt; \c host=localhost port=5432 dbname=mydb connect_timeout=10
sslmode=disable
+/programlisting
+para
+and URIs:
+/para
+programlisting
+=gt; \c postgresql://tom@localhost/mydb?application_name=myapp
+/programlisting
/listitem
/varlistentry
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..ec86e52 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ boolkeep_password = true;
+ boolhas_connection_string = false;
if (!o_conn (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0)
+ (strcmp(host, PQhost(o_conn)) == 0)
+ (strcmp(port, PQport(o_conn)) == 0)
+!has_connection_string);
+
+ /*
+* Unlike the previous stanzas, changing only the dbname shouldn't
+* trigger throwing away the password.
+*/
+ if (!dbname)
+
Re: [HACKERS] pg_dump gets attributes from tables in extensions
On Fri, Feb 20, 2015 at 5:33 AM, Peter Eisentraut pete...@gmx.net wrote: On 2/16/15 2:45 AM, Michael Paquier wrote: While looking at the patch to fix pg_dump with extensions containing tables referencing each other, I got surprised by the fact that getTableAttrs tries to dump table attributes even for tables that are part of an extension. Is that normal? Attached is a patch that I think makes things right, but not dumping any tables that are part of ext_member. Can you provide an example/test case? (e.g., which publicly available extension contains tables with attributes?) Sure. Attached is a simplified version of the extension I used for the other patch on pg_dump. $ psql -c 'create extension dump_test' CREATE EXTENSION $ psql -At -c '\dx+ dump_test' table aa_tab_fkey table bb_tab_fkey $ pg_dump -v 21 | grep columns and types pg_dump: finding the columns and types of table public.bb_tab_fkey pg_dump: finding the columns and types of table public.aa_tab_fkey -- Michael dump_test.tar.gz Description: GNU Zip compressed 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] POLA violation with \c service=
On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: Hi I am happy with doc changes now. When I test last patch, I found sigfault bug, because host = PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 If you are agree with fix, I'll mark this patch as ready for commit. Thanks for fixing the bug. Let's go with this. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed no issues with last 007 patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] POLA violation with \c service=
Hi
I am happy with doc changes now.
When I test last patch, I found sigfault bug, because host =
PQhost(o_conn); returns NULL. I fexed it - please, see patch 007
If you are agree with fix, I'll mark this patch as ready for commit.
Regards
Pavel
2015-02-19 23:33 GMT+01:00 David Fetter da...@fetter.org:
On Thu, Feb 19, 2015 at 09:32:29PM +0100, Pavel Stehule wrote:
2015-02-19 19:51 GMT+01:00 David Fetter da...@fetter.org:
On Sun, Feb 01, 2015 at 08:38:24AM +0100, Pavel Stehule wrote:
I'm not sure how best to illustrate those. Are you thinking of one
example each for the URI and conninfo cases?
some like
most common form is:
\c mydb
but you can use any connection format described
(libpq-connect.html#LIBPQ-PARAMKEYWORDS) like
\c postgresql://tom@localhost/mydb?application_name=myapp
Examples added.
Cheers,
David.
--
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
commit f0e71c50590b94da5dafb72a377c7c70b49ce488
Author: root root@localhost.localdomain
Date: Fri Feb 20 07:04:53 2015 +0100
fix segfault due empty variable host
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index a637001..340a5d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -796,18 +796,20 @@ testdb=gt;
/varlistentry
varlistentry
-termliteral\c/literal or literal\connect/literal literal[ replaceable class=parameterdbname/replaceable [ replaceable class=parameterusername/replaceable ] [ replaceable class=parameterhost/replaceable ] [ replaceable class=parameterport/replaceable ] ]/literal/term
+termliteral\c/literal or literal\connect/literal literal [ { [ replaceable class=parameterdbname/replaceable [ replaceable class=parameterusername/replaceable ] [ replaceable class=parameterhost/replaceable ] [ replaceable class=parameterport/replaceable ] ] | replaceable class=parameterconninfo/replaceable string | replaceable class=parameterURI/replaceable } ] /literal/term
listitem
para
Establishes a new connection to a productnamePostgreSQL/
-server. If the new connection is successfully made, the
-previous connection is closed. If any of replaceable
+server using positional parameters as described below, a
+parameterconninfo/parameter string, or a acronymURI/acronym. If the new connection is
+successfully made, the
+previous connection is closed. When using positional parameters, if any of replaceable
class=parameterdbname/replaceable, replaceable
class=parameterusername/replaceable, replaceable
class=parameterhost/replaceable or replaceable
class=parameterport/replaceable are omitted or specified
as literal-/literal, the value of that parameter from the
-previous connection is used. If there is no previous
+previous connection is used. If using positional parameters and there is no previous
connection, the applicationlibpq/application default for
the parameter's value is used.
/para
@@ -822,6 +824,27 @@ testdb=gt;
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
/para
+
+para
+Positional syntax:
+programlisting
+=gt; \c mydb myuser host.dom 6432
+/programlisting
+/para
+
+para
+The conninfo form detailed in xref linkend=LIBPQ-CONNSTRING takes two forms: keyword-value pairs
+/para
+programlisting
+=gt; \c service=foo
+=gt; \c host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable
+/programlisting
+para
+and URIs:
+/para
+programlisting
+=gt; \c postgresql://tom@localhost/mydb?application_name=myapp
+/programlisting
/listitem
/varlistentry
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7c9f28d..dd9350e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1607,6 +1607,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn (!dbname || !user || !host || !port))
{
@@ -1620,15 +1622,31 @@ do_connect(char *dbname, char *user, char *host, char *port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+
if (!host)
host = PQhost(o_conn);
+
if (!port)
port = PQport(o_conn);
+ if (dbname)
+ has_connection_string = recognized_connection_string(dbname);
+
+ keep_password = (
+ (strcmp(user, PQuser(o_conn)) == 0)
+ (!host || strcmp(host, PQhost(o_conn)) == 0)
+ (strcmp(port, PQport(o_conn))
Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)
On Fri, Feb 20, 2015 at 5:50 AM, Peter Eisentraut pete...@gmx.net wrote: On 1/14/15 11:31 PM, Michael Paquier wrote: pg_regress will fail with test suites using only source files if the destination folders do not exist in the code tree. This is annoying because this forces to maintain empty folders sql/ and expected/ with a .gitignore ignoring everything. We'd still need the .gitignore files somewhere. Do you want to move them one directory up? I am not sure I am getting what you are pointing to... For extensions that already have non-empty sql/ and expected/, they should have their own ignore entries as sql/.gitignore and expected/.gitignore. The point of the patch is to simplify the code tree of extensions that need to keep empty sql/ and expected/, for example to be able to run regression tests after a fresh repository clone for example. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] FDW for Oracle
On Fri, Feb 20, 2015 at 7:15 AM, Gilberto Castillo wrote: This is me problem: fdw_test=# SELECT oracle_diag(); oracle_diag --- oracle_fdw 1.2.0, PostgreSQL 9.1.8, Oracle client 11.2.0.1.0, ORACLE_HOME=/opt/oracle/, TNS_ADMIN=/opt/oracle (1 row) [...] My error is: OCIEnvCreate ¿What is the solution? Please. If you have an issue with this fdw, you had better contact the project maintainers here: https://github.com/laurenz/oracle_fdw Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
Michael Paquier michael.paqu...@gmail.com writes: Thanks for the clarifications and the review. Attached is a new set. I've reviewed and pushed the 0001 patch (you missed a few things :-(). Let's see how unhappy the buildfarm is with this before we start on the rest of them. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Question about durability and postgresql.
Hello, We have a combination of 9.3 and 9.4 databases used for logging of data. We do not need a strong durability guarantee, meaning it is ok if on crash a minute or two of data is lost from our logs. (This is just stats for our internal tool). I am looking at this page: http://www.postgresql.org/docs/9.4/static/non-durability.html And it's not clear which setting I should turn on. What we do NOT want is to lose the entire table or corrupt the database. We do want to gain speed though by not making DATA writes durable. Which setting is appropriate for this use case? At a glance it looks like a combination of 1) Turn off synchronous_commit and possibly: 2) Increase checkpoint_segments and checkpoint_timeout ; this reduces the frequency of checkpoints, but increases the storage requirements of /pg_xlog. 3) Turn off full_page_writes; there is no need to guard against partial page writes. The point here is to never get a corrupt database, but in case of crash we might lose a few minutes of last transactions. Any suggestions please? thank you, -Alfred
Re: [HACKERS] dblink: add polymorphic functions.
On Fri, Feb 20, 2015 at 2:50 PM, Corey Huinker corey.huin...@gmail.com wrote: Thanks - completely new to this process, so I'm going to need walking-through of it. I promise to document what I learn and try to add that to the commitfest wiki. Where can I go for guidance about documentation format and regression tests? Here are some guidelines about how to submit a patch: https://wiki.postgresql.org/wiki/Submitting_a_Patch You can have as well a look here to see how extensions like dblink are structured: http://www.postgresql.org/docs/devel/static/extend-pgxs.html What you need to do in the case of your patch is to add necessary test cases to in sql/dblink.sql for the new functions you have added and then update the output in expected/. Be sure to not break any existing things as well. After running the tests in your development environment output results are available in results then pg_regress generates a differential file in regressions.diffs. For the documentation, updating dblink.sgml with the new function prototypes would be sufficient. With perhaps an example(s) to show that what you want to add is useful. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] inherit support for foreign tables
On 2015/01/15 16:35, Etsuro Fujita wrote:
On 2014/12/23 0:36, Tom Lane wrote:
Yeah, we need to do something about the PlanRowMark data structure.
Aside from the pre-existing issue in postgres_fdw, we need to fix it
to support inheritance trees in which more than one rowmark method
is being used. That rte.hasForeignChildren thing is a crock,
The idea I'd had about that was to convert the markType field into a
bitmask, so that a parent node's markType could represent the logical
OR of the rowmark methods being used by all its children.
As I said before, that seems to me like a good idea. So I'll update the
patch based on that if you're okey with it.
Done based on your ideas: (a) add a field to PlanRowMark to record the
original lock strength to fix the postgres_fdw issue and (b) convert its
markType field into a bitmask to support the inheritance trees. I think
that both work well and that (a) is useful for the other places.
Patch attached, which has been created on top of [1].
Best regards,
Etsuro Fujita
[1] http://www.postgresql.org/message-id/54bcbbf8.3020...@lab.ntt.co.jp
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
***
*** 148,153 EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_csv WHERE a 0;
--- 148,167
SELECT * FROM agg_csv WHERE a 0;
RESET constraint_exclusion;
+ -- table inheritance tests
+ CREATE TABLE agg (a int2, b float4);
+ ALTER FOREIGN TABLE agg_csv INHERIT agg;
+ SELECT tableoid::regclass, * FROM agg;
+ SELECT tableoid::regclass, * FROM agg_csv;
+ SELECT tableoid::regclass, * FROM ONLY agg;
+ -- updates aren't supported
+ UPDATE agg SET a = 1;
+ DELETE FROM agg WHERE a = 100;
+ -- but this should be ignored
+ SELECT tableoid::regclass, * FROM agg FOR UPDATE;
+ ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
+ DROP TABLE agg;
+
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
***
*** 249,254 SELECT * FROM agg_csv WHERE a 0;
--- 249,294
(0 rows)
RESET constraint_exclusion;
+ -- table inheritance tests
+ CREATE TABLE agg (a int2, b float4);
+ ALTER FOREIGN TABLE agg_csv INHERIT agg;
+ SELECT tableoid::regclass, * FROM agg;
+ tableoid | a |b
+ --+-+-
+ agg_csv | 100 | 99.097
+ agg_csv | 0 | 0.09561
+ agg_csv | 42 | 324.78
+ (3 rows)
+
+ SELECT tableoid::regclass, * FROM agg_csv;
+ tableoid | a |b
+ --+-+-
+ agg_csv | 100 | 99.097
+ agg_csv | 0 | 0.09561
+ agg_csv | 42 | 324.78
+ (3 rows)
+
+ SELECT tableoid::regclass, * FROM ONLY agg;
+ tableoid | a | b
+ --+---+---
+ (0 rows)
+
+ -- updates aren't supported
+ UPDATE agg SET a = 1;
+ ERROR: cannot update foreign table agg_csv
+ DELETE FROM agg WHERE a = 100;
+ ERROR: cannot delete from foreign table agg_csv
+ -- but this should be ignored
+ SELECT tableoid::regclass, * FROM agg FOR UPDATE;
+ tableoid | a |b
+ --+-+-
+ agg_csv | 100 | 99.097
+ agg_csv | 0 | 0.09561
+ agg_csv | 42 | 324.78
+ (3 rows)
+
+ ALTER FOREIGN TABLE agg_csv NO INHERIT agg;
+ DROP TABLE agg;
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***
*** 3027,3032 NOTICE: NEW: (13,test triggered !)
--- 3027,3544
(1 row)
-- ===
+ -- test inheritance features
+ -- ===
+ CREATE TABLE a (aa TEXT);
+ CREATE TABLE loct (aa TEXT, bb TEXT);
+ CREATE FOREIGN TABLE b (bb TEXT) INHERITS (a)
+ SERVER loopback OPTIONS (table_name 'loct');
+ INSERT INTO a(aa) VALUES('aaa');
+ INSERT INTO a(aa) VALUES('');
+ INSERT INTO a(aa) VALUES('a');
+ INSERT INTO a(aa) VALUES('aa');
+ INSERT INTO a(aa) VALUES('aaa');
+ INSERT INTO a(aa) VALUES('');
+ INSERT INTO b(aa) VALUES('bbb');
+ INSERT INTO b(aa) VALUES('');
+ INSERT INTO b(aa) VALUES('b');
+ INSERT INTO b(aa) VALUES('bb');
+ INSERT INTO b(aa) VALUES('bbb');
+ INSERT INTO b(aa) VALUES('');
+ SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
+ relname |aa
+ -+--
+ a | aaa
+ a |
+ a | a
+ a | aa
+ a | aaa
+ a |
+ b | bbb
+ b |
+ b | b
+ b | bb
+ b | bbb
+ b |
+ (12 rows)
+
+ SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
+ relname |aa| bb
+ -+--+
+ b | bbb |
+ b | |
+ b | b|
+ b | bb |
+ b | bbb |
+ b |
Re: [HACKERS] dblink: add polymorphic functions.
Thanks - completely new to this process, so I'm going to need walking-through of it. I promise to document what I learn and try to add that to the commitfest wiki. Where can I go for guidance about documentation format and regression tests? Author field is presently being finicky, reported that to Magnus already. On Thu, Feb 19, 2015 at 11:00 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker corey.huin...@gmail.com wrote: Proposed patch attached. At quick glance, this patch lacks two things: - regression test coverage - documentation (Note: do not forget to add your name in the field Author when adding a new patch in the CF app). -- Michael
Re: [HACKERS] POLA violation with \c service=
2015-02-20 8:22 GMT+01:00 David Fetter da...@fetter.org: On Fri, Feb 20, 2015 at 07:10:29AM +0100, Pavel Stehule wrote: Hi I am happy with doc changes now. When I test last patch, I found sigfault bug, because host = PQhost(o_conn); returns NULL. I fexed it - please, see patch 007 If you are agree with fix, I'll mark this patch as ready for commit. Thanks for fixing the bug. Let's go with this. marked as ready for commit Thank you for patch Regards Pavel Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: [HACKERS] dblink: add polymorphic functions.
On Fri, Feb 20, 2015 at 7:06 AM, Corey Huinker corey.huin...@gmail.com wrote: Proposed patch attached. At quick glance, this patch lacks two things: - regression test coverage - documentation (Note: do not forget to add your name in the field Author when adding a new patch in the CF app). -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] A better translation version of Chinese for psql/po/zh_CN.po file
Hello all, This is my first patch to postgreSQL, so I think I'd better start from a easy one. I am a native Chinese speaker. A lot of words and expressions are different between mainsand Chinese and TaiWan/HongKong Chinese. For instance TaiWan Chinese will use 档案 to represent file, but in mainland Chinese we use 文件. Things like that make a huge difference in 'src/bin/psql/po/zh_CN.po', after check this file, I manually translate and modify the entire file to make it more native Chinese like, but I think there still have some space to improve. If any of you think this version of translation is useful or have any question , please let me know. Rugal Bernstein Graduate Student @ University of Manitoba Data Mining; Machine Learning; Neural Network better-translation-of-psql-po.diff 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] Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
On Fri, Feb 20, 2015 at 2:14 PM, Tom Lane wrote: Michael Paquier writes: Thanks for the clarifications and the review. Attached is a new set. I've reviewed and pushed the 0001 patch (you missed a few things :-(). My apologies. I completely forgot to check for any calls of offsetof with the structures changed... -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_check_dir comments and implementation mismatch
On Mon, Feb 02, 2015 at 03:48:33PM -0500, Robert Haas wrote: On Sat, Jan 31, 2015 at 8:28 AM, Marco Nenciarini marco.nenciar...@2ndquadrant.it wrote: I've attached a new version of the patch fixing the missing closedir on readdir error. If readir() fails and closedir() succeeds, the return will be -1 but errno will be 0. Out of curiosity, have you seen a closedir() implementation behave that way? It would violate C99 (The value of errno is zero at program startup, but is never set to zero by any library function.) and POSIX. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c
On Fri, Feb 20, 2015 at 5:41 AM, Peter Eisentraut wrote: That's cool if you want to add those assertions, but please make them separate statements each, like Assert(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE) || vacstmt-freeze_min_age == -1); Assert(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE) || vacstmt-freeze_table_age == -1); ... Besides being more readable, this will give you more useful output if the assertion fails. It makes sense. When a manual VACUUM FREEZE without options specified without parenthesis, VACOPT_FREEZE is not used in gram.y, so ISTM that we should change them to that instead: Assert((vacstmt-options VACOPT_VACUUM) || vacstmt-multixact_freeze_table_age == -1); At least this would check that an ANALYZE does not set those parameters inappropriately... -- Michael diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 2f3f79d..d2f5baa 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -111,6 +111,14 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast, Assert(vacstmt-options (VACOPT_VACUUM | VACOPT_ANALYZE)); Assert((vacstmt-options VACOPT_VACUUM) || !(vacstmt-options (VACOPT_FULL | VACOPT_FREEZE))); + Assert((vacstmt-options VACOPT_VACUUM) || + vacstmt-freeze_min_age == -1); + Assert((vacstmt-options VACOPT_VACUUM) || + vacstmt-freeze_table_age == -1); + Assert((vacstmt-options VACOPT_VACUUM) || + vacstmt-multixact_freeze_min_age == -1); + Assert((vacstmt-options VACOPT_VACUUM) || + vacstmt-multixact_freeze_table_age == -1); Assert((vacstmt-options VACOPT_ANALYZE) || vacstmt-va_cols == NIL); stmttype = (vacstmt-options VACOPT_VACUUM) ? VACUUM : ANALYZE; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
Tomas Vondra tomas.von...@2ndquadrant.com writes: I see the patch only works with the top-level snapshot timestamp, stored in globalStats, but since 9.3 (when the stats were split into per-db files) we track per-database timestamps too. Shouldn't we make those timestamps accessible too? It's not necessary for detecting unresponsive statistics collector (if it's stuck it's not writing anything, so the global timestamp will be old too), but it seems more appropriate for querying database-level stats to query database-level timestamp too. I'm inclined to say not; I think that's exposing an implementation detail that we might regret exposing, later. (IOW, it's not hard to think of rearrangements that might mean there wasn't a per-database stamp anymore.) But maybe that's not necessary, because to query database stats you have to be connected to that particular database and that should write fresh stats, so the timestamps should not be very different. Yeah. The only use-case that's been suggested is detecting an unresponsive stats collector, and the main timestamp should be plenty for that. 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] pg_upgrade and rsync
On 2/19/15 11:57 AM, Bruce Momjian wrote: On Wed, Jan 28, 2015 at 09:26:11PM -0800, Josh Berkus wrote: 3. Check that the replica is not very lagged. If it is, wait for traffic to die down and for it to catch up. Now that 9.4.1 is released, I would like to get this doc patch applied --- it will close the often-requested feature of how to pg_upgrade slave clusters. I wasn't happy with Josh's specification above that the replica is not very lagged, so I added a bullet point to check the pg_controldata output to verify that the primary and standby servers are synchronized. Yes, this adds even more complication to the pg_upgrade instructions, but it is really more of the same complexity. pg_upgrade really needs an install-aware and OS-aware tool on top of it to automate much of this. #3 bothered me as well because it was not specific enough. I like what you've added to clarify the procedure. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
Tomas Vondra tomas.von...@2ndquadrant.com writes:
Well, the patch also does this:
*** 28,34 SELECT pg_sleep_for('2 seconds');
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
--- 28,35
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
!pg_stat_snapshot_timestamp() as snap_ts
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
***
That's merely a regression test to verify that the value appears to
advance from time to time ... I don't think it has any larger meaning.
(It will be interesting to see what this new test query reports in the
transient buildfarm failures we still occasionally see in the stats
test.)
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] Allow snapshot too old error, to prevent bloat
Stephen Frost sfr...@snowman.net wrote: Kevin Grittner (kgri...@ymail.com) wrote: Stephen Frost sfr...@snowman.net wrote: In the end, with a single long-running transaction, the worst bloat you would have is double the size of the system at the time the long-running transaction started. I agree that limiting bloat to one dead tuple for every live one for each old snapshot is a limit that has value, and it was unfair of me to characterize that as not being a limit. Sorry for that. This possible solution was discussed with the user whose feedback caused me to write this patch, but there were several reasons they dismissed that as a viable total solution for them, two of which I can share: (1) They have a pool of connections each of which can have several long-running cursors, so the limit from that isn't just doubling the size of their database, it is limiting it to some two-or-three digit multiple of the necessary size. This strikes me as a bit off-the-cuff; was an analysis done which deteremined that would be the result? If it sounds like off-the-cuff it is because I am summarizing the mountain of data which has been accumulated over weeks of discussions and many rounds of multi-day tests using their actual software driven by a software test driver that simulates users, with think time and all. To be usable, we need to run on a particular set of hardware with a simulated load of 3000 users. In a two-day test without the patches I'm proposing, their actual, working production software running against PostgreSQL bloated the database by 37% and was on a linear rate of increase. Unpatched, CPU time consumed at a linear rate due to the bloat until in the second day it saturated the CPUs and the driver was unable to get acceptable user performance. Because of that we haven't moved on to test what the bloat rate would be like with 3000 users, but I assume it would be higher than with 300 users. With the two patches I submitted, bloat stabilized at less than 5% except for some btree indexes which followed pattern of inserting at the end and deleting most (but not all) of the entries over time. I've been working on a separate patch for that, but it's not ready to present, so I probably won't post that until the first CF of the next release -- unless someone's interested enough to want to discuss it before then. (2) They are already prepared to deal with snapshot too old errors on queries that run more than about 20 minutes and which access tables which are modified. They would rather do that than suffer the bloat beyond that point. That, really, is the crux here- they've already got support for dealing with it the way Oracle did and they'd like PG to do that too. Unfortunately, that, by itself, isn't a good reason for a particular capability (we certainly aren't going to be trying to duplicate PL/SQL in PG any time soon). I understand that, and if the community doesn't want this style of limitation on bloat we can make it part of PPAS (which *does* duplicate PL/SQL, among other things). We are offering it to the community first. That said, there are capabilities in other RDBMS's which are valuable and which we *do* want, so the fact that Oracle does this also isn't a reason to not include it. :-) I'm not against having a knob like this, which is defaulted to off, Thanks! I'm not sure that amounts to a +1, but at least it doesn't sound like a -1. :-) So, at the time I wrote that, I wasn't sure if it was a +1 or not myself. I've been thinking about it since then, however, and I'm leaning more towards having the capability than not, so perhaps that's a +1, but it doesn't excuse the need to come up with an implementation that everyone can be happy with and what you've come up with so far doesn't have a lot of appeal, based on the feedback (I've only glanced through it myself, but I agree with Andres and Tom that it's a larger footprint than we'd want for this and the number of places having to be touched is concerning as it could lead to future bugs). This conversation has gotten a little confusing because some of my posts seem to have been held up in the email systems for some reason, and are arriving out-of-order (and at least one I don't see yet). But I have been responding to these points. For one thing I stated four days ago that I now feel that the metric for determining that a snapshot is old should be *time*, not transactions IDs consumed, and offered to modify that patch in that direction if people agreed. I now see one person agreeing and several people arguing that I should do just that (as though they had not seem me propose it), so I'll try to get a new version out today like that. In any event, I am sure it is possible and feel that it would be better. (Having posted that now at least 4 times, hopefully people will pick up on it.) For another thing, as mentioned earlier, this is the btree footprint:
Re: [HACKERS] Allow snapshot too old error, to prevent bloat
On 02/19/2015 09:44 AM, Kevin Grittner wrote: On the 15th I said this: | What this discussion has made me reconsider is the metric for | considering a transaction too old. The number of transaction IDs | consumed seems inferior as the unit of measure for that to LSN or | time. | | It looks to me to be pretty trivial (on the order of maybe 30 lines | of code) to specify this GUC in minutes rather than transaction | IDs. At first glance this seems like it would be vulnerable to the | usual complaints about mismanaged clocks, but those are easily | answered by using a cached time in shared memory that we populate | in such a way that it can never move backward. Done right, this | could even allow the GUC to be changeable on reload rather than | only at restart. A badly mismanaged system clock could not cause a | query to generate incorrect results; the worst that could happen is | that this feature would fail to control bloat as much as expected | or reads of modified data could generate the snapshot too old | error around the time of the clock adjustment. | | As before, this would default to a magic value to mean that you | want the historical PostgreSQL behavior. | | If that makes the idea more palatable to anyone, I can submit a | patch to that effect within the next 24 hours. Until yesterday I didn't get any feedback suggesting that such a change would make the patch more palatable. Now that I have had, I'll try to post a patch to that effect today. Thanks! I understand why this make people nervous. I wonder if it might be more palatable if there were a per-table setting that could enable it? If we could ensure that this was only applied to high churn queue tables, say, while tables touched by the report writer would not have it applied, that would calm a lot of my fears. I'm also interested in handling the case Stephen Frost described, where a tuple is effectively dead but we don't currently have the means of discovering the fact, because there is an older long running transaction which is never in fact going to be able to see the tuple. 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] Allow snapshot too old error, to prevent bloat
Greg Stark st...@mit.edu wrote: On Sun, Feb 15, 2015 at 8:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: There might be something in that, but again it's not much like this patch. The key point I think we're both making is that nondeterministic failures are bad, especially when you're talking about long-running, expensive-to-retry transactions. But I think Kevin would agree with you there. That's why he's interested in having the errors not occur if you don't read from the volatile tables. Ie, your reporting query reading from read-only tables would be guaranteed to succeed even if some other table had had some rows vacuumed away. I'm not sure that's really going to work because of things like hint bit updates or hot pruning. That is, say your table is read-only now but last week some of the records were updated. You might reasonably expect those rows to be safe and indeed the rows themselves will still be in your report. But the earlier versions of the rows could still be sitting on some pages invisible to every query but not vacuumable quite yet and then suddenly they get vacuumed away and the LSN on the page gets bumped. That's a very interesting point. In the case that the reporting tables are like that (versus being created right before the run, for the report), it would argue for either very aggressive autovacuum settings or an explicit vacuum on those tables before starting the report. Fwiw I would strongly suggest that instead of having a number of transactions to have a time difference. On the 15th I said this: | What this discussion has made me reconsider is the metric for | considering a transaction too old. The number of transaction IDs | consumed seems inferior as the unit of measure for that to LSN or | time. | | It looks to me to be pretty trivial (on the order of maybe 30 lines | of code) to specify this GUC in minutes rather than transaction | IDs. At first glance this seems like it would be vulnerable to the | usual complaints about mismanaged clocks, but those are easily | answered by using a cached time in shared memory that we populate | in such a way that it can never move backward. Done right, this | could even allow the GUC to be changeable on reload rather than | only at restart. A badly mismanaged system clock could not cause a | query to generate incorrect results; the worst that could happen is | that this feature would fail to control bloat as much as expected | or reads of modified data could generate the snapshot too old | error around the time of the clock adjustment. | | As before, this would default to a magic value to mean that you | want the historical PostgreSQL behavior. | | If that makes the idea more palatable to anyone, I can submit a | patch to that effect within the next 24 hours. Until yesterday I didn't get any feedback suggesting that such a change would make the patch more palatable. Now that I have had, I'll try to post a patch to that effect today. Thanks! -- Kevin Grittner EDB: 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] Fetch zero result rows when executing a query?
Sorry to revive this thread, I just had one additional thought... To those advocating the deprecation of the max_rows parameter of Execute, there's another argument to consider. max_rows isn't just there in order to fetch, say, a single row of the result set and discard the rest (which is what I originally asked about). There's also the function of retrieving the resultset in chunks: getting 5 rows, then 10, etc. etc. Deprecating max_rows would leave the user/driver only with the option of retrieving the entire resultset in one go (for example, no option for the interleaving of rows from several resultsets). And the lack of the ability to execute and retrieve 0 rows hurts this scenario as well. Just wanted to put it out there as another argument against deprecation. On Wed, Feb 11, 2015 at 2:05 AM, Shay Rojansky r...@roji.org wrote: Thanks for understanding Robert, that's more or less what I had in mind, I was mainly wondering if I were missing some deeper explanation for the absence of the possibility of requesting 0 rows. Regardless of all of the above, it's really no big deal. I'll go ahead and use max_rows=1 for now, hopefully you guys don't decide to deprecate it. Shay On Tue, Feb 10, 2015 at 3:00 PM, Robert Haas robertmh...@gmail.com wrote: On Sun, Feb 8, 2015 at 3:56 AM, Shay Rojansky r...@roji.org wrote: Just to be precise: what is strange to me is that the max_rows feature exists but has no 0 value. You and Marko are arguing that the whole feature should be deprecated (i.e. always return all rows). I think the fact that it has no zero value is probably just a historical accident; most likely, whoever designed it originally (probably twenty years ago) didn't think about queries with side-effects and therefore didn't consider that wanting 0 rows would ever be sensible. Meanwhile, a sentinel value was needed to request all rows, so they used 0. If they'd thought of it, they might have picked -1 and we'd not be having this discussion. FWIW, I'm in complete agreement that it would be good if we had this feature. I believe this is not the first report we've had of PostgreSQL doing things in ways that mesh nicely with standardized driver interfaces. Whether we think those interfaces are well-designed or not, they are standardized. When people use $OTHERDB and have a really great driver, and then they move to PostgreSQL and get one with more warts, it does not encourage them to stick with PostgreSQL. .NET is not some fringe user community that we can dismiss as irrelevant. We need users of all languages to want to use PostgreSQL, not just users of languages any one of us happens to personally like. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0
On 02/16/2015 11:31 AM, Andres Freund wrote: On 2015-02-16 10:00:24 +0200, Heikki Linnakangas wrote: I'm starting to think that we should bite the bullet and consume an infomask bit for this. The infomask bits are a scarce resource, but we should use them when it makes sense. It would be good for forensic purposes too, to leave a trace that a super-deletion happened. Well, we IIRC don't have any left right now. We could reuse MOVED_IN|MOVED_OUT, as that never was set in the past. But it'd essentially use two infomask bits forever... t_infomask is all used, but t_infomask2 has two bits left: /* * information stored in t_infomask2: */ #define HEAP_NATTS_MASK 0x07FF /* 11 bits for number of attributes */ /* bits 0x1800 are available */ #define HEAP_KEYS_UPDATED 0x2000 /* tuple was updated and key cols * modified, or tuple deleted */ #define HEAP_HOT_UPDATED0x4000 /* tuple was HOT-updated */ #define HEAP_ONLY_TUPLE 0x8000 /* this is heap-only tuple */ #define HEAP2_XACT_MASK 0xE000 /* visibility-related bits */ - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: disallow operator = and use it for named parameters
On 19/01/15 17:14, Pavel Stehule wrote: 2015-01-19 14:27 GMT+01:00 Robert Haas robertmh...@gmail.com mailto:robertmh...@gmail.com: On Mon, Jan 19, 2015 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote: I think you should just remove the WARNING, not change it to an error. If somebody wants to quote the operator name to be able to continue using it, I think that's OK. It looks so quoting doesn't help here + CREATE OPERATOR = ( +leftarg = int8,-- right unary +procedure = numeric_fac + ); + ERROR: syntax error at or near ( + LINE 1: CREATE OPERATOR = ( + ^ Well then the error check is just dead code. Either way, you don't need it. yes, I removed it I am marking this as Ready For Committer, the patch is trivial and works as expected, there is nothing to be added to it IMHO. The = operator was deprecated for several years so it should not be too controversial either. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] pg_upgrade and rsync
On Wed, Jan 28, 2015 at 09:26:11PM -0800, Josh Berkus wrote: So, for my 2c, I'm on the fence about it. On the one hand, I agree, it's a bit of a complex process to get right. On the other hand, it's far better if we put something out there along the lines of if you really want to, this is how to do it than having folks try to fumble through to find the correct steps themselves. So, here's the correct steps for Bruce, because his current doc does not cover all of these. I really think this should go in as a numbered set of steps; the current doc has some steps as steps, and other stuff buried in paragraphs. 1. Install the new version binaries on both servers, alongside the old version. 2. If not done by the package install, initdb the new version's data directory. 3. Check that the replica is not very lagged. If it is, wait for traffic to die down and for it to catch up. Now that 9.4.1 is released, I would like to get this doc patch applied --- it will close the often-requested feature of how to pg_upgrade slave clusters. I wasn't happy with Josh's specification above that the replica is not very lagged, so I added a bullet point to check the pg_controldata output to verify that the primary and standby servers are synchronized. Yes, this adds even more complication to the pg_upgrade instructions, but it is really more of the same complexity. pg_upgrade really needs an install-aware and OS-aware tool on top of it to automate much of this. Patch attached. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml new file mode 100644 index 07ca0dc..e25e0d0 *** a/doc/src/sgml/backup.sgml --- b/doc/src/sgml/backup.sgml *** tar -cf backup.tar /usr/local/pgsql/data *** 438,445 Another option is to use applicationrsync/ to perform a file system backup. This is done by first running applicationrsync/ while the database server is running, then shutting down the database !server just long enough to do a second applicationrsync/. The !second applicationrsync/ will be much quicker than the first, because it has relatively little data to transfer, and the end result will be consistent because the server was down. This method allows a file system backup to be performed with minimal downtime. --- 438,447 Another option is to use applicationrsync/ to perform a file system backup. This is done by first running applicationrsync/ while the database server is running, then shutting down the database !server long enough to do an commandrsync --checksum/. !(option--checksum/ is necessary because commandrsync/ only !has file modification-time granularity of one second.) The !second applicationrsync/ will be quicker than the first, because it has relatively little data to transfer, and the end result will be consistent because the server was down. This method allows a file system backup to be performed with minimal downtime. diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml new file mode 100644 index e1cd260..d1c26df *** a/doc/src/sgml/pgupgrade.sgml --- b/doc/src/sgml/pgupgrade.sgml *** NET STOP postgresql-8.4 *** 315,320 --- 315,324 NET STOP postgresql-9.0 /programlisting /para + + para + Log-shipping standby servers can remain running until a later step. + /para /step step *** pg_upgrade.exe *** 399,404 --- 403,525 /step step + titleUpgrade any Log-Shipping Standby Servers/title + + para + If you have Log-Shipping Standby Servers (xref + linkend=warm-standby), follow these steps to upgrade them (before + starting any servers): + /para + + procedure + + step + titleInstall the new PostgreSQL binaries on standby servers/title + + para +Make sure the new binaries and support files are installed on all +standby servers. + /para + /step + + step + titleMake sure the new standby data directories do emphasisnot/ + exist/title + + para +Make sure the new standby data directories do emphasisnot/ +exist or are empty. If applicationinitdb/ was run, delete +the standby server data directories. + /para + /step + + step + titleInstall custom shared object files/title + + para +Install the same custom shared object files on the new standbys +that you installed in the new master cluster. + /para + /step + + step + titleStop standby servers/title + + para +If the standby servers are still running, stop them now using the +above instructions. + /para + /step + + step +
Re: [HACKERS] deparsing utility commands
Stephen Frost wrote:
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
Now, we probably don't want to hack *all* the utility commands to return
ObjectAddress instead of OID, because it many cases that's just not
going to be convenient (not to speak of the code churn); so I think for
most objtypes the ProcessUtilitySlow stanza would look like this:
That'd be fine with me, though for my 2c, I wouldn't object to changing
them all to return ObjectAddress either. I agree that it'd cause a fair
bit of code churn to do so, but there's a fair bit of code churn
happening here anyway (looking at what 0008 does to ProcessUtilitySlow
anyway).
Well, that would make my life easier I think (even if it's a bit more
work), so unless there are objections I will do things this way. It's a
bit of a pity that Robert and Dimitri went to huge lengths to have these
functions return OID and we're now changing it all to ObjAddress
instead, but oh well.
Not sure that I see it as that huge a deal.. They're still returning an
Oid, it's just embedded in the ObjAddress to provide a complete
statement of what the object is.
I've been looking at this idea some more. There's some churn, but it's
not that bad. For instance, here's the patch to have RENAME return
ObjectAddress rather than OIDs.
For instance, the get_objtype_catalog_id() function, provided in a later
patch in the series, will no longer be necessary; instead, each
execution code path in the src/backend/command code must set the correct
catalog ID in the ObjectAddress it returns. So the event triggers code
will have the catalog ID at the ready, without having to figure it out
from the ObjectType it gets from the parse node.
btw, the hunk in 0026 which adds a 'break;' into standard_ProcessUtility
caught me by surprise. Looks like that 'break;' was missing from 0003
(for T_GrantStmt).
Thanks for pointing this out -- that was broken rebase.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
From 69d0b62787c2a2a514f70247ccde98c4c43d70a0 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Thu, 19 Feb 2015 14:38:09 -0300
Subject: [PATCH] Have RENAME routines return ObjectAddress rather than OID
This lets them include an objectSubId when appropriate (i.e. when
renaming relation columns and composite type attributes), and
additionally they make the catalog OID available for further processing.
The object OID that was previously returned is still available in the
objectId field of the returned ObjectAddress.
This isn't terribly exciting in itself but it supports future event
trigger changes.
Discussion: 20150218213255.gc6...@tamriel.snowman.net
Reviewed-By: Stephen Frost
---
src/backend/catalog/dependency.c| 6 ++-
src/backend/catalog/objectaddress.c | 6 +++
src/backend/commands/alter.c| 14 +--
src/backend/commands/dbcommands.c | 7 +++-
src/backend/commands/policy.c | 7 +++-
src/backend/commands/schemacmds.c | 7 +++-
src/backend/commands/tablecmds.c| 77 +
src/backend/commands/tablespace.c | 7 +++-
src/backend/commands/trigger.c | 7 +++-
src/backend/commands/typecmds.c | 6 ++-
src/backend/commands/user.c | 7 +++-
src/backend/rewrite/rewriteDefine.c | 7 +++-
src/include/catalog/objectaddress.h | 16
src/include/commands/alter.h| 3 +-
src/include/commands/dbcommands.h | 3 +-
src/include/commands/policy.h | 2 +-
src/include/commands/schemacmds.h | 2 +-
src/include/commands/tablecmds.h| 9 +++--
src/include/commands/tablespace.h | 3 +-
src/include/commands/trigger.h | 3 +-
src/include/commands/typecmds.h | 2 +-
src/include/commands/user.h | 2 +-
src/include/rewrite/rewriteDefine.h | 3 +-
23 files changed, 155 insertions(+), 51 deletions(-)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index bacb242..25ff326 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2269,8 +2269,9 @@ free_object_addresses(ObjectAddresses *addrs)
ObjectClass
getObjectClass(const ObjectAddress *object)
{
- /* only pg_class entries can have nonzero objectSubId */
- if (object-classId != RelationRelationId
+ /* only pg_class and pg_type entries can have nonzero objectSubId */
+ if ((object-classId != RelationRelationId
+ object-classId != TypeRelationId)
object-objectSubId != 0)
elog(ERROR, invalid non-zero objectSubId for object class %u,
object-classId);
@@ -2285,6 +2286,7 @@ getObjectClass(const ObjectAddress *object)
return OCLASS_PROC;
case TypeRelationId:
+ /* caller must check objectSubId */
return OCLASS_TYPE;
case CastRelationId:
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index d899dd7..2bbc15d 100644
---
Re: [HACKERS] CATUPDATE confusion?
Peter, Thanks for the review and feedback. One of the interesting behaviors (or perhaps not) is how 'pg_class_aclmask' handles an invalid role id when checking permissions against 'rolsuper' instead of 'rolcatupdate'. I'd get rid of that whole check, not just replace rolcatupdate by rolsuper. Ah yes, that's a good point. I have updated and attached a new version of the patch. Thanks, Adam -- Adam Brightwell - adam.brightw...@crunchydatasolutions.com Database Engineer - www.crunchydatasolutions.com remove-catupdate-v2.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] Allow snapshot too old error, to prevent bloat
Kevin, * Kevin Grittner (kgri...@ymail.com) wrote: Stephen Frost sfr...@snowman.net wrote: Kevin Grittner (kgri...@ymail.com) wrote: (1) They have a pool of connections each of which can have several long-running cursors, so the limit from that isn't just doubling the size of their database, it is limiting it to some two-or-three digit multiple of the necessary size. This strikes me as a bit off-the-cuff; was an analysis done which deteremined that would be the result? If it sounds like off-the-cuff it is because I am summarizing the mountain of data which has been accumulated over weeks of discussions and many rounds of multi-day tests using their actual software driven by a software test driver that simulates users, with think time and all. My apologies for, unintentionally, implying that you hadn't done sufficient research or testing. That certainly was *not* my intent and it's absolutely clear to me that you've spent quite a bit of time analyzing this problem. What I was trying to get at is that the approach which I outlined was, perhaps, not closely analyzed. Now, analyzing a problem based on a solution which doesn't actually exist isn't exactly trivial, and while it might be possible to do in this case it'd probably be easier to simply write the code and the review the results than perform an independent analysis. I do feel that the solution might have been dismissed on the basis of doubling the database for each long-running transaction is not acceptable or even having bloat because of a long-running transaction is not acceptable, but really analyzing what would actually happen is a much more difficult exercise. With the two patches I submitted, bloat stabilized at less than 5% except for some btree indexes which followed pattern of inserting at the end and deleting most (but not all) of the entries over time. I've been working on a separate patch for that, but it's not ready to present, so I probably won't post that until the first CF of the next release -- unless someone's interested enough to want to discuss it before then. Understood. I'd encourage you to post your thoughts on it- I'm certainly interested in it, even if it isn't something we'll be able to really work on for 9.5. That, really, is the crux here- they've already got support for dealing with it the way Oracle did and they'd like PG to do that too. Unfortunately, that, by itself, isn't a good reason for a particular capability (we certainly aren't going to be trying to duplicate PL/SQL in PG any time soon). I understand that, and if the community doesn't want this style of limitation on bloat we can make it part of PPAS (which *does* duplicate PL/SQL, among other things). We are offering it to the community first. That is certainly appreciated and I'd encourage you to continue to do so. I do feel that the community, in general, is interested in these kinds of knobs- it's just a question of implemenation and making sure that it's a carefully considered solution and not a knee-jerk reaction to match what another RDBMS does. I'm not against having a knob like this, which is defaulted to off, Thanks! I'm not sure that amounts to a +1, but at least it doesn't sound like a -1. :-) So, at the time I wrote that, I wasn't sure if it was a +1 or not myself. I've been thinking about it since then, however, and I'm leaning more towards having the capability than not, so perhaps that's a +1, but it doesn't excuse the need to come up with an implementation that everyone can be happy with and what you've come up with so far doesn't have a lot of appeal, based on the feedback (I've only glanced through it myself, but I agree with Andres and Tom that it's a larger footprint than we'd want for this and the number of places having to be touched is concerning as it could lead to future bugs). This conversation has gotten a little confusing because some of my posts seem to have been held up in the email systems for some reason, and are arriving out-of-order (and at least one I don't see yet). But I have been responding to these points. For one thing I stated four days ago that I now feel that the metric for determining that a snapshot is old should be *time*, not transactions IDs consumed, and offered to modify that patch in that direction if people agreed. I now see one person agreeing and several people arguing that I should do just that (as though they had not seem me propose it), so I'll try to get a new version out today like that. In any event, I am sure it is possible and feel that it would be better. (Having posted that now at least 4 times, hopefully people will pick up on it.) I agree with others that having it be time-based would be better. I did see that but it's really an independent consideration from the footprint of the patch. For another thing, as mentioned earlier, this is the btree footprint:
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
Yeah. The only use-case that's been suggested is detecting an unresponsive stats collector, and the main timestamp should be plenty for that. Lately, I've spent most of my time doing investigation into increasing qps. Turned out we've been able to triple our throughput by monitoring experiments at highly granular time steps (1 to 2 seconds). Effects that were invisible with 30 second polls of the stats were obvious with 2 second polls. The problem with doing highly granular snapshots is that the postgres counters are monotonically increasing, but only when stats are published. Currently you have no option except to divide by the delta of now() between the polling intervals. If you poll every 2 seconds the max error is about .5/2 or 25%. It makes reading those numbers a bit noisy. Using (snapshot_timestamp_new - snapshot_timestamp_old) as the denominator in that calculation should help to smooth out that noise and show a clearer picture. However, I'm happy with the committed version. Thanks Tom. - Matt K. On Thu, Feb 19, 2015 at 9:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: Matt Kelly mkell...@gmail.com writes: Attached is the fixed version. (hopefully with the right mime-type and wrong extension. Alas, gmail doesn't let you set mime-types; time to find a new email client...) Committed with a couple of changes: * I changed the function name from pg_stat_snapshot_timestamp to pg_stat_get_snapshot_timestamp, which seemed to me to be the style in general use in the stats stuff for inquiry functions. * The function should be marked stable not volatile, since its value doesn't change any faster than all the other stats inquiry functions. regards, tom lane
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
Matt Kelly mkell...@gmail.com writes: Attached is the fixed version. (hopefully with the right mime-type and wrong extension. Alas, gmail doesn't let you set mime-types; time to find a new email client...) Committed with a couple of changes: * I changed the function name from pg_stat_snapshot_timestamp to pg_stat_get_snapshot_timestamp, which seemed to me to be the style in general use in the stats stuff for inquiry functions. * The function should be marked stable not volatile, since its value doesn't change any faster than all the other stats inquiry functions. 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] Exposing the stats snapshot timestamp to SQL
Matt Kelly mkell...@gmail.com writes: Yeah. The only use-case that's been suggested is detecting an unresponsive stats collector, and the main timestamp should be plenty for that. The problem with doing highly granular snapshots is that the postgres counters are monotonically increasing, but only when stats are published. Currently you have no option except to divide by the delta of now() between the polling intervals. If you poll every 2 seconds the max error is about .5/2 or 25%. It makes reading those numbers a bit noisy. Using (snapshot_timestamp_new - snapshot_timestamp_old) as the denominator in that calculation should help to smooth out that noise and show a clearer picture. Ah, interesting! Thanks for pointing out another use case. 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] Proposal : REINDEX xxx VERBOSE
Hello,
I showed an extreme number of examples to include *almost of all*
variations of existing syntax of option specification. And showed
what if all variations could be used for all commands. It was
almost a mess. Sorry for the confusion.
I think the issues at our hands are,
- Options location: at-the-end or right-after-the-keyword?
- FORCE options to be removed?
- Decide whether to allow bare word option if the options are to
be located right after the keyword.
Optinions or thoughts?
Rethinking from here.
At Wed, 11 Feb 2015 14:34:17 -0600, Jim Nasby jim.na...@bluetreble.com wrote
in 54dbbcc9.1020...@bluetreble.com
On 2/5/15 12:01 PM, Tom Lane wrote:
...
Meh. Options-at-the-end seems by far the most sensible style to me.
The options-right-after-the-keyword style is a mess, both logically
and from a parsing standpoint, and the only reason we have it at all
is historical artifact (ask Bruce about the origins of VACUUM ANALYZE
over a beer sometime).
...
I know you were worried about accepting options anywhere because it
leads to reserved words, but perhaps we could support it just for
EXPLAIN and VACUUM, and then switch to trailing options if people
think that would be better.
According to the above discussion, VACUUM and REINDEX should have
trailing options. Tom seems (to me) suggesting that SQL-style
(bare word preceded by WITH) options and Jim suggesting '()'
style options? (Anyway VACUUM gets the *third additional* option
sytle, but it is the different discussion from this)
VACUUM [tname [(cname, ...)]] [({FULL [bool]|FREEZE | [bool]|...})]
=# VACUUM t1 (FULL, FREEZE);
VACUUM [tname [(cname, ...)]] [WITH [FULL [bool]]|[FREEZE | [bool]|...]
=# VACUUM t1 WITH FULL;
IMHO, we are so accustomed to call by the names VACUUM FULL or
VACUUM FREEZE that the both of them look a bit uneasy.
If the new syntax above is added, REINDEX should have *only* the
trailing style.
REINDEX [{INDEX|TABLE|...}] name [(VERBOSE [bool]|...)]
=# REINDEX TABLE t1 (VERBOSE);
REINDEX [{INDEX|TABLE|...}] name [WITH {VERBOSE [bool]|...}]
=# REINDEX INDEX i_t1_pkey WITH VERBOSE;
Also, both of them seems to be unintuitive..
EXPLAIN.. it seems to be preferred to be as it is..
As the result, it seems the best way to go on the current syntax
for all of those commands.
Optinions?
At Wed, 18 Feb 2015 23:58:15 +0900, Sawada Masahiko sawada.m...@gmail.com
wrote in cad21aobkjndqaq2z-wbscmdhox257bjj6suythwwfs2il8y...@mail.gmail.com
From consistency perspective, I tend to agree with Robert to put
option at immediately after command name as follows.
REINDEX [(VERBOSE | FORCE)] {INDEX | ...} name;
I don't object against it if you prefer it. The remaining issue
is the choice between options-at-the-end or this
options-right-after-the-keyword mentioned above. I prefer the
more messy(:-) one..
Btw how long will the FORCE command available?
The options is obsolete since 7.4. I think it should have been
fade away long since and it's the time to remove it. But once the
ancient option removed, the above syntax looks a bit uneasy and
the more messy syntax looks natural.
REINDEX [VERBOSE] {INDEX | ...} name;
That do you think about this?
regards,
At Tue, 17 Feb 2015 12:00:13 -0600, Jim Nasby jim.na...@bluetreble.com wrote
in 54e381ad.1090...@bluetreble.com
VACUUM [FULL] [FREEZE] ... [ANALYZE] [tname [(cname, ...)]
| VACUUM [({FULL [bool]|FREEZE [bool]|...}[,...])] [tname [(cname, ...)]
| VACUUM [tname [(cname, ...)] [[WITH ]({FULL [bool]|FREEZE
| [bool]|...})]
REINDEX [{INDEX|TABLE|...}] name [[WITH] (VERBOSE [bool]|...)]
EXPLAIN [[WITH] ({ANALYZE [bool]|VERBOSE [bool]|... [,...]})]
statement
For concrete examples, the lines prefixed by asterisk are in new
syntax.
If I could choose only one for explain, I would find it easier to be
up front. That way you do the explain part on one line and just paste
the query after that.
..
VACUUM FULL table1;
VACUUM ANALYZE table1 (col1);
VACUUM (ANALYZE, VERBOSE) table1 (col1);
*VACUUM table1 WITH (FREEZE on)
*VACUUM table1 (cola) WITH (ANALYZE)
*VACUUM table1 WITH (ANALYZE)
*VACUUM table1 (FREEZE on)
The fifth example looks quite odd.
I don't think we need to allow both () and WITH... I'd say one or the
other, preferably ().
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Exposing the stats snapshot timestamp to SQL
On 20.2.2015 02:58, Tom Lane wrote:
Tomas Vondra tomas.von...@2ndquadrant.com writes:
I see the patch only works with the top-level snapshot timestamp,
stored in globalStats, but since 9.3 (when the stats were split
into per-db files) we track per-database timestamps too.
Shouldn't we make those timestamps accessible too? It's not
necessary for detecting unresponsive statistics collector (if it's
stuck it's not writing anything, so the global timestamp will be
old too), but it seems more appropriate for querying database-level
stats to query database-level timestamp too.
I'm inclined to say not; I think that's exposing an implementation
detail that we might regret exposing, later. (IOW, it's not hard to
think of rearrangements that might mean there wasn't a per-database
stamp anymore.)
Fair point, but isn't the global timestamp an implementation detail too?
Although we're less likely to remove the global timestamp, no doubt
about that ...
But maybe that's not necessary, because to query database stats you
have to be connected to that particular database and that should
write fresh stats, so the timestamps should not be very different.
Yeah. The only use-case that's been suggested is detecting an
unresponsive stats collector, and the main timestamp should be plenty
for that.
Well, the patch also does this:
*** 28,34 SELECT pg_sleep_for('2 seconds');
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
--- 28,35
CREATE TEMP TABLE prevstats AS
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
!(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
!pg_stat_snapshot_timestamp() as snap_ts
FROM pg_catalog.pg_stat_user_tables AS t,
pg_catalog.pg_statio_user_tables AS b
WHERE t.relname='tenk2' AND b.relname='tenk2';
***
i.e. it adds the timestamp into queries selecting from database-level
catalogs. But OTOH we're usually using now() here, so the global
snapshot is an improvement here, and if the collector is stuck it's not
going to update of the timestamps.
So I'm OK with this patch.
--
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training 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] Allow snapshot too old error, to prevent bloat
On 2/19/15 1:54 PM, Kevin Grittner wrote: Rod Taylor rod.tay...@gmail.com wrote: Would pg_dump be able to opt-out of such a restriction? I don't see how, since vacuum would be removing recently dead tuples that are still visible; the alternative to getting a snapshot too old error when reading a page which could be affected is to return incorrect results, and nobody wants that. The best you could do if you wanted to run pg_dump (or similar) and it might take more time than your old_snapshot_threshold would be to increase the threshold, reload, dump, set it back to the normal setting, and reload again. While I think pg_dump is a great solution for small to medium installations, there are a number of excellent file-based backup options available. Anyone who is seriously worried about bloat (or locking) should be looking to those solutions. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL
On 2/18/15 10:29 PM, Fujii Masao wrote: On Thu, Feb 19, 2015 at 12:25 AM, David Steele da...@pgmasters.net wrote: The pg_audit doesn't log BIND parameter values when prepared statement is used. Seems this is an oversight of the patch. Or is this intentional? It's actually intentional - following the model I talked about in my earlier emails, the idea is to log statements only. Is this acceptable for audit purpose in many cases? Without the values, I'm afraid that it's hard to analyze what table records are affected by the statements from the audit logs. I was thinking that identifying the data affected is one of important thing for the audit. If I'm malicious DBA, I will always use the extended protocol to prevent the values from being audited when I execute the statement. I agree with you, but I wonder how much is practical at this stage. Let me think about it and see what I can come up with. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
