Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Wed, Apr 16, 2014 at 11:01:56PM -0400, Bruce Momjian wrote: On Tue, Apr 1, 2014 at 10:32:01AM -0400, Tom Lane wrote: Adrian Vondendriesch adrian.vondendrie...@credativ.de writes: I patched the function conninfo_array_parse() which is used by PQconnectStartParams to behave like PQsetdbLogin. The patch also contains a document patch which clarify unspecified parameters. I see no documentation update here. I'm also fairly concerned about the implication that no connection parameter, now or in future, can ever have an empty string as the correct value. I thought about this. We have never needed PQsetdbLogin() to handle zero-length strings specially in all the years we used it, so I am not sure why we would ever need PQconnectdbParams() to handle it. I am thinking we should make PQconnectdbParams() handle zero-length strings the same as NULL, and document it. Attached is a slightly-modified version of Adrian Vondendriesch's patch. Patch applied. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Tue, Apr 1, 2014 at 05:06:08PM +0200, Adrian Vondendriesch wrote: Am 01.04.2014 16:32, schrieb Tom Lane: Adrian Vondendriesch adrian.vondendrie...@credativ.de writes: I patched the function conninfo_array_parse() which is used by PQconnectStartParams to behave like PQsetdbLogin. The patch also contains a document patch which clarify unspecified parameters. I see no documentation update here. I'm also fairly concerned about the implication that no connection parameter, now or in future, can ever have an empty string as the correct value. If we want to preserve the possibility to except an empty string as correct value, then pgbench should initialise some variables with NULL instead of empty string. Moreover it should be documented that unspecified means NULL and not empty string, like in PQsetdbLogin. However, attached you will find the whole patch, including documentation. Where do we want to go with this? Right now, PQsetdbLogin() and PQconnectdbParams() handle zero-length strings differently. PQsetdbLogin() treats it as unspecified, while PQconnectdbParams() does not, and our documentation on PQconnectdbParams() is unclear on this. It seems we can either change pgbench or libpq, and we should document libpq in either case. Also, the second sentence seems wrong: The passed arrays can be empty to use all default parameters, or can contain one or more parameter settings. They should be matched in length. Processing will stop with the last non-symbolNULL/symbol element of the literalkeywords/literal array. Doesn't it stop with first NULL value? For example, if the array is a, NULL, b, NULL, it stops at the first NULL, not the second one. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Tue, Apr 1, 2014 at 10:32:01AM -0400, Tom Lane wrote:
Adrian Vondendriesch adrian.vondendrie...@credativ.de writes:
I patched the function conninfo_array_parse() which is used by
PQconnectStartParams to behave like PQsetdbLogin. The patch also
contains a document patch which clarify unspecified parameters.
I see no documentation update here. I'm also fairly concerned about the
implication that no connection parameter, now or in future, can ever have
an empty string as the correct value.
I thought about this. We have never needed PQsetdbLogin() to handle
zero-length strings specially in all the years we used it, so I am not
sure why we would ever need PQconnectdbParams() to handle it. I am
thinking we should make PQconnectdbParams() handle zero-length strings
the same as NULL, and document it.
Attached is a slightly-modified version of Adrian Vondendriesch's patch.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
new file mode 100644
index be0d602..1f0975a
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*** PGconn *PQconnectdbParams(const char * c
*** 131,145
para
The passed arrays can be empty to use all default parameters, or can
contain one or more parameter settings. They should be matched in length.
!Processing will stop with the last non-symbolNULL/symbol element
!of the literalkeywords/literal array.
/para
para
!If any parameter is unspecified, then the corresponding
!environment variable (see xref linkend=libpq-envars)
!is checked. If the environment variable is not set either,
!then the indicated built-in defaults are used.
/para
para
--- 131,145
para
The passed arrays can be empty to use all default parameters, or can
contain one or more parameter settings. They should be matched in length.
!Processing will stop at the first symbolNULL/symbol element
!in the literalkeywords/literal array.
/para
para
!If any parameter is NULL or an emptry string, the corresponding
!environment variable (see xref linkend=libpq-envars) is checked.
!If the environment variable is not set either, then the indicated
!built-in defaults are used.
/para
para
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index 10cc0e6..45df6ce
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*** conninfo_array_parse(const char *const *
*** 4352,4358
const char *pname = keywords[i];
const char *pvalue = values[i];
! if (pvalue != NULL)
{
/* Search for the param record */
for (option = options; option-keyword != NULL; option++)
--- 4352,4358
const char *pname = keywords[i];
const char *pvalue = values[i];
! if (pvalue != NULL pvalue[0] != '\0')
{
/* Search for the param record */
for (option = options; option-keyword != NULL; option++)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Hi,
Am Wed, 12 Feb 2014 13:47:41 -0500
schrieb Robert Haas robertmh...@gmail.com:
On Mon, Feb 10, 2014 at 12:14 PM, Jeff Janes jeff.ja...@gmail.com
wrote:
Presumably whatever behavior difference you are seeing is somehow
related to the use of PQconnectdbParams() rather than
PQsetdbLogin(), but the fine manual does not appear to document a
different between those functions as regards password-prompting
behavior or .pgpass usage.
It looks like PQsetdbLogin() has either NULL or empty string passed
to it match 5432 in pgpass, while PQconnectdbParams() only has NULL
match 5432 and empty string does not. pgbench uses empty string if
no -p is specified.
This make pgbench behave the way I think is correct, but it hardly
seems like the right way to fix it.
[ kludge ]i
Well, it seems to me that the threshold issue here is whether or not
we should try to change the behavior of libpq. If not, then your
kludge might be the best option. But if so then it isn't necessary.
However, I don't feel confident to pass judgement on the what the
libpq semantics should be.
I noticed that pgbnech doesn't use all variables from my PGSERVICE
definition. Then I came around and find this Thread.
export PGSERVICE=test_db_head
cat ~/.pg_service.conf
[test_db_head]
host=/tmp
user=avo
port=5496
dbname=pgbench
/usr/local/postgresql/head/bin/pgbench -s 1 -i
Connection to database failed:
could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket /tmp/.s.PGSQL.5432?
As you noticed before pgbench initialises some of its connection
parameters with empty string, this overwrites variables defined
in .pg_service.conf.
I patched the function conninfo_array_parse() which is used by
PQconnectStartParams to behave like PQsetdbLogin. The patch also
contains a document patch which clarify unspecified parameters.
Now, PQconnectStartParams will handle empty strings in exactly the same
way as it handles NULL and pgbench run as expected:
usr/local/postgresql/head/bin/pgbench -s 1 -i
NOTICE: table pgbench_history does not exist, skipping
NOTICE: table pgbench_tellers does not exist, skipping
NOTICE: table pgbench_accounts does not exist, skipping
NOTICE: table pgbench_branches does not exist, skipping
creating tables...
10 of 10 tuples (100%) done (elapsed 0.21 s, remaining 0.00
s). vacuum...
set primary keys...
done.
Kind Regards
- Adrian
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index d53c41f..253615e 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4321,7 +4321,7 @@ conninfo_array_parse(const char *const * keywords, const char *const * values,
const char *pname = keywords[i];
const char *pvalue = values[i];
- if (pvalue != NULL)
+ if (pvalue != NULL pvalue[0] != '\0')
{
/* Search for the param record */
for (option = options; option-keyword != NULL; option++)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Adrian Vondendriesch adrian.vondendrie...@credativ.de writes: I patched the function conninfo_array_parse() which is used by PQconnectStartParams to behave like PQsetdbLogin. The patch also contains a document patch which clarify unspecified parameters. I see no documentation update here. I'm also fairly concerned about the implication that no connection parameter, now or in future, can ever have an empty string as the correct value. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Am 01.04.2014 16:32, schrieb Tom Lane:
Adrian Vondendriesch adrian.vondendrie...@credativ.de writes:
I patched the function conninfo_array_parse() which is used by
PQconnectStartParams to behave like PQsetdbLogin. The patch also
contains a document patch which clarify unspecified parameters.
I see no documentation update here. I'm also fairly concerned about the
implication that no connection parameter, now or in future, can ever have
an empty string as the correct value.
If we want to preserve the possibility to except an empty string as
correct value, then pgbench should initialise some variables with
NULL instead of empty string.
Moreover it should be documented that unspecified means NULL and not
empty string, like in PQsetdbLogin.
However, attached you will find the whole patch, including documentation.
Kind Regards
- Adrian
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index be0d602..0bac166 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -136,10 +136,11 @@ PGconn *PQconnectdbParams(const char * const *keywords,
/para
para
- If any parameter is unspecified, then the corresponding
- environment variable (see xref linkend=libpq-envars)
- is checked. If the environment variable is not set either,
- then the indicated built-in defaults are used.
+ If any parameter is unspecified, then the corresponding environment
+ variable (see xref linkend=libpq-envars) is checked. Parameters are
+ treated as unspecified if they are either NULL or contain an empty string
+ (). If the environment variable is not set either, then the
+ indicated built-in defaults are used.
/para
para
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index d53c41f..253615e 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4321,7 +4321,7 @@ conninfo_array_parse(const char *const * keywords, const char *const * values,
const char *pname = keywords[i];
const char *pvalue = values[i];
- if (pvalue != NULL)
+ if (pvalue != NULL pvalue[0] != '\0')
{
/* Search for the param record */
for (option = options; option-keyword != NULL; option++)
signature.asc
Description: OpenPGP digital signature
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Mon, Feb 10, 2014 at 12:14 PM, Jeff Janes jeff.ja...@gmail.com wrote: Presumably whatever behavior difference you are seeing is somehow related to the use of PQconnectdbParams() rather than PQsetdbLogin(), but the fine manual does not appear to document a different between those functions as regards password-prompting behavior or .pgpass usage. It looks like PQsetdbLogin() has either NULL or empty string passed to it match 5432 in pgpass, while PQconnectdbParams() only has NULL match 5432 and empty string does not. pgbench uses empty string if no -p is specified. This make pgbench behave the way I think is correct, but it hardly seems like the right way to fix it. [ kludge ] Well, it seems to me that the threshold issue here is whether or not we should try to change the behavior of libpq. If not, then your kludge might be the best option. But if so then it isn't necessary. However, I don't feel confident to pass judgement on the what the libpq semantics should be. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Sun, Feb 9, 2014 at 4:56 PM, Robert Haas robertmh...@gmail.com wrote: On Sun, Feb 9, 2014 at 6:33 PM, Jeff Janes jeff.ja...@gmail.com wrote: Since this commit (17676c785a95b2598c573), pgbench no longer uses .pgpass to obtain passwords, but instead prompts for a password This problem is in 9.3 and 9.4dev According to strace, it is reading the .pgpass file, it just seem like it is not using it. Hmm. I tried pgbench with the following .pgpass file and it worked OK. Removing the file caused pgbench to prompt for a password. *:*:*:*:foo OK, that works for me. I had it completely specified. Playing with variations on this, I see that the key is pgport. Set to * it works, set to 5432 it prompts for the password. (If I specify -p 5432 to pgbench, that would work with the original file) Presumably whatever behavior difference you are seeing is somehow related to the use of PQconnectdbParams() rather than PQsetdbLogin(), but the fine manual does not appear to document a different between those functions as regards password-prompting behavior or .pgpass usage. It looks like PQsetdbLogin() has either NULL or empty string passed to it match 5432 in pgpass, while PQconnectdbParams() only has NULL match 5432 and empty string does not. pgbench uses empty string if no -p is specified. This make pgbench behave the way I think is correct, but it hardly seems like the right way to fix it. *** a/contrib/pgbench/pgbench.c --- b/contrib/pgbench/pgbench.c *** doConnect(void) *** 528,533 --- 528,535 new_pass = false; + if (values[1][0] == 0) values[1]=NULL; + conn = PQconnectdbParams(keywords, values, true); if (!conn) Cheers, Jeff
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Wed, Jul 4, 2012 at 12:41 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 3, 2012 at 11:36 PM, Amit Kapila amit.kap...@huawei.com wrote: Hi Shigeru/Robert, The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Shall I consider following 2 points to update the patch: 1. Apply changes similar to pgbench and oid2name for vacuumlo 2. Remove the modifications for dblink. I've done these two things and committed this. Along the way, I also fixed it to use a stack-allocated array instead of using malloc, since there's no need to malloc a fixed-size array with 7 elements. Thanks for the patch. Since this commit (17676c785a95b2598c573), pgbench no longer uses .pgpass to obtain passwords, but instead prompts for a password This problem is in 9.3 and 9.4dev According to strace, it is reading the .pgpass file, it just seem like it is not using it. Cheers, Jeff
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Sun, Feb 9, 2014 at 6:33 PM, Jeff Janes jeff.ja...@gmail.com wrote: On Wed, Jul 4, 2012 at 12:41 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jul 3, 2012 at 11:36 PM, Amit Kapila amit.kap...@huawei.com wrote: Hi Shigeru/Robert, The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Shall I consider following 2 points to update the patch: 1. Apply changes similar to pgbench and oid2name for vacuumlo 2. Remove the modifications for dblink. I've done these two things and committed this. Along the way, I also fixed it to use a stack-allocated array instead of using malloc, since there's no need to malloc a fixed-size array with 7 elements. Thanks for the patch. Since this commit (17676c785a95b2598c573), pgbench no longer uses .pgpass to obtain passwords, but instead prompts for a password This problem is in 9.3 and 9.4dev According to strace, it is reading the .pgpass file, it just seem like it is not using it. Hmm. I tried pgbench with the following .pgpass file and it worked OK. Removing the file caused pgbench to prompt for a password. *:*:*:*:foo Presumably whatever behavior difference you are seeing is somehow related to the use of PQconnectdbParams() rather than PQsetdbLogin(), but the fine manual does not appear to document a different between those functions as regards password-prompting behavior or .pgpass usage. My guess is that it's getting as far as PasswordFromFile() and then judging whatever entry you have in the file not to match the connection attempt. If you're correct about this commit being to blame, then my guess is that one of hostname, port, dbname, and username must end up initialized differently when PQconnectdbParams() rather than PQsetdbLogin() is used... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Tue, Jul 3, 2012 at 11:36 PM, Amit Kapila amit.kap...@huawei.com wrote: Hi Shigeru/Robert, The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Shall I consider following 2 points to update the patch: 1. Apply changes similar to pgbench and oid2name for vacuumlo 2. Remove the modifications for dblink. I've done these two things and committed this. Along the way, I also fixed it to use a stack-allocated array instead of using malloc, since there's no need to malloc a fixed-size array with 7 elements. Thanks for the patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On fre, 2012-06-08 at 17:14 +, Amit kapila wrote: This patch is to provide support for fallback application name for contrib/pgbench, oid2name, and dblink. vacuumlo should also be treated, I think. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
(2012/06/28 11:16), Robert Haas wrote: If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, and fallback_application_name is only going to apply in cases where the user doesn't care enough to bother setting application_name. Let's not knock ourselves out to solve a problem that may not be that important to begin with. Thanks for clarification. I got the point. The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Regards, -- Shigeru HANADA -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Hi Shigeru/Robert, -Original Message- From: Shigeru HANADA [mailto:shigeru.han...@gmail.com] Sent: Wednesday, July 04, 2012 6:57 AM (2012/06/28 11:16), Robert Haas wrote: If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, and fallback_application_name is only going to apply in cases where the user doesn't care enough to bother setting application_name. Let's not knock ourselves out to solve a problem that may not be that important to begin with. Thanks for clarification. I got the point. The way fixing oid2name and pgbench seems reasonable, so applying it to vacuumlo (as Peter mentioned) would be enough for this issue. Shall I consider following 2 points to update the patch: 1. Apply changes similar to pgbench and oid2name for vacuumlo 2. Remove the modifications for dblink. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Thursday, June 28, 2012 7:46 AM On Wed, Jun 27, 2012 at 9:57 PM, Shigeru HANADA shigeru.han...@gmail.com wrote: On Thu, Jun 14, 2012 at 10:55 PM, Robert Haas robertmh...@gmail.com wrote: Indeed reparsing connection string is not cheap, but dblink does it for checking password requirement for non-in dblink_connstr_check when the local user was not a superuser. So Amit's idea doesn't seem unreasonable to me, if we can avoid extra PQconninfoParse call. Just an idea, but how about pushing fallback_application_name handling into dblink_connstr_check? We reparse connection string unless local user was a superuser, so it would not be serious overhead in most cases. Although it might require changes in DBLINK_GET_CONN macro... If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, In some cases like when DBA does the monitoring in night or sometimes when he doesn't expect much load on database to do some maintenance tasks, it can be helpful for him to know if there is any application which he doesn't expect to be there. This can be one of the ways he can know the which applications are currently active. Users are not bothered to set application_name in general cases as it doesn't server any big purpose for them. I understand this is good to have feature, but if it doesn't cost much then according to me it can be done. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Thu, Jun 14, 2012 at 10:55 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Jun 13, 2012 at 12:07 AM, Amit Kapila amit.kap...@huawei.com wrote: To achieve the same in dblink, we need to parse the passed connection string and check if it contains fallback_application_name, if yes then its okay, otherwise we need to append fallback_application_name in connection string. That seems undesirable. I don't think this is important enough to be worth reparsing the connection string for. I'd just forget about doing it for dblink if there's no cheaper way. Indeed reparsing connection string is not cheap, but dblink does it for checking password requirement for non-in dblink_connstr_check when the local user was not a superuser. So Amit's idea doesn't seem unreasonable to me, if we can avoid extra PQconninfoParse call. Just an idea, but how about pushing fallback_application_name handling into dblink_connstr_check? We reparse connection string unless local user was a superuser, so it would not be serious overhead in most cases. Although it might require changes in DBLINK_GET_CONN macro... Regards, -- Shigeru Hanada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Wed, Jun 27, 2012 at 9:57 PM, Shigeru HANADA shigeru.han...@gmail.com wrote: On Thu, Jun 14, 2012 at 10:55 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Jun 13, 2012 at 12:07 AM, Amit Kapila amit.kap...@huawei.com wrote: To achieve the same in dblink, we need to parse the passed connection string and check if it contains fallback_application_name, if yes then its okay, otherwise we need to append fallback_application_name in connection string. That seems undesirable. I don't think this is important enough to be worth reparsing the connection string for. I'd just forget about doing it for dblink if there's no cheaper way. Indeed reparsing connection string is not cheap, but dblink does it for checking password requirement for non-in dblink_connstr_check when the local user was not a superuser. So Amit's idea doesn't seem unreasonable to me, if we can avoid extra PQconninfoParse call. Just an idea, but how about pushing fallback_application_name handling into dblink_connstr_check? We reparse connection string unless local user was a superuser, so it would not be serious overhead in most cases. Although it might require changes in DBLINK_GET_CONN macro... *shrug* If it can be done without costing anything meaningful, I don't object, but I would humbly suggest that this is not hugely important one way or the other. application_name is primarily a monitoring convenience, so it's not hugely important to have it set anyway, and fallback_application_name is only going to apply in cases where the user doesn't care enough to bother setting application_name. Let's not knock ourselves out to solve a problem that may not be that important to begin with. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Wed, Jun 13, 2012 at 12:07 AM, Amit Kapila amit.kap...@huawei.com wrote: Why not 'dblink'? We can do for dblink as well. I just wanted to check before implementing in dblink. I have checked the dblink_connect() function, it receives the connect string and used in most cases that string to call libpq connect which is different from pgbench and oid2name where connection parameters are formed in main function and then call libpq connect. To achieve the same in dblink, we need to parse the passed connection string and check if it contains fallback_application_name, if yes then its okay, otherwise we need to append fallback_application_name in connection string. That seems undesirable. I don't think this is important enough to be worth reparsing the connection string for. I'd just forget about doing it for dblink if there's no cheaper way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
That seems undesirable. I don't think this is important enough to be worth reparsing the connection string for. The connect string should not be long, so parsing is not a big cost performance wise. I have currently modified the code for dblink in the patch I have uploaded in CF. However as per your suggestion, I will remove it during handling of other Review comments for patch unless somebody asks to keep it. -Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Thursday, June 14, 2012 7:25 PM To: Amit Kapila Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink On Wed, Jun 13, 2012 at 12:07 AM, Amit Kapila amit.kap...@huawei.com wrote: Why not 'dblink'? We can do for dblink as well. I just wanted to check before implementing in dblink. I have checked the dblink_connect() function, it receives the connect string and used in most cases that string to call libpq connect which is different from pgbench and oid2name where connection parameters are formed in main function and then call libpq connect. To achieve the same in dblink, we need to parse the passed connection string and check if it contains fallback_application_name, if yes then its okay, otherwise we need to append fallback_application_name in connection string. That seems undesirable. I don't think this is important enough to be worth reparsing the connection string for. I'd just forget about doing it for dblink if there's no cheaper way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
I have created the patch by including fallback_application_name for dblink as well. In this I have used the name of fallback_application_name as dblink. Please let me know your suggestions regarding the same. -Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Wednesday, June 13, 2012 12:13 AM To: Amit Kapila Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink On Mon, Jun 11, 2012 at 5:30 AM, Amit Kapila amit.kap...@huawei.com wrote: As per the previous discussion in link below, it seems that fallback application name needs to be provided for only pgbench and oid2name. http://archives.postgresql.org/message-id/w2g9837222c1004070216u3bc46b3ahbdd fdffdbfb46...@mail.gmail.com However the title of Todo Item suggests it needs to be done for dblink as well. Please suggest if it needs to be done for dblink, if yes then what should be fallback_application_name for it? Why not 'dblink'? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company fallback_application_name.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
On Mon, Jun 11, 2012 at 5:30 AM, Amit Kapila amit.kap...@huawei.com wrote: As per the previous discussion in link below, it seems that fallback application name needs to be provided for only pgbench and oid2name. http://archives.postgresql.org/message-id/w2g9837222c1004070216u3bc46b3ahbddfdffdbfb46...@mail.gmail.com However the title of Todo Item suggests it needs to be done for dblink as well. Please suggest if it needs to be done for dblink, if yes then what should be fallback_application_name for it? Why not 'dblink'? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
Why not 'dblink'? We can do for dblink as well. I just wanted to check before implementing in dblink. I have checked the dblink_connect() function, it receives the connect string and used in most cases that string to call libpq connect which is different from pgbench and oid2name where connection parameters are formed in main function and then call libpq connect. To achieve the same in dblink, we need to parse the passed connection string and check if it contains fallback_application_name, if yes then its okay, otherwise we need to append fallback_application_name in connection string. The doubt I have is that what name to use as fallback_application_name because here we cannot have argv as in pgbench or oid2name? The 2 options which I can think of are: 1. Hard-coded name - dblink 2. postgres - which I think will be caller of dblink functionality Please suggest if any of this option is okay or what is other way to get the program name. -Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Wednesday, June 13, 2012 12:13 AM To: Amit Kapila Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink On Mon, Jun 11, 2012 at 5:30 AM, Amit Kapila amit.kap...@huawei.com wrote: As per the previous discussion in link below, it seems that fallback application name needs to be provided for only pgbench and oid2name. http://archives.postgresql.org/message-id/w2g9837222c1004070216u3bc46b3ahbdd fdffdbfb46...@mail.gmail.com However the title of Todo Item suggests it needs to be done for dblink as well. Please suggest if it needs to be done for dblink, if yes then what should be fallback_application_name for it? Why not 'dblink'? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
As per the previous discussion in link below, it seems that fallback application name needs to be provided for only pgbench and oid2name. http://archives.postgresql.org/message-id/w2g9837222c1004070216u3bc46b3ahbdd fdffdbfb46...@mail.gmail.com However the title of Todo Item suggests it needs to be done for dblink as well. Please suggest if it needs to be done for dblink, if yes then what should be fallback_application_name for it? From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit kapila Sent: Friday, June 08, 2012 10:45 PM To: pgsql-hackers@postgresql.org Subject: [HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink This patch is to provide support for fallback application name for contrib/pgbench, oid2name, and dblink. Currently I have done the implementation for pgbench. The implementation is same as in psql. Before creating a final patch, I wanted to check whether my direction for creating a patch is what is expected from this Todo item. I have done the basic testing for following 2 scenario's 1. After implementation, if during run of pgbench, we query pg_stat_activity it displays the application name as pgbench 2. It displays the application name in log file also. Suggestions?
[HACKERS] WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink
This patch is to provide support for fallback application name for contrib/pgbench, oid2name, and dblink. Currently I have done the implementation for pgbench. The implementation is same as in psql. Before creating a final patch, I wanted to check whether my direction for creating a patch is what is expected from this Todo item. I have done the basic testing for following 2 scenario's 1. After implementation, if during run of pgbench, we query pg_stat_activity it displays the application name as pgbench 2. It displays the application name in log file also. Suggestions? fallback_application_name.patch Description: fallback_application_name.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
