Repository: pgjdbc/pgjdbc (java)

Issue:
Add a setURL method to BaseDataSource
I have an application that runs in JBoss and works across H2, MySQL, Oracle and, now, PostgreSQL.  I leverage XA data sources because I have a single transaction that spans multiple data sources.

So far I've been able to parameterize my configuration in such a way that I can easily switch between H2, MySQL and Oracle.  The configuration in question is the JBoss XA data source configuration where I specify the URL to the database.  For my previous databases the following configuration works:

``` xml
<xa-datasource-property name="URL">${database.url}</xa-datasource-property>
```

where `${database.url}` is replaced by my database URL.

Unfortunately when I ran this for PostgreSQL, JBoss threw an exception:

> Caused by: java.lang.NoSuchMethodException: Method setURL not found

Searching around I found this old [JBoss forum](https://developer.jboss.org/thread/26776?tstart=0) post where they instead recommend configuring the driver like this:

``` xml
<xa-datasource-property name="ServerName">hostname</xa-datasource-property>
<xa-datasource-property name="PortNumber">5432</xa-datasource-property>
<xa-datasource-property name="DatabaseName">dbname</xa-datasource-property>
```

This works, however I'd rather use the more succinct URL-style configuration.  The [BaseDataSource](https://github.com/pgjdbc/pgjdbc/blob/master/org/postgresql/ds/common/BaseDataSource.java) class currently has a setUrl method.  As with [H2](http://www.h2database.com/javadoc/org/h2/jdbcx/JdbcDataSource.html#setURL_String) and [MySQL](http://www.javafind.net/gate.jsp?q=/library/16/mysql-connector-java-5.1.29/apidocs/com/mysql/jdbc/jdbc2/optional/MysqlDataSource.html#setURL%28java.lang.String%29), I would like to have a setURL method in BaseDataSource that calls setUrl.


Pull request:
feat: add setURL method to BaseDataSource
Add setURL method to BaseDateSource in order to follow convention used in other DBMS drivers.
Improves JBoss support, allowing to configure XA data source to switch between various DBMS
drivers. Most of vendors use uppercase 'URL' parameter, contrary to PostgreSQL.

Resolves #250. Previous solution was reverted due to issue with tests.

Patch:
diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 00000000..02d10616
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,31 @@
+# appveyor.yml
+init: 
+- ps: iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
+- ps: Add-Content "c:\program files\postgresql\9.6\data\postgresql.conf" "wal_level=logical"
+- ps: Add-Content "c:\program files\postgresql\9.6\data\postgresql.conf" "max_wal_senders=3"
+- ps: Add-Content "c:\program files\postgresql\9.6\data\postgresql.conf" "wal_keep_segments=10"
+- ps: Add-Content "c:\program files\postgresql\9.6\data\postgresql.conf" "wal_sender_timeout=5s"
+- ps: Add-Content "c:\program files\postgresql\9.6\data\postgresql.conf" "max_replication_slots=10"
+- ps: Add-Content "c:\program files\postgresql\9.6\data\pg_hba.conf" "host    replication     all             127.0.0.1/32            trust"
+
+services: 
+  - postgresql96
+
+before_build:
+  - SET PATH=C:\Program Files\PostgreSQL\9.6\bin\;%PATH% 
+  - SET PGUSER=postgres 
+  - SET PGPASSWORD=Password12! 
+  - createuser -U postgres test
+  - psql -U postgres -c "alter user test with password 'test'" postgres 
+  - createdb -U postgres -O test test
+
+build_script:
+  - mvn clean package -DskipTests
+
+test_script:
+  - echo redirect escape ^> foo.bar
+  - echo privilegedPassword=Password12!>c:\projects\pgjdbc\build.local.properties
+  - mvn package
+
+cache:
+  - C:\Users\appveyor\.m2
diff --git a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
index 41b01eca..2716aac5 100644
--- a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
+++ b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
@@ -1075,6 +1075,15 @@ public abstract class BaseDataSource implements CommonDataSource, Referenceable
     return url.toString();
   }
 
+  /**
+   * Generates a {@link DriverManager} URL from the other properties supplied.
+   *
+   * @return {@link DriverManager} URL from the other properties supplied
+   */
+  public String getURL() {
+    return getUrl();
+  }
+
   /**
    * Sets properties from a {@link DriverManager} URL.
    *
@@ -1089,6 +1098,16 @@ public abstract class BaseDataSource implements CommonDataSource, Referenceable
     }
   }
 
+  /**
+   * Sets properties from a {@link DriverManager} URL.
+   * Added to follow convention used in other DBMS.
+   *
+   * @param url properties to set
+   */
+  public void setURL(String url) {
+    setUrl(url);
+  }
+
   public String getProperty(String name) throws SQLException {
     PGProperty pgProperty = PGProperty.forName(name);
     if (pgProperty != null) {
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSetMetaData.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSetMetaData.java
index d075b236..cdf25f40 100644
--- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSetMetaData.java
+++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSetMetaData.java
@@ -8,6 +8,7 @@ package org.postgresql.jdbc;
 import org.postgresql.PGResultSetMetaData;
 import org.postgresql.core.BaseConnection;
 import org.postgresql.core.Field;
+import org.postgresql.core.ServerVersion;
 import org.postgresql.util.GT;
 import org.postgresql.util.JdbcBlackHole;
 import org.postgresql.util.LruCache;
@@ -233,9 +234,14 @@ public class PgResultSetMetaData implements ResultSetMetaData, PGResultSetMetaDa
 
     StringBuilder sql = new StringBuilder(
         "SELECT c.oid, a.attnum, a.attname, c.relname, n.nspname, "
-            + "a.attnotnull OR (t.typtype = 'd' AND t.typnotnull), "
-            + "pg_catalog.pg_get_expr(d.adbin, d.adrelid) LIKE '%nextval(%' "
-            + "FROM pg_catalog.pg_class c "
+            + "a.attnotnull OR (t.typtype = 'd' AND t.typnotnull), ");
+
+    if ( connection.haveMinimumServerVersion(ServerVersion.v10)) {
+      sql.append("a.attidentity != '' OR pg_catalog.pg_get_expr(d.adbin, d.adrelid) LIKE '%nextval(%' ");
+    } else {
+      sql.append("pg_catalog.pg_get_expr(d.adbin, d.adrelid) LIKE '%nextval(%' ");
+    }
+    sql.append( "FROM pg_catalog.pg_class c "
             + "JOIN pg_catalog.pg_namespace n ON (c.relnamespace = n.oid) "
             + "JOIN pg_catalog.pg_attribute a ON (c.oid = a.attrelid) "
             + "JOIN pg_catalog.pg_type t ON (a.atttypid = t.oid) "


Test patch:
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ResultSetMetaDataTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ResultSetMetaDataTest.java
index 39649b43..98f9b78c 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ResultSetMetaDataTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ResultSetMetaDataTest.java
@@ -10,9 +10,11 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import org.postgresql.PGResultSetMetaData;
+import org.postgresql.core.ServerVersion;
 import org.postgresql.jdbc.PreferQueryMode;
 import org.postgresql.test.TestUtil;
 
+import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Test;
 
@@ -38,6 +40,11 @@ public class ResultSetMetaDataTest extends BaseTest4 {
 
     TestUtil.dropSequence(conn, "serialtest_a_seq");
     TestUtil.dropSequence(conn, "serialtest_b_seq");
+
+    if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v10)) {
+      TestUtil.createTable(conn, "identitytest", "id int GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY");
+    }
+
     TestUtil.createTable(conn, "serialtest", "a serial, b bigserial, c int");
     TestUtil.createTable(conn, "alltypes",
         "bool boolean, i2 int2, i4 int4, i8 int8, num numeric(10,2), re real, fl float, ch char(3), vc varchar(3), tx text, d date, t time without time zone, tz time with time zone, ts timestamp without time zone, tsz timestamp with time zone, bt bytea");
@@ -52,6 +59,9 @@ public class ResultSetMetaDataTest extends BaseTest4 {
     TestUtil.dropTable(conn, "rsmd1");
     TestUtil.dropTable(conn, "timetest");
     TestUtil.dropTable(conn, "serialtest");
+    if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v10)) {
+      TestUtil.dropTable(conn, "identitytest");
+    }
     TestUtil.dropTable(conn, "alltypes");
     TestUtil.dropTable(conn, "sizetest");
     TestUtil.dropSequence(conn, "serialtest_a_seq");
@@ -248,6 +258,16 @@ public class ResultSetMetaDataTest extends BaseTest4 {
     assertEquals("rsmd1", rsmd.getColumnTypeName(1));
   }
 
+  @Test
+  public void testIdentityColumn() throws Exception {
+    assumeMinimumServerVersion(ServerVersion.v10);
+    assumePreparedStatementMetadataSupported();
+    PreparedStatement pstmt = conn.prepareStatement("SELECT id FROM identitytest");
+    ResultSet rs = pstmt.executeQuery();
+    ResultSetMetaData rsmd = pstmt.getMetaData();
+    Assert.assertTrue(rsmd.isAutoIncrement(1));
+  }
+
   private void assumePreparedStatementMetadataSupported() {
     Assume.assumeTrue("prepared statement metadata is not supported for simple protocol",
         preferQueryMode.compareTo(PreferQueryMode.EXTENDED_FOR_PREPARED) >= 0);
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/OptionalTestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/OptionalTestSuite.java
index fe59de08..00c5d1a0 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/OptionalTestSuite.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/OptionalTestSuite.java
@@ -17,6 +17,7 @@ import org.junit.runners.Suite;
 @RunWith(Suite.class)
 @Suite.SuiteClasses({SimpleDataSourceTest.class,
         SimpleDataSourceWithUrlTest.class,
+        SimpleDataSourceWithSetURLTest.class,
         ConnectionPoolTest.class,
         PoolingDataSourceTest.class,
         CaseOptimiserDataSourceTest.class})
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/SimpleDataSourceWithSetURLTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/SimpleDataSourceWithSetURLTest.java
new file mode 100644
index 00000000..f9bbd5a9
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/SimpleDataSourceWithSetURLTest.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.jdbc2.optional;
+
+import static org.junit.Assert.assertEquals;
+import static org.postgresql.Driver.parseURL;
+
+import org.postgresql.PGProperty;
+import org.postgresql.jdbc2.optional.SimpleDataSource;
+import org.postgresql.test.TestUtil;
+
+import org.junit.Test;
+
+import java.util.Properties;
+
+/**
+ * Performs the basic tests defined in the superclass. Just adds the configuration logic.
+ */
+public class SimpleDataSourceWithSetURLTest extends BaseDataSourceTest {
+  /**
+   * Creates and configures a new SimpleDataSource using setURL method.
+   */
+  @Override
+  protected void initializeDataSource() {
+    if (bds == null) {
+      bds = new SimpleDataSource();
+      bds.setURL(String.format("jdbc:postgresql://%s:%d/%s?prepareThreshold=%d&loggerLevel=%s", TestUtil.getServer(), TestUtil.getPort(), TestUtil.getDatabase(), TestUtil.getPrepareThreshold(),
+              TestUtil.getLogLevel()));
+      bds.setUser(TestUtil.getUser());
+      bds.setPassword(TestUtil.getPassword());
+      bds.setProtocolVersion(TestUtil.getProtocolVersion());
+    }
+  }
+
+  @Test
+  public void testGetURL() throws Exception {
+    con = getDataSourceConnection();
+
+    String url = bds.getURL();
+    Properties properties = parseURL(url, null);
+
+    assertEquals(TestUtil.getServer(), properties.getProperty(PGProperty.PG_HOST.getName()));
+    assertEquals(Integer.toString(TestUtil.getPort()), properties.getProperty(PGProperty.PG_PORT.getName()));
+    assertEquals(TestUtil.getDatabase(), properties.getProperty(PGProperty.PG_DBNAME.getName()));
+    assertEquals(Integer.toString(TestUtil.getPrepareThreshold()), properties.getProperty(PGProperty.PREPARE_THRESHOLD.getName()));
+    assertEquals(TestUtil.getLogLevel(), properties.getProperty(PGProperty.LOGGER_LEVEL.getName()));
+  }
+
+  @Test
+  public void testSetURL() throws Exception {
+    initializeDataSource();
+
+    assertEquals(TestUtil.getServer(), bds.getServerName());
+    assertEquals(TestUtil.getPort(), bds.getPortNumber());
+    assertEquals(TestUtil.getDatabase(), bds.getDatabaseName());
+    assertEquals(TestUtil.getPrepareThreshold(), bds.getPrepareThreshold());
+    assertEquals(TestUtil.getLogLevel(), bds.getLoggerLevel());
+  }
+}