You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2018/12/22 00:54:54 UTC

[geode] branch feature/GEODE-6225 updated: table meta data now figures out the catalog and schema based on the RegionMapping and on the jdbc meta data. This checkin is a WIP and still needs testing. TableMetaDataManager in particular needs unit tests added to the code that figures out the catalog and schema. Integration tests need to be added for mysql and postgres to TableMetaDataManagerIntegrationTest

This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch feature/GEODE-6225
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-6225 by this push:
     new 49f2d2d  table meta data now figures out the catalog and schema based on the RegionMapping and on the jdbc meta data. This checkin is a WIP and still needs testing. TableMetaDataManager in particular needs unit tests added to the code that figures out the catalog and schema. Integration tests need to be added for mysql and postgres to TableMetaDataManagerIntegrationTest
49f2d2d is described below

commit 49f2d2d04ede7aaebc78d02e3abc1c07e33c1761
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Fri Dec 21 16:48:29 2018 -0800

    table meta data now figures out the catalog and schema based
    on the RegionMapping and on the jdbc meta data.
    This checkin is a WIP and still needs testing.
    TableMetaDataManager in particular needs unit tests added
    to the code that figures out the catalog and schema.
    Integration tests need to be added for mysql and postgres
    to TableMetaDataManagerIntegrationTest
---
 .../connectors/jdbc/internal/TableMetaData.java    |  13 +-
 .../jdbc/internal/TableMetaDataManager.java        | 149 ++++++++++++++-------
 .../jdbc/internal/configuration/RegionMapping.java |   7 -
 .../jdbc/internal/RegionMappingTest.java           |  12 --
 .../connectors/jdbc/internal/SqlHandlerTest.java   |   1 -
 .../jdbc/internal/TableMetaDataManagerTest.java    |  14 +-
 6 files changed, 117 insertions(+), 79 deletions(-)

diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java
index 904acba..5020e12 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java
@@ -16,21 +16,22 @@
  */
 package org.apache.geode.connectors.jdbc.internal;
 
-import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 public class TableMetaData implements TableMetaDataView {
 
   private final String tableName;
   private final List<String> keyColumnNames;
-  private final HashMap<String, Integer> columnNameToTypeMap;
+  private final Map<String, Integer> columnNameToTypeMap;
   private final String identifierQuoteString;
 
-  public TableMetaData(String tableName, List<String> keyColumnNames, String quoteString) {
+  public TableMetaData(String tableName, List<String> keyColumnNames, String quoteString,
+      Map<String, Integer> dataTypes) {
     this.tableName = tableName;
     this.keyColumnNames = keyColumnNames;
-    this.columnNameToTypeMap = new HashMap<>();
+    this.columnNameToTypeMap = dataTypes;
     this.identifierQuoteString = quoteString;
   }
 
@@ -58,10 +59,6 @@ public class TableMetaData implements TableMetaDataView {
     return columnNameToTypeMap.keySet();
   }
 
-  public void addDataType(String columnName, int dataType) {
-    this.columnNameToTypeMap.put(columnName, dataType);
-  }
-
   @Override
   public String getIdentifierQuoteString() {
     return this.identifierQuoteString;
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java
index 89e2c6b..b3ed8aa 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java
@@ -20,7 +20,9 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -42,69 +44,121 @@ public class TableMetaDataManager {
 
   public TableMetaDataView getTableMetaDataView(Connection connection,
       RegionMapping regionMapping) {
-    return tableToMetaDataMap.computeIfAbsent(regionMapping.getTableName(),
-        k -> computeTableMetaDataView(connection, regionMapping));
+    return tableToMetaDataMap.computeIfAbsent(computeTableName(regionMapping),
+        k -> computeTableMetaDataView(connection, k, regionMapping));
+  }
+
+  /**
+   * If the region mapping has been given a table name then return it.
+   * Otherwise return the region mapping's region name as the table name.
+   */
+  private String computeTableName(RegionMapping regionMapping) {
+    String result = regionMapping.getTableName();
+    if (result == null) {
+      result = regionMapping.getRegionName();
+    }
+    return result;
   }
 
   private TableMetaDataView computeTableMetaDataView(Connection connection,
-      RegionMapping regionMapping) {
-    TableMetaData result;
+      String tableName, RegionMapping regionMapping) {
     try {
       DatabaseMetaData metaData = connection.getMetaData();
-      String catalogFilter = DEFAULT_CATALOG;
-      String schemaFilter = DEFAULT_SCHEMA;
-      if ("PostgreSQL".equals(metaData.getDatabaseProductName())) {
-        schemaFilter = "public";
-      }
-      try (ResultSet tables = metaData.getTables(catalogFilter, schemaFilter, "%", null)) {
-        String realTableName = getTableNameFromMetaData(regionMapping.getTableName(), tables);
-        List<String> keys = getPrimaryKeyColumnNamesFromMetaData(realTableName, metaData,
-            catalogFilter, schemaFilter, regionMapping.getIds());
-        String quoteString = metaData.getIdentifierQuoteString();
-        if (quoteString == null) {
-          quoteString = "";
-        }
-        result = new TableMetaData(realTableName, keys, quoteString);
-        getDataTypesFromMetaData(realTableName, metaData, catalogFilter, schemaFilter, result);
-      }
+      String catalogFilter = getCatalogNameFromMetaData(metaData, regionMapping);
+      String schemaFilter = getSchemaNameFromMetaData(metaData, regionMapping, catalogFilter);
+      String realTableName =
+          getTableNameFromMetaData(metaData, catalogFilter, schemaFilter, tableName);
+      List<String> keys = getPrimaryKeyColumnNamesFromMetaData(metaData, catalogFilter,
+          schemaFilter, realTableName, regionMapping.getIds());
+      String quoteString = getQuoteStringFromMetaData(metaData);
+      Map<String, Integer> dataTypes =
+          getDataTypesFromMetaData(metaData, catalogFilter, schemaFilter, realTableName);
+      return new TableMetaData(realTableName, keys, quoteString, dataTypes);
     } catch (SQLException e) {
       throw JdbcConnectorException.createException(e);
     }
-    return result;
   }
 
-  private String getTableNameFromMetaData(String tableName, ResultSet tables) throws SQLException {
-    String result = null;
-    int inexactMatches = 0;
-    int exactMatches = 0;
+  private String getQuoteStringFromMetaData(DatabaseMetaData metaData) throws SQLException {
+    String quoteString = metaData.getIdentifierQuoteString();
+    if (quoteString == null) {
+      quoteString = "";
+    }
+    return quoteString;
+  }
 
-    while (tables.next()) {
-      String name = tables.getString("TABLE_NAME");
-      if (name.equals(tableName)) {
-        exactMatches++;
-        result = name;
-      } else if (name.equalsIgnoreCase(tableName)) {
-        inexactMatches++;
-        result = name;
-      }
+  private String getCatalogNameFromMetaData(DatabaseMetaData metaData, RegionMapping regionMapping)
+      throws SQLException {
+    String catalogFilter = DEFAULT_CATALOG;
+    if (regionMapping.getCatalog() != null && regionMapping.getCatalog().length() > 0) {
+      catalogFilter = regionMapping.getCatalog();
+    }
+    if (catalogFilter.equals("")) {
+      return catalogFilter;
+    }
+    try (ResultSet catalogs = metaData.getCatalogs()) {
+      return findMatchInResultSet(catalogFilter, catalogs, "TABLE_CAT", "catalog");
     }
+  }
 
-    if (exactMatches == 1) {
-      return result;
+  private String getSchemaNameFromMetaData(DatabaseMetaData metaData, RegionMapping regionMapping,
+      String catalogFilter) throws SQLException {
+    String schemaFilter = DEFAULT_SCHEMA;
+    if (regionMapping.getSchema() != null && regionMapping.getSchema().length() > 0) {
+      schemaFilter = regionMapping.getSchema();
+    } else if ("PostgreSQL".equals(metaData.getDatabaseProductName())) {
+      schemaFilter = "public";
     }
+    if (schemaFilter.equals("")) {
+      return schemaFilter;
+    }
+    try (ResultSet schemas = metaData.getSchemas(catalogFilter, "%")) {
+      return findMatchInResultSet(schemaFilter, schemas, "TABLE_SCHEM", "schema");
+    }
+  }
 
-    if (inexactMatches > 1 || exactMatches > 1) {
-      throw new JdbcConnectorException("Duplicate tables that match region name");
+  private String getTableNameFromMetaData(DatabaseMetaData metaData, String catalogFilter,
+      String schemaFilter, String tableName) throws SQLException {
+    try (ResultSet tables = metaData.getTables(catalogFilter, schemaFilter, "%", null)) {
+      return findMatchInResultSet(tableName, tables, "TABLE_NAME", "table");
     }
+  }
 
-    if (result == null) {
-      throw new JdbcConnectorException("no table was found that matches " + tableName);
+  String findMatchInResultSet(String stringToFind, ResultSet resultSet, String column,
+      String description)
+      throws SQLException {
+    int exactMatches = 0;
+    String exactMatch = null;
+    int inexactMatches = 0;
+    String inexactMatch = null;
+    if (resultSet != null) {
+      while (resultSet.next()) {
+        String name = resultSet.getString(column);
+        if (name.equals(stringToFind)) {
+          exactMatches++;
+          exactMatch = name;
+        } else if (name.equalsIgnoreCase(stringToFind)) {
+          inexactMatches++;
+          inexactMatch = name;
+        }
+      }
     }
-    return result;
+    if (exactMatches == 1) {
+      return exactMatch;
+    }
+    if (inexactMatches > 1 || exactMatches > 1) {
+      throw new JdbcConnectorException(
+          "Multiple " + description + "s were found that match \"" + stringToFind + '"');
+    }
+    if (inexactMatches == 1) {
+      return inexactMatch;
+    }
+    throw new JdbcConnectorException(
+        "No " + description + " was found that matches \"" + stringToFind + '"');
   }
 
-  private List<String> getPrimaryKeyColumnNamesFromMetaData(String tableName,
-      DatabaseMetaData metaData, String catalogFilter, String schemaFilter,
+  private List<String> getPrimaryKeyColumnNamesFromMetaData(DatabaseMetaData metaData,
+      String catalogFilter, String schemaFilter, String tableName,
       String ids)
       throws SQLException {
     List<String> keys = new ArrayList<>();
@@ -131,16 +185,19 @@ public class TableMetaDataManager {
     return keys;
   }
 
-  private void getDataTypesFromMetaData(String tableName, DatabaseMetaData metaData,
-      String catalogFilter, String schemaFilter, TableMetaData result) throws SQLException {
+  private Map<String, Integer> getDataTypesFromMetaData(DatabaseMetaData metaData,
+      String catalogFilter,
+      String schemaFilter, String tableName) throws SQLException {
+    Map<String, Integer> result = new HashMap<>();
     try (ResultSet columnData =
         metaData.getColumns(catalogFilter, schemaFilter, tableName, "%")) {
       while (columnData.next()) {
         String columnName = columnData.getString("COLUMN_NAME");
         int dataType = columnData.getInt("DATA_TYPE");
-        result.addDataType(columnName, dataType);
+        result.put(columnName, dataType);
       }
     }
+    return result;
   }
 
   private void checkColumnExistsInTable(String tableName, DatabaseMetaData metaData,
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/configuration/RegionMapping.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/configuration/RegionMapping.java
index 8d19a88..cc668df 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/configuration/RegionMapping.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/configuration/RegionMapping.java
@@ -152,13 +152,6 @@ public class RegionMapping implements CacheElement {
     return tableName;
   }
 
-  public String getRegionToTableName() {
-    if (tableName == null) {
-      return regionName;
-    }
-    return tableName;
-  }
-
   public String getColumnNameForField(String fieldName, TableMetaDataView tableMetaDataView) {
     Set<String> columnNames = tableMetaDataView.getColumnNames();
     if (columnNames.contains(fieldName)) {
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
index 8e58448..cd29c4b 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
@@ -57,7 +57,6 @@ public class RegionMappingTest {
     assertThat(mapping.getRegionName()).isNull();
     assertThat(mapping.getDataSourceName()).isNull();
     assertThat(mapping.getPdxName()).isEqualTo("pdxClassName");
-    assertThat(mapping.getRegionToTableName()).isNull();
     assertThat(mapping.getIds()).isNull();
     assertThat(mapping.getCatalog()).isNull();
     assertThat(mapping.getSchema()).isNull();
@@ -70,16 +69,6 @@ public class RegionMappingTest {
     mapping = new RegionMapping(null, null, name, null, null, null, null);
 
     assertThat(mapping.getTableName()).isEqualTo(name);
-    assertThat(mapping.getRegionToTableName()).isEqualTo(name);
-  }
-
-  @Test
-  public void hasCorrectTableNameWhenRegionNameIsSet() {
-    mapping = new RegionMapping("regionName", null, "tableName", null, null, null, null);
-
-    assertThat(mapping.getRegionName()).isEqualTo("regionName");
-    assertThat(mapping.getTableName()).isEqualTo("tableName");
-    assertThat(mapping.getRegionToTableName()).isEqualTo("tableName");
   }
 
   @Test
@@ -87,7 +76,6 @@ public class RegionMappingTest {
     mapping = new RegionMapping(name, null, null, null, null, null, null);
 
     assertThat(mapping.getRegionName()).isEqualTo(name);
-    assertThat(mapping.getRegionToTableName()).isEqualTo(name);
   }
 
   @Test
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
index 5dd884a..fc8c861 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java
@@ -108,7 +108,6 @@ public class SqlHandlerTest {
     when(regionMapping.getRegionName()).thenReturn(REGION_NAME);
     when(regionMapping.getTableName()).thenReturn(TABLE_NAME);
     when(regionMapping.getIds()).thenReturn(IDS);
-    when(regionMapping.getRegionToTableName()).thenReturn(TABLE_NAME);
     when(connectorService.getMappingForRegion(REGION_NAME)).thenReturn(regionMapping);
 
     when(dataSource.getConnection()).thenReturn(this.connection);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java
index 4e0f244..e626150f 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java
@@ -95,14 +95,18 @@ public class TableMetaDataManagerTest {
   @Test
   public void verifyPostgreUsesPublicSchemaByDefault() throws Exception {
     setupCompositePrimaryKeysMetaData();
-    when(databaseMetaData.getDatabaseProductName()).thenReturn("PostgreSQL");
     when(regionMapping.getIds()).thenReturn("");
+    ResultSet schemas = mock(ResultSet.class);
+    when(schemas.next()).thenReturn(true).thenReturn(false);
+    when(schemas.getString("TABLE_SCHEM")).thenReturn("PUBLIC");
+    when(databaseMetaData.getSchemas(any(), any())).thenReturn(schemas);
+    when(databaseMetaData.getDatabaseProductName()).thenReturn("PostgreSQL");
 
     TableMetaDataView data = tableMetaDataManager.getTableMetaDataView(connection, regionMapping);
 
     assertThat(data.getKeyColumnNames()).isEqualTo(Arrays.asList(KEY_COLUMN, KEY_COLUMN2));
     verify(connection).getMetaData();
-    verify(databaseMetaData).getTables("", "public", "%", null);
+    verify(databaseMetaData).getTables("", "PUBLIC", "%", null);
   }
 
   @Test
@@ -246,7 +250,7 @@ public class TableMetaDataManagerTest {
     assertThatThrownBy(
         () -> tableMetaDataManager.getTableMetaDataView(connection, regionMapping))
             .isInstanceOf(JdbcConnectorException.class)
-            .hasMessage("no table was found that matches testTable");
+            .hasMessage("No table was found that matches \"" + TABLE_NAME + '"');
   }
 
   @Test
@@ -290,7 +294,7 @@ public class TableMetaDataManagerTest {
     assertThatThrownBy(
         () -> tableMetaDataManager.getTableMetaDataView(connection, regionMapping))
             .isInstanceOf(JdbcConnectorException.class)
-            .hasMessage("Duplicate tables that match region name");
+            .hasMessage("Multiple tables were found that match \"" + TABLE_NAME + '"');
   }
 
   @Test
@@ -304,7 +308,7 @@ public class TableMetaDataManagerTest {
     assertThatThrownBy(
         () -> tableMetaDataManager.getTableMetaDataView(connection, regionMapping))
             .isInstanceOf(JdbcConnectorException.class)
-            .hasMessage("Duplicate tables that match region name");
+            .hasMessage("Multiple tables were found that match \"" + TABLE_NAME + '"');
   }
 
   @Test