Add more regression tests for dbcommands

Started by Robins Tharakanover 12 years ago26 messages
#1Robins Tharakan
tharakan@gmail.com
1 attachment(s)

Hi,

Please find attached a patch to take code-coverage of DBCommands (CREATE
DATABASE / ALTER DATABASE / DROP DATABASE) from 36% to 71%.

Any and all feedback is obviously welcome.

--
Robins Tharakan

Attachments:

regress_db_v2.patchapplication/octet-stream; name=regress_db_v2.patchDownload
diff --git a/src/test/regress/expected/database.out b/src/test/regress/expected/database.out
new file mode 100644
index 0000000..f8f30b7
--- /dev/null
+++ b/src/test/regress/expected/database.out
@@ -0,0 +1,203 @@
+--
+-- tests functions in dbcommands.c script
+--
+-- Should work. Create Database
+CREATE DATABASE db_db1;
+DROP DATABASE db_db1;
+-- Should fail. Can't DROP DATABASE that doesn't exist / is reserved
+DROP DATABASE db_db2;
+ERROR:  database "db_db2" does not exist
+DROP DATABASE IF EXISTS db_db2;
+NOTICE:  database "db_db2" does not exist, skipping
+DROP DATABASE template1;
+ERROR:  cannot drop a template database
+DROP DATABASE regression;
+ERROR:  cannot drop the currently open database
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ERROR:  CREATE DATABASE cannot run inside a transaction block
+DROP DATABASE db_db3;
+ERROR:  current transaction is aborted, commands ignored until end of transaction block
+ROLLBACK;
+-- Should fail. DROP DATABASE requires CREATEDB permissions
+CREATE ROLE rol_db4;
+CREATE DATABASE db_db4;
+SET ROLE rol_db4;
+DROP DATABASE db_db4;
+ERROR:  must be owner of database db_db4
+RESET ROLE;
+DROP DATABASE db_db4;
+DROP ROLE rol_db4;
+-- Should work. ALTER DATABASE RENAME TO
+CREATE DATABASE db_db5;
+ALTER DATABASE db_db5 RENAME TO db_db5b;
+DROP DATABASE db_db5b;
+-- Should fail. ALTER DATABASE RENAME TO on non-existent DB
+CREATE DATABASE db_db6;
+ALTER DATABASE asdf RENAME TO db_db6a;
+ERROR:  database "asdf" does not exist
+ALTER DATABASE db_db6 RENAME TO db_db6;
+ERROR:  database "db_db6" already exists
+ALTER DATABASE db_db6 RENAME TO db_db6a;
+DROP DATABASE db_db6a;
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+CREATE DATABASE db_db7;
+SET ROLE rol_db7;
+ALTER DATABASE db_db7 RENAME TO db_db7a;
+ERROR:  must be owner of database db_db7
+RESET ROLE;
+DROP DATABASE db_db7;
+DROP ROLE rol_db7;
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+CREATE ROLE rol_db8;
+CREATE DATABASE db_db8 WITH OWNER rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE db_db8 RENAME TO db_db8a;
+ERROR:  permission denied to rename database
+ALTER DATABASE db_db8 RENAME TO db_db8a;
+ERROR:  permission denied to rename database
+RESET ROLE;
+DROP DATABASE db_db8;
+DROP ROLE rol_db8;
+-- Should work. ALTER DATABASE RENAME TO with OWNER and CREATEDB privilege
+CREATE ROLE rol_db9 CREATEDB;
+CREATE DATABASE db_db9 WITH OWNER rol_db9;
+SET ROLE rol_db9;
+ALTER DATABASE db_db9 RENAME TO regression;
+ERROR:  database "regression" already exists
+ALTER DATABASE db_db9 RENAME TO db_db9a;
+RESET ROLE;
+DROP DATABASE db_db9a;
+DROP ROLE rol_db9;
+-- Should fail. Can't rename current database
+ALTER DATABASE regression RENAME TO regression_a;
+ERROR:  current database cannot be renamed
+-- Should work. ALTER DATABASE SET with some valid / invalid values
+CREATE DATABASE db_db10;
+ALTER DATABASE db_db10 SET ASDF;
+ERROR:  syntax error at or near ";"
+LINE 1: ALTER DATABASE db_db10 SET ASDF;
+                                       ^
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT = -100;
+ERROR:  invalid connection limit: -100
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  0;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  100;
+ALTER DATABASE db_db10 SET SEED = 0.5;
+DROP DATABASE db_db10;
+-- Should fail. Can't set Tablespace within transaction
+CREATE DATABASE db_db11;
+BEGIN TRANSACTION;
+ALTER DATABASE db_db11 SET TABLESPACE asdf;
+ERROR:  ALTER DATABASE SET TABLESPACE cannot run inside a transaction block
+ROLLBACK;
+DROP DATABASE db_db11;
+-- Should fail. ALTER DATABASE with invalid connection limit
+CREATE DATABASE db_db12;
+BEGIN TRANSACTION;
+ALTER DATABASE db_db12 WITH CONNECTION LIMIT = 8ASDF;
+ERROR:  syntax error at or near "ASDF"
+LINE 1: ALTER DATABASE db_db12 WITH CONNECTION LIMIT = 8ASDF;
+                                                        ^
+ROLLBACK;
+DROP DATABASE db_db12;
+-- Should work. ALTER DATABASE SET with OWNER rights without CREATEDB
+CREATE ROLE rol_db13;
+CREATE DATABASE db_db13 WITH OWNER rol_db13;
+SET ROLE rol_db13;
+ALTER DATABASE db_db13 SET SEED=0.5;
+RESET ROLE;
+DROP DATABASE db_db13;
+DROP ROLE rol_db13;
+-- Should work. ALTER DATABASE OWNER TO with OWNER rights
+CREATE ROLE rol_db14;
+CREATE ROLE rol_db15;
+CREATE DATABASE db_db14 WITH OWNER rol_db14;
+SET ROLE rol_db15;
+ALTER DATABASE db_db14 OWNER TO rol_db15; -- OWNERship required
+ERROR:  must be owner of database db_db14
+RESET ROLE;
+SET ROLE rol_db14;
+ALTER DATABASE db_db14 OWNER TO rol_db15; -- CREATEDB required for grantor
+ERROR:  must be member of role "rol_db15"
+RESET ROLE;
+ALTER DATABASE asdf OWNER TO rol_db15;  -- bogus database
+ERROR:  database "asdf" does not exist
+ALTER DATABASE db_db14 OWNER TO asdf;  -- bogus to owner
+ERROR:  role "asdf" does not exist
+ALTER DATABASE db_db14 OWNER TO rol_db14;  -- reassign to self
+ALTER DATABASE db_db14 OWNER TO rol_db15;
+DROP DATABASE db_db14;
+DROP ROLE rol_db15;
+DROP ROLE rol_db14;
+-- Should fail. ALTER DATABASE SET TABLESPACE with invalid name
+ALTER DATABASE db_db16 SET TABLESPACE asdf;
+ERROR:  database "db_db16" does not exist
+-- Should fail. ALTER DATABASE SET TABLESPACE without OWNER privilege
+CREATE ROLE rol_db16;
+CREATE DATABASE db_db16;
+SET ROLE rol_db16;
+ALTER DATABASE db_db16 SET TABLESPACE asdf;
+ERROR:  must be owner of database db_db16
+RESET ROLE;
+DROP DATABASE db_db16;
+DROP ROLE rol_db16;
+-- Should fail. ALTER DATABASE SET TABLESPACE without OWNER privilege
+ALTER DATABASE regression SET TABLESPACE asdf;
+ERROR:  cannot change the tablespace of the currently open database
+-- Should fail. ALTER DATABASE SET TABLESPACE with OWNER / CREATEDB privilege
+CREATE ROLE rol_db17 CREATEDB;
+CREATE DATABASE db_db17 OWNER rol_db17;
+SET ROLE rol_db17;
+ALTER DATABASE db_db17 SET TABLESPACE DEFAULT;
+ERROR:  syntax error at or near "DEFAULT"
+LINE 1: ALTER DATABASE db_db17 SET TABLESPACE DEFAULT;
+                                              ^
+RESET ROLE;
+DROP DATABASE db_db17;
+DROP ROLE rol_db17;
+-- Should work. CREATE DATABASE with valid values
+CREATE DATABASE db_db18a CONNECTION LIMIT 10;
+DROP DATABASE db_db18a;
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+ERROR:  invalid_encoding is not a valid encoding name
+CREATE DATABASE db_db18 ENCODING 123456789;
+ERROR:  123456789 is not a valid encoding code
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+ERROR:  invalid connection limit: -10
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+ERROR:  template database "invalid_template" does not exist
+CREATE DATABASE regression;
+ERROR:  database "regression" already exists
+-- Should fail. CREATE DATABASE using non-template as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 template db_db19;
+ERROR:  permission denied to copy database "db_db19"
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+-- Should fail. Self trying CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+CREATE DATABASE db_db21a;
+ERROR:  permission denied to create database
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+ERROR:  must be owner of database db_db21
+DROP DATABASE db_db21;
+ERROR:  must be owner of database db_db21
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+DROP DATABASE db_db22a;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 2af28b1..243f377 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -88,7 +88,7 @@ test: privileges security_label collate matview
 # ----------
 # Another group of parallel tests
 # ----------
-test: alter_generic misc psql
+test: alter_generic misc psql database
 
 # rules cannot run concurrently with any test that creates a view
 test: rules
diff --git a/src/test/regress/sql/database.sql b/src/test/regress/sql/database.sql
new file mode 100644
index 0000000..e4d6481
--- /dev/null
+++ b/src/test/regress/sql/database.sql
@@ -0,0 +1,187 @@
+--
+-- tests functions in dbcommands.c script
+--
+
+-- Should work. Create Database
+CREATE DATABASE db_db1;
+DROP DATABASE db_db1;
+
+-- Should fail. Can't DROP DATABASE that doesn't exist / is reserved
+DROP DATABASE db_db2;
+DROP DATABASE IF EXISTS db_db2;
+DROP DATABASE template1;
+DROP DATABASE regression;
+
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+DROP DATABASE db_db3;
+ROLLBACK;
+
+-- Should fail. DROP DATABASE requires CREATEDB permissions
+CREATE ROLE rol_db4;
+CREATE DATABASE db_db4;
+SET ROLE rol_db4;
+DROP DATABASE db_db4;
+RESET ROLE;
+DROP DATABASE db_db4;
+DROP ROLE rol_db4;
+
+-- Should work. ALTER DATABASE RENAME TO
+CREATE DATABASE db_db5;
+ALTER DATABASE db_db5 RENAME TO db_db5b;
+DROP DATABASE db_db5b;
+
+-- Should fail. ALTER DATABASE RENAME TO on non-existent DB
+CREATE DATABASE db_db6;
+ALTER DATABASE asdf RENAME TO db_db6a;
+ALTER DATABASE db_db6 RENAME TO db_db6;
+ALTER DATABASE db_db6 RENAME TO db_db6a;
+DROP DATABASE db_db6a;
+
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+CREATE DATABASE db_db7;
+SET ROLE rol_db7;
+ALTER DATABASE db_db7 RENAME TO db_db7a;
+RESET ROLE;
+DROP DATABASE db_db7;
+DROP ROLE rol_db7;
+
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+CREATE ROLE rol_db8;
+CREATE DATABASE db_db8 WITH OWNER rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE db_db8 RENAME TO db_db8a;
+ALTER DATABASE db_db8 RENAME TO db_db8a;
+RESET ROLE;
+DROP DATABASE db_db8;
+DROP ROLE rol_db8;
+
+-- Should work. ALTER DATABASE RENAME TO with OWNER and CREATEDB privilege
+CREATE ROLE rol_db9 CREATEDB;
+CREATE DATABASE db_db9 WITH OWNER rol_db9;
+SET ROLE rol_db9;
+ALTER DATABASE db_db9 RENAME TO regression;
+ALTER DATABASE db_db9 RENAME TO db_db9a;
+RESET ROLE;
+DROP DATABASE db_db9a;
+DROP ROLE rol_db9;
+
+-- Should fail. Can't rename current database
+ALTER DATABASE regression RENAME TO regression_a;
+
+-- Should work. ALTER DATABASE SET with some valid / invalid values
+CREATE DATABASE db_db10;
+ALTER DATABASE db_db10 SET ASDF;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT = -100;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  0;
+ALTER DATABASE db_db10 WITH CONNECTION LIMIT =  100;
+ALTER DATABASE db_db10 SET SEED = 0.5;
+DROP DATABASE db_db10;
+
+-- Should fail. Can't set Tablespace within transaction
+CREATE DATABASE db_db11;
+BEGIN TRANSACTION;
+ALTER DATABASE db_db11 SET TABLESPACE asdf;
+ROLLBACK;
+DROP DATABASE db_db11;
+
+-- Should fail. ALTER DATABASE with invalid connection limit
+CREATE DATABASE db_db12;
+BEGIN TRANSACTION;
+ALTER DATABASE db_db12 WITH CONNECTION LIMIT = 8ASDF;
+ROLLBACK;
+DROP DATABASE db_db12;
+
+-- Should work. ALTER DATABASE SET with OWNER rights without CREATEDB
+CREATE ROLE rol_db13;
+CREATE DATABASE db_db13 WITH OWNER rol_db13;
+SET ROLE rol_db13;
+ALTER DATABASE db_db13 SET SEED=0.5;
+RESET ROLE;
+DROP DATABASE db_db13;
+DROP ROLE rol_db13;
+
+-- Should work. ALTER DATABASE OWNER TO with OWNER rights
+CREATE ROLE rol_db14;
+CREATE ROLE rol_db15;
+CREATE DATABASE db_db14 WITH OWNER rol_db14;
+
+SET ROLE rol_db15;
+ALTER DATABASE db_db14 OWNER TO rol_db15; -- OWNERship required
+RESET ROLE;
+
+SET ROLE rol_db14;
+ALTER DATABASE db_db14 OWNER TO rol_db15; -- CREATEDB required for grantor
+RESET ROLE;
+
+ALTER DATABASE asdf OWNER TO rol_db15;  -- bogus database
+ALTER DATABASE db_db14 OWNER TO asdf;  -- bogus to owner
+ALTER DATABASE db_db14 OWNER TO rol_db14;  -- reassign to self
+ALTER DATABASE db_db14 OWNER TO rol_db15;
+DROP DATABASE db_db14;
+DROP ROLE rol_db15;
+DROP ROLE rol_db14;
+
+-- Should fail. ALTER DATABASE SET TABLESPACE with invalid name
+ALTER DATABASE db_db16 SET TABLESPACE asdf;
+
+-- Should fail. ALTER DATABASE SET TABLESPACE without OWNER privilege
+CREATE ROLE rol_db16;
+CREATE DATABASE db_db16;
+SET ROLE rol_db16;
+ALTER DATABASE db_db16 SET TABLESPACE asdf;
+RESET ROLE;
+DROP DATABASE db_db16;
+DROP ROLE rol_db16;
+
+-- Should fail. ALTER DATABASE SET TABLESPACE without OWNER privilege
+ALTER DATABASE regression SET TABLESPACE asdf;
+
+-- Should fail. ALTER DATABASE SET TABLESPACE with OWNER / CREATEDB privilege
+CREATE ROLE rol_db17 CREATEDB;
+CREATE DATABASE db_db17 OWNER rol_db17;
+SET ROLE rol_db17;
+ALTER DATABASE db_db17 SET TABLESPACE DEFAULT;
+RESET ROLE;
+DROP DATABASE db_db17;
+DROP ROLE rol_db17;
+
+-- Should work. CREATE DATABASE with valid values
+CREATE DATABASE db_db18a CONNECTION LIMIT 10;
+DROP DATABASE db_db18a;
+
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+CREATE DATABASE db_db18 ENCODING 123456789;
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+CREATE DATABASE regression;
+
+-- Should fail. CREATE DATABASE using non-template as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 template db_db19;
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+
+-- Should fail. Self trying CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+CREATE DATABASE db_db21a;
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+DROP DATABASE db_db21;
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+DROP DATABASE db_db22a;
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robins Tharakan (#1)
Re: Add more regression tests for dbcommands

Robins Tharakan <tharakan@gmail.com> writes:

Please find attached a patch to take code-coverage of DBCommands (CREATE
DATABASE / ALTER DATABASE / DROP DATABASE) from 36% to 71%.

I wish to object strenuously to adding any more CREATE/DROP DATABASE
commands to the core regression tests. Those are at least one order of
magnitude more expensive than any other commands we test, and the added
code coverage is precisely zero because creation and dropping of a
database is already done repeatedly in the contrib regression tests.

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

#3Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#2)
Re: Add more regression tests for dbcommands

Please find attached a patch to take code-coverage of DBCommands (CREATE
DATABASE / ALTER DATABASE / DROP DATABASE) from 36% to 71%.

I wish to object strenuously to adding any more CREATE/DROP DATABASE
commands to the core regression tests. Those are at least one order of
magnitude more expensive than any other commands we test, and the added
code coverage is precisely zero because creation and dropping of a
database is already done repeatedly in the contrib regression tests.

Would you be okay if there is one/a few effective create/drop (some tests
check that the create or drop fails e.g. depending on permissions, which
ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#3)
Re: Add more regression tests for dbcommands

Fabien COELHO <coelho@cri.ensmp.fr> writes:

Would you be okay if there is one/a few effective create/drop (some tests
check that the create or drop fails e.g. depending on permissions, which
ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

TBH, I do not see that such tests are worth adding, if they are going to
significantly slow down the core regression tests. Those tests are run
probably hundreds of times a day, in aggregate across all Postgres
developers. Adding ten seconds or whatever this would add is a major
cost, while the benefit appears trivial.

We could consider adding expensive low-value tests like these to some
alternate regression target that's only exercised by buildfarm members,
perhaps. But I think there's probably a point of diminishing returns
even in that context.

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

#5Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#4)
Re: Add more regression tests for dbcommands

Hello,

Would you be okay if there is one/a few effective create/drop (some tests
check that the create or drop fails e.g. depending on permissions, which
ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

TBH, I do not see that such tests are worth adding, if they are going to
significantly slow down the core regression tests. Those tests are run
probably hundreds of times a day, in aggregate across all Postgres
developers. Adding ten seconds or whatever this would add is a major
cost, while the benefit appears trivial.

We could consider adding expensive low-value tests like these to some
alternate regression target that's only exercised by buildfarm members,
perhaps. But I think there's probably a point of diminishing returns
even in that context.

I'm not sure that the tests are "low value", because a commit that would
generate a failure on a permission check test would be a potential
security issue for Pg.

My personal experience in other contexts is that small sanity checks help
avoid blunders at a small cost. It is also worthwhile to have significant
functional tests, such as replication and so on...

As for the cost, if the proposed tests are indeed too costly, what is not
necessarily the case for what I have seen, I do not think that it would be
a great problem to have two set of tests, with one a superset of the
other, with some convention.

It is enough that these tests are run once in a while to raise an alert if
need be, especially before a release, and not necessarily on every "make
check" of every developer in the world, so that we get some value at very
low cost. So, as you suggest implicitely, maybe "make check" and "make
longcheck" or make "shortcheck" in the test infrastructure would do the
trick?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Andres Freund
andres@2ndquadrant.com
In reply to: Fabien COELHO (#5)
Re: Add more regression tests for dbcommands

On 2013-05-13 16:52:08 +0200, Fabien COELHO wrote:

Hello,

Would you be okay if there is one/a few effective create/drop (some tests
check that the create or drop fails e.g. depending on permissions, which
ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

TBH, I do not see that such tests are worth adding, if they are going to
significantly slow down the core regression tests. Those tests are run
probably hundreds of times a day, in aggregate across all Postgres
developers. Adding ten seconds or whatever this would add is a major
cost, while the benefit appears trivial.

We could consider adding expensive low-value tests like these to some
alternate regression target that's only exercised by buildfarm members,
perhaps. But I think there's probably a point of diminishing returns
even in that context.

I'm not sure that the tests are "low value", because a commit that would
generate a failure on a permission check test would be a potential security
issue for Pg.

As for the cost, if the proposed tests are indeed too costly, what is not
necessarily the case for what I have seen, I do not think that it would be a
great problem to have two set of tests, with one a superset of the other,
with some convention.

Well, tests like permission tests aren't the expensive part. The actual
CREATE/DROP DATABASE you do is. The latter essentially are already
tested by the buildfarm already.
So, trimming the patch to do only the fast stuff should be less
controversial?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Robins Tharakan
tharakan@gmail.com
In reply to: Andres Freund (#6)
Re: Add more regression tests for dbcommands

I believe Tom / Andres and Fabien all have valid points.

Net-net, I believe the tests although non-negotiable, are not required to
be in make-check. For now, its the slow tests that are the pain points
here, and then I would soon try to prune them and commit once again.

Whether it goes in make-check or not is obviously not discretion but to me
their importance is undoubted since I am sure they increase code-coverage.
Actually that is 'how' I create those tests, I look at untouched code and
create new tests that check untouched code.

Would try to revert with a faster script (preferably with minimal
CREATE/DROP).

--
Robins Tharakan

On 13 May 2013 20:30, Andres Freund <andres@2ndquadrant.com> wrote:

Show quoted text

On 2013-05-13 16:52:08 +0200, Fabien COELHO wrote:

Hello,

Would you be okay if there is one/a few effective create/drop (some

tests

check that the create or drop fails e.g. depending on permissions,

which

ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

TBH, I do not see that such tests are worth adding, if they are going to
significantly slow down the core regression tests. Those tests are run
probably hundreds of times a day, in aggregate across all Postgres
developers. Adding ten seconds or whatever this would add is a major
cost, while the benefit appears trivial.

We could consider adding expensive low-value tests like these to some
alternate regression target that's only exercised by buildfarm members,
perhaps. But I think there's probably a point of diminishing returns
even in that context.

I'm not sure that the tests are "low value", because a commit that would
generate a failure on a permission check test would be a potential

security

issue for Pg.

As for the cost, if the proposed tests are indeed too costly, what is not
necessarily the case for what I have seen, I do not think that it would

be a

great problem to have two set of tests, with one a superset of the other,
with some convention.

Well, tests like permission tests aren't the expensive part. The actual
CREATE/DROP DATABASE you do is. The latter essentially are already
tested by the buildfarm already.
So, trimming the patch to do only the fast stuff should be less
controversial?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#8Fabien COELHO
fabien.coelho@mines-paristech.fr
In reply to: Andres Freund (#6)
Re: Add more regression tests for dbcommands

As for the cost, if the proposed tests are indeed too costly, what is not
necessarily the case for what I have seen, I do not think that it would be a
great problem to have two set of tests, with one a superset of the other,
with some convention.

Well, tests like permission tests aren't the expensive part. The actual
CREATE/DROP DATABASE you

Not me, but "Robins Tharakan".

do is. The latter essentially are already tested by the buildfarm
already.

Yep, that is what Tom was arguing.

So, trimming the patch to do only the fast stuff should be less
controversial?

Yes, I think so.

I think that allowing "expensive" tests such as a full archive or
replication setup would be nice as well, so maybe having several level of
tests would be a good thing anyway.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Robins Tharakan (#7)
Re: Add more regression tests for dbcommands

Would try to revert with a faster script (preferably with minimal
CREATE/DROP).

Yes. I just checked with a few create database/drop database. One cycle
takes about 1 full second on my laptop, so I must agree that it is slow.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Robins Tharakan
tharakan@gmail.com
In reply to: Robins Tharakan (#7)
1 attachment(s)
Re: Add more regression tests for dbcommands

Hi,

Attached is an updated patch that does only 1 CREATE DATABASE and reuses
that for all other tests.
The code-coverage with this patch goes up from 36% to 70%.

--
Robins Tharakan

On 13 May 2013 21:04, Robins Tharakan <tharakan@gmail.com> wrote:

Show quoted text

I believe Tom / Andres and Fabien all have valid points.

Net-net, I believe the tests although non-negotiable, are not required to
be in make-check. For now, its the slow tests that are the pain points
here, and then I would soon try to prune them and commit once again.

Whether it goes in make-check or not is obviously not discretion but to me
their importance is undoubted since I am sure they increase code-coverage.
Actually that is 'how' I create those tests, I look at untouched code and
create new tests that check untouched code.

Would try to revert with a faster script (preferably with minimal
CREATE/DROP).

--
Robins Tharakan

On 13 May 2013 20:30, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-05-13 16:52:08 +0200, Fabien COELHO wrote:

Hello,

Would you be okay if there is one/a few effective create/drop (some

tests

check that the create or drop fails e.g. depending on permissions,

which

ISTM is not tested anywhere else), so that tests for various ALTER
DATABASE commands are combined together onto these databases?

TBH, I do not see that such tests are worth adding, if they are going

to

significantly slow down the core regression tests. Those tests are run
probably hundreds of times a day, in aggregate across all Postgres
developers. Adding ten seconds or whatever this would add is a major
cost, while the benefit appears trivial.

We could consider adding expensive low-value tests like these to some
alternate regression target that's only exercised by buildfarm members,
perhaps. But I think there's probably a point of diminishing returns
even in that context.

I'm not sure that the tests are "low value", because a commit that would
generate a failure on a permission check test would be a potential

security

issue for Pg.

As for the cost, if the proposed tests are indeed too costly, what is

not

necessarily the case for what I have seen, I do not think that it would

be a

great problem to have two set of tests, with one a superset of the

other,

with some convention.

Well, tests like permission tests aren't the expensive part. The actual
CREATE/DROP DATABASE you do is. The latter essentially are already
tested by the buildfarm already.
So, trimming the patch to do only the fast stuff should be less
controversial?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

regress_db_v3.patchapplication/octet-stream; name=regress_db_v3.patchDownload
diff --git a/src/test/regress/expected/database.out b/src/test/regress/expected/database.out
new file mode 100644
index 0000000..518c941
--- /dev/null
+++ b/src/test/regress/expected/database.out
@@ -0,0 +1,158 @@
+--
+-- tests functions in dbcommands.c script
+--
+-- Should work. Check CREATE DATABASE. Use a valid CONNECTION LIMIT while at it.
+-- Instead of using a separate Database for each test, would be using this
+-- database in some tests below (as per feedback on email-thread)
+CREATE DATABASE db_db1 CONNECTION LIMIT 10;
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+ERROR:  database "db_db2" does not exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+NOTICE:  database "db_db2" does not exist, skipping
+DROP DATABASE template1;    -- can't drop a template database
+ERROR:  cannot drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+ERROR:  cannot drop the currently open database
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ERROR:  CREATE DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE db_db1;
+ERROR:  DROP DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should work. ALTER DATABASE RENAME TO
+ALTER DATABASE db_db1 RENAME TO db_db2;
+ALTER DATABASE db_db2 RENAME TO db_db1; -- For future tests
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1;   -- Database does not exist
+ERROR:  database "db_db1_nonexistent" does not exist
+ALTER DATABASE db_db1 RENAME TO db_db1; -- Database can't be renamed to itself
+ERROR:  database "db_db1" already exists
+--ALTER DATABASE db_db1 RENAME TO db_db1a;   -- Can't rename current database
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+SET ROLE rol_db7;
+ALTER DATABASE db_db1 RENAME TO db_db7;
+ERROR:  must be owner of database db_db1
+RESET ROLE;
+DROP ROLE rol_db7;
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE rol_db8;
+ALTER DATABASE db_db1 OWNER TO rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE db_db1 RENAME TO db_db8a;
+ERROR:  permission denied to rename database
+ROLLBACK;
+-- Should work. ALTER DATABASE RENAME TO with OWNER and CREATEDB privilege
+BEGIN TRANSACTION;
+CREATE ROLE rol_db9 CREATEDB;
+ALTER DATABASE db_db1 OWNER TO rol_db9;
+SET ROLE rol_db9;
+ALTER DATABASE db_db1 RENAME TO db_db9a;
+ROLLBACK;
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE db_db1 SET invalid_parameter;
+ERROR:  syntax error at or near ";"
+LINE 1: ALTER DATABASE db_db1 SET invalid_parameter;
+                                                   ^
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT = -100;
+ERROR:  invalid connection limit: -100
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT = 8ASDF;
+ERROR:  syntax error at or near "ASDF"
+LINE 1: ALTER DATABASE db_db1 WITH CONNECTION LIMIT = 8ASDF;
+                                                       ^
+ALTER DATABASE db_db1 SET TABLESPACE tablespace_does_not_exist;   -- invalid tablespace
+ERROR:  tablespace "tablespace_does_not_exist" does not exist
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+ERROR:  conflicting or redundant options
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  0;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  100;
+ALTER DATABASE db_db1 SET SEED = 0.5;
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE db_db1 SET TABLESPACE pg_default;
+ERROR:  ALTER DATABASE SET TABLESPACE cannot run inside a transaction block
+ROLLBACK;
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE rol_db15;
+CREATE ROLE rol_db14 IN ROLE rol_db15;  -- CREATE rol_db14 to be a member of rol_db15
+ALTER DATABASE db_db1 OWNER TO rol_db14;
+SET ROLE rol_db14;
+ALTER DATABASE db_db1 OWNER TO rol_db15;
+ERROR:  permission denied to change owner of database
+ROLLBACK;
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+ERROR:  invalid_encoding is not a valid encoding name
+CREATE DATABASE db_db18 ENCODING 123456789;
+ERROR:  123456789 is not a valid encoding code
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+ERROR:  invalid connection limit: -10
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+ERROR:  template database "invalid_template" does not exist
+CREATE DATABASE db_db1; -- DATABASE already exists
+ERROR:  database "db_db1" already exists
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 OWNER rol_db18a OWNER rol_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+ERROR:  conflicting or redundant options
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+ERROR:  permission denied to copy database "db_db19"
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+ALTER DATABASE db_db1 SET TABLESPACE tablespace_db16;
+ERROR:  must be owner of database db_db1
+CREATE DATABASE db_db21a;
+ERROR:  permission denied to create database
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+ERROR:  must be owner of database db_db21
+DROP DATABASE db_db21;
+ERROR:  must be owner of database db_db21
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+DROP DATABASE db_db22a;
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE db_db1 OWNER TO rol_db23_does_not_exist;
+ERROR:  role "rol_db23_does_not_exist" does not exist
+ROLLBACK;
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE rol_db24;
+ALTER DATABASE db_db1 OWNER TO rol_db24;
+ALTER DATABASE db_db1 OWNER TO rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
+-- Cleanup. DROP any test databases created
+DROP DATABASE db_db1;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 2af28b1..243f377 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -88,7 +88,7 @@ test: privileges security_label collate matview
 # ----------
 # Another group of parallel tests
 # ----------
-test: alter_generic misc psql
+test: alter_generic misc psql database
 
 # rules cannot run concurrently with any test that creates a view
 test: rules
diff --git a/src/test/regress/sql/database.sql b/src/test/regress/sql/database.sql
new file mode 100644
index 0000000..2a948c2
--- /dev/null
+++ b/src/test/regress/sql/database.sql
@@ -0,0 +1,141 @@
+--
+-- tests functions in dbcommands.c script
+--
+
+-- Should work. Check CREATE DATABASE. Use a valid CONNECTION LIMIT while at it.
+-- Instead of using a separate Database for each test, would be using this
+-- database in some tests below (as per feedback on email-thread)
+CREATE DATABASE db_db1 CONNECTION LIMIT 10;
+
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+DROP DATABASE template1;    -- can't drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ROLLBACK;
+
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE db_db1;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE RENAME TO
+ALTER DATABASE db_db1 RENAME TO db_db2;
+ALTER DATABASE db_db2 RENAME TO db_db1; -- For future tests
+
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1;   -- Database does not exist
+ALTER DATABASE db_db1 RENAME TO db_db1; -- Database can't be renamed to itself
+--ALTER DATABASE db_db1 RENAME TO db_db1a;   -- Can't rename current database
+
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+SET ROLE rol_db7;
+ALTER DATABASE db_db1 RENAME TO db_db7;
+RESET ROLE;
+DROP ROLE rol_db7;
+
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE rol_db8;
+ALTER DATABASE db_db1 OWNER TO rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE db_db1 RENAME TO db_db8a;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE RENAME TO with OWNER and CREATEDB privilege
+BEGIN TRANSACTION;
+CREATE ROLE rol_db9 CREATEDB;
+ALTER DATABASE db_db1 OWNER TO rol_db9;
+SET ROLE rol_db9;
+ALTER DATABASE db_db1 RENAME TO db_db9a;
+ROLLBACK;
+
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE db_db1 SET invalid_parameter;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT = -100;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT = 8ASDF;
+ALTER DATABASE db_db1 SET TABLESPACE tablespace_does_not_exist;   -- invalid tablespace
+
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  0;
+ALTER DATABASE db_db1 WITH CONNECTION LIMIT =  100;
+ALTER DATABASE db_db1 SET SEED = 0.5;
+
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE db_db1 SET TABLESPACE pg_default;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE rol_db15;
+CREATE ROLE rol_db14 IN ROLE rol_db15;  -- CREATE rol_db14 to be a member of rol_db15
+ALTER DATABASE db_db1 OWNER TO rol_db14;
+SET ROLE rol_db14;
+ALTER DATABASE db_db1 OWNER TO rol_db15;
+ROLLBACK;
+
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+CREATE DATABASE db_db18 ENCODING 123456789;
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+CREATE DATABASE db_db1; -- DATABASE already exists
+
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+CREATE DATABASE db_db18 OWNER rol_db18a OWNER rol_db18b;
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+ALTER DATABASE db_db1 SET TABLESPACE tablespace_db16;
+CREATE DATABASE db_db21a;
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+DROP DATABASE db_db21;
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+DROP DATABASE db_db22a;
+
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE db_db1 OWNER TO rol_db23_does_not_exist;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE rol_db24;
+ALTER DATABASE db_db1 OWNER TO rol_db24;
+ALTER DATABASE db_db1 OWNER TO rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
+
+-- Cleanup. DROP any test databases created
+DROP DATABASE db_db1;
#11Andres Freund
andres@2ndquadrant.com
In reply to: Robins Tharakan (#10)
Re: Add more regression tests for dbcommands

On 2013-05-21 02:58:25 +0530, Robins Tharakan wrote:

Attached is an updated patch that does only 1 CREATE DATABASE and reuses
that for all other tests.
The code-coverage with this patch goes up from 36% to 70%.

Even creating one database seems superfluous. The plain CREATE DATABASE
has been tested due to the creation of the regression database itself
anyway?
All the other tests should be doable using the normal regression db?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Robins Tharakan
tharakan@gmail.com
In reply to: Andres Freund (#11)
1 attachment(s)
Re: Add more regression tests for dbcommands

Hi Andres,

Attached is a patch which
does not CREATE DATABASE, but now the regression tests do not test the
following:

- ALTER DATABASE RENAME TO is not allowed on a database in use. Had to
remove two tests that were using this.

- ALTER DATABASE SET TABLESPACE is also not allowed on a database in use,
so removed it (the existing test was supposed to fail too, but it was to
fail at a later stage in the function when checking for whether the
tablespace exists, but with the 'regression' database, it just doesn't even
reach that part)

-
The CREATE DATABASE test itself was checking whether the 'CONNECTION LIMIT'
was working. Removed that as well.

Code coverage improved from 36% to 68%.

--
Robins Tharakan

On 24 June 2013 07:47, Andres Freund <andres@2ndquadrant.com> wrote:

Show quoted text

On 2013-05-21 02:58:25 +0530, Robins Tharakan wrote:

Attached is an updated patch that does only 1 CREATE DATABASE and reuses
that for all other tests.
The code-coverage with this patch goes up from 36% to 70%.

Even creating one database seems superfluous. The plain CREATE DATABASE
has been tested due to the creation of the regression database itself
anyway?
All the other tests should be doable using the normal regression db?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

regress_db_v4.patchapplication/octet-stream; name=regress_db_v4.patchDownload
diff --git a/src/test/regress/expected/database.out b/src/test/regress/expected/database.out
new file mode 100644
index 0000000..f6bb833
--- /dev/null
+++ b/src/test/regress/expected/database.out
@@ -0,0 +1,140 @@
+--
+-- tests functions in dbcommands.c script
+--
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+ERROR:  database "db_db2" does not exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+NOTICE:  database "db_db2" does not exist, skipping
+DROP DATABASE template1;    -- can't drop a template database
+ERROR:  cannot drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+ERROR:  cannot drop the currently open database
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ERROR:  CREATE DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE regression;
+ERROR:  DROP DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1a;   -- Database does not exist
+ERROR:  database "db_db1_nonexistent" does not exist
+ALTER DATABASE regression RENAME TO regression; -- Database can't be renamed to itself
+ERROR:  database "regression" already exists
+--ALTER DATABASE regression RENAME TO db_db1a;   -- Can't rename current database
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+SET ROLE rol_db7;
+ALTER DATABASE regression RENAME TO db_db7;
+ERROR:  must be owner of database regression
+RESET ROLE;
+DROP ROLE rol_db7;
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE rol_db8;
+ALTER DATABASE regression OWNER TO rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE regression RENAME TO db_db8a;
+ERROR:  permission denied to rename database
+ROLLBACK;
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE regression SET invalid_parameter;
+ERROR:  syntax error at or near ";"
+LINE 1: ALTER DATABASE regression SET invalid_parameter;
+                                                       ^
+ALTER DATABASE regression WITH CONNECTION LIMIT = -100;
+ERROR:  invalid connection limit: -100
+ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+ERROR:  syntax error at or near "ASDF"
+LINE 1: ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+                                                           ^
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE regression WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+ERROR:  conflicting or redundant options
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE regression WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  0;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  100;
+ALTER DATABASE regression SET SEED = 0.5;
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE regression SET TABLESPACE pg_default;
+ERROR:  ALTER DATABASE SET TABLESPACE cannot run inside a transaction block
+ROLLBACK;
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE rol_db15;
+CREATE ROLE rol_db14 IN ROLE rol_db15;  -- CREATE rol_db14 to be a member of rol_db15
+ALTER DATABASE regression OWNER TO rol_db14;
+SET ROLE rol_db14;
+ALTER DATABASE regression OWNER TO rol_db15;
+ERROR:  permission denied to change owner of database
+ROLLBACK;
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+ERROR:  invalid_encoding is not a valid encoding name
+CREATE DATABASE db_db18 ENCODING 123456789;
+ERROR:  123456789 is not a valid encoding code
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+ERROR:  invalid connection limit: -10
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+ERROR:  template database "invalid_template" does not exist
+CREATE DATABASE regression; -- DATABASE already exists
+ERROR:  database "regression" already exists
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 OWNER rol_db18a OWNER rol_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+ERROR:  conflicting or redundant options
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+ERROR:  permission denied to copy database "db_db19"
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+ALTER DATABASE regression SET TABLESPACE tablespace_db16;
+ERROR:  must be owner of database regression
+CREATE DATABASE db_db21a;
+ERROR:  permission denied to create database
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+ERROR:  must be owner of database db_db21
+DROP DATABASE db_db21;
+ERROR:  must be owner of database db_db21
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+DROP DATABASE db_db22a;
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE regression OWNER TO rol_db23_does_not_exist;
+ERROR:  role "rol_db23_does_not_exist" does not exist
+ROLLBACK;
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE rol_db24;
+ALTER DATABASE regression OWNER TO rol_db24;
+ALTER DATABASE regression OWNER TO rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 2af28b1..243f377 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -88,7 +88,7 @@ test: privileges security_label collate matview
 # ----------
 # Another group of parallel tests
 # ----------
-test: alter_generic misc psql
+test: alter_generic misc psql database
 
 # rules cannot run concurrently with any test that creates a view
 test: rules
diff --git a/src/test/regress/sql/database.sql b/src/test/regress/sql/database.sql
new file mode 100644
index 0000000..b911e89
--- /dev/null
+++ b/src/test/regress/sql/database.sql
@@ -0,0 +1,120 @@
+--
+-- tests functions in dbcommands.c script
+--
+
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+DROP DATABASE template1;    -- can't drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ROLLBACK;
+
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE regression;
+ROLLBACK;
+
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1a;   -- Database does not exist
+ALTER DATABASE regression RENAME TO regression; -- Database can't be renamed to itself
+--ALTER DATABASE regression RENAME TO db_db1a;   -- Can't rename current database
+
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE rol_db7;
+SET ROLE rol_db7;
+ALTER DATABASE regression RENAME TO db_db7;
+RESET ROLE;
+DROP ROLE rol_db7;
+
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE rol_db8;
+ALTER DATABASE regression OWNER TO rol_db8;
+SET ROLE rol_db8;
+ALTER DATABASE regression RENAME TO db_db8a;
+ROLLBACK;
+
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE regression SET invalid_parameter;
+ALTER DATABASE regression WITH CONNECTION LIMIT = -100;
+ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE regression WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE regression WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  0;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  100;
+ALTER DATABASE regression SET SEED = 0.5;
+
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE regression SET TABLESPACE pg_default;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE rol_db15;
+CREATE ROLE rol_db14 IN ROLE rol_db15;  -- CREATE rol_db14 to be a member of rol_db15
+ALTER DATABASE regression OWNER TO rol_db14;
+SET ROLE rol_db14;
+ALTER DATABASE regression OWNER TO rol_db15;
+ROLLBACK;
+
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+CREATE DATABASE db_db18 ENCODING 123456789;
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+CREATE DATABASE regression; -- DATABASE already exists
+
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+CREATE DATABASE db_db18 OWNER rol_db18a OWNER rol_db18b;
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE rol_db19;
+
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE rol_db21;
+ALTER DATABASE regression SET TABLESPACE tablespace_db16;
+CREATE DATABASE db_db21a;
+ALTER DATABASE db_db21 OWNER TO rol_db21;
+DROP DATABASE db_db21;
+RESET ROLE;
+DROP ROLE rol_db21;
+DROP DATABASE db_db21;
+
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+DROP DATABASE db_db22a;
+
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE regression OWNER TO rol_db23_does_not_exist;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE rol_db24;
+ALTER DATABASE regression OWNER TO rol_db24;
+ALTER DATABASE regression OWNER TO rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
#13Andres Freund
andres@2ndquadrant.com
In reply to: Robins Tharakan (#12)
Re: Add more regression tests for dbcommands

Hi,

I am generally a bit unsure whether the regression tests you propose
aren't a bit too verbose. Does any of the committers have an opinion
about this?

My feeling is that they are ok if they aren't slowing down things much.

On 2013-06-26 01:55:53 -0500, Robins Tharakan wrote:

The CREATE DATABASE test itself was checking whether the 'CONNECTION LIMIT'
was working. Removed that as well.

You should be able to test that by setting the connection limit to 1 and
then try to connect using \c. The old connection is only dropped after
the new one has been successfully performed.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#13)
Re: Add more regression tests for dbcommands

Andres Freund <andres@2ndquadrant.com> writes:

I am generally a bit unsure whether the regression tests you propose
aren't a bit too verbose. Does any of the committers have an opinion
about this?

My feeling is that they are ok if they aren't slowing down things much.

Yeah, I'm concerned about speed too. If the regression tests take a
long time it makes it harder for developers to run them. (I like to
point at mysql's regression tests, which take well over an hour even on
fast machines. If lots of tests are so helpful, why is their bug rate
no better than ours?)

I was intending to suggest that much of what Robins has submitted
doesn't belong in the core regression tests, but could usefully be put
into an optional set of "big" regression tests. We already have a
"numeric_big" test in that spirit. What seems to be lacking is an
organizational principle for this (should the "big" tests live with the
regular ones, or in a separate directory?) and a convenient "make"
target for running the big ones. Once we had a setup like that, we
could get some or all of the buildfarm machines to run the "big" tests,
but we wouldn't be penalizing development work.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#14)
Re: Add more regression tests for dbcommands

I was intending to suggest that much of what Robins has submitted
doesn't belong in the core regression tests, but could usefully be put
into an optional set of "big" regression tests. We already have a
"numeric_big" test in that spirit. What seems to be lacking is an
organizational principle for this (should the "big" tests live with the
regular ones, or in a separate directory?) and a convenient "make"
target for running the big ones. Once we had a setup like that, we
could get some or all of the buildfarm machines to run the "big" tests,
but we wouldn't be penalizing development work.

I have been suggesting something upon that line in some of the reviews
I've posted about Robins non regression tests, if they were to be rejected
on the basis that they add a few seconds for checks. They are well made to
test corner cases quite systematically, and I feel that it would be sad if
they were lost.

Looking at the GNUmakefile, ISTM that there could be a
{parallel,serial}_big_schedule derived from {parallel,serial}_schedule by
appending a few things in a separate file:

parallel_big_schedule: parallel_schedule parallel_big
cat $^ > $@

bigcheck: ... parallel_big_schedule
... --schedule=$(srcdir)/parallel_big_schedule ...

and idem for serial & bigtest.

Also, the serial_schedule should be automatically derived from the
parallel one, which does not seem to me the case. ISTM that currently
Robins patches only update the parallel version.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Josh Berkus
josh@agliodbs.com
In reply to: Robins Tharakan (#1)
Re: Add more regression tests for dbcommands

On 06/26/2013 12:08 PM, Fabien COELHO wrote:

I have been suggesting something upon that line in some of the reviews
I've posted about Robins non regression tests, if they were to be
rejected on the basis that they add a few seconds for checks. They are
well made to test corner cases quite systematically, and I feel that it
would be sad if they were lost.

My thinking was that someone should add all of his new tests at once,
and then see how much of a time difference they make. If it's 7
seconds, who cares?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#16)
Re: Add more regression tests for dbcommands

Josh Berkus <josh@agliodbs.com> writes:

On 06/26/2013 12:08 PM, Fabien COELHO wrote:

I have been suggesting something upon that line in some of the reviews
I've posted about Robins non regression tests, if they were to be
rejected on the basis that they add a few seconds for checks. They are
well made to test corner cases quite systematically, and I feel that it
would be sad if they were lost.

My thinking was that someone should add all of his new tests at once,
and then see how much of a time difference they make. If it's 7
seconds, who cares?

Making that measurement on the current set of tests doesn't seem to me
to prove much. I assume Robins' eventual goal is to make a significant
improvement in the tests' code coverage across the entire backend, and
what we see submitted now is just as much as he's been able to do yet
in that line. So even if the current cost is negligible, I don't think
it'll stay that way.

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

#18Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#17)
Re: Add more regression tests for dbcommands

On Wed, Jun 26, 2013 at 11:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Josh Berkus <josh@agliodbs.com> writes:

On 06/26/2013 12:08 PM, Fabien COELHO wrote:

I have been suggesting something upon that line in some of the reviews
I've posted about Robins non regression tests, if they were to be
rejected on the basis that they add a few seconds for checks. They are
well made to test corner cases quite systematically, and I feel that it
would be sad if they were lost.

My thinking was that someone should add all of his new tests at once,
and then see how much of a time difference they make. If it's 7
seconds, who cares?

Making that measurement on the current set of tests doesn't seem to me
to prove much. I assume Robins' eventual goal is to make a significant
improvement in the tests' code coverage across the entire backend, and
what we see submitted now is just as much as he's been able to do yet
in that line. So even if the current cost is negligible, I don't think
it'll stay that way.

Well, this is a discussion we've had before. I'm certainly in favor
of having a bigger set of regression tests, so that people can just
run the small suite if they want to. But I'm not keen on pushing
tests into that extended suite when they run in a fractions of a
second. That's likely to create more hassle than it solves, since
people will forget to update the expected outputs if they don't run
those tests, and if they do run those tests, then having them in a
separate suite isn't saving anything. The place for a separate test
suite, IMHO, is when you have tests that run for a long time
individually.

FWIW, internally to EDB, we have it broken down just about exactly
that way. Our core set of regression tests for PPAS takes about 5
minutes to run on my laptop, and unfortunately quite a bit longer on
some older laptops. It's a nuisance, but it's a manageable nuisance,
and it finds a lot of coding errors. Then we have separate schedules
for test suites that contain long-running tests or have dependencies
on which compiler flags you use; most developers don't run that suite
regularly, but it gets run every night. That keeps those annoyances
out of the foreground path of developers. I really don't see why we
shouldn't take the same approach here.

I don't think we want as large a regression test suite for PostgreSQL
as we have for PPAS, because among other things, PostgreSQL doesn't
have a paid staff of people who can be roped into helping manage it
when needed. But the management effort for the kind of expansion that
Robins Tharakan is proposing here is not going to be all that large,
at least if my EDB experience is any indication. And I think it will
help find bugs. I also think (and this is also something we've seen
internally) that part of the value of a regression suite is that it
exposes when you've changed the behavior. The regression tests fail
and you have to change the expected outputs; fine. But now it's much
less likely that you're going to make those kinds of changes without
*realizing* that you've changed the behavior. I don't have a specific
example top of mind right now, but I am pretty sure there have been at
least a few cases where PostgreSQL changes have had knock-on
consequences that weren't obvious at the time they were made. The
kind of work that Robins is doing here is not going to fix that
problem entirely, but I think it will help, and I think that has a lot
of value.

So I'd like to endorse Josh's idea: subject to appropriate review,
let's add these test cases. Then, if it really turns out to be too
burdensome, we can take them out, or figure out a sensible way to
split the suite. Pushing all of Robins work into a secondary suite,
or throwing it out altogether, both seem to me to be things which will
not be to the long-term benefit of the project.

--
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

#19Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#18)
Re: Add more regression tests for dbcommands

On 6/27/13 10:20 AM, Robert Haas wrote:

So I'd like to endorse Josh's idea: subject to appropriate review,
let's add these test cases. Then, if it really turns out to be too
burdensome, we can take them out, or figure out a sensible way to
split the suite. Pushing all of Robins work into a secondary suite,
or throwing it out altogether, both seem to me to be things which will
not be to the long-term benefit of the project.

I agree. If it gets too big, let's deal with it then. But let's not
make things more complicated because they *might* get too big later.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#14)
Re: Add more regression tests for dbcommands

On 6/26/13 12:17 PM, Tom Lane wrote:

(I like to
point at mysql's regression tests, which take well over an hour even on
fast machines. If lots of tests are so helpful, why is their bug rate
no better than ours?)

Tests are not (primarily) there to prevent bugs.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#20)
Re: Add more regression tests for dbcommands

Peter Eisentraut <peter_e@gmx.net> writes:

On 6/26/13 12:17 PM, Tom Lane wrote:

(I like to
point at mysql's regression tests, which take well over an hour even on
fast machines. If lots of tests are so helpful, why is their bug rate
no better than ours?)

Tests are not (primarily) there to prevent bugs.

Really? Pray tell, what do you think they're for? Particularly code
coverage tests, which is what these are?

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

#22Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#21)
Re: Add more regression tests for dbcommands

On 6/27/13 10:57 AM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On 6/26/13 12:17 PM, Tom Lane wrote:

(I like to
point at mysql's regression tests, which take well over an hour even on
fast machines. If lots of tests are so helpful, why is their bug rate
no better than ours?)

Tests are not (primarily) there to prevent bugs.

Really? Pray tell, what do you think they're for? Particularly code
coverage tests, which is what these are?

Tests document the existing functionality of the code, to facilitate
refactoring. (paraphrased, Uncle Bob)

Case in point, the existing ALTER DDL code could arguably use even more
refactoring. But no one wants to do it because it's unclear what will
break. With the proposed set of tests (which I have not read to
completion), this could become quite a bit easier, both for the coder
and the reviewer. But these tests probably won't detect, say, locking
bugs in such new code. That can only be prevented by careful code
review and a clean architecture. Perhaps, MySQL has neither. ;-)

Code coverage is not an end itself, it's a tool.

In this sense, tests prevent existing functionality being broken, which
might be classified as a bug. But it's wrong to infer that because
system X has a lot of bugs and a lot of tests, having a lot of tests
must be useless.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#23Kevin Grittner
kgrittn@ymail.com
In reply to: Peter Eisentraut (#19)
Re: Add more regression tests for dbcommands

Peter Eisentraut <peter_e@gmx.net> wrote:

On 6/27/13 10:20 AM, Robert Haas wrote:

So I'd like to endorse Josh's idea: subject to appropriate review,
let's add these test cases.  Then, if it really turns out to be too
burdensome, we can take them out, or figure out a sensible way to
split the suite.  Pushing all of Robins work into a secondary suite,
or throwing it out altogether, both seem to me to be things which will
not be to the long-term benefit of the project.

I agree.  If it gets too big, let's deal with it then.  But let's not
make things more complicated because they *might* get too big later.

+1

--
Kevin Grittner
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

#24Robins Tharakan
tharakan@gmail.com
In reply to: Andres Freund (#13)
Re: Add more regression tests for dbcommands

Hi Andres,

Just an aside, your point about CONNECTION LIMIT was something that just
didn't come to my mind and is probably a good way to test ALTER DATABASE
with CONNECTION LIMIT.

Its just that that actually wasn't what I was testing there. That
'CONNECTION LIMIT' test was coupled with CREATE DATABASE because I wanted
to test that 'branch' in the CREATE DATABASE function just to ensure that
there was a regression test that tests CONNECTION LIMIT specifically with
CREATE DATABASE. That's all. A check to confirm whether connection limit
restrictions actually got enforced was something I missed, but well, its
out of the window for now anyway.

--
Robins Tharakan

On 26 June 2013 06:34, Andres Freund <andres@2ndquadrant.com> wrote:

Show quoted text

Hi,

I am generally a bit unsure whether the regression tests you propose
aren't a bit too verbose. Does any of the committers have an opinion
about this?

My feeling is that they are ok if they aren't slowing down things much.

On 2013-06-26 01:55:53 -0500, Robins Tharakan wrote:

The CREATE DATABASE test itself was checking whether the 'CONNECTION

LIMIT'

was working. Removed that as well.

You should be able to test that by setting the connection limit to 1 and
then try to connect using \c. The old connection is only dropped after
the new one has been successfully performed.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#25Robins Tharakan
tharakan@gmail.com
In reply to: Robins Tharakan (#12)
1 attachment(s)
Re: Add more regression tests for dbcommands

On 26 June 2013 01:55, Robins Tharakan <tharakan@gmail.com> wrote:

Code coverage improved from 36% to 68%.

Updated patch:
- Renamed ROLEs as per Robert's feedback (prepend regress_xxx)
- Added test to serial_schedule (missed out earlier).

--
Robins Tharakan

Attachments:

regress_db_v5.patchapplication/octet-stream; name=regress_db_v5.patchDownload
diff --git a/src/test/regress/expected/database.out b/src/test/regress/expected/database.out
new file mode 100644
index 0000000..1f5e80e
--- /dev/null
+++ b/src/test/regress/expected/database.out
@@ -0,0 +1,140 @@
+--
+-- tests functions in dbcommands.c script
+--
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+ERROR:  database "db_db2" does not exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+NOTICE:  database "db_db2" does not exist, skipping
+DROP DATABASE template1;    -- can't drop a template database
+ERROR:  cannot drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+ERROR:  cannot drop the currently open database
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ERROR:  CREATE DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE regression;
+ERROR:  DROP DATABASE cannot run inside a transaction block
+ROLLBACK;
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1a;   -- Database does not exist
+ERROR:  database "db_db1_nonexistent" does not exist
+ALTER DATABASE regression RENAME TO regression; -- Database can't be renamed to itself
+ERROR:  database "regression" already exists
+--ALTER DATABASE regression RENAME TO db_db1a;   -- Can't rename current database
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE regress_rol_db7;
+SET ROLE regress_rol_db7;
+ALTER DATABASE regression RENAME TO db_db7;
+ERROR:  must be owner of database regression
+RESET ROLE;
+DROP ROLE regress_rol_db7;
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db8;
+ALTER DATABASE regression OWNER TO regress_rol_db8;
+SET ROLE regress_rol_db8;
+ALTER DATABASE regression RENAME TO db_db8a;
+ERROR:  permission denied to rename database
+ROLLBACK;
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE regression SET invalid_parameter;
+ERROR:  syntax error at or near ";"
+LINE 1: ALTER DATABASE regression SET invalid_parameter;
+                                                       ^
+ALTER DATABASE regression WITH CONNECTION LIMIT = -100;
+ERROR:  invalid connection limit: -100
+ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+ERROR:  syntax error at or near "ASDF"
+LINE 1: ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+                                                           ^
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE regression WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+ERROR:  conflicting or redundant options
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE regression WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  0;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  100;
+ALTER DATABASE regression SET SEED = 0.5;
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE regression SET TABLESPACE pg_default;
+ERROR:  ALTER DATABASE SET TABLESPACE cannot run inside a transaction block
+ROLLBACK;
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db15;
+CREATE ROLE regress_rol_db14 IN ROLE regress_rol_db15;  -- CREATE regress_rol_db14 to be a member of regress_rol_db15
+ALTER DATABASE regression OWNER TO regress_rol_db14;
+SET ROLE regress_rol_db14;
+ALTER DATABASE regression OWNER TO regress_rol_db15;
+ERROR:  permission denied to change owner of database
+ROLLBACK;
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+ERROR:  invalid_encoding is not a valid encoding name
+CREATE DATABASE db_db18 ENCODING 123456789;
+ERROR:  123456789 is not a valid encoding code
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+ERROR:  invalid connection limit: -10
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+ERROR:  template database "invalid_template" does not exist
+CREATE DATABASE regression; -- DATABASE already exists
+ERROR:  database "regression" already exists
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 OWNER regress_rol_db18a OWNER regress_rol_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+ERROR:  conflicting or redundant options
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+ERROR:  conflicting or redundant options
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE regress_rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE regress_rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+ERROR:  permission denied to copy database "db_db19"
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE regress_rol_db19;
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE regress_rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE regress_rol_db21;
+ALTER DATABASE regression SET TABLESPACE tablespace_db16;
+ERROR:  must be owner of database regression
+CREATE DATABASE db_db21a;
+ERROR:  permission denied to create database
+ALTER DATABASE db_db21 OWNER TO regress_rol_db21;
+ERROR:  must be owner of database db_db21
+DROP DATABASE db_db21;
+ERROR:  must be owner of database db_db21
+RESET ROLE;
+DROP ROLE regress_rol_db21;
+DROP DATABASE db_db21;
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+ERROR:  pg_global cannot be used as default tablespace
+DROP DATABASE db_db22a;
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE regression OWNER TO regress_rol_db23_does_not_exist;
+ERROR:  role "regress_rol_db23_does_not_exist" does not exist
+ROLLBACK;
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db24;
+ALTER DATABASE regression OWNER TO regress_rol_db24;
+ALTER DATABASE regression OWNER TO regress_rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 3e6b306..be426fa 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -88,7 +88,7 @@ test: privileges security_label collate matview
 # ----------
 # Another group of parallel tests
 # ----------
-test: alter_generic misc psql async
+test: alter_generic misc psql async database
 
 # rules cannot run concurrently with any test that creates a view
 test: rules
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 3ad289f..6c15500 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -139,3 +139,4 @@ test: largeobject
 test: with
 test: xml
 test: stats
+test: database
diff --git a/src/test/regress/sql/database.sql b/src/test/regress/sql/database.sql
new file mode 100644
index 0000000..4266dbf
--- /dev/null
+++ b/src/test/regress/sql/database.sql
@@ -0,0 +1,120 @@
+--
+-- tests functions in dbcommands.c script
+--
+
+-- Should fail. Check for cases when DROP DATABASE is not allowed
+DROP DATABASE db_db2;   -- doesn't exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+DROP DATABASE template1;    -- can't drop a template database
+DROP DATABASE regression;   -- can't drop a database in use
+
+-- Should fail. Can't CREATE DATABASE inside a transaction
+BEGIN TRANSACTION;
+CREATE DATABASE db_db3;
+ROLLBACK;
+
+-- Should fail. Can't DROP DATABASE inside a transaction
+BEGIN TRANSACTION;
+DROP DATABASE regression;
+ROLLBACK;
+
+-- Should fail. ALTER DATABASE RENAME TO on invalid DB
+ALTER DATABASE db_db1_nonexistent RENAME TO db_db1a;   -- Database does not exist
+ALTER DATABASE regression RENAME TO regression; -- Database can't be renamed to itself
+--ALTER DATABASE regression RENAME TO db_db1a;   -- Can't rename current database
+
+-- Should fail. ALTER DATABASE RENAME TO without permission
+CREATE ROLE regress_rol_db7;
+SET ROLE regress_rol_db7;
+ALTER DATABASE regression RENAME TO db_db7;
+RESET ROLE;
+DROP ROLE regress_rol_db7;
+
+-- Should fail. ALTER DATABASE RENAME TO with OWNER rights without CREATEDB
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db8;
+ALTER DATABASE regression OWNER TO regress_rol_db8;
+SET ROLE regress_rol_db8;
+ALTER DATABASE regression RENAME TO db_db8a;
+ROLLBACK;
+
+-- Should fail. ALTER DATABASE SET / WITH with invalid values
+ALTER DATABASE regression SET invalid_parameter;
+ALTER DATABASE regression WITH CONNECTION LIMIT = -100;
+ALTER DATABASE regression WITH CONNECTION LIMIT = 8ASDF;
+
+-- Should fail. ALTER DATABASE with redundant options
+ALTER DATABASE regression WITH CONNECTION LIMIT 1 CONNECTION LIMIT 2;
+
+-- Should work. ALTER DATABASE SET / WITH with valid values
+ALTER DATABASE regression WITH CONNECTION LIMIT =  -1;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  0;
+ALTER DATABASE regression WITH CONNECTION LIMIT =  100;
+ALTER DATABASE regression SET SEED = 0.5;
+
+-- Should fail. Can't set Tablespace within transaction
+BEGIN TRANSACTION;
+ALTER DATABASE regression SET TABLESPACE pg_default;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE OWNER TO requires CREATEDB rights for grantor
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db15;
+CREATE ROLE regress_rol_db14 IN ROLE regress_rol_db15;  -- CREATE regress_rol_db14 to be a member of regress_rol_db15
+ALTER DATABASE regression OWNER TO regress_rol_db14;
+SET ROLE regress_rol_db14;
+ALTER DATABASE regression OWNER TO regress_rol_db15;
+ROLLBACK;
+
+-- Should fail. CREATE DATABASE with invalid values
+CREATE DATABASE db_db18 ENCODING 'invalid_encoding';
+CREATE DATABASE db_db18 ENCODING 123456789;
+CREATE DATABASE db_db18 CONNECTION LIMIT -10;
+CREATE DATABASE db_db18 TEMPLATE invalid_template;
+CREATE DATABASE regression; -- DATABASE already exists
+
+-- Should fail. CREATE DATABASE with redundant options
+CREATE DATABASE db_db18 TABLESPACE tablespace_db18a TABLESPACE tablespace_db18b;
+CREATE DATABASE db_db18 OWNER regress_rol_db18a OWNER regress_rol_db18b;
+CREATE DATABASE db_db18 TEMPLATE template_db18a TEMPLATE template_db18b;
+CREATE DATABASE db_db18 ENCODING 'UTF8' ENCODING 'UTF8';
+CREATE DATABASE db_db18 CONNECTION LIMIT 10 CONNECTION LIMIT 10;
+
+-- Should fail. CREATE DATABASE using non-template DB as template without SUPERUSER
+CREATE ROLE regress_rol_db19 CREATEDB;
+CREATE DATABASE db_db19;
+SET ROLE regress_rol_db19;
+CREATE DATABASE db_db20 TEMPLATE db_db19;
+RESET ROLE;
+DROP DATABASE db_db19;
+DROP ROLE regress_rol_db19;
+
+-- Should fail. Trying to CREATE / ALTER / DROP DATABASE without rights
+CREATE ROLE regress_rol_db21;
+CREATE DATABASE db_db21;
+SET ROLE regress_rol_db21;
+ALTER DATABASE regression SET TABLESPACE tablespace_db16;
+CREATE DATABASE db_db21a;
+ALTER DATABASE db_db21 OWNER TO regress_rol_db21;
+DROP DATABASE db_db21;
+RESET ROLE;
+DROP ROLE regress_rol_db21;
+DROP DATABASE db_db21;
+
+-- Should fail. CREATE / ALTER DATABASE should not have pg_global as TABLESPACE
+CREATE DATABASE db_db22 TABLESPACE pg_global;
+CREATE DATABASE db_db22a;
+ALTER DATABASE db_db22a SET TABLESPACE pg_global;
+DROP DATABASE db_db22a;
+
+-- Should work. ALTER DATABASE SET with bogus OWNER
+BEGIN TRANSACTION;
+ALTER DATABASE regression OWNER TO regress_rol_db23_does_not_exist;
+ROLLBACK;
+
+-- Should work. ALTER DATABASE SET OWNER to self!
+BEGIN TRANSACTION;
+CREATE ROLE regress_rol_db24;
+ALTER DATABASE regression OWNER TO regress_rol_db24;
+ALTER DATABASE regression OWNER TO regress_rol_db24; -- Set OWNER TO previous OWNER
+ROLLBACK;
#26Robert Haas
robertmhaas@gmail.com
In reply to: Robins Tharakan (#25)
Re: Add more regression tests for dbcommands

On Sun, Jul 7, 2013 at 10:35 AM, Robins Tharakan <tharakan@gmail.com> wrote:

On 26 June 2013 01:55, Robins Tharakan <tharakan@gmail.com> wrote:

Code coverage improved from 36% to 68%.

Updated patch:
- Renamed ROLEs as per Robert's feedback (prepend regress_xxx)
- Added test to serial_schedule (missed out earlier).

Databases - like roles - are global objects, so those should be
similarly prefixed with "regress". This is very dangerous:

+DROP DATABASE db_db2;   -- doesn't exist
+DROP DATABASE IF EXISTS db_db2; -- doesn't exist with IF EXISTS;
+DROP DATABASE template1;    -- can't drop a template database
+DROP DATABASE regression;   -- can't drop a database in use

I think we should drop the first three of these. The first one risks
dropping actual user data in the "make installcheck" case, as does the
second one. We can reduce the risk of death and destruction by
choosing a better database name, but I don't think it's really worth
it. If DROP DATABASE gets broken, we'll notice that soon enough.

I don't think the third one is a good test, either. There's no
requirement that the user keep template1 around, although nearly
everyone probably does.

Please see if you can't also do something to minimize the number of
create/drop role cycles this patch performs - like maybe to just one.

--
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