From d56817f2fec1629767cb522a19ee32f20bb67451 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Tue, 10 Nov 2020 18:12:18 +0100 Subject: [PATCH] Fix: Rework sql type gathering to use OID instead of typname. This does not have the issue of name shadowing / qual-names, and has the added benefit of fixing #1948. 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 --- .../java/org/postgresql/core/TypeInfo.java | 2 + .../postgresql/jdbc/PgDatabaseMetaData.java | 8 +-- .../org/postgresql/jdbc/TypeInfoCache.java | 49 +++++++++++------- .../test/jdbc42/DatabaseMetaDataTest.java | 50 +++++++++++++++++++ 4 files changed, 88 insertions(+), 21 deletions(-) diff --git a/pgjdbc/src/main/java/org/postgresql/core/TypeInfo.java b/pgjdbc/src/main/java/org/postgresql/core/TypeInfo.java index 1bae8bed50..d8d7e9e740 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/TypeInfo.java +++ b/pgjdbc/src/main/java/org/postgresql/core/TypeInfo.java @@ -85,6 +85,8 @@ void addCoreType(String pgTypeName, Integer oid, Integer sqlType, String javaCla Iterator getPGTypeNamesWithSQLTypes(); + Iterator getPGTypeOidsWithSQLTypes(); + @Nullable Class getPGobject(String type); String getJavaClass(int oid) throws SQLException; diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java index e0ca8c8285..a0656ea71f 100644 --- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java +++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java @@ -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 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 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(); diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java b/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java index c05b3fd18f..86494eec49 100644 --- a/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java +++ b/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java @@ -38,6 +38,8 @@ public class TypeInfoCache implements TypeInfo { // pgname (String) -> java.sql.Types (Integer) private Map pgNameToSQLType; + private Map oidToSQLType; + // pgname (String) -> java class name (String) // ie "text" -> "java.lang.String" private Map pgNameToJavaClass; @@ -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((int) Math.round(types.length * 1.5))); + oidToSQLType = Collections.synchronizedMap(new HashMap((int) Math.round(types.length * 1.5))); for (Object[] type : types) { String pgTypeName = (String) type[0]; @@ -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 @@ -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)) { @@ -185,7 +190,11 @@ public Iterator getPGTypeNamesWithSQLTypes() { return pgNameToSQLType.keySet().iterator(); } - private String getSQLTypeQuery(boolean typnameParam) { + public Iterator 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. @@ -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 "); @@ -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(); @@ -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) { @@ -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) @@ -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 { diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc42/DatabaseMetaDataTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc42/DatabaseMetaDataTest.java index d1f81ee399..7a4c9071c3 100644 --- a/pgjdbc/src/test/java/org/postgresql/test/jdbc42/DatabaseMetaDataTest.java +++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc42/DatabaseMetaDataTest.java @@ -18,6 +18,7 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; +import java.sql.Types; public class DatabaseMetaDataTest { @@ -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); } @@ -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()); + } }