Repository: googleapis/java-spanner-jdbc
Pull request #1375 (2023-09-28 09:05:38)

feat: support default schema and catalog for PostgreSQL databases
The JDBC driver would return the empty string as the current catalog and schema for PostgreSQL databases. This is incorrect, as the default catalog and schema for PostgreSQL is the database name and the 'public' schema.


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
index 6031afe..9248dac 100644
--- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
+++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcConnection.java
@@ -47,6 +47,7 @@ import java.util.Iterator;
 import java.util.Map;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
+import javax.annotation.Nonnull;
 
 /** Jdbc Connection class for Google Cloud Spanner */
 class JdbcConnection extends AbstractJdbcConnection {
@@ -394,31 +395,66 @@ class JdbcConnection extends AbstractJdbcConnection {
   @Override
   public void setCatalog(String catalog) throws SQLException {
     // This method could be changed to allow the user to change to another database.
-    // For now we only support setting an empty string in order to support frameworks
+    // For now, we only support setting the default catalog in order to support frameworks
     // and applications that set this when no catalog has been specified in the connection
     // URL.
     checkClosed();
-    JdbcPreconditions.checkArgument("".equals(catalog), "Only catalog \"\" is supported");
+    checkValidCatalog(catalog);
+  }
+
+  void checkValidCatalog(String catalog) throws SQLException {
+    String defaultCatalog = getDefaultCatalog();
+    JdbcPreconditions.checkArgument(
+        defaultCatalog.equals(catalog),
+        String.format("Only catalog %s is supported", defaultCatalog));
   }
 
   @Override
   public String getCatalog() throws SQLException {
     checkClosed();
-    return "";
+    return getDefaultCatalog();
+  }
+
+  @Nonnull
+  String getDefaultCatalog() {
+    switch (getDialect()) {
+      case POSTGRESQL:
+        String database = getConnectionOptions().getDatabaseName();
+        // It should not be possible that database is null, but it's better to be safe than sorry.
+        return database == null ? "" : database;
+      case GOOGLE_STANDARD_SQL:
+      default:
+        return "";
+    }
   }
 
   @Override
   public void setSchema(String schema) throws SQLException {
     checkClosed();
-    // Cloud Spanner does not support schemas, but does contain a pseudo 'empty string' schema that
-    // might be set by frameworks and applications that read the database metadata.
-    JdbcPreconditions.checkArgument("".equals(schema), "Only schema \"\" is supported");
+    checkValidSchema(schema);
+  }
+
+  void checkValidSchema(String schema) throws SQLException {
+    String defaultSchema = getDefaultSchema();
+    JdbcPreconditions.checkArgument(
+        defaultSchema.equals(schema), String.format("Only schema %s is supported", defaultSchema));
   }
 
   @Override
   public String getSchema() throws SQLException {
     checkClosed();
-    return "";
+    return getDefaultSchema();
+  }
+
+  @Nonnull
+  String getDefaultSchema() {
+    switch (getDialect()) {
+      case POSTGRESQL:
+        return "public";
+      case GOOGLE_STANDARD_SQL:
+      default:
+        return "";
+    }
   }
 
   @Override
diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java
index b9c7cb8..4463395 100644
--- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java
+++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java
@@ -777,11 +777,12 @@ class JdbcDatabaseMetaData extends AbstractJdbcWrapper implements DatabaseMetaDa
   }
 
   @Override
-  public ResultSet getCatalogs() {
+  public ResultSet getCatalogs() throws SQLException {
     return JdbcResultSet.of(
         ResultSets.forRows(
             Type.struct(StructField.of("TABLE_CAT", Type.string())),
-            Collections.singletonList(Struct.newBuilder().set("TABLE_CAT").to("").build())));
+            Collections.singletonList(
+                Struct.newBuilder().set("TABLE_CAT").to(getConnection().getCatalog()).build())));
   }
 
   @Override
@@ -1524,9 +1525,10 @@ class JdbcDatabaseMetaData extends AbstractJdbcWrapper implements DatabaseMetaDa
   @Override
   public ResultSet getSchemas(String catalog, String schemaPattern) throws SQLException {
     String sql = readSqlFromFile("DatabaseMetaData_GetSchemas.sql", connection.getDialect());
-    JdbcPreparedStatement statement =
-        prepareStatementReplaceNullWithAnyString(sql, catalog, schemaPattern);
-    return statement.executeQueryWithOptions(InternalMetadataQuery.INSTANCE);
+    try (JdbcPreparedStatement statement =
+        prepareStatementReplaceNullWithAnyString(sql, catalog, schemaPattern)) {
+      return statement.executeQueryWithOptions(InternalMetadataQuery.INSTANCE);
+    }
   }
 
   @Override


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
index 592d404..ff811f3 100644
--- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
+++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcConnectionTest.java
@@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.any;
@@ -700,34 +701,33 @@ public class JdbcConnectionTest {
     ConnectionOptions options = mockOptions();
     when(options.getDatabaseName()).thenReturn("test");
     try (JdbcConnection connection = createConnection(options)) {
-      // The connection should always return the empty string as the current catalog, as no other
+      // The connection should always return the default catalog as the current catalog, as no other
       // catalogs exist in the INFORMATION_SCHEMA.
-      assertThat(connection.getCatalog()).isEqualTo("");
+      // The default catalog is the empty string for GoogleSQL databases.
+      // The default catalog is the database name for PostgreSQL databases.
+      assertEquals(connection.getDefaultCatalog(), connection.getCatalog());
       // This should be allowed.
-      connection.setCatalog("");
-      try {
-        // This should cause an exception.
-        connection.setCatalog("other");
-        fail("missing expected exception");
-      } catch (JdbcSqlExceptionImpl e) {
-        assertThat(e.getCode()).isEqualTo(Code.INVALID_ARGUMENT);
-      }
+      connection.setCatalog(connection.getDefaultCatalog());
+      // This should cause an exception.
+      JdbcSqlExceptionImpl exception =
+          assertThrows(JdbcSqlExceptionImpl.class, () -> connection.setCatalog("other"));
+      assertEquals(Code.INVALID_ARGUMENT, exception.getCode());
     }
   }
 
   @Test
   public void testSchema() throws SQLException {
     try (JdbcConnection connection = createConnection(mockOptions())) {
-      assertThat(connection.getSchema()).isEqualTo("");
+      // The connection should always return the default schema as the current schema, as we
+      // currently do not support setting the connection to a different schema.
+      // The default schema is the empty string for GoogleSQL databases.
+      // The default schema is 'public' for PostgreSQL databases.
+      assertEquals(connection.getDefaultSchema(), connection.getSchema());
       // This should be allowed.
-      connection.setSchema("");
-      try {
-        // This should cause an exception.
-        connection.setSchema("other");
-        fail("missing expected exception");
-      } catch (JdbcSqlExceptionImpl e) {
-        assertThat(e.getCode()).isEqualTo(Code.INVALID_ARGUMENT);
-      }
+      connection.setSchema(connection.getDefaultSchema());
+      JdbcSqlExceptionImpl exception =
+          assertThrows(JdbcSqlExceptionImpl.class, () -> connection.setSchema("other"));
+      assertEquals(Code.INVALID_ARGUMENT, exception.getCode());
     }
   }
 
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java
index 6a2b0ca..9782e96 100644
--- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java
+++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java
@@ -41,10 +41,19 @@ import java.sql.Types;
 import java.util.Objects;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public class JdbcDatabaseMetaDataTest {
+  @Parameter public Dialect dialect;
+
+  @Parameters(name = "dialect = {0}")
+  public static Object[] data() {
+    return Dialect.values();
+  }
+
   private static final String DEFAULT_CATALOG = "";
   private static final String DEFAULT_SCHEMA = "";
   private static final String TEST_TABLE = "FOO";
@@ -314,10 +323,12 @@ public class JdbcDatabaseMetaDataTest {
   @Test
   public void testGetCatalogs() throws SQLException {
     JdbcConnection connection = mock(JdbcConnection.class);
+    when(connection.getDialect()).thenReturn(dialect);
+    when(connection.getCatalog()).thenCallRealMethod();
     DatabaseMetaData meta = new JdbcDatabaseMetaData(connection);
     try (ResultSet rs = meta.getCatalogs()) {
       assertThat(rs.next(), is(true));
-      assertThat(rs.getString("TABLE_CAT"), is(equalTo("")));
+      assertThat(rs.getString("TABLE_CAT"), is(equalTo(connection.getDefaultCatalog())));
       assertThat(rs.next(), is(false));
       ResultSetMetaData rsmd = rs.getMetaData();
       assertThat(rsmd.getColumnCount(), is(equalTo(1)));
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcDatabaseMetaDataTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcDatabaseMetaDataTest.java
index 7e2481a..820bdfa 100644
--- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcDatabaseMetaDataTest.java
+++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcDatabaseMetaDataTest.java
@@ -822,6 +822,7 @@ public class ITJdbcDatabaseMetaDataTest extends ITAbstractJdbcTest {
   @Test
   public void testGetSchemas() throws SQLException {
     try (Connection connection = createConnection(env, database)) {
+      assertEquals("", connection.getSchema());
       try (ResultSet rs = connection.getMetaData().getSchemas()) {
         assertThat(rs.next(), is(true));
         assertThat(rs.getString("TABLE_SCHEM"), is(equalTo(DEFAULT_SCHEMA)));
@@ -829,11 +830,22 @@ public class ITJdbcDatabaseMetaDataTest extends ITAbstractJdbcTest {
         assertThat(rs.next(), is(true));
         assertThat(rs.getString("TABLE_SCHEM"), is(equalTo("INFORMATION_SCHEMA")));
         assertThat(rs.getString("TABLE_CATALOG"), is(equalTo(DEFAULT_CATALOG)));
-        if (!EmulatorSpannerHelper.isUsingEmulator()) {
-          assertThat(rs.next(), is(true));
-          assertThat(rs.getString("TABLE_SCHEM"), is(equalTo("SPANNER_SYS")));
-          assertThat(rs.getString("TABLE_CATALOG"), is(equalTo(DEFAULT_CATALOG)));
-        }
+        assertThat(rs.next(), is(true));
+        assertThat(rs.getString("TABLE_SCHEM"), is(equalTo("SPANNER_SYS")));
+        assertThat(rs.getString("TABLE_CATALOG"), is(equalTo(DEFAULT_CATALOG)));
+        assertFalse(rs.next());
+      }
+    }
+  }
+
+  @Test
+  public void testGetCatalogs() throws SQLException {
+    try (Connection connection = createConnection(env, database)) {
+      assertEquals("", connection.getCatalog());
+      try (ResultSet rs = connection.getMetaData().getCatalogs()) {
+        assertTrue(rs.next());
+        assertEquals("", rs.getString("TABLE_CAT"));
+
         assertFalse(rs.next());
       }
     }
diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPgDatabaseMetaDataTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPgDatabaseMetaDataTest.java
index ff0bff7..56d75db 100644
--- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPgDatabaseMetaDataTest.java
+++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPgDatabaseMetaDataTest.java
@@ -752,6 +752,7 @@ public class ITJdbcPgDatabaseMetaDataTest extends ITAbstractJdbcTest {
   @Test
   public void testGetSchemas() throws SQLException {
     try (Connection connection = createConnection(env, database)) {
+      assertEquals("public", connection.getSchema());
       try (ResultSet rs = connection.getMetaData().getSchemas()) {
         assertTrue(rs.next());
         assertEquals(getDefaultCatalog(database), rs.getString("TABLE_CATALOG"));
@@ -774,6 +775,19 @@ public class ITJdbcPgDatabaseMetaDataTest extends ITAbstractJdbcTest {
     }
   }
 
+  @Test
+  public void testGetCatalogs() throws SQLException {
+    try (Connection connection = createConnection(env, database)) {
+      assertEquals(database.getId().getDatabase(), connection.getCatalog());
+      try (ResultSet rs = connection.getMetaData().getCatalogs()) {
+        assertTrue(rs.next());
+        assertEquals(database.getId().getDatabase(), rs.getString("TABLE_CAT"));
+
+        assertFalse(rs.next());
+      }
+    }
+  }
+
   private static final class Table {
     private final String name;
     private final String type;