Fixes #1164 - Peloton connects to nonexistent database

[eribeiro]

[coveralls]

Coverage increased (+0.006%) to 76.973% when pulling 47f529d012784d4892f7e0644e3183cd5452fd89 on eribeiro:1164-inexistent-db into a9d83a04918bd8423bc218090b06bdd52291526d on cmu-db:master.

[schedutron]

@eribeiro Thanks for the PR, I just forgot to inform you about doing a separate PR as I’m busy with a conference this week. However, please fix the undeclared identifier error for client. Then the code should compile fine.

[ChTimTsubasa]

I would review this when this PR is ready.

[eribeiro]

@schedutron @ChTimTsubasa My bad! I was rebasing with current master late last night and didn't see this "little" detail. Thanks for pointing out.

update: gonna fix the new test asap and then I think it will be ready to review. Thanks!

[tcm-marcel]

@eribeiro Can you add the changes @ChTimTsubasa requested?

[eribeiro]

@tcm-marcel yes, gonna change the PR tomorrow. Thanks for the review @ChTimTsubasa!

[eribeiro]

@ChTimTsubasa ~Could you please give me some pointers about adding the JDBC test? Which file should I insert this new test? There's an automated way of testing the jdbc tests via CLI?~

I see the script test_jdbc.sh should be run with something along these lines: ./test_jdbc.sh PelotonBasicTest localhost 15721. It run, but looks like it doesn't execute the test.

[eribeiro]

Ok, I am gonna add those tests asap. Btw, I was able to get the a basic jdbc tests running. Some observations below when running with the following JDBC URL: jdbc:postgresql://localhost:<port>/foo

1. This is the stack trace on Postgres connecting to a non existent database (foo):

Exception in thread "main" org.postgresql.util.PSQLException: FATAL: database "foo" does not exist
	at org.postgresql.core.v3.ConnectionFactoryImpl.readStartupMessages(ConnectionFactoryImpl.java:730)
	at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:231)
	at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:66)
	at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:211)
	at org.postgresql.Driver.makeConnection(Driver.java:407)
	at org.postgresql.Driver.connect(Driver.java:275)
	at java.sql.DriverManager.getConnection(DriverManager.java:664)
	at java.sql.DriverManager.getConnection(DriverManager.java:247)
	at PelotonBasicTest.makeConnection(PelotonBasicTest.java:149)
	at PelotonBasicTest.<init>(PelotonBasicTest.java:144)
	at PelotonBasicTest.SingleTest(PelotonBasicTest.java:807)
	at PelotonBasicTest.main(PelotonBasicTest.java:801)

2. This is the stack trace of Peloton after the patch connecting to same non existent database:

Exception in thread "main" org.postgresql.util.PSQLException: FATAL: Database "foo" doesn't exist
	at org.postgresql.core.v3.ConnectionFactoryImpl.doAuthentication(ConnectionFactoryImpl.java:451)
	at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:223)
	at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:66)
	at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:211)
	at org.postgresql.Driver.makeConnection(Driver.java:407)
	at org.postgresql.Driver.connect(Driver.java:275)
	at java.sql.DriverManager.getConnection(DriverManager.java:664)
	at java.sql.DriverManager.getConnection(DriverManager.java:247)
	at PelotonBasicTest.makeConnection(PelotonBasicTest.java:149)
	at PelotonBasicTest.<init>(PelotonBasicTest.java:144)
	at PelotonBasicTest.SingleTest(PelotonBasicTest.java:807)
	at PelotonBasicTest.main(PelotonBasicTest.java:801)

• There was a code and message for severity ('S' - "FATAL") as described here: https://www.postgresql.org/docs/9.4/static/protocol-error-fields.html that I added on the lastest commit, while the "3D000" error code came from here: https://www.postgresql.org/docs/9.4/static/errcodes-appendix.html

I see that if I don't provide the database name after the slash (jdbc:postgresql://localhost:15721/) then Peloton returns a empty value for the database name. Any idea of why this is happening?

Exception in thread "main" org.postgresql.util.PSQLException: Database "" doesn't exist
	at org.postgresql.core.v3.ConnectionFactoryImpl.doAuthentication(ConnectionFactoryImpl.java:451)
	at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:223)
	at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:66)
	at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:211)
	at org.postgresql.Driver.makeConnection(Driver.java:407)
	at org.postgresql.Driver.connect(Driver.java:275)
	at java.sql.DriverManager.getConnection(DriverManager.java:664)
	at java.sql.DriverManager.getConnection(DriverManager.java:247)
	at PelotonBasicTest.makeConnection(PelotonBasicTest.java:149)
	at PelotonBasicTest.<init>(PelotonBasicTest.java:144)
	at PelotonBasicTest.SingleTest(PelotonBasicTest.java:807)
	at PelotonBasicTest.main(PelotonBasicTest.java:801)

[ChTimTsubasa]

The exception looks like that you are implementing the database check correctly. Can you try some test cases with "default_database", which is the default existing database?

[eribeiro]

@ChTimTsubasa Firstly, thanks for the patience and support. Well, I have added a quite simple test -- simple_connection_test.cpp -- in test/network directory. It tries two connections with a invalid and a valid database name. Please, let me know what I can improve on this file. There is a need for JDBC test?

But I must say that I was unable to register it as a valid test so I resorted to a quick-and-dirty hack of renaming to an already registered name and then I could test it. Could you tell me how can I register a new test file to execute via ctest?

Thanks!

[pervazea]

I don't understand the comment about "not being able to register a test". How did you try to add the new test?

[pmenon]

@eribeiro What is the status of this?

[edwardoliveira]

@pmenon Hi,

TL;DR: it is stalled. :(

The code works(ed), but it lacks the last (?) round of review since I've got swamped by work commitments.

I can try to resume work on it if it is already relevant, but if anyone else would like to adopt this PR feel free to do so, no problem. Afaik, it is almost ready to commit once the tests are in place and conflicts resolved. :)