Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Hi, This is revised patch including document. I confused three identifiers to be compared, names in the catalog, those in pg_hba lines and those given from the client under connecting. This patch concerns the comparison between pg_hba and client names. Finally all the additional pg_strcasecmp() or whole catalog scanning are eliminated. This version works as following. Tokenize every hba tokens and categorize having two attributes, One is whether the case is preserved or not. Case of a word is preserved in the returned token if the word is enclosed with double quotes. Another is token type, Leading bare '+' indicates the token is a group name, and '@' indicates file inclusion. The string in returned token is stripped of the special characters. A double quoted region which does not begin at the beginning of the word was handled in its own way from before this change. I don't know it is right or not. (horiguti stored as horiguti by the orignal next_token() and it is not changed) Matching names are performed as following, Tokens corrensponding to keywords should be 'normal' ones (not a group name or file inclusion) and should not be case-preserved ones, which were enclosed by double quotes. The tokens are lowercased so token_is_keyword() macro compares them by strcmp(). Database name and user name should be 'normal' tokens and the cases of the names are preserved or not according to the notaion in hba line so token_matches() compares them with the name given from client by strcmp(). The patch size is far reduced from the previous version. At Wed, 10 Sep 2014 11:32:22 +0200, Florian Pflug f...@phlo.org wrote in 7d70ee06-1e80-44d6-9428-5f60ad796...@phlo.org So foo, Foo and FOO would all match the user called foo, but Foo would match the user called Foo, and FOO the user called FOO. This patch does so. An unquoted + would cause whatever follows it to be interpreted as a group name, whereas a quoted + would simply become part of the user name (or group name, if there's an additional unquoted + before it). So +foo would refer to the group foo, +FOO to the group FOO, and ++A to the group +A. I think this behaves so. I haven't checked if such an approach would be sufficiently backwards-compatible, though. One obveous breaking which affects the existing sane pg_hba.conf is that db and user names not surrounded by double quotes became to match the lowercased names, not the original name containing uppercase characters. But this is just what this patch intended. I think all behaviors for other cases appear in existing pg_hba.conf are unchanged including the behaviors for string consists of single character '+' or '@'. # '+' is treated as a group name '' and '@' is treated as a # user/db name '@' but they seems meanless.. Any suggestions? regards, -- Kyotaro Horiguchi NTT Open Source Software Center From b02cea3ead352a198f341c0f2a9f6ab93f439077 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi horiguchi.kyot...@lab.ntt.co.jp Date: Thu, 18 Sep 2014 17:06:21 +0900 Subject: [PATCH 2/2] Make pg_hba.conf case insensitive. --- src/backend/libpq/hba.c | 101 ++ 1 files changed, 83 insertions(+), 18 deletions(-) diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 84da823..e4b1635 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -60,9 +60,18 @@ typedef struct check_network_data bool result; /* set to true if match */ } check_network_data; +typedef enum TokenType +{ + NORMAL, + GROUP_NAME, /* this token had leading '+' */ + FILE_INCLUSION, /* this token had leading '@' */ +} TokenType; -#define token_is_keyword(t, k) (!t-quoted strcmp(t-string, k) == 0) -#define token_matches(t, k) (strcmp(t-string, k) == 0) +#define token_is_keyword(tk, kw) \ + ((tk)-type == NORMAL !(tk)-case_preserved \ + (strcmp((tk)-string, (kw)) == 0)) +#define token_matches(t, k) \ + ((t)-type == NORMAL (strcmp((t)-string, (k)) == 0)) /* * A single string token lexed from the HBA config file, together with whether @@ -71,7 +80,8 @@ typedef struct check_network_data typedef struct HbaToken { char *string; - bool quoted; + TokenType type; + bool case_preserved; } HbaToken; /* @@ -123,8 +133,14 @@ pg_isblank(const char c) * the first character. (We use that to prevent @x from being treated * as a file inclusion request. Note that @x should be so treated; * we want to allow that to support embedded spaces in file paths.) + * type is one of NORMAL, GROUP_NAME, FILE_INCLUSION. GROUP_NAME is set if the + * token is prefix by '+', FILE_INCLUSION if prefixed by '@', NORMAL + * otherwise. * We set *terminating_comma to indicate whether the token is terminated by a * comma (which is not returned.) + * case_preserved is set if the token's case of every character is preserved + * when the whole word is enclosed by double quotes. Elsewise
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Hi, At Thu, 11 Sep 2014 08:10:54 -0400, Robert Haas robertmh...@gmail.com wrote in ca+tgmoz9xinc_ca23-p1dmihmv0zhckef6_rv6v3s+oxrla...@mail.gmail.com On Wed, Sep 10, 2014 at 4:54 AM, Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp wrote: Finally I think that we need case-insensitive version of get_role_id and() get_database_id() to acoomplish this patch'es objective. (This runs full-scans on pg_database or pg_authid X() Any such thing is certainly grounds for rejecting the patch outright. It may be that pg_hba.conf should follow the same case-folding rules we use elsewhere, but it should not invent novel semantics, especially ones that make connecting to the database a far more expensive operation than it is today. No wonder. I wondered why such things are needed for this 'case-insensitive matcing'. I've misunderstood the meaning of 'case-insensitive'. There's no need to scanning catalogues for the 'case-insensitive' matching. Thank you for suggestion. - Non-quoted names are matched with the names in the catalog after lowercased. - Quoted names are matched as is. This is archieved by simply downcase the identifier if not case-insensitive notation, and remove case-insensitive version catalog stuff. I'll show you more reasonable version sooner. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On Wed, Sep 10, 2014 at 4:54 AM, Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp wrote: Finally I think that we need case-insensitive version of get_role_id and() get_database_id() to acoomplish this patch'es objective. (This runs full-scans on pg_database or pg_authid X() Any such thing is certainly grounds for rejecting the patch outright. It may be that pg_hba.conf should follow the same case-folding rules we use elsewhere, but it should not invent novel semantics, especially ones that make connecting to the database a far more expensive operation than it is today. -- 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] [TODO] Process pg_hba.conf keywords as case-insensitive
Hello, I had a closer look on this patch. Finally I think that we need case-insensitive version of get_role_id and() get_database_id() to acoomplish this patch'es objective. (This runs full-scans on pg_database or pg_authid X() And I'd like to propose to change token categorization from notation-base to how-to-treat base. Concretely this patch categorizes tokens using 'special quote is used' and 'quote from the first' but it seems making logics clearer to categorize them using 'case sensive or not' and 'it represents group name'. The attached patch is a revised version of your original patch regarding to the above point. (Sorry in advance that this is a quick hack, especially the code related to file-inclusion is not tested at all) I have tested this only superficial level but it seems works as expected. Under the new specifications, next_token will work as following, - USER : token: USER , case-insensitive - USeR: token: USeR , case-SENSITIVE - +uSeR : token: +uSeR , case-SENSITIVE - +UsER : token: +UsEr , case-insensitive - USeR : token: USeR , case-insensitive - +USER : token: USER , case-insensitive, group_name - +uSeR : token: uSeR , case_SENSITIVE, group_name - +UsEr : token: UsEr , case-insensitive, group_name - + : token: + , (useless?) - @ : token: @ , (useless?) - @hoge: token: hoge, file_inclusion (not confirmed) There's a concern that Case-insensitive matching is accomplished by full-scan on pg_database or pg_authid so it would be rather slow than case-sensitive matching. This might not be acceptable by the community. And one known defect is that you will get a bit odd message if you put an hba line having keywords quoted or prefixed with '+', for example +locAl postgres +sUstRust The server complains for the line above that *| LOG: invalid connection type locAl | CONTEXT: line 84 of configuration file /home/horiguti/data/data_work/pg_hba.conf The prefixing '+' is omitted. To correct this, either deparsing token into original string or storing original string into tokens is needed, I think. What do you think about the changes, Viswanatham or all ? regards, -- Kyotaro Horiguchi NTT Open Source Software Center diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index f480be8..db73dd9 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -1991,6 +1991,50 @@ get_database_oid(const char *dbname, bool missing_ok) return oid; } +/* + * get_database_oid - given a database name, look up the OID in + * case-insensitive manner. + * + * If missing_ok is false, throw an error if database name not found. If + * true, just return InvalidOid. + */ +Oid +get_database_oid_case_insensitive(const char *dbname, bool missing_ok) +{ + Relation relation; + SysScanDesc scandesc; + HeapTuple tuple; + Oid oid = InvalidOid; + + /* + * SysCache has no abirility to case insensitive match, so we have no + * means except scanning whole the systable. + */ + relation = heap_open(DatabaseRelationId, AccessShareLock); + + scandesc = systable_beginscan(relation, InvalidOid, false, + NULL, 0, NULL); + while (HeapTupleIsValid(tuple = systable_getnext(scandesc))) + { + Form_pg_database dbForm = (Form_pg_database) GETSTRUCT(tuple); + + if (pg_strcasecmp(dbname, dbForm-datname.data) == 0) + { + oid = HeapTupleGetOid(tuple); + break; + } + } + systable_endscan(scandesc); + heap_close(relation, AccessShareLock); + + if (!OidIsValid(oid) !missing_ok) + ereport(ERROR, +(errcode(ERRCODE_UNDEFINED_DATABASE), + errmsg(database \%s\ does not exist, + dbname))); + + return oid; +} /* * get_database_name - given a database OID, look up the name diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 84da823..2d3a059 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -60,9 +60,20 @@ typedef struct check_network_data bool result; /* set to true if match */ } check_network_data; - -#define token_is_keyword(t, k) (!t-quoted strcmp(t-string, k) == 0) -#define token_matches(t, k) (strcmp(t-string, k) == 0) +typedef enum TokenType +{ + NORMAL, + GROUP_NAME, /* this token had leading '+' */ + FILE_INCLUSION, /* this token had leading '@' */ +} TokenType; + +#define token_is_keyword(tk, kw) \ + ((tk)-type != NORMAL || (tk)-case_sensitive ? false : \ + (pg_strcasecmp((tk)-string, (kw)) == 0)) +#define token_matches(t, k) \ + ((t)-type != NORMAL ? false :\ + ((t)-case_sensitive ? (strcmp((t)-string, (k)) == 0): \ + (pg_strcasecmp((t)-string, (k)) == 0))) /* * A single string token lexed from the HBA config file, together with whether @@ -71,7 +82,8 @@ typedef struct check_network_data typedef struct HbaToken { char *string; - bool quoted; + TokenType type; + bool case_sensitive; } HbaToken; /* @@ -111,6 +123,7 @@ pg_isblank(const char
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Hmm... case-insensitive mathing could get multiple matches, which should be an error but I've forgot to do so. regards, 2014/09/10 17:54 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp: And one known defect is that you will get a bit odd message if you put an hba line having keywords quoted or prefixed with '+', for example +locAl postgres +sUstRust The server complains for the line above that *| LOG: invalid connection type locAl | CONTEXT: line 84 of configuration file /home/horiguti/data/data_work/pg_hba.conf The prefixing '+' is omitted. To correct this, either deparsing token into original string or storing original string into tokens is needed, I think. What do you think about the changes, Viswanatham or all ? -- Kyotaro Horiguchi NTT Open Source Software Center
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On Sep10, 2014, at 10:54 , Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp wrote: Under the new specifications, next_token will work as following, - USER : token: USER , case-insensitive - USeR: token: USeR , case-SENSITIVE - +uSeR : token: +uSeR , case-SENSITIVE - +UsER : token: +UsEr , case-insensitive - USeR : token: USeR , case-insensitive - +USER : token: USER , case-insensitive, group_name - +uSeR : token: uSeR , case_SENSITIVE, group_name - +UsEr : token: UsEr , case-insensitive, group_name - + : token: + , (useless?) - @ : token: @ , (useless?) - @hoge: token: hoge, file_inclusion (not confirmed) There's a concern that Case-insensitive matching is accomplished by full-scan on pg_database or pg_authid so it would be rather slow than case-sensitive matching. This might not be acceptable by the community. That does indeed sound bad. Couldn't we handle this the same way we handle SQL identifiers, i.e. simply downcase unquoted identifiers, and then compare case-sensitively? So foo, Foo and FOO would all match the user called foo, but Foo would match the user called Foo, and FOO the user called FOO. An unquoted + would cause whatever follows it to be interpreted as a group name, whereas a quoted + would simply become part of the user name (or group name, if there's an additional unquoted + before it). So +foo would refer to the group foo, +FOO to the group FOO, and ++A to the group +A. I haven't checked if such an approach would be sufficiently backwards-compatible, though. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Hello, I will be the reviewer of this patch. You approach that coloring tokens seems right, but you have broken the parse logic by adding your code. Other than the mistakes others pointed, I found that - non-SQL-ident like tokens are ignored by their token style, quoted or not, so the following line works. | local All aLL trust I suppose this is not what you intended. This is because you have igonred the attribute of a token when comparing it as non-SQL-ident tokens. - '+' at the head of the sequence '+' is treated as the first character of the *quoted* string. e.g. +hoge is tokenized as +hoge:special_quoted. This is why you simply continued processing for '+' without discarding and skipping the '+', and not setting in_quote so the following parser code works as it is not intended. You should understand what the original code does and insert or modify logics not braeking the assumptions. With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. Actually it should have matched neither, as both lines will get folded downcase: local mixeddb all trust local mixeddb all reject regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Sorry for wrong suggestion. Setting in_quote is wrong there because it's before the beginning quote. Although, advancing read pointer and replacing c with the next value is still needed. regards, -- Kyotaro Horiguchi NTT Open Source Software Center 2014/09/09 20:49 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp: Hello, I will be the reviewer of this patch. You approach that coloring tokens seems right, but you have broken the parse logic by adding your code. Other than the mistakes others pointed, I found that - non-SQL-ident like tokens are ignored by their token style, quoted or not, so the following line works. | local All aLL trust I suppose this is not what you intended. This is because you have igonred the attribute of a token when comparing it as non-SQL-ident tokens. - '+' at the head of the sequence '+' is treated as the first character of the *quoted* string. e.g. +hoge is tokenized as +hoge:special_quoted. This is why you simply continued processing for '+' without discarding and skipping the '+', and not setting in_quote so the following parser code works as it is not intended. You should understand what the original code does and insert or modify logics not braeking the assumptions. With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. Actually it should have matched neither, as both lines will get folded downcase: local mixeddb all trust local mixeddb all reject regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On 07/23/2014 09:14 AM, Viswanatham kirankumar wrote: On 16 July 2014 23:12, Tom Lane wrote Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. Re-reading the original thread, there was also concern about whether we should try to make quoting/casefolding behave more like it does in SQL, specifically for matching pg_hba.conf items to SQL identifiers (database and role names). This patch doesn't seem to have addressed that part of it, but I think we need to think those things through before we just do a blind s/strcmp/pg_strcasecmp/g. Otherwise we might find that we've added ambiguity that will give us trouble when we do try to fix that. I had updated as per you review comments 1) database and role names behave similar to SQL identifiers (case-sensitive / case-folding). 2) users and user-groups only requires special handling and behavior as follows Normal user : A. unquoted ( USER ) will be treated as user ( downcase ). B. quoted ( USeR ) will be treated as USeR (case-sensitive). C. quoted ( +USER ) will be treated as normal user +USER (i.e. will not be considered as user-group) and case-sensitive as string is quoted. User Group : A. unquoted ( +USERGROUP ) will be treated as +usergruop ( downcase ). B. plus quoted ( +UserGROUP ) will be treated as +UserGROUP (case-sensitive). 3) Host name is not a SQL object so it will be treated as case-sensitive except for all, samehost, samenet are considered as keywords. For these user need to use quotes to differentiate between hostname and keywords. 4) All the fixed keywords mention in pg_hba.conf and Client Authentication section will be considered as keywords Eg: host, local, hostssl etc.. With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. PS. Please update the docs. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Re: Heikki Linnakangas 2014-08-21 53f5a2d6.2050...@vmware.com 1) database and role names behave similar to SQL identifiers (case-sensitive / case-folding). 2) users and user-groups only requires special handling and behavior as follows Normal user : A. unquoted ( USER ) will be treated as user ( downcase ). B. quoted ( USeR ) will be treated as USeR (case-sensitive). With this patch, database (and role?) names are compared case-insensitively. For example: local MixedDB all trust local mixedDB all reject psql -d mixedDB psql (9.5devel) Type help for help. mixedDB=# That connection should've matched that 2nd line, and be rejected. Actually it should have matched neither, as both lines will get folded downcase: local mixeddb all trust local mixeddb all reject Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On 16 July 2014 23:12, Tom Lane wrote Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. Re-reading the original thread, there was also concern about whether we should try to make quoting/casefolding behave more like it does in SQL, specifically for matching pg_hba.conf items to SQL identifiers (database and role names). This patch doesn't seem to have addressed that part of it, but I think we need to think those things through before we just do a blind s/strcmp/pg_strcasecmp/g. Otherwise we might find that we've added ambiguity that will give us trouble when we do try to fix that. I had updated as per you review comments 1) database and role names behave similar to SQL identifiers (case-sensitive / case-folding). 2) users and user-groups only requires special handling and behavior as follows Normal user : A. unquoted ( USER ) will be treated as user ( downcase ). B. quoted ( USeR ) will be treated as USeR (case-sensitive). C. quoted ( +USER ) will be treated as normal user +USER (i.e. will not be considered as user-group) and case-sensitive as string is quoted. User Group : A. unquoted ( +USERGROUP ) will be treated as +usergruop ( downcase ). B. plus quoted ( +UserGROUP ) will be treated as +UserGROUP (case-sensitive). 3) Host name is not a SQL object so it will be treated as case-sensitive except for all, samehost, samenet are considered as keywords. For these user need to use quotes to differentiate between hostname and keywords. 4) All the fixed keywords mention in pg_hba.conf and Client Authentication section will be considered as keywords Eg: host, local, hostssl etc.. Thanks Regards, VISWANATHAM KIRAN KUMAR HUAWEI TECHNOLOGIES INDIA PVT. LTD. pg_hba.conf_keywords_as_case-insensitive_v2.patch Description: pg_hba.conf_keywords_as_case-insensitive_v2.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Re: Viswanatham kirankumar 2014-07-23 ec867def52699d4189b584a14baa7c2165442...@blreml504-mbx.china.huawei.com 3) Host name is not a SQL object so it will be treated as case-sensitive except for all, samehost, samenet are considered as keywords. For these user need to use quotes to differentiate between hostname and keywords. DNS is case-insensitive, though most of the time case-preserving (nothing guarantees that it won't down-up-whatever-case the answer you get). (FTR, I'll retract my original complaint, the idea of using SQL-like case folding is nice.) Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Re: Tom Lane 2014-07-16 30956.1405532...@sss.pgh.pa.us Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. One place that's been bugging me where case-insensitivity would really make sense is this: # set work_mem = '1mb'; ERROR: 22023: invalid value for parameter work_mem: 1mb HINT: Valid units for this parameter are kB, MB, and GB. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Christoph Berg c...@df7cb.de writes: One place that's been bugging me where case-insensitivity would really make sense is this: # set work_mem = '1mb'; ERROR: 22023: invalid value for parameter work_mem: 1mb HINT: Valid units for this parameter are kB, MB, and GB. Yeah ... there was some pedantry about how kB and KB mean different things. IMO that's mere pedantry, but ... 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] [TODO] Process pg_hba.conf keywords as case-insensitive
On 18/07/14 04:08, Tom Lane wrote: Christoph Berg c...@df7cb.de writes: One place that's been bugging me where case-insensitivity would really make sense is this: # set work_mem = '1mb'; ERROR: 22023: invalid value for parameter work_mem: 1mb HINT: Valid units for this parameter are kB, MB, and GB. Yeah ... there was some pedantry about how kB and KB mean different things. IMO that's mere pedantry, but ... regards, tom lane But kb kB do mean different things: kilobits vs kilobytes! :-) (Network throughput seems to be always in bits per second - my broadband download is quoted at 100Mb/s, whereas I get 12MB/s download at best.) Cheers, Gavin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On 07/17/2014 01:41 AM, Tom Lane wrote: Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. Re-reading the original thread, there was also concern about whether we should try to make quoting/casefolding behave more like it does in SQL, specifically for matching pg_hba.conf items to SQL identifiers (database and role names). This patch doesn't seem to have addressed that part of it, but I think we need to think those things through before we just do a blind s/strcmp/pg_strcasecmp/g. Otherwise we might find that we've added ambiguity that will give us trouble when we do try to fix that. It's worth noting that pg_ident.conf uses SQL-like case-folding and quoting, though I don't think it's documented. We should certainly be using the same thing in pg_hba.conf IMO. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive * More robust pg_hba.conf parsing/error logginghttp://archives.postgresql.org/pgsql-hackers/2009-09/msg00432.php Thanks Regards, Viswanatham Kiran Kumar pg_hba.conf_keywords_as_case-insensitive.patch Description: pg_hba.conf_keywords_as_case-insensitive.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
On Wed, Jul 16, 2014 at 6:23 PM, Viswanatham kirankumar viswanatham.kiranku...@huawei.com wrote: Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive More robust pg_hba.conf parsing/error logging You should consider adding this patch to the next commit fest: https://commitfest.postgresql.org/action/commitfest_view?id=23 Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive * More robust pg_hba.conf parsing/error logginghttp://archives.postgresql.org/pgsql-hackers/2009-09/msg00432.php Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. Possibly things like MD5 and GSSAPI are naturally spelled in upper case, but I have my doubts about the rest. Christoph -- c...@df7cb.de | http://www.df7cb.de/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [TODO] Process pg_hba.conf keywords as case-insensitive
Christoph Berg c...@df7cb.de writes: Re: Viswanatham kirankumar 2014-07-16 ec867def52699d4189b584a14baa7c2165440...@blreml504-mbx.china.huawei.com Attached patch is implementing following TODO item Process pg_hba.conf keywords as case-insensitive Hmm. I see a case for accepting ALL (as in hosts.allow(5)), so +1 on that, but I don't think the other keywords like host and peer should be valid in upper case. I think the argument was that SQL users are accustomed to thinking that keywords are case-insensitive. It makes sense to me that we should adopt that same convention in pg_hba.conf. Re-reading the original thread, there was also concern about whether we should try to make quoting/casefolding behave more like it does in SQL, specifically for matching pg_hba.conf items to SQL identifiers (database and role names). This patch doesn't seem to have addressed that part of it, but I think we need to think those things through before we just do a blind s/strcmp/pg_strcasecmp/g. Otherwise we might find that we've added ambiguity that will give us trouble when we do try to fix that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers