Re: [HACKERS] SQL access to database attributes
Vik Fearing vik.fear...@dalibo.com writes: Okay, here is version two of the refactoring patch that documents that the with-space version is deprecated but still accepted. The feature patch is not affected by this and so I am not attaching a new version of that. I've committed this without the changes to expose the CONNECTION_LIMIT spelling, and with some other minor fixes --- the only one of substance being that you'd broken the foo = DEFAULT variants of the options by removing the checks on whether defel-arg was provided. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
2014-06-29 21:09 GMT+02:00 Tom Lane t...@sss.pgh.pa.us: Vik Fearing vik.fear...@dalibo.com writes: On 06/21/2014 10:11 PM, Pavel Stehule wrote: Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. While I agree that this patch wouldn't break backwards compatibility, I don't really see what the argument is for changing the recommended spelling of the command. The difficulty with doing what you've done here is that it creates unnecessary cross-version incompatibilities; for example a 9.5 psql being used against a 9.4 server would tab-complete the wrong spelling of the option. Back-patching would change the set of versions for which the problem exists, but it wouldn't remove the problem altogether. And in fact it'd add new problems, e.g. pg_dumpall output from a 9.3.5 pg_dumpall failing to load into a 9.3.4 server. This is not the kind of change we customarily back-patch anyway. So personally I'd have just made connection_limit be an undocumented internal equivalent for CONNECTION LIMIT, and kept the latter as the preferred spelling, with no client-side changes. +1 There is no important reason do hard changes in this moment Pavel regards, tom lane
Re: [HACKERS] SQL access to database attributes
Vik Fearing vik.fear...@dalibo.com writes: On 06/21/2014 10:11 PM, Pavel Stehule wrote: Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. While I agree that this patch wouldn't break backwards compatibility, I don't really see what the argument is for changing the recommended spelling of the command. The difficulty with doing what you've done here is that it creates unnecessary cross-version incompatibilities; for example a 9.5 psql being used against a 9.4 server would tab-complete the wrong spelling of the option. Back-patching would change the set of versions for which the problem exists, but it wouldn't remove the problem altogether. And in fact it'd add new problems, e.g. pg_dumpall output from a 9.3.5 pg_dumpall failing to load into a 9.3.4 server. This is not the kind of change we customarily back-patch anyway. So personally I'd have just made connection_limit be an undocumented internal equivalent for CONNECTION LIMIT, and kept the latter as the preferred spelling, with no client-side changes. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On 06/23/2014 06:45 PM, Pavel Stehule wrote:
2014-06-23 18:39 GMT+02:00 Vik Fearing vik.fear...@dalibo.com
mailto:vik.fear...@dalibo.com:
On 06/23/2014 06:21 PM, Robert Haas wrote:
On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule
pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote:
I found only one problem - first patch introduce a new property
CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in
documentation. But CONNECTION LIMIT is still supported, but it
is not
documented. So for some readers it can look like breaking
compatibility, but
it is false. This should be documented better.
Yeah, I think the old syntax should be documented also.
Why do we want to document syntax that should eventually be deprecated?
It is fair to our users. It can be deprecated, ok, we can write in doc -
this feature will be deprecated in next three years. Don't use it, but
this should be documentated.
Okay, here is version two of the refactoring patch that documents that
the with-space version is deprecated but still accepted.
The feature patch is not affected by this and so I am not attaching a
new version of that.
--
Vik
*** a/contrib/sepgsql/expected/alter.out
--- b/contrib/sepgsql/expected/alter.out
***
*** 110,116 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 110,116
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/contrib/sepgsql/sql/alter.sql
--- b/contrib/sepgsql/sql/alter.sql
***
*** 85,91 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 85,91
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/doc/src/sgml/ref/alter_database.sgml
--- b/doc/src/sgml/ref/alter_database.sgml
***
*** 25,31 ALTER DATABASE replaceable class=PARAMETERname/replaceable [ [ WITH ] rep
phrasewhere replaceable class=PARAMETERoption/replaceable can be:/phrase
! CONNECTION LIMIT replaceable class=PARAMETERconnlimit/replaceable
ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO replaceablenew_name/replaceable
--- 25,31
phrasewhere replaceable class=PARAMETERoption/replaceable can be:/phrase
! CONNECTION_LIMIT replaceable class=PARAMETERconnection_limit/replaceable
ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO replaceablenew_name/replaceable
***
*** 107,117 ALTER DATABASE replaceable class=PARAMETERname/replaceable RESET ALL
/varlistentry
varlistentry
! termreplaceable class=parameterconnlimit/replaceable/term
listitem
para
How many concurrent connections can be made
to this database. -1 means no limit.
/para
/listitem
/varlistentry
--- 107,119
/varlistentry
varlistentry
! termreplaceable class=parameterconnection_limit/replaceable/term
listitem
para
How many concurrent connections can be made
to this database. -1 means no limit.
+ The deprecated spelling commandCONNECTION LIMIT/command is
+ still accepted.
/para
/listitem
/varlistentry
*** a/doc/src/sgml/ref/create_database.sgml
--- b/doc/src/sgml/ref/create_database.sgml
***
*** 28,34 CREATE DATABASE replaceable class=PARAMETERname/replaceable
[ LC_COLLATE [=] replaceable class=parameterlc_collate/replaceable ]
[ LC_CTYPE [=] replaceable class=parameterlc_ctype/replaceable ]
[ TABLESPACE [=] replaceable
Re: [HACKERS] SQL access to database attributes
On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I found only one problem - first patch introduce a new property CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in documentation. But CONNECTION LIMIT is still supported, but it is not documented. So for some readers it can look like breaking compatibility, but it is false. This should be documented better. Yeah, I think the old syntax should be documented also. See, e.g., what we do for COPY. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On 06/23/2014 06:21 PM, Robert Haas wrote: On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I found only one problem - first patch introduce a new property CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in documentation. But CONNECTION LIMIT is still supported, but it is not documented. So for some readers it can look like breaking compatibility, but it is false. This should be documented better. Yeah, I think the old syntax should be documented also. Why do we want to document syntax that should eventually be deprecated? See, e.g., what we do for COPY. Exactly. We're still carrying around baggage from 7.2! Backward compatibility: yes. Backward documentation: no. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
2014-06-23 18:39 GMT+02:00 Vik Fearing vik.fear...@dalibo.com: On 06/23/2014 06:21 PM, Robert Haas wrote: On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I found only one problem - first patch introduce a new property CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in documentation. But CONNECTION LIMIT is still supported, but it is not documented. So for some readers it can look like breaking compatibility, but it is false. This should be documented better. Yeah, I think the old syntax should be documented also. Why do we want to document syntax that should eventually be deprecated? It is fair to our users. It can be deprecated, ok, we can write in doc - this feature will be deprecated in next three years. Don't use it, but this should be documentated. Pavel See, e.g., what we do for COPY. Exactly. We're still carrying around baggage from 7.2! Backward compatibility: yes. Backward documentation: no. -- Vik
Re: [HACKERS] SQL access to database attributes
On Mon, Jun 23, 2014 at 12:39 PM, Vik Fearing vik.fear...@dalibo.com wrote: On 06/23/2014 06:21 PM, Robert Haas wrote: On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule pavel.steh...@gmail.com wrote: I found only one problem - first patch introduce a new property CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in documentation. But CONNECTION LIMIT is still supported, but it is not documented. So for some readers it can look like breaking compatibility, but it is false. This should be documented better. Yeah, I think the old syntax should be documented also. Why do we want to document syntax that should eventually be deprecated? Because otherwise existing users will wonder if their dumps can still be restored on newer systems. And also, because documentation is, in general, a good thing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
Hello I returned to review this patch after sleeping - and I have to say, these patches doesn't break a compatibility. This feature has two patches: createdb_alterdb_grammar_refactoring.v1-1.patch and database_attributes.v2-1.patch. First patch do some cleaning in gram rules a CREATE DATABASE and ALTER DATABASE statements (and introduce a CONNECTION_LIMIT property). Second patch introduces ALLOW_CONNECTIONS and IS_TEMPLATE database properties. A motivation for these patches is cleaning alterdb/createdb grammars and drop necessity to directly modify pg_database table. 1. these patch does what was proposed, there was not any objection in related discussion 2. I can apply these patches cleanly, a compilation was without new warnings and without errors 3. all tests was passed 4. there is a necessary documentation (for new features) 5. a new syntax is actively used in initdb and pg_upgrade. I am not sure, if some special test is necessary and if we are able to test it. Refactoring of alterdb/createdb grammars has sense and we would it. I found only one problem - first patch introduce a new property CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in documentation. But CONNECTION LIMIT is still supported, but it is not documented. So for some readers it can look like breaking compatibility, but it is false. This should be documented better. Regards Pavel 2014-06-21 23:14 GMT+02:00 Vik Fearing vik.fear...@dalibo.com: On 06/21/2014 10:11 PM, Pavel Stehule wrote: Hello I am looking createdb_alterdb_grammar_refactoring.v1.patch http://www.postgresql.org/message-id/53868e57.3030...@dalibo.com Thank you for looking at this. Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. Surely this patch cannot be backported what is proposed there. There are reasons I can think of not to backport this first patch, but breaking compatibility isn't one of them. -- Vik
[HACKERS] SQL access to database attributes
Hello I am looking createdb_alterdb_grammar_refactoring.v1.patch http://www.postgresql.org/message-id/53868e57.3030...@dalibo.com Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? Surely this patch cannot be backported what is proposed there. Regards Pavel
Re: [HACKERS] SQL access to database attributes
Second question related to second patch: Must be new syntax ALLOW_CONNECTIONS? Should not be (ENABLE | DISABLE) CONNECTION ? This doesn't need any new keyword. Regards Pavel 2014-06-21 22:11 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com: Hello I am looking createdb_alterdb_grammar_refactoring.v1.patch http://www.postgresql.org/message-id/53868e57.3030...@dalibo.com Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? Surely this patch cannot be backported what is proposed there. Regards Pavel
Re: [HACKERS] SQL access to database attributes
On 06/21/2014 10:11 PM, Pavel Stehule wrote: Hello I am looking createdb_alterdb_grammar_refactoring.v1.patch http://www.postgresql.org/message-id/53868e57.3030...@dalibo.com Thank you for looking at this. Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. Surely this patch cannot be backported what is proposed there. There are reasons I can think of not to backport this first patch, but breaking compatibility isn't one of them. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On 06/21/2014 10:21 PM, Pavel Stehule wrote: Second question related to second patch: Must be new syntax ALLOW_CONNECTIONS? It doesn't *have* to be called that, but that's what the corresponding column in pg_database is called so why add confusion? (Actually, it's called datallowconn but that would be a silly name on the SQL level.) Should not be (ENABLE | DISABLE) CONNECTION ? I don't think it should be, no. This doesn't need any new keyword. None of this requires any new keywords. That's the whole point of the refactoring patch. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
2014-06-21 23:14 GMT+02:00 Vik Fearing vik.fear...@dalibo.com: On 06/21/2014 10:11 PM, Pavel Stehule wrote: Hello I am looking createdb_alterdb_grammar_refactoring.v1.patch http://www.postgresql.org/message-id/53868e57.3030...@dalibo.com Thank you for looking at this. Is any reason or is acceptable incompatible change CONNECTION_LIMIT instead CONNECTION LIMIT? Is decreasing parser size about 1% good enough for breaking compatibility? How is compatibility broken? The grammar still accepts the old way, I just changed the documentation to promote the new way. Surely this patch cannot be backported what is proposed there. There are reasons I can think of not to backport this first patch, but breaking compatibility isn't one of them. I am sorry, tomorrow I have to read it again Pavel -- Vik
Re: [HACKERS] SQL access to database attributes
On 05/26/2014 08:19 PM, Vik Fearing wrote:
On 05/26/2014 07:10 AM, Stephen Frost wrote:
* Tom Lane (t...@sss.pgh.pa.us) wrote:
I don't really object to doing an unlocked check for another such
database, but I'm not convinced that additional locking to try to
prevent a race is worth its keep.
+1 on the nannyism, and +1 to ignoring the race.
Okay, I'll submit a new patch with racy nannyism and some grammar
changes that Tom and I have been discussing in private.
Attached are two patches.
The first is a refactoring of the createdb/alterdb grammars mostly by
Tom which makes all of the options non-keywords that don't otherwise
need to be. Not only does this remove the two unreserved keywords I had
added (ALLOW and CONNECTIONS) but also removes two existing ones
(LC_COLLATE and LC_TYPE), reducing gram.o by about half a percent by
Tom's calculations. That's much better than increasing it like my
original patch did.
The problem is we foolishly adopted a two-word option name (CONNECTION
LIMIT) which complicates the grammar. My aping of that for IS TEMPLATE
and ALLOW CONNECTIONS only aggravated the situation. And so I changed
all the documentation (and pg_dumpall etc) to use CONNECTION_LIMIT
instead. We might hopefully one day deprecate the with-space version so
the sooner the documentation recommends the without-space version, the
better. The old syntax is of course still valid.
I also changed the documentation to say connection_limit instead of
connlimit. Documentation is for humans, something like connlimit (and
later as we'll see allowconn) is for programmers. It also indirectly
reminds us that we should not add another multi-word option like I
initially did.
Now that anything goes grammar-wise, the elog for unknown option is now
ereport.
I am hoping this gets backpatched.
---
The second patch adds my original functionality, except this time the
syntax is IS_TEMPLATE and ALLOW_CONNECTIONS (both one word, neither
being keywords). It also changes all the catalog updates we used to do
because we didn't have this patch.
As for the nannyism, the point was to find another database having
datallowconn=true but since we have to be connected to a database to
issue this command, the simplest is just to disallow the current
database (credit Andres on IRC) so that's what I've done as explained
with this in-code comment:
/*
* In order to avoid getting locked out and having to go through
standalone
* mode, we refuse to disallow connections on the database we're
currently
* connected to. Lockout can still happen with concurrent
sessions but the
* likeliness of that is not high enough to worry about.
*/
I also changed the C variable connlimit to dbconnlimit in
AlterDatabase() to be consistent with its analog in createdb().
---
The third and non-existent patch is about regression tests. Tom put in
gram.y that we should have some now that the grammar doesn't regulate
it, and I wanted some anyway in my original patch; but I don't know what
they should look like or where they should go so I'm asking for help on
that.
--
Vik
*** a/contrib/sepgsql/expected/alter.out
--- b/contrib/sepgsql/expected/alter.out
***
*** 110,116 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 110,116
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/contrib/sepgsql/sql/alter.sql
--- b/contrib/sepgsql/sql/alter.sql
***
*** 85,91 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 85,91
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/doc/src/sgml/ref/alter_database.sgml
---
Re: [HACKERS] SQL access to database attributes
On 05/26/2014 07:10 AM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: I don't really object to doing an unlocked check for another such database, but I'm not convinced that additional locking to try to prevent a race is worth its keep. +1 on the nannyism, and +1 to ignoring the race. Okay, I'll submit a new patch with racy nannyism and some grammar changes that Tom and I have been discussing in private. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
* Tom Lane (t...@sss.pgh.pa.us) wrote: I don't really object to doing an unlocked check for another such database, but I'm not convinced that additional locking to try to prevent a race is worth its keep. +1 on the nannyism, and +1 to ignoring the race. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] SQL access to database attributes
Vik Fearing vik.fear...@dalibo.com writes: On 05/24/2014 12:03 AM, Jaime Casanova wrote: Which lead us to the question: you need to connect to the database to modify it, don't you? then, how do you change ALLOW CONNECTIONS to true? You can ALTER DATABASE from anywhere. Perhaps it'd be wise to have a safety check to disallow turning off datallowconn for the last connectable database? Although it couldn't be bulletproof due to race conditions, so maybe that'd just be nannyism. (If you do shoot yourself in the foot that way, I think we ignore datallowconn in standalone mode.) As with the patch we were discussing yesterday, -1 for inventing new parser keywords for this. I wonder if we couldn't refactor the grammar so it thinks all of CREATE DATABASE's WITH options are identifier = value and none of them have to be keywords. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On 5/24/14, 8:14 AM, Tom Lane wrote: Vik Fearingvik.fear...@dalibo.com writes: On 05/24/2014 12:03 AM, Jaime Casanova wrote: Which lead us to the question: you need to connect to the database to modify it, don't you? then, how do you change ALLOW CONNECTIONS to true? You can ALTER DATABASE from anywhere. Perhaps it'd be wise to have a safety check to disallow turning off datallowconn for the last connectable database? Although it couldn't be bulletproof due to race conditions, so maybe that'd just be nannyism. (If you do shoot yourself in the foot that way, I think we ignore datallowconn in standalone mode.) I think this is nannyism that would be well placed. Most people don't know about standalone, and I don't think we want to change that. BTW, I think the race condition could be eliminated if we did something like (forgive the user-space semantics): SELECT datallowconn FROM pg_database WHERE datallowconn AND datname $$database we're disallowing connections on$$ LIMIT 1 FOR UPDATE; If you don't get a record back from that you abort; meanwhile no one else can disallow connections on that database until you commit or rollback. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
Jim Nasby j...@nasby.net writes: On 5/24/14, 8:14 AM, Tom Lane wrote: Perhaps it'd be wise to have a safety check to disallow turning off datallowconn for the last connectable database? Although it couldn't be bulletproof due to race conditions, so maybe that'd just be nannyism. BTW, I think the race condition could be eliminated if we did something like (forgive the user-space semantics): SELECT datallowconn FROM pg_database WHERE datallowconn AND datname $$database we're disallowing connections on$$ LIMIT 1 FOR UPDATE; If you don't get a record back from that you abort; meanwhile no one else can disallow connections on that database until you commit or rollback. Meh. That would take out a rowlock on a database unrelated to the one we're modifying, which would be (a) surprising and (b) subject to deadlocks. I don't really object to doing an unlocked check for another such database, but I'm not convinced that additional locking to try to prevent a race is worth its keep. 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] SQL access to database attributes
We try to tell our clients not to update the catalogs directly, but there are at least two instances where it's not possible to do otherwise (pg_database.datistemplate and .datallowconn). This patch aims to remedy that. For example, it is now possible to say ALTER DATABASE d ALLOW CONNECTIONS = false; and ALTER DATABASE d IS TEMPLATE = true; This syntax matches that of CONNECTION LIMIT but unfortunately required me to make ALLOW and CONNECTIONS unreserved keywords. I know we try not to do that but I didn't see any other way. The two new options are of course also available on CREATE DATABASE. There is a slight change in behavior with this patch in that previously one had to be superuser or have rolcatupdate appropriately set, and now the owner of the database is also allowed to change these settings. I believe this is for the better. It was suggested to me that these options should either error out if there are existing connections or terminate said connections. I don't agree with that because there is no harm in connecting to a template database (how else do you modify it?), and adding a reject rule in pg_hba.conf doesn't disconnect existing users so why should turning off ALLOW CONNECTIONS do it? As for regression tests, I couldn't figure out how to make CREATE/ALTER DATABASE play nice with make installcheck and so I haven't provided any. Other than that, I think this patch is complete and so I'm adding it the next commitfest. -- Vik *** a/doc/src/sgml/ref/alter_database.sgml --- b/doc/src/sgml/ref/alter_database.sgml *** *** 25,30 ALTER DATABASE replaceable class=PARAMETERname/replaceable [ [ WITH ] rep --- 25,32 phrasewhere replaceable class=PARAMETERoption/replaceable can be:/phrase + IS TEMPLATE replaceable class=PARAMETERistemplate/replaceable + ALLOW CONNECTIONS replaceable class=PARAMETERallowconn/replaceable CONNECTION LIMIT replaceable class=PARAMETERconnlimit/replaceable ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO replaceablenew_name/replaceable *** *** 107,112 ALTER DATABASE replaceable class=PARAMETERname/replaceable RESET ALL --- 109,134 /varlistentry varlistentry + termreplaceable class=parameteristemplate/replaceable/term + listitem +para + If true, then this database can be cloned by any user with CREATEDB + privileges; if false, then only superusers or the owner of the + database can clone it. +/para + /listitem + /varlistentry + + varlistentry + termreplaceable class=parameterallowconn/replaceable/term + listitem +para + If false then no one can connect to this database. +/para + /listitem + /varlistentry + + varlistentry termreplaceable class=parameterconnlimit/replaceable/term listitem para *** a/doc/src/sgml/ref/create_database.sgml --- b/doc/src/sgml/ref/create_database.sgml *** *** 28,33 CREATE DATABASE replaceable class=PARAMETERname/replaceable --- 28,35 [ LC_COLLATE [=] replaceable class=parameterlc_collate/replaceable ] [ LC_CTYPE [=] replaceable class=parameterlc_ctype/replaceable ] [ TABLESPACE [=] replaceable class=parametertablespace_name/replaceable ] +[ IS TEMPLATE [=] replaceable class=parameteristemplate/replaceable] +[ ALLOW CONNECTIONS [=] replaceable class=parameterallowconn/replaceable] [ CONNECTION LIMIT [=] replaceable class=parameterconnlimit/replaceable ] ] /synopsis /refsynopsisdiv *** *** 148,153 CREATE DATABASE replaceable class=PARAMETERname/replaceable --- 150,175 /varlistentry varlistentry + termreplaceable class=parameteristemplate/replaceable/term + listitem +para + If true, then this database can be cloned by any user with CREATEDB + privileges; if false, then only superusers or the owner of the + database can clone it. +/para + /listitem + /varlistentry + + varlistentry + termreplaceable class=parameterallowconn/replaceable/term + listitem +para + If false then no one can connect to this database. +/para + /listitem + /varlistentry + + varlistentry termreplaceable class=parameterconnlimit/replaceable/term listitem para *** a/src/backend/commands/dbcommands.c --- b/src/backend/commands/dbcommands.c *** *** 39,44 --- 39,45 #include catalog/pg_tablespace.h #include commands/comment.h #include commands/dbcommands.h + #include commands/defrem.h #include commands/seclabel.h #include commands/tablespace.h #include mb/pg_wchar.h *** *** 122,127 createdb(const CreatedbStmt *stmt) --- 123,130 DefElem*dencoding = NULL; DefElem
Re: [HACKERS] SQL access to database attributes
On Fri, May 23, 2014 at 10:53 PM, Vik Fearing vik.fear...@dalibo.com wrote: It was suggested to me that these options should either error out if there are existing connections or terminate said connections. I don't agree with that because there is no harm in connecting to a template database (how else do you modify it?), and adding a reject rule in pg_hba.conf doesn't disconnect existing users so why should turning off ALLOW CONNECTIONS do it? Which lead us to the question: you need to connect to the database to modify it, don't you? then, how do you change ALLOW CONNECTIONS to true? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On 05/24/2014 12:03 AM, Jaime Casanova wrote: On Fri, May 23, 2014 at 10:53 PM, Vik Fearing vik.fear...@dalibo.com wrote: It was suggested to me that these options should either error out if there are existing connections or terminate said connections. I don't agree with that because there is no harm in connecting to a template database (how else do you modify it?), and adding a reject rule in pg_hba.conf doesn't disconnect existing users so why should turning off ALLOW CONNECTIONS do it? Which lead us to the question: you need to connect to the database to modify it, don't you? then, how do you change ALLOW CONNECTIONS to true? You can ALTER DATABASE from anywhere. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SQL access to database attributes
On Fri, May 23, 2014 at 11:06 PM, Vik Fearing vik.fear...@dalibo.com wrote: On 05/24/2014 12:03 AM, Jaime Casanova wrote: On Fri, May 23, 2014 at 10:53 PM, Vik Fearing vik.fear...@dalibo.com wrote: It was suggested to me that these options should either error out if there are existing connections or terminate said connections. I don't agree with that because there is no harm in connecting to a template database (how else do you modify it?), and adding a reject rule in pg_hba.conf doesn't disconnect existing users so why should turning off ALLOW CONNECTIONS do it? Which lead us to the question: you need to connect to the database to modify it, don't you? then, how do you change ALLOW CONNECTIONS to true? You can ALTER DATABASE from anywhere. ah! doh! right! don't know why i was convinced you need to connect to the database to execute ALTER DATABASE -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
