Skip to content

Commit

Permalink
Fix: Rework sql type gathering to use OID instead of typname. This do…
Browse files Browse the repository at this point in the history 
…es not have the issue of name shadowing / qual-names, and has the added benefit of fixing #1948. (#1949)

Types that are not on the search path (e.g. they are shadowed, or in a schema that is not included in the search path) are stored in the caches as fully qualified type names and OIDs. As we cannot easily query the pg_type catalog using qualified type names, we replace the pgTypeName with the oid of the type name to query properties of the type.

Testcases are added to improve coverage of correctly detecting SQL types that are not on the path, but are available through OID or qualified lookup. These types are stored internally as a fully qualified type, but we cannot use this name for lookup in pg_type.

Special consideration has been given to Oid.UNSPECIFIED, as that needs to be mapped to Types.OTHER without first hitting the database. That mapping is static, but is not in `types` because it is not an actual type.

Fixes #1948

Co-authored-by: Matthias van de Meent <matthias.vandemeent@cofano.nl>
  • Loading branch information
@MMeent
MMeent and Matthias van de Meent committed Nov 11, 2020
1 parent 18cb3f6 commit 375cb37
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 21 deletions.
2 changes: 2 additions & 0 deletions pgjdbc/src/main/java/org/postgresql/core/TypeInfo.java
View file
Expand Up @@ -85,6 +85,8 @@ void addCoreType(String pgTypeName, Integer oid, Integer sqlType, String javaCla

Iterator<String> getPGTypeNamesWithSQLTypes();

Iterator<Integer> getPGTypeOidsWithSQLTypes();

@Nullable Class<? extends PGobject> getPGobject(String type);

String getJavaClass(int oid) throws SQLException;
Expand Down
8 changes: 4 additions & 4 deletions pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java
View file
Expand Up @@ -2608,10 +2608,10 @@ public ResultSet getUDTs(@Nullable String catalog, @Nullable String schemaPatter
+ "as remarks, CASE WHEN t.typtype = 'd' then (select CASE";

StringBuilder sqlwhen = new StringBuilder();
for (Iterator<String> i = connection.getTypeInfo().getPGTypeNamesWithSQLTypes(); i.hasNext(); ) {
String pgType = i.next();
int sqlType = connection.getTypeInfo().getSQLType(pgType);
sqlwhen.append(" when typname = ").append(escapeQuotes(pgType)).append(" then ").append(sqlType);
for (Iterator<Integer> i = connection.getTypeInfo().getPGTypeOidsWithSQLTypes(); i.hasNext(); ) {
Integer typOid = i.next();
int sqlType = connection.getTypeInfo().getSQLType(typOid);
sqlwhen.append(" when t.oid = ").append(typOid).append(" then ").append(sqlType);
}
sql += sqlwhen.toString();

Expand Down
49 changes: 32 additions & 17 deletions pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java
View file
Expand Up @@ -38,6 +38,8 @@ public class TypeInfoCache implements TypeInfo {
// pgname (String) -> java.sql.Types (Integer)
private Map<String, Integer> pgNameToSQLType;

private Map<Integer, Integer> oidToSQLType;

// pgname (String) -> java class name (String)
// ie "text" -> "java.lang.String"
private Map<String, String> pgNameToJavaClass;
Expand Down Expand Up @@ -132,6 +134,7 @@ public TypeInfoCache(BaseConnection conn, int unknownLength) {
// needs to be synchronized because the iterator is returned
// from getPGTypeNamesWithSQLTypes()
pgNameToSQLType = Collections.synchronizedMap(new HashMap<String, Integer>((int) Math.round(types.length * 1.5)));
oidToSQLType = Collections.synchronizedMap(new HashMap<Integer, Integer>((int) Math.round(types.length * 1.5)));

for (Object[] type : types) {
String pgTypeName = (String) type[0];
Expand All @@ -153,6 +156,7 @@ public synchronized void addCoreType(String pgTypeName, Integer oid, Integer sql
oidToPgName.put(oid, pgTypeName);
pgArrayToPgType.put(arrayOid, oid);
pgNameToSQLType.put(pgTypeName, sqlType);
oidToSQLType.put(oid, sqlType);

// Currently we hardcode all core types array delimiter
// to a comma. In a stock install the only exception is
Expand All @@ -165,6 +169,7 @@ public synchronized void addCoreType(String pgTypeName, Integer oid, Integer sql
String pgArrayTypeName = pgTypeName + "[]";
pgNameToJavaClass.put(pgArrayTypeName, "java.sql.Array");
pgNameToSQLType.put(pgArrayTypeName, Types.ARRAY);
oidToSQLType.put(arrayOid, Types.ARRAY);
pgNameToOid.put(pgArrayTypeName, arrayOid);
pgArrayTypeName = "_" + pgTypeName;
if (!pgNameToJavaClass.containsKey(pgArrayTypeName)) {
Expand All @@ -185,7 +190,11 @@ public Iterator<String> getPGTypeNamesWithSQLTypes() {
return pgNameToSQLType.keySet().iterator();
}

private String getSQLTypeQuery(boolean typnameParam) {
public Iterator<Integer> getPGTypeOidsWithSQLTypes() {
return oidToSQLType.keySet().iterator();
}

private String getSQLTypeQuery(boolean typoidParam) {
// There's no great way of telling what's an array type.
// People can name their own types starting with _.
// Other types use typelem that aren't actually arrays, like box.
Expand All @@ -196,7 +205,7 @@ private String getSQLTypeQuery(boolean typnameParam) {
// (keeping old behaviour of finding types, that should not be found without correct search
// path)
StringBuilder sql = new StringBuilder();
sql.append("SELECT typinput='array_in'::regproc as is_array, typtype, typname ");
sql.append("SELECT typinput='array_in'::regproc as is_array, typtype, typname, pg_type.oid ");
sql.append(" FROM pg_catalog.pg_type ");
sql.append(" LEFT JOIN (select ns.oid as nspoid, ns.nspname, r.r ");
sql.append(" from pg_namespace as ns ");
Expand All @@ -206,8 +215,8 @@ private String getSQLTypeQuery(boolean typnameParam) {
sql.append(" using ( nspname ) ");
sql.append(" ) as sp ");
sql.append(" ON sp.nspoid = typnamespace ");
if (typnameParam) {
sql.append(" WHERE typname = ? ");
if (typoidParam) {
sql.append(" WHERE pg_type.oid = ? ");
}
sql.append(" ORDER BY sp.r, pg_type.oid DESC;");
return sql.toString();
Expand Down Expand Up @@ -256,14 +265,15 @@ public void cacheSQLTypes() throws SQLException {
if (!pgNameToSQLType.containsKey(typeName)) {
pgNameToSQLType.put(typeName, type);
}

Integer typeOid = castNonNull(rs.getInt("oid"));
if (!oidToSQLType.containsKey(typeOid)) {
oidToSQLType.put(typeOid, type);
}
}
rs.close();
}

public int getSQLType(int oid) throws SQLException {
return getSQLType(castNonNull(getPGType(oid)));
}

private PreparedStatement prepareGetTypeInfoStatement() throws SQLException {
PreparedStatement getTypeInfoStatement = this.getTypeInfoStatement;
if (getTypeInfoStatement == null) {
Expand All @@ -274,19 +284,24 @@ private PreparedStatement prepareGetTypeInfoStatement() throws SQLException {
}

public synchronized int getSQLType(String pgTypeName) throws SQLException {
if (pgTypeName.endsWith("[]")) {
return Types.ARRAY;
return getSQLType(castNonNull(getPGType(pgTypeName)));
}

public synchronized int getSQLType(int typeOid) throws SQLException {
if (typeOid == Oid.UNSPECIFIED) {
return Types.OTHER;
}
Integer i = pgNameToSQLType.get(pgTypeName);

Integer i = oidToSQLType.get(typeOid);
if (i != null) {
return i;
}

LOGGER.log(Level.FINEST, "querying SQL typecode for pg type '{0}'", pgTypeName);
LOGGER.log(Level.FINEST, "querying SQL typecode for pg type oid '{0}'", typeOid);

PreparedStatement getTypeInfoStatement = prepareGetTypeInfoStatement();

getTypeInfoStatement.setString(1, pgTypeName);
getTypeInfoStatement.setInt(1, typeOid);

// Go through BaseStatement to avoid transaction start.
if (!((BaseStatement) getTypeInfoStatement)
Expand All @@ -296,14 +311,14 @@ public synchronized int getSQLType(String pgTypeName) throws SQLException {

ResultSet rs = castNonNull(getTypeInfoStatement.getResultSet());

int type = Types.OTHER;
int sqlType = Types.OTHER;
if (rs.next()) {
type = getSQLTypeFromQueryResult(rs);
sqlType = getSQLTypeFromQueryResult(rs);
}
rs.close();

pgNameToSQLType.put(pgTypeName, type);
return type;
oidToSQLType.put(typeOid, sqlType);
return sqlType;
}

private PreparedStatement getOidStatement(String pgTypeName) throws SQLException {
Expand Down
50 changes: 50 additions & 0 deletions pgjdbc/src/test/java/org/postgresql/test/jdbc42/DatabaseMetaDataTest.java
View file
Expand Up @@ -18,6 +18,7 @@
import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
import java.sql.Types;

public class DatabaseMetaDataTest {

Expand All @@ -26,12 +27,22 @@ public class DatabaseMetaDataTest {
@Before
public void setUp() throws Exception {
conn = TestUtil.openDB();
TestUtil.createSchema(conn, "test_schema");
TestUtil.createEnumType(conn, "test_schema.test_enum", "'val'");
TestUtil.createTable(conn, "test_schema.off_path_table", "var test_schema.test_enum[]");
TestUtil.createEnumType(conn, "_test_enum", "'evil'");
TestUtil.createEnumType(conn, "test_enum", "'other'");
TestUtil.createTable(conn, "on_path_table", "a test_schema.test_enum[], b _test_enum, c test_enum[]");
TestUtil.createTable(conn, "decimaltest", "a decimal, b decimal(10, 5)");
}

@After
public void tearDown() throws Exception {
TestUtil.dropTable(conn, "decimaltest");
TestUtil.dropTable(conn, "on_path_table");
TestUtil.dropType(conn, "test_enum");
TestUtil.dropType(conn, "_test_enum");
TestUtil.dropSchema(conn, "test_schema");
TestUtil.closeDB(conn);
}

Expand All @@ -52,4 +63,43 @@ public void testGetColumnsForNullScale() throws Exception {

assertTrue(!rs.next());
}

@Test
public void testGetCorrectSQLTypeForOffPathTypes() throws Exception {
DatabaseMetaData dbmd = conn.getMetaData();

ResultSet rs = dbmd.getColumns("%", "%", "off_path_table", "%");
assertTrue(rs.next());
assertEquals("var", rs.getString("COLUMN_NAME"));
assertEquals("Detects correct off-path type name", "\"test_schema\".\"_test_enum\"", rs.getString("TYPE_NAME"));
assertEquals("Detects correct SQL type for off-path types", Types.ARRAY, rs.getInt("DATA_TYPE"));

assertFalse(rs.next());
}

@Test
public void testGetCorrectSQLTypeForShadowedTypes() throws Exception {
DatabaseMetaData dbmd = conn.getMetaData();

ResultSet rs = dbmd.getColumns("%", "%", "on_path_table", "%");

assertTrue(rs.next());
assertEquals("a", rs.getString("COLUMN_NAME"));
assertEquals("Correctly maps types from other schemas","\"test_schema\".\"_test_enum\"", rs.getString("TYPE_NAME"));
assertEquals(Types.ARRAY, rs.getInt("DATA_TYPE"));

assertTrue(rs.next());
assertEquals("b", rs.getString("COLUMN_NAME"));
// = TYPE _test_enum AS ENUM ('evil')
assertEquals( "_test_enum", rs.getString("TYPE_NAME"));
assertEquals(Types.VARCHAR, rs.getInt("DATA_TYPE"));

assertTrue(rs.next());
assertEquals("c", rs.getString("COLUMN_NAME"));
// = array of TYPE test_enum AS ENUM ('value')
assertEquals("Correctly detects shadowed array type name","___test_enum", rs.getString("TYPE_NAME"));
assertEquals("Correctly detects type of shadowed name", Types.ARRAY, rs.getInt("DATA_TYPE"));

assertFalse(rs.next());
}
}

0 comments on commit 375cb37

Please sign in to comment.