[HACKERS] some possible parser cleaning: drop support column(table) syntax
Hello this is syntax column(table) necessary still? postgres=# select a(x) from x; a 10 (1 row) Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
Simon Riggs wrote: > On Thu, 2009-10-22 at 07:55 +0300, Heikki Linnakangas wrote: >> Making some effort to transfer locks instead of acquiring+releasing >> would eliminate the need for having extra lock space available when >> switching from hot standby mode to normal operation. > > This isn't very clear. You started by saying you were quite eager to > always grant and then release; this sounds like you don't want that now, > but you now again like the approach I had already attempted to take. Yeah, I haven't made up my mind. What's in there now is certainly broken, so we need to do something. The simplest approach would be to revert the changes in lock_twophase_recover(), while transfering the locks with something like AtPrepare_Locks() would be more robust in the face of shared memory shortage. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
On Thu, 2009-10-22 at 07:55 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Wed, 2009-10-21 at 23:02 +0300, Heikki Linnakangas wrote: > >> Hmm, dunno about that, but there is one problem with the "grant to dummy > >> proc, then release in startup process" approach. What if there isn't > >> enough shared memory available to re-acquire the lock for the dummy > >> proc? It would be rather unfortunate to throw an error and shut down the > >> standby, instead of promoting it to a new master. > > > > Any error would be unfortunate at that point. That particular error > > seems unlikely, since we are only talking about AccessExclusiveLocks. If > > the server has a problem with that many locks, then it is severely in > > danger from prepared transactions in the general case, since such errors > > could be also be thrown by the current code in mildly different > > circumstances. > > Note that read-only backends also occupy lock space when they run > queries, taking AccessShareLocks. > > > Do you see any alternative approaches to the one taken? > > Making some effort to transfer locks instead of acquiring+releasing > would eliminate the need for having extra lock space available when > switching from hot standby mode to normal operation. This isn't very clear. You started by saying you were quite eager to always grant and then release; this sounds like you don't want that now, but you now again like the approach I had already attempted to take. Please explain a little more. Well spotted on the pending bug, BTW. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
Simon Riggs wrote: > On Wed, 2009-10-21 at 23:02 +0300, Heikki Linnakangas wrote: >> Hmm, dunno about that, but there is one problem with the "grant to dummy >> proc, then release in startup process" approach. What if there isn't >> enough shared memory available to re-acquire the lock for the dummy >> proc? It would be rather unfortunate to throw an error and shut down the >> standby, instead of promoting it to a new master. > > Any error would be unfortunate at that point. That particular error > seems unlikely, since we are only talking about AccessExclusiveLocks. If > the server has a problem with that many locks, then it is severely in > danger from prepared transactions in the general case, since such errors > could be also be thrown by the current code in mildly different > circumstances. Note that read-only backends also occupy lock space when they run queries, taking AccessShareLocks. > Do you see any alternative approaches to the one taken? Making some effort to transfer locks instead of acquiring+releasing would eliminate the need for having extra lock space available when switching from hot standby mode to normal operation. >> In fact, what happens if you ran out of shared memory when replaying a >> relation_redo_lock record? Panic? > > An ERROR in the startup process will cause it to upgrade to FATAL, > AFAIK. That means the server will do a crash shutdown, AIUI. That is the > equivalent of a PANIC, I guess. How else could it behave? It could wait for backends to release locks. If there isn't any, then you don't have much choice. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prelim specs for parser hooks for plpgsql
2009/10/22 Itagaki Takahiro : > > Tom Lane wrote: > >> 3. The pre-transform hook would have a signature like >> Node *hook(ParseState *pstate, ColumnRef *cref) >> >> 4. The post-transform hook would have a signature like >> Node *hook(ParseState *pstate, ColumnRef *cref, Node *var) > > Are there any relationships between the hooks and > Function Call Hook/parser hook submitted by Pavel-san? > https://commitfest.postgresql.org/action/patch_view?id=160 No, "my" hook was little bit more general and global. And I was to able inject running environment. > > If we can also use them in normal SQL parsing, they are very useful > to implement SYSDATE global variable for porting from Oracle. > I thing, so you need a global hook, and it isn't what Tom propose. But Tom's proposal is correct - we don't need plpgsql behave ouside plpgsql. Simply, if you like add some special variables like SYSDATE or others, then you need some add new global hook. Pavel Regards Pavel Stehule > Regards, > --- > ITAGAKI Takahiro > NTT Open Source Software Center > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
2009/10/21 Tom Lane : > Josh Berkus writes: >> Making this GUC suset would make it far less useful to users trying to >> gradually upgrade their infrastructures, and make it more likely that >> many/most of our users would just set it to backwards-compatible in >> their postgresql.conf and not fix anything. In fact, I'd go so far as >> to say that if it's suset, we might as well make it sighup because >> postgresql.conf is the *only* place anyone will touch it. > > No, they might reasonably also set it via ALTER DATABASE or ALTER USER. > >> In a secure database setup, none of the functions belong to the >> superuser. Yet an suset would mean that the function owner couldn't set >> the parameter for their own functions, which is the most obvious place >> to set it. > > That's what the #option alternative is for. Yes, it's a bit ugly, but > it's perfectly functional, and secure too. > >> Tom has proposed some kind of odd special "options" syntax to get around >> this, but I think that's unnecessary. So far on this thread, I haven't >> seen anyone engineer an actual function exploit by using this setting; > > It would take a coincidence of names to make anything happen, so the > details would depend a lot on the exact contents of the function you > were hoping to break. But since you insist: > > create function foo(id int) ... > ... > select * into rec from tab where tab.id = id; > if found then do-something else do-something-else end if; > > Under old-style semantics this will do what the programmer thought. > Under Oracle semantics it will return the first table row. If > do-something is security critical then this is enough to call it > an exploit. The reverse direction (code meant for Oracle behavior > breaks under old-style) is not difficult to cons up either; I think > you can find some examples in pgsql-bugs archives from people trying > to port Oracle code to PG. > > Given that people are very prone to intentionally naming things as above > (for a particularly egregious example try > http://archives.postgresql.org/pgsql-bugs/2009-10/msg00054.php) > I think it's entirely foolish to imagine this isn't a real hazard. > If name collisions were improbable we'd not have so many complaints > about the current behavior in our archives, and probably wouldn't be > thinking about changing the behavior at all. > > As for the analogy to search_path, I think if we were doing that over > knowing what we know today, we'd have locked it down a lot more from the > beginning. We might yet decide to lock it down more. It's not a good > example to cite when arguing that this setting doesn't need any > protection. > > regards, tom lane > Although I am one who like to see Oracle behave in Pg I have to say, so Oracle's behave is a significant risk. PL/pgSQL is more dynamic then PL/SQL and it should be very strange, when somebody add new table and for some functions will be broken. What I know, PL/SQL programmers knows this problem and protects self. In light of this I see more important default to raise error. Oracle mode is good for stable environments, for migration from Oracle, but for others is similar risk like current plpgsql, only with different risks. Personally I prefer #option. This option isn't possible to change inside run of function, and this is exactly what we need. It's local only plpgsql problem - it isn't related to postgres, it is related only to plpgsql. regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Prelim specs for parser hooks for plpgsql
Tom Lane wrote: > 3. The pre-transform hook would have a signature like > Node *hook(ParseState *pstate, ColumnRef *cref) > > 4. The post-transform hook would have a signature like > Node *hook(ParseState *pstate, ColumnRef *cref, Node *var) Are there any relationships between the hooks and Function Call Hook/parser hook submitted by Pavel-san? https://commitfest.postgresql.org/action/patch_view?id=160 If we can also use them in normal SQL parsing, they are very useful to implement SYSDATE global variable for porting from Oracle. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
On Wed, Oct 21, 2009 at 5:02 PM, Tom Lane wrote: > Robert Haas writes: >> I actually think that we should not have a GUC for this at all. We >> should have a compiled-in default, and it should be error. If you >> want some other behavior, decorate your functions with #option. > > We've agreed that the factory default should be "error", but I don't > think we can go much further than that in the direction of breaking > peoples' code. We need to provide a backwards-compatible option, > IMHO, so that this isn't seen as a roadblock to updating to 8.5. > (You can argue all you want about whether it really is one, but it'll > be seen as one.) Hmm... I wouldn't see inserting a line at the beginning of every function as a roadblock, but I don't rule out the possibility that I'm thick-skinned. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
Josh Berkus writes: >> That's what the #option alternative is for. Yes, it's a bit ugly, but >> it's perfectly functional, and secure too. > I still don't see why it's needed. If the function owner simply sets > the option in the function definitions (as a userset), it doesn't matter > what the calling user sets, does it? If we do it that way, it is safe only if *every* *single* plpgsql function has an attached SET option for this. Otherwise a function's own setting will propagate to its callees. This is error-prone and will be pretty bad for performance too --- the per-function SET mechanism isn't especially cheap and was never meant to be used by every last function. 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] Controlling changes in plpgsql variable resolution
Robert Haas writes: > I actually think that we should not have a GUC for this at all. We > should have a compiled-in default, and it should be error. If you > want some other behavior, decorate your functions with #option. We've agreed that the factory default should be "error", but I don't think we can go much further than that in the direction of breaking peoples' code. We need to provide a backwards-compatible option, IMHO, so that this isn't seen as a roadblock to updating to 8.5. (You can argue all you want about whether it really is one, but it'll be seen as one.) And the Oracle-compatible option will be attractive to people coming in from that side. Reviewing megabytes of pl/sql code for this kind of gotcha is not fun, and the "error" default would only help a bit. One advantage of making the GUC be SUSET is that those who want to take a paranoid approach here have an easy answer: don't allow it to be changed from "error". We are allowed to assume that those who do change it are responsible adults. 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] alpha2 bundled -- please verify
Can't find it ... and it doesn't look like anyone has moved it ... On Wed, 21 Oct 2009, Peter Eisentraut wrote: Alpha2 has been bundled and is available at http://developer.postgresql.org/~petere/alpha/ Please check that it is sane. Then, someone please move this to an appropriate place on the FTP server and make an announcement. Josh Berkus is coordinating the announcement. See http://wiki.postgresql.org/wiki/Alpha_release_process for process details. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Marc G. FournierHub.Org Hosting Solutions S.A. scra...@hub.org http://www.hub.org Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
On Wed, Oct 21, 2009 at 4:28 PM, Merlin Moncure wrote: > On Wed, Oct 21, 2009 at 3:09 PM, Josh Berkus wrote: >> Tom has proposed some kind of odd special "options" syntax to get around >> this, but I think that's unnecessary. So far on this thread, I haven't >> seen anyone engineer an actual function exploit by using this setting; I >> personally can't come up with one off the top of my head which doesn't >> require the attacker to be the table owner, the function owner, or the >> superuser themselves. Also keep in mind what we're patching here is an >> unmaintanable and insecure practice anyway, which is the ambiguous use >> of variable names which match column names. >> >> So, I'm saying: make it a userset. > > I couldn't disagree more strongly. .conf entries that adjust how > plpgsql funtions operate in a very central way will 'fork' plpgsql > develoeprs so that you have one group of people using method 'a' and > another using method 'b'. Nobody bothers to fix legacy code and now > we have a first class mess. All code intended to run on servers you > don't control (like library code) now needs to be decorated with 'set > local...' which defeats the whole purpose. IMO, guc settings that > control how sql behaves should be avoided at all costs. You should be > able to look at a piece of code and explicitly determine what it does. > At least with #option, knowing the server version and the function > body is enough. if you want to support multiple behaviors but don't > like #option, i think the only alternative is to version the plpgsql > language somehow and decorate 'create function' with the version. Tom > didn't like that idea, but it still beats GUC imo. I agree. We already have one case (search_path) where you potentially can't accurately predict the semantics of the function without knowing something about the calling environment. That means every security-definer function, to be secure, has to explicitly control the search path. That's bad. I actually think that we should not have a GUC for this at all. We should have a compiled-in default, and it should be error. If you want some other behavior, decorate your functions with #option. The only way this is not a train wreck is if the semantics are set *from within the function*. Having any portion of the semantics, no matter how seemingly trivial, imported from the outside is just asking to get whacked on the head with a stream of difficult-to-diagnose misbehavior. Even if we assume, with a heavy dose of unjustified optimism, that no security vulnerabilities will result, it's not going to be fun or pleasant. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
On Wed, 2009-10-21 at 23:02 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Wed, 2009-10-21 at 19:37 +0300, Heikki Linnakangas wrote: > > > >> So, I'm quite eager to just revert all those lock_twophase_recover() > >> changes, and always rely on the "grant lock to dummy proc, then > >> release > >> it in startup process" method. If we don't want to rely on that, > >> PostPrepare_Locks is an example of how to transfer lock ownership from > >> one process to another correctly. > > > > Yes, I realised after I wrote it that PostPrepare already does that > > switch, just been busy with other stuff to switch over the code. > > > > I think we do need some special code because of handling whole lock > > queues. i.e. if there is a backend requesting an AEL but a prepared xact > > has it, the lock queue will initially be Backend->Startup and needs to > > end up looking like Backend->DummyProc. > > Hmm, dunno about that, but there is one problem with the "grant to dummy > proc, then release in startup process" approach. What if there isn't > enough shared memory available to re-acquire the lock for the dummy > proc? It would be rather unfortunate to throw an error and shut down the > standby, instead of promoting it to a new master. Any error would be unfortunate at that point. That particular error seems unlikely, since we are only talking about AccessExclusiveLocks. If the server has a problem with that many locks, then it is severely in danger from prepared transactions in the general case, since such errors could be also be thrown by the current code in mildly different circumstances. Do you see any alternative approaches to the one taken? I have documented the requirement for max_locks_per_transaction to be as high or higher than on master, as is the case for other parameters. > In fact, what happens if you ran out of shared memory when replaying a > relation_redo_lock record? Panic? An ERROR in the startup process will cause it to upgrade to FATAL, AFAIK. That means the server will do a crash shutdown, AIUI. That is the equivalent of a PANIC, I guess. How else could it behave? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
On Wed, Oct 21, 2009 at 3:09 PM, Josh Berkus wrote: > Tom has proposed some kind of odd special "options" syntax to get around > this, but I think that's unnecessary. So far on this thread, I haven't > seen anyone engineer an actual function exploit by using this setting; I > personally can't come up with one off the top of my head which doesn't > require the attacker to be the table owner, the function owner, or the > superuser themselves. Also keep in mind what we're patching here is an > unmaintanable and insecure practice anyway, which is the ambiguous use > of variable names which match column names. > > So, I'm saying: make it a userset. I couldn't disagree more strongly. .conf entries that adjust how plpgsql funtions operate in a very central way will 'fork' plpgsql develoeprs so that you have one group of people using method 'a' and another using method 'b'. Nobody bothers to fix legacy code and now we have a first class mess. All code intended to run on servers you don't control (like library code) now needs to be decorated with 'set local...' which defeats the whole purpose. IMO, guc settings that control how sql behaves should be avoided at all costs. You should be able to look at a piece of code and explicitly determine what it does. At least with #option, knowing the server version and the function body is enough. if you want to support multiple behaviors but don't like #option, i think the only alternative is to version the plpgsql language somehow and decorate 'create function' with the version. Tom didn't like that idea, but it still beats GUC imo. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] alpha2 bundled -- please verify
Alpha2 has been bundled and is available at http://developer.postgresql.org/~petere/alpha/ Please check that it is sane. Then, someone please move this to an appropriate place on the FTP server and make an announcement. Josh Berkus is coordinating the announcement. See http://wiki.postgresql.org/wiki/Alpha_release_process for process details. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
It is solved. I'll propose my work the next weeks. :-) Regards, Samuel. Le mercredi 21 octobre 2009 à 17:31 +0200, Samuel ROZE a écrit : > Hi, > > I'm writing two functions "parse_url_key" and "parse_url_record" which > will have one text argument and will return a record or a specific > column of it. Theses functions are calling "parse_url_exec" which parse > the URL. When theses function will works, i'll purpose them to > PostgreSQL community. > > The problem is that they don't work fine... :/ > > Prototypes of function/struct used by them: > > typedef struct url { > char *scheme; > char *user; > char *pass; > char *host; > unsigned short port; > char *path; > char *query; > char *fragment; > } url; > > url *parse_url_exec (char* str); > > > The parse_url_key function: > > PG_FUNCTION_INFO_V1(parse_url_key); > Datum parse_url_key (PG_FUNCTION_ARGS) > { > char str[] = "http://www.ovh.com/intenal.html";; > //text *my_url = PG_GETARG_TEXT_P(0); > //char *char_url = DatumGetCString(my_url); > > url *ret = parse_url_exec(str); > > PG_RETURN_TEXT_P(ret->host); > } > > Note: I'm using built-in strings to be sure that the recuperation > doesn't change anything.. > > This function works well: > > postgres=# CREATE OR REPLACE FUNCTION parse_url_key(text) RETURNS text > AS '/home/samuel/parse_url.so', 'parse_url_key' LANGUAGE C; > CREATE FUNCTION > postgres=# SELECT parse_url_key('') as scheme; >scheme > > ww.ovh.com > (1 row) > > Note: there's a little problem here but not important. :-) > > The problem is that the other function, "parse_url_record" doesn't > return values ! The code is: > > PG_FUNCTION_INFO_V1(parse_url_record); > Datum parse_url_record (PG_FUNCTION_ARGS) > { > // Vars about the params > //text *str2 = PG_GETARG_TEXT_P(0); > char str[] = "http://www.ovh.com/intenal.html";; > > // Some vars which will used to create the composite output type > TupleDesc tupdesc; > Datum values[2]; // 8 values > HeapTuple tuple; > boolnulls[2]; > int tuplen; > > // Check NULLs values > if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { > PG_RETURN_NULL(); > } > > url *ret = parse_url_exec(str); > > // Add datas into the values Datum > values[0] = PointerGetDatum(ret->scheme); > values[1] = PointerGetDatum(ret->host); > > // Convert values into a composite type > /*tuplen = tupdesc->natts; > nulls = palloc(tuplen * sizeof(bool));*/ > memset(nulls, 0, sizeof(nulls)); > > // build tuple from datum array > tuple = heap_form_tuple(tupdesc, values, nulls); > // Free null values > /*pfree(nulls);*/ > > // Return the composite type > PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); > } > > Note: I'm just returning scheme and host fields for test, but others are > too completed by parse_url_exec. > > It doesn't works fine: > > postgres=# CREATE OR REPLACE FUNCTION parse_url_record(text) RETURNS > record AS '/home/samuel/parse_url.so', 'parse_url_record' LANGUAGE C; > CREATE FUNCTION > postgres=# SELECT * FROM parse_url_record('') as ("scheme" text, "host" > text); > scheme | host > +-- > | > (1 row) > > > Is there anybody here who can help me ? > > Thanks you very much ! > Samuel ROZE. > http://www.d-sites.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] Controlling changes in plpgsql variable resolution
On 10/21/09 1:02 PM, Josh Berkus wrote: >> That's what the #option alternative is for. Yes, it's a bit ugly, but >> it's perfectly functional, and secure too. > > I still don't see why it's needed. If the function owner simply sets > the option in the function definitions (as a userset), it doesn't matter > what the calling user sets, does it? > > All that's needed is a scripting example to set this in all function > definitions as a regular setting. Actually, to follow this up: there would be *huge* utility in ALTER FUNCTION name SET setting=value statement. This would help people do all the "lock down" things we want to do with function definitions. There'd also be utility in a default set of guc settings for new functions, but maybe we should put that off ... --Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
Simon Riggs wrote: > On Wed, 2009-10-21 at 19:37 +0300, Heikki Linnakangas wrote: > >> So, I'm quite eager to just revert all those lock_twophase_recover() >> changes, and always rely on the "grant lock to dummy proc, then >> release >> it in startup process" method. If we don't want to rely on that, >> PostPrepare_Locks is an example of how to transfer lock ownership from >> one process to another correctly. > > Yes, I realised after I wrote it that PostPrepare already does that > switch, just been busy with other stuff to switch over the code. > > I think we do need some special code because of handling whole lock > queues. i.e. if there is a backend requesting an AEL but a prepared xact > has it, the lock queue will initially be Backend->Startup and needs to > end up looking like Backend->DummyProc. Hmm, dunno about that, but there is one problem with the "grant to dummy proc, then release in startup process" approach. What if there isn't enough shared memory available to re-acquire the lock for the dummy proc? It would be rather unfortunate to throw an error and shut down the standby, instead of promoting it to a new master. In fact, what happens if you ran out of shared memory when replaying a relation_redo_lock record? Panic? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
> That's what the #option alternative is for. Yes, it's a bit ugly, but > it's perfectly functional, and secure too. I still don't see why it's needed. If the function owner simply sets the option in the function definitions (as a userset), it doesn't matter what the calling user sets, does it? All that's needed is a scripting example to set this in all function definitions as a regular setting. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
Josh Berkus writes: > Making this GUC suset would make it far less useful to users trying to > gradually upgrade their infrastructures, and make it more likely that > many/most of our users would just set it to backwards-compatible in > their postgresql.conf and not fix anything. In fact, I'd go so far as > to say that if it's suset, we might as well make it sighup because > postgresql.conf is the *only* place anyone will touch it. No, they might reasonably also set it via ALTER DATABASE or ALTER USER. > In a secure database setup, none of the functions belong to the > superuser. Yet an suset would mean that the function owner couldn't set > the parameter for their own functions, which is the most obvious place > to set it. That's what the #option alternative is for. Yes, it's a bit ugly, but it's perfectly functional, and secure too. > Tom has proposed some kind of odd special "options" syntax to get around > this, but I think that's unnecessary. So far on this thread, I haven't > seen anyone engineer an actual function exploit by using this setting; It would take a coincidence of names to make anything happen, so the details would depend a lot on the exact contents of the function you were hoping to break. But since you insist: create function foo(id int) ... ... select * into rec from tab where tab.id = id; if found then do-something else do-something-else end if; Under old-style semantics this will do what the programmer thought. Under Oracle semantics it will return the first table row. If do-something is security critical then this is enough to call it an exploit. The reverse direction (code meant for Oracle behavior breaks under old-style) is not difficult to cons up either; I think you can find some examples in pgsql-bugs archives from people trying to port Oracle code to PG. Given that people are very prone to intentionally naming things as above (for a particularly egregious example try http://archives.postgresql.org/pgsql-bugs/2009-10/msg00054.php) I think it's entirely foolish to imagine this isn't a real hazard. If name collisions were improbable we'd not have so many complaints about the current behavior in our archives, and probably wouldn't be thinking about changing the behavior at all. As for the analogy to search_path, I think if we were doing that over knowing what we know today, we'd have locked it down a lot more from the beginning. We might yet decide to lock it down more. It's not a good example to cite when arguing that this setting doesn't need any protection. 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] Controlling changes in plpgsql variable resolution
Robert, >> H. I don't see any reason why this couldn't be set by any user at >> runtime, really. From a security standpoint, it's less of a risk than >> search_path, and we allow anyone to mess with that. > > That's like saying that it's less of a risk than a group of rabid > tyrannosaurs in a kindergarten classroom. No, it's like saying "I don't see a point in putting a burglar alarm on the windows when the door isn't even locked." Making this GUC suset would make it far less useful to users trying to gradually upgrade their infrastructures, and make it more likely that many/most of our users would just set it to backwards-compatible in their postgresql.conf and not fix anything. In fact, I'd go so far as to say that if it's suset, we might as well make it sighup because postgresql.conf is the *only* place anyone will touch it. In a secure database setup, none of the functions belong to the superuser. Yet an suset would mean that the function owner couldn't set the parameter for their own functions, which is the most obvious place to set it. Tom has proposed some kind of odd special "options" syntax to get around this, but I think that's unnecessary. So far on this thread, I haven't seen anyone engineer an actual function exploit by using this setting; I personally can't come up with one off the top of my head which doesn't require the attacker to be the table owner, the function owner, or the superuser themselves. Also keep in mind what we're patching here is an unmaintanable and insecure practice anyway, which is the ambiguous use of variable names which match column names. So, I'm saying: make it a userset. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
Robert Haas writes: > On Wed, Oct 21, 2009 at 2:41 PM, Tom Lane wrote: >> Only connections that are actually using the feature. It doesn't >> bother me that much --- before 7.4 we had *multiple* round trips >> involved in a connection start, > OK, but surely we're not saying that was good? I presume we fixed > that for a reason and want it to stay fixed. Well, that was one of multiple things we fixed in the 7.4 protocol version bump. The *right* way to fix this would probably be another protocol bump, but I don't see us doing one now ... there are not enough things broken to justify it, yet. I think that leaving this as a separate SET until such time as that happens is the most reasonable thing to do from a project management standpoint. This feature is a nice-to-have, it's not any kind of "must"; so we should not be boxing ourselves in with weird kluges we'll have to stay compatible with till the end of time in order to make it work slightly better. If you want to avoid an extra round trip, that could probably be managed with some effort within libpq by not waiting for the response to come back after the SET. It'd just need to retain some state to remind itself to discard the first CommandComplete message from the server. I'm not convinced it's worth the trouble though. 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] Controlling changes in plpgsql variable resolution
On Oct 21, 2009, at 11:37 AM, Robert Haas wrote: That's like saying that it's less of a risk than a group of rabid tyrannosaurs in a kindergarten classroom. I'm not sure, but I kind of doubt that tyrannosaurs can get rabies. I mean, if they were even around anymore. Which, you know, they're not. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 2:41 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Oct 21, 2009 at 12:27 PM, Tom Lane wrote: >>> The post-connect SET still seems like the best choice to me. > >> Are we really thinking about interposing an additional server >> round-trip on every connection for such a marginal feature (to >> paraphrase yourself)? That doesn't seem like a good trade-off. > > Only connections that are actually using the feature. It doesn't > bother me that much --- before 7.4 we had *multiple* round trips > involved in a connection start, OK, but surely we're not saying that was good? I presume we fixed that for a reason and want it to stay fixed. > and anyway backend startup is a > pretty dang heavyweight operation. > > If you are concerned about that you should certainly not be advocating > multiple connection tries instead. That's a lot of round trips too, > plus you are paying repeated fork and backend-startup overhead. Yeah, I don't like that either, but at least it only happens when you have a server-version mismatch, and even then (if we accept Dave's other suggestion) only for releases prior to 8.5. I'd rather it be dog-slow in a rare situation that I can avoid through proper configuration than moderately slow in a common situation that I can't escape short of not using the feature. Of course, the difficulty of implementation is another issue... ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
Robert Haas writes: > On Wed, Oct 21, 2009 at 12:27 PM, Tom Lane wrote: >> The post-connect SET still seems like the best choice to me. > Are we really thinking about interposing an additional server > round-trip on every connection for such a marginal feature (to > paraphrase yourself)? That doesn't seem like a good trade-off. Only connections that are actually using the feature. It doesn't bother me that much --- before 7.4 we had *multiple* round trips involved in a connection start, and anyway backend startup is a pretty dang heavyweight operation. If you are concerned about that you should certainly not be advocating multiple connection tries instead. That's a lot of round trips too, plus you are paying repeated fork and backend-startup overhead. 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] Controlling changes in plpgsql variable resolution
On Wed, Oct 21, 2009 at 1:59 PM, Josh Berkus wrote: > Tom, > >> 1. Invent a GUC that has the settings backwards-compatible, >> oracle-compatible, throw-error (exact spellings TBD). Factory default, >> at least for a few releases, will be throw-error. Make it SUSET so that >> unprivileged users can't break things by twiddling it; but it's still >> possible for the DBA to set it per-database or per-user. >> >> 2. Also invent a #option syntax that allows the GUC to be overridden >> per-function. (Since the main GUC is SUSET, we can't just use a >> per-function SET to override it. There are other ways we could do this >> but none seem less ugly than #option...) > > H. I don't see any reason why this couldn't be set by any user at > runtime, really. From a security standpoint, it's less of a risk than > search_path, and we allow anyone to mess with that. That's like saying that it's less of a risk than a group of rabid tyrannosaurs in a kindergarten classroom. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 12:27 PM, Tom Lane wrote: > Dave Page writes: >> BTW, any thoughts on Heikki's suggestions of hacking about the >> 'options' value or retrying the connection vs. just doing a SET >> post-connection in libpq? It's pretty certain that whatever I choose >> you probably won't like :-p > > The post-connect SET still seems like the best choice to me. > It's mildly annoying that that won't help for log_connections > messages, but in the big scheme of things that's really not a > killer problem. Are we really thinking about interposing an additional server round-trip on every connection for such a marginal feature (to paraphrase yourself)? That doesn't seem like a good trade-off. > The retry approach is not too bad from a user perspective: it would > only actually happen during a server version mismatch, which isn't > *that* common. My recollection though is that there's no graceful way > to implement a retry in libpq; you'd need a significant amount of new, > ugly, special-purpose code, with the complexity rising very fast if > there's more than one reason to retry. If you can figure out a clean > implementation this one would be okay with me, but I'm dubious that > it's worth the work. > > That options hack was just an ugly hack, I don't like it at all --- > mainly because I don't believe that approach scales to solve more > than one case either. Either way, seems like you can try it with all the options and the back up one major release at a time until you find compatibility. It's only O(features), not O(2^features). ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Controlling changes in plpgsql variable resolution
Tom, > 1. Invent a GUC that has the settings backwards-compatible, > oracle-compatible, throw-error (exact spellings TBD). Factory default, > at least for a few releases, will be throw-error. Make it SUSET so that > unprivileged users can't break things by twiddling it; but it's still > possible for the DBA to set it per-database or per-user. > > 2. Also invent a #option syntax that allows the GUC to be overridden > per-function. (Since the main GUC is SUSET, we can't just use a > per-function SET to override it. There are other ways we could do this > but none seem less ugly than #option...) H. I don't see any reason why this couldn't be set by any user at runtime, really. From a security standpoint, it's less of a risk than search_path, and we allow anyone to mess with that. Then we'd have the simple factor of setting it in postgresql.conf or setting it in the function definitions via WITH. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
Tom Lane wrote: > That options hack was just an ugly hack, I don't like it at all --- > mainly because I don't believe that approach scales to solve more > than one case either. It does if you hack it even more: don't pass the (first) options directly as command line arguments, but parse it in ProcessStartupPacket into a list of GUC settings. Add the ones that the server version understands into the usual list of GUCs to set, and ignore others. Yeah, ugly as hell, but does scale if used wisely. One possible issue is that there might be 3rd party applications out there that speak the fe/be protocol, like proxies. They might not like an extra options line. I don't think it's a show-stopper, but would need to check at least the popular ones out there. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Prelim specs for parser hooks for plpgsql
Here's what I'm thinking of doing to enable plpgsql to resolve variable references during the main SQL parser processing, instead of its current hack of replacing references with $n in advance: 1. Add some fields to ParseState to carry hook function pointers as well as "void *" passthrough arguments for the hook functions. 2. transformColumnRef will need two hooks, a pre-transform hook and a post-transform hook. The pre-transform hook would be used to implement "old style" semantics, ie, the plpgsql variable definition takes precedence over query definition. The post hook would be used to implement either "oracle style" semantics or throwing error for conflicts. 3. The pre-transform hook would have a signature like Node *hook(ParseState *pstate, ColumnRef *cref) If it returns a node, we immediately return that as the analyzed value of the ColumnRef. If there's no hook or it returns NULL, we proceed with the normal analysis of the ColumnRef. Note: I don't see any big need to pass the "void *passthrough" argument to the hook explicitly --- if it needs it, it can fetch it out of the ParseState for itself. 4. The post-transform hook would have a signature like Node *hook(ParseState *pstate, ColumnRef *cref, Node *var) "var" is either the analyzed equivalent of the ColumnRef (usually, but not necessarily, a Var node) or NULL if the system couldn't find a referent. If "var" is NULL then the hook can return a substitute node (probably a Param), or it can return NULL to indicate that the normal no-such-column error should be thrown. If "var" is not null then the hook should either return "var", or throw error if it wants to complain that the reference is ambiguous. (We will disallow the third possibility of returning an override value when var isn't null. This is because the normal processing may have already made changes to the parse tree, which the hook won't have enough information to undo.) It will take a certain amount of code rearrangement to implement the post-transform hook --- we can't just add a call at the bottom of transformColumnRef, because we will need to retain enough state to throw the correct error if neither the core parser nor the hook can resolve the ColumnRef. This seems reasonably do-able, though the actual hook call may end up in some weird places like ParseFuncOrColumn. 5. We will also need a hook in transformParamRef() so that plpgsql can implement old-style $n references to function parameters. In this case there doesn't seem to be any real need for before/after hooks, since any given parser caller should only need p_paramtypes or a hook, not both. In fact, what I'd like to do is see if we can rip out p_paramtypes and the parser's built-in processing of ParamRefs *entirely*, and re-implement it as a hook supplied by PREPARE or Bind-message processing. So the hook would just be Node *hook(ParseState *pstate, ParamRef *pref) and the core parser would just throw error if the hook's not there or returns NULL. 6. Callers will need a way to set the hook pointers in a ParseState before invoking parsing. I think we can just have them do the equivalent steps to parse_analyze themselves, ie, call make_parsestate, set the hooks, call transformStmt, call free_parsestate; all three of those are exported already anyway. The separate entry point parse_analyze_varparams should go away entirely since the functionality it's setting up will move to hooks. 7. plpgsql will have a bit more of a problem since it's currently a couple of call levels away from the parser --- it goes through SPI. So we're going to have to modify SPI's API to some extent. I'm a bit tempted to add an entry point that lets the caller supply the parsed/analyzed/planned plantree, instead of assuming that that work must be done internally to SPI. Any thoughts about that? Any comments or objections to this plan of work? 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] URL Managment - C Function help
Samuel ROZE wrote: Le mercredi 21 octobre 2009 à 12:59 -0400, Andrew Dunstan a écrit : On 8.3 you might need to put a #define for it directly in your C file. I can't: cstring_to_text isn't defined. I'll develop on 8.4. or try: #define CStringGetTextP(c) DatumGetTextP(DirectFunctionCall1(textin, CStringGetDatum(c))) 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] URL Managment - C Function help
Le mercredi 21 octobre 2009 à 12:59 -0400, Andrew Dunstan a écrit : > On 8.3 you might need to put a #define for it directly in your C file. I can't: cstring_to_text isn't defined. I'll develop on 8.4. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot standby, prepared xacts, locks
On Wed, 2009-10-21 at 19:37 +0300, Heikki Linnakangas wrote: > So, I'm quite eager to just revert all those lock_twophase_recover() > changes, and always rely on the "grant lock to dummy proc, then > release > it in startup process" method. If we don't want to rely on that, > PostPrepare_Locks is an example of how to transfer lock ownership from > one process to another correctly. Yes, I realised after I wrote it that PostPrepare already does that switch, just been busy with other stuff to switch over the code. I think we do need some special code because of handling whole lock queues. i.e. if there is a backend requesting an AEL but a prepared xact has it, the lock queue will initially be Backend->Startup and needs to end up looking like Backend->DummyProc. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Samuel ROZE wrote: I've done it but I had no results... strange. I've a 8.3 version and this lines are NOT in the file: You neglected to tell us you were in 8.3 before, I think. On 8.3 you might need to put a #define for it directly in your C file. 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] Could regexp_matches be immutable?
On Oct 21, 2009, at 9:48 AM, Tom Lane wrote: I was wondering whether you could query pg_proc to look for functions with the same name and different arguments/results. It's a bit tricky though because you'd expect s/citext/text/ in at least some positions (maybe not all)? Yeah, almost all. I'll poke around, though it might be a day or two… Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Samuel ROZE writes: > I've done it but I had no results... strange. > I've a 8.3 version and this lines are NOT in the file: Oh, it was changed in 8.4 IIRC. If you are thinking of submitting code to the project you should not be developing against a back release anyway ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
"David E. Wheeler" writes: > Is there a straight-foward way to check such a thing > programmatically, with a query perhaps? Or should I just put aside an > hour to do an audit? I was wondering whether you could query pg_proc to look for functions with the same name and different arguments/results. It's a bit tricky though because you'd expect s/citext/text/ in at least some positions (maybe not all)? 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] URL Managment - C Function help
I'm now using C strings. I don't need to use CStringGetTextDatum, but it still don't works. There's the code: --- PG_FUNCTION_INFO_V1(parse_url_record); Datum parse_url_record (PG_FUNCTION_ARGS) { // Vars about the params //text *str2 = PG_GETARG_TEXT_P(0); char str[] = "http://www.ovh.com/intenal.html";; // Some vars which will used to create the composite output type TupleDesc tupdesc; char**values; HeapTuple tuple; AttInMetadata *attinmeta; boolnulls[2]; url *ret; // Check NULLs values if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { PG_RETURN_NULL(); } ret = parse_url_exec(str); if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("function returning record called in context that cannot accept type record"))); } attinmeta = TupleDescGetAttInMetadata(tupdesc); // ... values = (char **) palloc(2 * sizeof(char *)); // Add datas into the values Datum values[0] = (char *) ret->scheme; values[1] = (char *) ret->host; // Convert values into a composite type memset(nulls, 0, sizeof(nulls)); // build tuple from datum array tuple = BuildTupleFromCStrings(attinmeta, values); // Return the composite type PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); } --- Thanks a lot ! Samuel ROZE. Le mercredi 21 octobre 2009 à 11:42 -0400, Tom Lane a écrit : > Samuel ROZE writes: > > The problem is that they don't work fine... :/ > > I think the problem is that you are passing C strings to code that > expects pointers to text datums --- which are not the same thing > at all. (text has a length word, not a null terminator byte.) > It's pure accident that your first example works, and entirely > unsurprising that the second one doesn't. Some CStringGetTextDatum > calls might help. > > 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] Could regexp_matches be immutable?
"David E. Wheeler" writes: > FWIW, I think that this is a bug, and that the variation from the text > version will be unexpected. I recommend fixing it for 8.4.2. Well, it's certainly a bug, but I don't think it's back-patchable. A back-patch will not affect existing installations anyway. What it will do is break user code that is expecting the existing behavior (for instance, Rod's index). I think it's something we can only change at a major version boundary. 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] Could regexp_matches be immutable?
On Oct 21, 2009, at 9:40 AM, David E. Wheeler wrote: On Oct 21, 2009, at 9:37 AM, Tom Lane wrote: Ooh, yeah, dunno how I missed that. I think we're probably stuck in 8.4, but we should fix it going forward. Would you make a quick check if any of the other citext functions have the same bug? I've fixed it in my [version for 8.3](https://svn.kineticode.com/citext/trunk ). Is there a straight-foward way to check such a thing programmatically, with a query perhaps? Or should I just put aside an hour to do an audit? FWIW, I think that this is a bug, and that the variation from the text version will be unexpected. I recommend fixing it for 8.4.2. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
On Oct 21, 2009, at 9:37 AM, Tom Lane wrote: Ooh, yeah, dunno how I missed that. I think we're probably stuck in 8.4, but we should fix it going forward. Would you make a quick check if any of the other citext functions have the same bug? I've fixed it in my [version for 8.3](https://svn.kineticode.com/citext/trunk ). Is there a straight-foward way to check such a thing programmatically, with a query perhaps? Or should I just put aside an hour to do an audit? Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
I've done it but I had no results... strange. I've a 8.3 version and this lines are NOT in the file: 00668 /* varlena.c */ 00669 extern text *cstring_to_text(const char *s); 00670 extern text *cstring_to_text_with_len(const char *s, int len); 00671 extern char *text_to_cstring(const text *t); 00672 extern void text_to_cstring_buffer(const text *src, char *dst, size_t dst_len); 00673 00674 #define CStringGetTextDatum(s) PointerGetDatum(cstring_to_text(s)) 00675 #define TextDatumGetCString(d) text_to_cstring((text *) DatumGetPointer(d)) Le mercredi 21 octobre 2009 à 12:28 -0400, Andrew Dunstan a écrit : > > Samuel ROZE wrote: > > Le mercredi 21 octobre 2009 à 11:42 -0400, Tom Lane a écrit : > > > >> CStringGetTextDatum > >> > > > > Can you give me more precisions ? > > > > I'm creating a "user C function", with shared library and > > CStringGetTextDatum is in "varlena.h" file which is not in the > > "src/include" dir... How can I include it ? > > > > > > > > It's in utils/builtins.h, as a simple grep search should have told you. > > cheers > > andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Hot standby, prepared xacts, locks
The Hot Standby patch changes lock_twophase_recover() so that when we're starting up from Hot Standby mode to normal operation, as opposed to crash recovery, we assume that all AccessExcusiveLocks are already held by the startup process and instead of acquiring them anew they are transferred from the startup process to the dummy PGPROC entry. As the patch stands, that works. However, if we don't see a running-xacts record, but only some XLOG_RELATION_LOCK records followed by a PREPARE record, the startup process will hold some of the transactions locks but not all of them. That's OK because we're not letting read-only backends in yet. If we then failover and bring the standby server up as a new master, lock_twophase_recover() will grant the locks directly for the dummy process. But the startup process is already holding some of them, so we briefly have a situation where two processes - the dummy process and the startup process - are both holding the same AccessExclusiveLock. AFAICS, the lock manager can cope with that situation just fine, even though it can't normally happen, but it's worth a mention. A while back in your branch you changed ProcArrayApplyRecoverInfo() so that the standby is put into "pending" mode when it sees a running-xacts record marked as 'locks overflowed', instead of just ignoring the record. If we see such a record, go into pending mode, and then try to bring up the standby as the new master, we will run into this error in lock_twophase_recover(): > /* >* We should already hold the desired lock. >*/ > if (!(proclock->holdMask & LOCKBIT_ON(lockmode))) > elog(ERROR, "lock %s on object %u/%u/%u by xid %u was > not held by startup process", >lockMethodTable->lockModeNames[lockmode], >locktag->locktag_field1, > locktag->locktag_field2, >locktag->locktag_field3, >xid); > lock_twophase_recover() assumes that all locks held by the prepared transactions are already held by the startup process, but that might not be true in the pending state. Given that even without that change in ProcArrayApplyRecoveryInfo(), we can end up in a situation where both the dummy process and the startup process hold the same AccessExclusiveLock, I think we could simply revert all the changes in lock_twophase_recover() and rely on that behavior whenever we end recovery. BTW, this self-proclaimed ugly hack in lock_twophase_recover() looks utterly broken: > /* >* Transfer the lock->procLocks entry so that the lock is now > owned >* by the new gxact proc. Nothing else need change. >* This can happen safely because the startup process has the > lock >* and wishes to give the lock to a gxact dummy process that has >* no associated backend. So neither proc can change > concurrently. >* This technique might normally be considered an ugly hack, yet >* managing dummy procs all the way through recovery resulted in >* more complex code, which was worse. Chiedere il permesso. >*/ > SHMQueueInit(&lock->procLocks); > SHMQueueInsertBefore(&lock->procLocks, &proclock->lockLink); > SHMQueueInsertBefore(&(proc->myProcLocks[partition]), >&proclock->procLink); It will blow away any PROCLOCKs belonging to other backends waiting for the lock, and you can't just transfer a PROCLOCK entry from one backend to another anyway, because the PGPROC pointer is part of its the hash key. But then again, I don't think it's actually transferring the startup process' PROCLOCK entry anyway, because the PROCLOCK entry searched/created just above that belongs to the dummy proc, not the startup process. So this is really just blowing away all other PROCLOCKs from the lock's procLock queue, and possibly inserting a duplicate entry into proc->myProcLocks[partition]. Ugh. So, I'm quite eager to just revert all those lock_twophase_recover() changes, and always rely on the "grant lock to dummy proc, then release it in startup process" method. If we don't want to rely on that, PostPrepare_Locks is an example of how to transfer lock ownership from one process to another correctly. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
"David E. Wheeler" writes: > On Oct 21, 2009, at 7:27 AM, Tom Lane wrote: >> Huh, it looks to me like that's an error in the declaration of the >> citext versions of regexp_matches --- they should be declared to >> return >> setof text[], the same as the underlying text functions. David, do >> you agree? > Ooh, yeah, dunno how I missed that. I think we're probably stuck in 8.4, but we should fix it going forward. Would you make a quick check if any of the other citext functions have the same bug? 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] URL Managment - C Function help
Samuel ROZE wrote: Le mercredi 21 octobre 2009 à 11:42 -0400, Tom Lane a écrit : CStringGetTextDatum Can you give me more precisions ? I'm creating a "user C function", with shared library and CStringGetTextDatum is in "varlena.h" file which is not in the "src/include" dir... How can I include it ? It's in utils/builtins.h, as a simple grep search should have told you. 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] Client application name
Dave Page writes: > BTW, any thoughts on Heikki's suggestions of hacking about the > 'options' value or retrying the connection vs. just doing a SET > post-connection in libpq? It's pretty certain that whatever I choose > you probably won't like :-p The post-connect SET still seems like the best choice to me. It's mildly annoying that that won't help for log_connections messages, but in the big scheme of things that's really not a killer problem. The retry approach is not too bad from a user perspective: it would only actually happen during a server version mismatch, which isn't *that* common. My recollection though is that there's no graceful way to implement a retry in libpq; you'd need a significant amount of new, ugly, special-purpose code, with the complexity rising very fast if there's more than one reason to retry. If you can figure out a clean implementation this one would be okay with me, but I'm dubious that it's worth the work. That options hack was just an ugly hack, I don't like it at all --- mainly because I don't believe that approach scales to solve more than one case either. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
On Oct 21, 2009, at 7:27 AM, Tom Lane wrote: Huh, it looks to me like that's an error in the declaration of the citext versions of regexp_matches --- they should be declared to return setof text[], the same as the underlying text functions. David, do you agree? Ooh, yeah, dunno how I missed that. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Le mercredi 21 octobre 2009 à 11:42 -0400, Tom Lane a écrit : > CStringGetTextDatum Can you give me more precisions ? I'm creating a "user C function", with shared library and CStringGetTextDatum is in "varlena.h" file which is not in the "src/include" dir... How can I include it ? Thanks. Samuel. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Application name patch - v2
Dave Page wrote: > Robert Haas wrote: > >> I also like PGAPPNAME better, for the same reasons as Tom. > > :-). Have to admit, I've mistyped it a few times too. Well, it would seem we have consensus on that. :-) I don't feel that the Java default issue reached the same level of consensus, though. I think we can rule out anything beyond an environment variable or system property as a non-null default. The options seem to be: (1) No non-null default. If they don't set it through the standard techniques, the default is null. (2) Use an environment variable. (3) Use a system property. (4) Some combination of (2) and (3), with one having precedence over the other. Here's what the Java documentation has to say about environment variables versus system properties: | System properties and environment variables are both conceptually | mappings between names and values. Both mechanisms can be used to | pass user-defined information to a Java process. Environment | variables have a more global effect, because they are visible to all | descendants of the process which defines them, not just the | immediate Java subprocess. They can have subtly different semantics, | such as case insensitivity, on different operating systems. For | these reasons, environment variables are more likely to have | unintended side effects. It is best to use system properties where | possible. Environment variables should be used when a global effect | is desired, or when an external system interface requires an | environment variable (such as PATH). It would be zero lines of programming to support *setting* either or both, just documenting it and very simple code to use either. For example, assuming the code to support the standard setting is written, we could allow both defaults for cases where it isn't explicitly set, with precedence given to the system property; and the entire programming effort would look something like this: if (appName == null) appName = System.getProperty("PGAPPNAME", System.getenv("PGAPPNAME")); So the coding involved isn't overwhelming. :-) The primary use case would be to allow someone with an existing application to set this without changing or recompiling any Java code -- they could just set the value in the launch script. Does anyone have an opinion on this? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Thanks for your reply. PG_FUNCTION_INFO_V1(parse_url_record); Datum parse_url_record (PG_FUNCTION_ARGS) { // Vars about the params //text *str2 = PG_GETARG_TEXT_P(0); char str[] = "http://www.ovh.com/intenal.html";; // Some vars which will used to create the composite output type TupleDesc tupdesc; char**values; HeapTuple tuple; AttInMetadata *attinmeta; boolnulls[2]; int tuplen; // Check NULLs values if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { PG_RETURN_NULL(); } url *ret = parse_url_exec(str); if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("function returning record called in context " "that cannot accept type record"))); } attinmeta = TupleDescGetAttInMetadata(tupdesc); // ... values = (char **) palloc(2 * sizeof(char *)); // Add datas into the values Datum values[0] = (char *) ret->scheme; values[1] = (char *) ret->host; // Convert values into a composite type memset(nulls, 0, sizeof(nulls)); // build tuple from datum array tuple = BuildTupleFromCStrings(attinmeta, values); // Return the composite type PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); } --- This code doesn't works better... :/ Le mercredi 21 octobre 2009 à 18:42 +0300, Heikki Linnakangas a écrit : > Samuel ROZE wrote: > > PG_FUNCTION_INFO_V1(parse_url_record); > > Datum parse_url_record (PG_FUNCTION_ARGS) > > { > > // Vars about the params > > //text *str2 = PG_GETARG_TEXT_P(0); > > char str[] = "http://www.ovh.com/intenal.html";; > > > > // Some vars which will used to create the composite output type > > TupleDesc tupdesc; > > Datum values[2]; // 8 values > > HeapTuple tuple; > > boolnulls[2]; > > int tuplen; > > > > // Check NULLs values > > if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { > > PG_RETURN_NULL(); > > } > > > > url *ret = parse_url_exec(str); > > > > // Add datas into the values Datum > > values[0] = PointerGetDatum(ret->scheme); > > values[1] = PointerGetDatum(ret->host); > > > > // Convert values into a composite type > > /*tuplen = tupdesc->natts; > > nulls = palloc(tuplen * sizeof(bool));*/ > > memset(nulls, 0, sizeof(nulls)); > > > > // build tuple from datum array > > tuple = heap_form_tuple(tupdesc, values, nulls); > > // Free null values > > /*pfree(nulls);*/ > > > > // Return the composite type > > PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); > > } > > You haven't initialized tupdesc. > > BTW, there's a fine example in the manual: > http://www.postgresql.org/docs/8.4/interactive/xfunc-c.html#AEN44968 > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 4:39 PM, Dave Page wrote: > On Wed, Oct 21, 2009 at 3:49 PM, Tom Lane wrote: >> Dave Page writes: >> This might be a good argument for changing that going forward, but >> it will be *years* before we can rely on it for anything. > > That's what I meant by 'a few releases' (major, not minor). BTW, any thoughts on Heikki's suggestions of hacking about the 'options' value or retrying the connection vs. just doing a SET post-connection in libpq? It's pretty certain that whatever I choose you probably won't like :-p -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Samuel ROZE wrote: > PG_FUNCTION_INFO_V1(parse_url_record); > Datum parse_url_record (PG_FUNCTION_ARGS) > { > // Vars about the params > //text *str2 = PG_GETARG_TEXT_P(0); > char str[] = "http://www.ovh.com/intenal.html";; > > // Some vars which will used to create the composite output type > TupleDesc tupdesc; > Datum values[2]; // 8 values > HeapTuple tuple; > boolnulls[2]; > int tuplen; > > // Check NULLs values > if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { > PG_RETURN_NULL(); > } > > url *ret = parse_url_exec(str); > > // Add datas into the values Datum > values[0] = PointerGetDatum(ret->scheme); > values[1] = PointerGetDatum(ret->host); > > // Convert values into a composite type > /*tuplen = tupdesc->natts; > nulls = palloc(tuplen * sizeof(bool));*/ > memset(nulls, 0, sizeof(nulls)); > > // build tuple from datum array > tuple = heap_form_tuple(tupdesc, values, nulls); > // Free null values > /*pfree(nulls);*/ > > // Return the composite type > PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); > } You haven't initialized tupdesc. BTW, there's a fine example in the manual: http://www.postgresql.org/docs/8.4/interactive/xfunc-c.html#AEN44968 -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] URL Managment - C Function help
Samuel ROZE writes: > The problem is that they don't work fine... :/ I think the problem is that you are passing C strings to code that expects pointers to text datums --- which are not the same thing at all. (text has a length word, not a null terminator byte.) It's pure accident that your first example works, and entirely unsurprising that the second one doesn't. Some CStringGetTextDatum calls might help. 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] Application name patch - v2
On Wed, Oct 21, 2009 at 4:29 PM, Robert Haas wrote: > I also like PGAPPNAME better, for the same reasons as Tom. :-). Have to admit, I've mistyped it a few times too. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 3:49 PM, Tom Lane wrote: > Dave Page writes: >> Should we perhaps change the behaviour of the backend to give a >> warning only for unknown settings in the startup packet? > > It's not going to help, unless you first invent a time machine so > we can retroactively cause all PG servers that are already in the field > to behave that way. > >> It doesn't >> seem beyond the realms of possibility that we might want to add >> something else in the future, and this will at least mean that in a >> few releases time it might be reasonably safe to do so. > > This might be a good argument for changing that going forward, but > it will be *years* before we can rely on it for anything. That's what I meant by 'a few releases' (major, not minor). -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] URL Managment - C Function help
Hi, I'm writing two functions "parse_url_key" and "parse_url_record" which will have one text argument and will return a record or a specific column of it. Theses functions are calling "parse_url_exec" which parse the URL. When theses function will works, i'll purpose them to PostgreSQL community. The problem is that they don't work fine... :/ Prototypes of function/struct used by them: typedef struct url { char *scheme; char *user; char *pass; char *host; unsigned short port; char *path; char *query; char *fragment; } url; url *parse_url_exec (char* str); The parse_url_key function: PG_FUNCTION_INFO_V1(parse_url_key); Datum parse_url_key (PG_FUNCTION_ARGS) { char str[] = "http://www.ovh.com/intenal.html";; //text *my_url = PG_GETARG_TEXT_P(0); //char *char_url = DatumGetCString(my_url); url *ret = parse_url_exec(str); PG_RETURN_TEXT_P(ret->host); } Note: I'm using built-in strings to be sure that the recuperation doesn't change anything.. This function works well: postgres=# CREATE OR REPLACE FUNCTION parse_url_key(text) RETURNS text AS '/home/samuel/parse_url.so', 'parse_url_key' LANGUAGE C; CREATE FUNCTION postgres=# SELECT parse_url_key('') as scheme; scheme ww.ovh.com (1 row) Note: there's a little problem here but not important. :-) The problem is that the other function, "parse_url_record" doesn't return values ! The code is: PG_FUNCTION_INFO_V1(parse_url_record); Datum parse_url_record (PG_FUNCTION_ARGS) { // Vars about the params //text *str2 = PG_GETARG_TEXT_P(0); char str[] = "http://www.ovh.com/intenal.html";; // Some vars which will used to create the composite output type TupleDesc tupdesc; Datum values[2]; // 8 values HeapTuple tuple; boolnulls[2]; int tuplen; // Check NULLs values if(PG_ARGISNULL(0) || PG_ARGISNULL(1)) { PG_RETURN_NULL(); } url *ret = parse_url_exec(str); // Add datas into the values Datum values[0] = PointerGetDatum(ret->scheme); values[1] = PointerGetDatum(ret->host); // Convert values into a composite type /*tuplen = tupdesc->natts; nulls = palloc(tuplen * sizeof(bool));*/ memset(nulls, 0, sizeof(nulls)); // build tuple from datum array tuple = heap_form_tuple(tupdesc, values, nulls); // Free null values /*pfree(nulls);*/ // Return the composite type PG_RETURN_DATUM(HeapTupleGetDatum(tuple)); } Note: I'm just returning scheme and host fields for test, but others are too completed by parse_url_exec. It doesn't works fine: postgres=# CREATE OR REPLACE FUNCTION parse_url_record(text) RETURNS record AS '/home/samuel/parse_url.so', 'parse_url_record' LANGUAGE C; CREATE FUNCTION postgres=# SELECT * FROM parse_url_record('') as ("scheme" text, "host" text); scheme | host +-- | (1 row) Is there anybody here who can help me ? Thanks you very much ! Samuel ROZE. http://www.d-sites.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] Application name patch - v2
Tom Lane wrote: Andrew Dunstan writes: Tom Lane wrote: FWIW, I would prefer PGAPPNAME to PGAPPLICATIONNAME which is what We don't usually use abbreviations, so how about PGCLIENTNAME or some such? Not sure I believe that argument. Among the set of existing libpq environment variables I see PGHOSTADDR PGSSLCERT PGSSLCRL PGKRBSRVNAME PGTZ PGSYSCONFDIR so it can hardly be said that there's a policy of avoiding abbreviations. PGCLIENTNAME would be better than PGAPPLICATIONNAME I guess, but I still prefer the other. OK. I don't have strong feelings on the subject. 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] Application name patch - v2
On Wed, Oct 21, 2009 at 11:25 AM, Tom Lane wrote: > Andrew Dunstan writes: >> Tom Lane wrote: >>> FWIW, I would prefer PGAPPNAME to PGAPPLICATIONNAME which is what > >> We don't usually use abbreviations, so how about PGCLIENTNAME or some such? > > Not sure I believe that argument. Among the set of existing libpq > environment variables I see > > PGHOSTADDR > PGSSLCERT > PGSSLCRL > PGKRBSRVNAME > PGTZ > PGSYSCONFDIR > > so it can hardly be said that there's a policy of avoiding > abbreviations. PGCLIENTNAME would be better than PGAPPLICATIONNAME > I guess, but I still prefer the other. I also like PGAPPNAME better, for the same reasons as Tom. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Application name patch - v2
Andrew Dunstan writes: > Tom Lane wrote: >> FWIW, I would prefer PGAPPNAME to PGAPPLICATIONNAME which is what > We don't usually use abbreviations, so how about PGCLIENTNAME or some such? Not sure I believe that argument. Among the set of existing libpq environment variables I see PGHOSTADDR PGSSLCERT PGSSLCRL PGKRBSRVNAME PGTZ PGSYSCONFDIR so it can hardly be said that there's a policy of avoiding abbreviations. PGCLIENTNAME would be better than PGAPPLICATIONNAME I guess, but I still prefer the other. 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] Application name patch - v2
Tom Lane wrote: "Kevin Grittner" writes: (or whatever name we choose for this in place of PGAPPNAME.) FWIW, I would prefer PGAPPNAME to PGAPPLICATIONNAME which is what Dave has been using in his examples. The latter is too frickin long, and the double N is a typo threat (I already mistyped it in composing this message...) Well, the latter argument suggests we should use an underscore ... We don't usually use abbreviations, so how about PGCLIENTNAME or some such? 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] Application name patch - v2
"Kevin Grittner" writes: > (or whatever name we choose for this in place of PGAPPNAME.) FWIW, I would prefer PGAPPNAME to PGAPPLICATIONNAME which is what Dave has been using in his examples. The latter is too frickin long, and the double N is a typo threat (I already mistyped it in composing this message...) 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] \du quite ugly in 8.4
Alvaro Herrera writes: > The reason it's an improvement of sorts is that there are now more > possible attributes, and if we had kept adding columns it would have > become wider than 80 columns. Yeah, 8.4 has two more possible entries, and adding them as separate columns would have guaranteed that the display doesn't fit in 80 cols. > If there's a way to have the attributes as a single line, > comma-separated, but that wraps around when exceeding the terminal > width, I'm all for it. We could also use that logic in a lot more > \-commands, so it'd be worthwhile I think. The hard part of that is the "wrap" bit. I wonder how badly we need that part though. If we just made it comma-separated instead of newline-separated, it would be a trivial code change, and I bet it would still look okay for the typical case where the user has only a subset of these properties. 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] Client application name
Dave Page writes: > Should we perhaps change the behaviour of the backend to give a > warning only for unknown settings in the startup packet? It's not going to help, unless you first invent a time machine so we can retroactively cause all PG servers that are already in the field to behave that way. > It doesn't > seem beyond the realms of possibility that we might want to add > something else in the future, and this will at least mean that in a > few releases time it might be reasonably safe to do so. This might be a good argument for changing that going forward, but it will be *years* before we can rely on it for anything. 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] postgres 8.3.8 and Solaris 10_x86 64 bit problems?
Andrew Chernow wrote: I'll hack the makefile and give it a shot. No need to hack it, set CFLAGS during configure: shell]# CFLAGS="-m64" ./configure (options) I tried that and it still built a 32 bit version of perl. When I checked the make file it didn't have CFLAGS anywhere. I manually added it but seems to be ignoring it. I'm regretting going with Solaris. I don't recall it being this difficult to work with. perhaps I can do something like CC="cc -m64" ./configure (options) I'll have to play with that. Very weird :-) Thanks -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] Re: BUG #5065: pg_ctl start fails as administrator, with "could not locate matching postgres executable"
Alvaro Herrera writes: > Magnus Hagander wrote: >> From a quick look, it looks fine to me. I don't have time to do a >> complete check right now, but I'll do that as soon as I can and then >> commit it - unless people feel it's more urgent than maybe a week >> worst case, in which case someone else has to pick it up :-) > Are we going to do an 8.4.2 release soon? There's been no discussion of one, so it's not happening in less than a week. I'm happy to wait for Magnus on this --- an extra set of eyeballs can't hurt, especially on something that (I assume) we're going to back-patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
Rod Taylor writes: > It is interesting that "citext" seems to be functional with exactly > the same statements. Huh, it looks to me like that's an error in the declaration of the citext versions of regexp_matches --- they should be declared to return setof text[], the same as the underlying text functions. David, do you agree? 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] Re: [BUGS] Re: BUG #5065: pg_ctl start fails as administrator, with "could not locate matching postgres executable"
Magnus Hagander wrote: > From a quick look, it looks fine to me. I don't have time to do a > complete check right now, but I'll do that as soon as I can and then > commit it - unless people feel it's more urgent than maybe a week > worst case, in which case someone else has to pick it up :-) Are we going to do an 8.4.2 release soon? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
> So, having dismissed my original off-the-cuff answer to Rod, the next > question is what's really going wrong for him. I get this from > a quick trial: I wish I had kept specific notes on what I was actually trying to do. I tried to_number first then the expression as seen below. I guess I saw the error again and assumed it was the same as for to_number. sk=# BEGIN; BEGIN sk=# sk=# create table t1 (col1 text); CREATE TABLE sk=# INSERT INTO t1 values ('Z342432'); INSERT 0 1 sk=# INSERT INTO t1 values ('REW9432'); INSERT 0 1 sk=# sk=# SELECT (regexp_matches(col1, '(\d+)$'))[1] from t1; regexp_matches 342432 9432 (2 rows) sk=# sk=# create index t1_idx ON t1 (( (regexp_matches(col1, '(\d+)$'))[1] )); ERROR: index expression cannot return a set sk=# sk=# ROLLBACK; ROLLBACK It is interesting that "citext" seems to be functional with exactly the same statements. sk=# BEGIN; BEGIN sk=# sk=# create table t1 (col1 citext); CREATE TABLE sk=# INSERT INTO t1 values ('Z342432'); INSERT 0 1 sk=# INSERT INTO t1 values ('REW9432'); INSERT 0 1 sk=# sk=# SELECT (regexp_matches(col1, '(\d+)$'))[1] from t1; regexp_matches 342432 9432 (2 rows) sk=# sk=# create index t1_idx ON t1 (( (regexp_matches(col1, '(\d+)$'))[1] )); CREATE INDEX sk=# sk=# ROLLBACK; ROLLBACK The function regexp_replace(col1, '^[^0-9]+', '') does seem to do the trick for text. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Application name patch - v2
Tom Lane wrote: > [ scratches head... ] I thought the JDBC spec already said exactly > how one would set this. Why would we go to significant effort to > make it behave contrary to spec? We certainly should allow it to be set as specified in the spec. The only question is whether it makes sense to provide a non-null default if it is not set in that way. I thought Magnus was arguing for that. I have no objection, and see potential use-cases where that would be convenient. To illustrate what I'm talking about, adding something like this to the command line which starts Java would provide the non-null default: -DPGAPPNAME="Receipting - Traffic" (or whatever name we choose for this in place of PGAPPNAME.) This seems similar to what is proposed for libpq. The effort to support that would not be significant -- something along the order of if (appName == NULL) appName = System.getProperty("PGAPPNAME"); Do you object? -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] \du quite ugly in 8.4
Peter Eisentraut wrote: > Could someone clarify why this (from PG 8.4) > > # \du > List of roles >Role name | Attributes | Member of > ---+-+ > admin | Create role | {} >: Create DB > postgres | Superuser | {} >: Create role >: Create DB > someotheruser | | {} > someuser | | {} > > is an improvement over this (from PG 8.3) The reason it's an improvement of sorts is that there are now more possible attributes, and if we had kept adding columns it would have become wider than 80 columns. If there's a way to have the attributes as a single line, comma-separated, but that wraps around when exceeding the terminal width, I'm all for it. We could also use that logic in a lot more \-commands, so it'd be worthwhile I think. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
Peter Eisentraut writes: > On Tue, 2009-10-20 at 20:48 -0400, Tom Lane wrote: >> So I went to see about making the changes to remove regex_flavor, and >> was astonished to find that all the regex-related functions are already >> marked immutable, and AFAICS always have been. This is clearly wrong, >> and we would have to fix it if we weren't about to remove the GUC. > Are you sure this wasn't intentional, because it breaks performance and > we doubted that many applications would change regex_flavor on the fly? Intentional or not, it's wrong :-( In practice I doubt there are many cases where constant-folding a regex would be possible or performance-critical. The real use of having it be immutable is probably Rod's, ie, using it in an index. And that is *obviously* really dangerous if there's a GUC affecting the results. 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] UTF8 with BOM support in psql
Peter Eisentraut wrote: On Wed, 2009-10-21 at 13:11 +0900, Itagaki Takahiro wrote: The attached patch replace BOM with while spaces, but it does not change client encoding automatically. I think we can always ignore client encoding at the replacement because SQL command cannot start with BOM sequence. If we don't ignore the sequence, execution of the script must fail with syntax error. I feel that psql is the wrong place to fix this. BOMs in UTF-8 should be ignored everywhere, all the time. I suggest you re-read the Unicode FAQ on the subject. That is not the conclusion I came to after I read it. Quite the reverse in fact. 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] \du quite ugly in 8.4
2009/10/21 Peter Eisentraut : > Could someone clarify why this (from PG 8.4) > > # \du > List of roles > Role name | Attributes | Member of > ---+-+ > admin | Create role | {} > : Create DB > postgres | Superuser | {} > : Create role > : Create DB > someotheruser | | {} > someuser | | {} > > is an improvement over this (from PG 8.3) > > # \du > List of roles > Role name | Superuser | Create role | Create DB | Connections | > ---+---+-+---+-+ > admin | no | yes | yes | no limit | {} > postgres | yes | yes | yes | no limit | {} > someotheruser | no | no | no | no limit | {} > someuser | no | no | no | no limit | {} > (4 rows) > > The way I see it, a perfectly clear, complete, and legible table has > been turned into a denormalized, unreadable, and ugly pile of ASCII > salad that moreover wastes valuable vertical screen space instead of > using the abundant horizontal screen space. > I can see a possible advantage in that there is more room to list the membership items (although I can't see that being useful most of the time), and it also provides flexibility by providing an undefined list of attributes compared to a defined list. For example, the new format allows the display of "No inheritance", something not shown in the old format. However, if that is the limit of how this more flexible approach is being taken advantage of, I'd tend to agree that the previous format was fine. Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] \du quite ugly in 8.4
Could someone clarify why this (from PG 8.4) # \du List of roles Role name | Attributes | Member of ---+-+ admin | Create role | {} : Create DB postgres | Superuser | {} : Create role : Create DB someotheruser | | {} someuser | | {} is an improvement over this (from PG 8.3) # \du List of roles Role name | Superuser | Create role | Create DB | Connections | ---+---+-+---+-+ admin | no| yes | yes | no limit| {} postgres | yes | yes | yes | no limit| {} someotheruser | no| no | no| no limit| {} someuser | no| no | no| no limit| {} (4 rows) The way I see it, a perfectly clear, complete, and legible table has been turned into a denormalized, unreadable, and ugly pile of ASCII salad that moreover wastes valuable vertical screen space instead of using the abundant horizontal screen space. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 12:01 PM, Simon Riggs wrote: > On Wed, 2009-10-21 at 12:49 +0200, Magnus Hagander wrote: > >> PGOPTIONS is the way to do that, no? It can be a bit tricky when you >> have to deal with quoting, but it is there and it works... > > Which will work for application name also. In earlier discussion on this, we spoke about having some way of overriding hard-coded values in the client, such that psql could report it's application name, but that could be overridden by a value in the environment as in the examples I gave earlier. This is the exact opposite behaviour of the existing mechanism. The solution implemented was to add 2 connection string options, one of which libpq would only use if the other wasn't set, thus allowing you to hardcode 'fallback_application_name=psql', which could be overriden with PGAPPLICATIONNAME. Overloading this feature on PGOPTIONS not only makes that desired behaviour impossible (at least without propagating the notion of fallback_application_name to the backend), but also potentially makes configuration/scripting more tricky for the user when that variable now need to be reset as otherwise unrelated options are changed. Besides, I really don't see the problem anyway. The patch is already written - I'd rather work out the kink (for which we already have three possible solutions) than rip out 50% of the functionality. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, 2009-10-21 at 12:49 +0200, Magnus Hagander wrote: > PGOPTIONS is the way to do that, no? It can be a bit tricky when you > have to deal with quoting, but it is there and it works... Which will work for application name also. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 12:45, Simon Riggs wrote: > On Wed, 2009-10-21 at 11:20 +0100, Dave Page wrote: >> On Wed, Oct 21, 2009 at 11:06 AM, Simon Riggs wrote: >> >> > The SET seems sufficient for me. All interfaces currently support it. >> >> SET alone will not allow what I see as one of the most useful uses of >> this - consider: >> >> PGAPPLICATIONNAME="Nightly backup" pg_dump mydb >> PGAPPLICATIONNAME="Sensor data import" psql < data.log > > This highlights a different issue. If you wish to pass arbitrary SET > parameter(s) to a client then it is difficult to do so. We would be > better off solving that generic problem than solving your specific one. > > Consider > > PGDEADLOCKTIMEOUT=1 pg_dump mydb > PGWORKMEM=32657 psql < data.log > > Same requirement as setting the appname. Specific code for each > parameter is the wrong way to do that. PGOPTIONS is the way to do that, no? It can be a bit tricky when you have to deal with quoting, but it is there and it works... >> Also, adding something to libpq means we have to alter all the clients >> > and that means they become incompatible with earlier versions. What >> > advantage comes from doing all of that work? Nothing even close to large >> > enough to warrant the pain and delay, AFAICS. >> >> I must be missing something - why do we have to alter the clients? As >> it stands, they can use SET with whatever libpq they currently have, >> however if they wish to use the environment or connection string >> they'll need to update to the new libpq version. Those apps that don't >> care won't be affected because the libpq API hasn't changed in any way >> that isn't fully backwards compatible. > > If they can use SET, why are we changing libpq? If we are changing > libpq, why would we ignore those changes in the clients? (We don't need > to change clients, but most clients expose some language specific > feature to set things rather than just ignore them and let them be set > via libpq). The idea is to provide a better default than an empty string, I think. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 11:49, Dave Page wrote: >> Another idea is to do something similar to the 'prefer' SSL mode, or if >> the server doesn't support protocol version 3: Try with the GUC in >> startup packet first, and if that fails, retry without it. >> >> I'm not sure if I like either of those better than the extra SET >> command, but I thought I'd mention it. > > The command line sure seems ugly if that's what you meant. Retrying > doesn't seem so bad, though it'll still litter server logs with > connection errors. It'd definitely be good if we can avoid littering the server logs with it. It's bad enough in some SSL situations already :( -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, 2009-10-21 at 11:20 +0100, Dave Page wrote: > On Wed, Oct 21, 2009 at 11:06 AM, Simon Riggs wrote: > > > The SET seems sufficient for me. All interfaces currently support it. > > SET alone will not allow what I see as one of the most useful uses of > this - consider: > > PGAPPLICATIONNAME="Nightly backup" pg_dump mydb > PGAPPLICATIONNAME="Sensor data import" psql < data.log This highlights a different issue. If you wish to pass arbitrary SET parameter(s) to a client then it is difficult to do so. We would be better off solving that generic problem than solving your specific one. Consider PGDEADLOCKTIMEOUT=1 pg_dump mydb PGWORKMEM=32657 psql < data.log Same requirement as setting the appname. Specific code for each parameter is the wrong way to do that. > Also, adding something to libpq means we have to alter all the clients > > and that means they become incompatible with earlier versions. What > > advantage comes from doing all of that work? Nothing even close to large > > enough to warrant the pain and delay, AFAICS. > > I must be missing something - why do we have to alter the clients? As > it stands, they can use SET with whatever libpq they currently have, > however if they wish to use the environment or connection string > they'll need to update to the new libpq version. Those apps that don't > care won't be affected because the libpq API hasn't changed in any way > that isn't fully backwards compatible. If they can use SET, why are we changing libpq? If we are changing libpq, why would we ignore those changes in the clients? (We don't need to change clients, but most clients expose some language specific feature to set things rather than just ignore them and let them be set via libpq). -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
Dave Page wrote: > On Wed, Oct 21, 2009 at 10:14 AM, Heikki Linnakangas > wrote: > >> Looking at the way we process the startup packet in >> ProcessStartupPacket, there's one dirty hack you could do. As the code >> stands, if you specify "options" key/value pair more than once, the >> latter value overrides the first one. If we change that in PG 8.5 so >> that the values are concatenated instead, you can put app name into the >> first "options" line, and other options (or an empty string) into the >> second. Pre-8.5 servers will silently ignore the first line. > > Not sure I follow - are you suggesting I set the appname via the > backend command line options? Yep! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 11:06 AM, Simon Riggs wrote: > The SET seems sufficient for me. All interfaces currently support it. SET alone will not allow what I see as one of the most useful uses of this - consider: PGAPPLICATIONNAME="Nightly backup" pg_dump mydb PGAPPLICATIONNAME="Sensor data import" psql < data.log > The reason we want this is because the app name is not available at > connection time. It will only ever be NULL in the connection log > messages in the cases where this is most needed. In other cases, the app > name is not uniquely associated with a session and can change over time. > What value is there in knowing the app name at connection? Could we not > just log the app name iff log_connections is enabled? It allows you to identify what application or script a connection came from. Sure, you can probably figure that out regardless (from PID and later log messages for example), but it's convenient and avoids the need to correlate multiple log entries to one another. > Also, adding something to libpq means we have to alter all the clients > and that means they become incompatible with earlier versions. What > advantage comes from doing all of that work? Nothing even close to large > enough to warrant the pain and delay, AFAICS. I must be missing something - why do we have to alter the clients? As it stands, they can use SET with whatever libpq they currently have, however if they wish to use the environment or connection string they'll need to update to the new libpq version. Those apps that don't care won't be affected because the libpq API hasn't changed in any way that isn't fully backwards compatible. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, 2009-10-21 at 10:45 +0100, Dave Page wrote: > On Wed, Oct 21, 2009 at 10:40 AM, Simon Riggs wrote: > > On Wed, 2009-10-21 at 10:19 +0100, Dave Page wrote: > >> On Wed, Oct 21, 2009 at 10:14 AM, Simon Riggs > >> wrote: > >> > >> > ISTM much of the complexity discussed relates to this second feature. > >> > Let's just concentrate on getting the connection-pool-identification > >> > aspect of this done right and then maybe add second part later. > >> > >> That side of the patch works fine already (it's only a GUC after all), > >> as does the during-connection option. The current problem is that old > >> servers will barf if it's set in the startup packet. > >> > >> And yes, the stats part also works :-). > > > > Then I say this feature is good to go. It's much needed. Well done. > > Thanks. > > > Strip out the during-connection option for now. Let the startup packet > > stuff come later, if ever. > > Either way there's a little work to do - if we forget about the > startup packet (which I'd rather not do - I can see the value in > having the app name in the connection log messages), I'll still need > to tweak things such that libpq can set the GUC post-connection, > either using the code Tom mentioned yesterday, or just issuing an > explicit SET. The SET seems sufficient for me. All interfaces currently support it. The reason we want this is because the app name is not available at connection time. It will only ever be NULL in the connection log messages in the cases where this is most needed. In other cases, the app name is not uniquely associated with a session and can change over time. What value is there in knowing the app name at connection? Could we not just log the app name iff log_connections is enabled? Also, adding something to libpq means we have to alter all the clients and that means they become incompatible with earlier versions. What advantage comes from doing all of that work? Nothing even close to large enough to warrant the pain and delay, AFAICS. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] UTF8 with BOM support in psql
On Wed, 2009-10-21 at 13:11 +0900, Itagaki Takahiro wrote: > The attached patch replace BOM with while spaces, but it does not > change client encoding automatically. I think we can always ignore > client encoding at the replacement because SQL command cannot start > with BOM sequence. If we don't ignore the sequence, execution of > the script must fail with syntax error. I feel that psql is the wrong place to fix this. BOMs in UTF-8 should be ignored everywhere, all the time. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Could regexp_matches be immutable?
On Tue, 2009-10-20 at 20:48 -0400, Tom Lane wrote: > So I went to see about making the changes to remove regex_flavor, and > was astonished to find that all the regex-related functions are already > marked immutable, and AFAICS always have been. This is clearly wrong, > and we would have to fix it if we weren't about to remove the GUC. > (In principle we should advise people to change the markings in existing > databases, but given the lack of complaints it's probably not worth the > trouble --- I doubt many applications change regex_flavor on the fly.) Are you sure this wasn't intentional, because it breaks performance and we doubted that many applications would change regex_flavor on the fly? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 10:14 AM, Heikki Linnakangas wrote: > Looking at the way we process the startup packet in > ProcessStartupPacket, there's one dirty hack you could do. As the code > stands, if you specify "options" key/value pair more than once, the > latter value overrides the first one. If we change that in PG 8.5 so > that the values are concatenated instead, you can put app name into the > first "options" line, and other options (or an empty string) into the > second. Pre-8.5 servers will silently ignore the first line. Not sure I follow - are you suggesting I set the appname via the backend command line options? Currently I just send the "application_name" as an explicit key/value pair. > Another idea is to do something similar to the 'prefer' SSL mode, or if > the server doesn't support protocol version 3: Try with the GUC in > startup packet first, and if that fails, retry without it. > > I'm not sure if I like either of those better than the extra SET > command, but I thought I'd mention it. The command line sure seems ugly if that's what you meant. Retrying doesn't seem so bad, though it'll still litter server logs with connection errors. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 10:40 AM, Simon Riggs wrote: > On Wed, 2009-10-21 at 10:19 +0100, Dave Page wrote: >> On Wed, Oct 21, 2009 at 10:14 AM, Simon Riggs wrote: >> >> > ISTM much of the complexity discussed relates to this second feature. >> > Let's just concentrate on getting the connection-pool-identification >> > aspect of this done right and then maybe add second part later. >> >> That side of the patch works fine already (it's only a GUC after all), >> as does the during-connection option. The current problem is that old >> servers will barf if it's set in the startup packet. >> >> And yes, the stats part also works :-). > > Then I say this feature is good to go. It's much needed. Well done. Thanks. > Strip out the during-connection option for now. Let the startup packet > stuff come later, if ever. Either way there's a little work to do - if we forget about the startup packet (which I'd rather not do - I can see the value in having the app name in the connection log messages), I'll still need to tweak things such that libpq can set the GUC post-connection, either using the code Tom mentioned yesterday, or just issuing an explicit SET. I don't see that being a problem though. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, 2009-10-21 at 10:19 +0100, Dave Page wrote: > On Wed, Oct 21, 2009 at 10:14 AM, Simon Riggs wrote: > > > ISTM much of the complexity discussed relates to this second feature. > > Let's just concentrate on getting the connection-pool-identification > > aspect of this done right and then maybe add second part later. > > That side of the patch works fine already (it's only a GUC after all), > as does the during-connection option. The current problem is that old > servers will barf if it's set in the startup packet. > > And yes, the stats part also works :-). Then I say this feature is good to go. It's much needed. Well done. Strip out the during-connection option for now. Let the startup packet stuff come later, if ever. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, Oct 21, 2009 at 10:14 AM, Simon Riggs wrote: > ISTM much of the complexity discussed relates to this second feature. > Let's just concentrate on getting the connection-pool-identification > aspect of this done right and then maybe add second part later. That side of the patch works fine already (it's only a GUC after all), as does the during-connection option. The current problem is that old servers will barf if it's set in the startup packet. And yes, the stats part also works :-). -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Wed, 2009-10-14 at 18:44 +0200, Magnus Hagander wrote: > On Wed, Oct 14, 2009 at 18:39, Dave Page wrote: > > On Wed, Oct 14, 2009 at 5:37 PM, Eric B. Ridge wrote: > >> On Oct 13, 2009, at 11:02 AM, Dave Page wrote: > >> > >>> A useful feature found in other DBMSs such as MS SQL Server that has > >>> been requested on these lists a few times, is the ability for a client > >>> application to report its name to the server. > >> > >> I've been following this thread closely and haven't seen mention of > >> including the setting as part of the process name, so a 'ps' (on Unix) > >> would > >> display it. Thoughts? > > > > Isn't that cluttered enough already? > > +1 for pg_stat_activity but not 'os' output. +1 to output application name in pg_stat_activity and also accessible to pg_stat_statements. We want to be able to set the application name *after* connection, so that session pool users can be more easily identified. Note that this *must* be set *after* connection, not during connection. So for me, the ability to set the application name at connection time is a related but separate feature - and a much less valuable one at that. ISTM much of the complexity discussed relates to this second feature. Let's just concentrate on getting the connection-pool-identification aspect of this done right and then maybe add second part later. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
Dave Page wrote: > On Tue, Oct 20, 2009 at 8:55 PM, Tom Lane wrote: >> Dave Page writes: >>> I just realised there's a nasty problem with this. In my client >>> application, I can use PQconninfoParse to determine if >>> application_name (or fallback_application_name) are valid connection >>> string options for the version of libpq that I have. >>> However, there is no way that I can see of doing a comparable test in >>> libpq itself, to ensure that the server understands the parameter, so >>> if I connect to an 8.5 server, everything works fine, but if connect >>> to an older server with my new libpq, the connection fails because of >>> the unknown parameter in the startup packet. >> Hmm, yeah, that's a good point. It seems like you will have to send the >> appname SET command as a separate packet, after you have gotten the >> initial response and know what version the server is. Kind of annoying >> from a performance standpoint, but I believe it won't be too hard to >> shoehorn into libpq --- it already has a code path for that for older >> servers, IIRC. > > Yeah - unfortunately that means the connection log messages won't be > able to include the appname (I had to tweak ProcessStatupPacket() to > deal with it earlier as it was). Looking at the way we process the startup packet in ProcessStartupPacket, there's one dirty hack you could do. As the code stands, if you specify "options" key/value pair more than once, the latter value overrides the first one. If we change that in PG 8.5 so that the values are concatenated instead, you can put app name into the first "options" line, and other options (or an empty string) into the second. Pre-8.5 servers will silently ignore the first line. Another idea is to do something similar to the 'prefer' SSL mode, or if the server doesn't support protocol version 3: Try with the GUC in startup packet first, and if that fails, retry without it. I'm not sure if I like either of those better than the extra SET command, but I thought I'd mention it. > Should we perhaps change the behaviour of the backend to give a > warning only for unknown settings in the startup packet? It doesn't > seem beyond the realms of possibility that we might want to add > something else in the future, and this will at least mean that in a > few releases time it might be reasonably safe to do so. Yeah, sounds like a good idea. But what if there's options that we would *want* to throw an error on if the server doesn't understand them? It would be better if the client could explicitly mark optional arguments somehow. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Client application name
On Tue, Oct 20, 2009 at 8:55 PM, Tom Lane wrote: > Dave Page writes: >> I just realised there's a nasty problem with this. In my client >> application, I can use PQconninfoParse to determine if >> application_name (or fallback_application_name) are valid connection >> string options for the version of libpq that I have. > >> However, there is no way that I can see of doing a comparable test in >> libpq itself, to ensure that the server understands the parameter, so >> if I connect to an 8.5 server, everything works fine, but if connect >> to an older server with my new libpq, the connection fails because of >> the unknown parameter in the startup packet. > > Hmm, yeah, that's a good point. It seems like you will have to send the > appname SET command as a separate packet, after you have gotten the > initial response and know what version the server is. Kind of annoying > from a performance standpoint, but I believe it won't be too hard to > shoehorn into libpq --- it already has a code path for that for older > servers, IIRC. Yeah - unfortunately that means the connection log messages won't be able to include the appname (I had to tweak ProcessStatupPacket() to deal with it earlier as it was). Should we perhaps change the behaviour of the backend to give a warning only for unknown settings in the startup packet? It doesn't seem beyond the realms of possibility that we might want to add something else in the future, and this will at least mean that in a few releases time it might be reasonably safe to do so. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com PGDay.EU 2009 Conference: http://2009.pgday.eu/start -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers