diff --git a/api/src/org/labkey/api/data/dialect/BackslashEscapingStringHandler.java b/api/src/org/labkey/api/data/dialect/BackslashEscapingStringHandler.java index 150c37a827d..342f6f32541 100644 --- a/api/src/org/labkey/api/data/dialect/BackslashEscapingStringHandler.java +++ b/api/src/org/labkey/api/data/dialect/BackslashEscapingStringHandler.java @@ -17,6 +17,9 @@ package org.labkey.api.data.dialect; import org.apache.commons.lang3.Strings; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.data.SQLFragment; // Adds support for backslash escaping in string literals public class BackslashEscapingStringHandler extends StandardDialectStringHandler @@ -56,4 +59,25 @@ else if (c == '\'') return current; } + + public static abstract class BackslashEscapingStringHandlerTestCase extends Assert + { + protected abstract SqlDialect getSqlDialect(); + + @Test + public void testAppendLiteral() + { + SqlDialect dialect = getSqlDialect(); + testAppendLiteral(dialect, "\\", "'\\\\'"); + testAppendLiteral(dialect, "C:\\Users\\Name\\Documents", "'C:\\\\Users\\\\Name\\\\Documents'"); + testAppendLiteral(dialect, "What's the buzz, tell me what's happening", "'What''s the buzz, tell me what''s happening'"); + testAppendLiteral(dialect, "\\d+", "'\\\\d+'"); + } + + private void testAppendLiteral(SqlDialect dialect , String literal, String expected) + { + String actual = new SQLFragment().appendStringLiteral(literal, dialect).toDebugString(dialect); + assertEquals(expected, actual); + } + } } diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 1bfa267bfb4..61ad4f9ffba 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -21,14 +21,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.collections.CaseInsensitiveMapWrapper; -import org.labkey.api.collections.CopyOnWriteHashMap; import org.labkey.api.collections.CsvSet; import org.labkey.api.collections.Sets; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.ConnectionWrapper; import org.labkey.api.data.ConnectionWrapper.Closer; import org.labkey.api.data.DatabaseIdentifier; -import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.LabKeyDataSource; import org.labkey.api.data.ExceptionFramework; @@ -37,16 +35,13 @@ import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.Selector; import org.labkey.api.data.SqlExecutingSelector.ConnectionFactory; import org.labkey.api.data.SqlExecutor; -import org.labkey.api.data.SqlSelector; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.dialect.LimitRowsSqlGenerator.LimitRowsCustomizer; import org.labkey.api.data.dialect.LimitRowsSqlGenerator.StandardLimitRowsCustomizer; import org.labkey.api.exp.PropertyType; -import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.view.template.Warnings; @@ -71,49 +66,20 @@ // if not, put it in PostgreSql92Dialect. public abstract class BasePostgreSqlDialect extends SqlDialect { - // Issue 52190: Expose troubleshooting data that supports postgreSQL-specific analysis + // Issue 52190: Expose troubleshooting data that supports postgreSQL-specific analysis. These names are also used + // by org.labkey.api.util.DebugInfoDumper, so they must stay in the api module even though the queries they back + // (see PostgreSql92Dialect) are Postgres-only, not Redshift. public static final String POSTGRES_SCHEMA_NAME = "postgres"; public static final String POSTGRES_STAT_ACTIVITY_TABLE_NAME = "pg_stat_activity"; public static final String POSTGRES_LOCKS_TABLE_NAME = "pg_locks"; public static final String POSTGRES_TABLE_SIZES_TABLE_NAME = "pg_tablesizes"; - private final Map _domainScaleMap = new CopyOnWriteHashMap<>(); - private HtmlString _adminWarning = null; // Default to 9 and let newer versions be refreshed later private int _majorVersion = 9; - // Specifies if this PostgreSQL server treats backslashes in string literals as normal characters (as per the SQL - // standard) or as escape characters (old, non-standard behavior). As of PostgreSQL 9.1, the setting - // standard_conforming_strings is on by default; before 9.1, it was off by default. We check the server setting - // when we prepare a new DbScope and use this when we escape and parse string literals. - private Boolean _standardConformingStrings = Boolean.TRUE; - private PostgreSqlServerType _serverType = PostgreSqlServerType.PostgreSQL; - - public boolean getStandardConformingStrings() - { - // make sure we're not calling this before finishing instance init - assert _standardConformingStrings != null; - return _standardConformingStrings == null || _standardConformingStrings; - } - - public void setStandardConformingStrings(boolean standardConformingStrings) - { - _standardConformingStrings = standardConformingStrings; - } - - public PostgreSqlServerType getServerType() - { - return _serverType; - } - - public void setServerType(PostgreSqlServerType serverType) - { - _serverType = serverType; - } - @Override protected @NotNull Set getReservedWords() { @@ -136,12 +102,6 @@ public StatementWrapper getStatementWrapper(ConnectionWrapper conn, Statement st return statementWrapper; } - @Override - public SQLFragment getDatabaseSizeSql(String databaseName) - { - return new SQLFragment("SELECT pg_database_size(?)", databaseName); - } - @Override public boolean cancelQueries(DbScope scope, Collection connections, boolean terminate) { @@ -458,12 +418,6 @@ public SQLFragment wrapBooleanExpression(SQLFragment booleanSql) return booleanSql; } - @Override - protected String getSystemTableNames() - { - return "pg_logdir_ls"; - } - @Override public boolean isSystemSchema(String schemaName) { @@ -617,69 +571,11 @@ public void prepare(LabKeyDataSource dataSource) } } - @Override - public String prepare(DbScope scope) - { - initializeUserDefinedTypes(scope); - determineSettings(scope); - return super.prepare(scope); - } - @Override public void prepareConnection(Connection conn) { } - // When a new PostgreSQL DbScope is created, we enumerate the domains (user-defined types) in the public schema - // of the datasource, determine their "scale," and stash that information in a map associated with the DbScope. - // When the PostgreSQLColumnMetaDataReader reads metadata, it returns these scale values for all domains. - private void initializeUserDefinedTypes(DbScope scope) - { - // Skip domains query if connecting to LabKey Server - it has no user-defined types - if (getServerType().supportsSpecialMetadataQueries()) - { - Selector selector = new SqlSelector(scope, "SELECT * FROM information_schema.domains"); - selector.forEach(rs -> { - String schemaName = rs.getString("domain_schema"); - String domainName = rs.getString("domain_name"); - String dataType = rs.getString("data_type"); - int scale; - - if (dataType.startsWith("character")) - { - String maxLength = rs.getString("character_maximum_length"); - - // VARCHAR with no specific size has null maxLength... but character_octet_length seems okay - scale = Integer.parseInt(null != maxLength ? maxLength : rs.getString("character_octet_length")); - } - else - { - // Assume everything else is an integer for now. We should support more types for better external schema handling. - scale = 4; - } - - String key = getDomainKey(schemaName, domainName); - _domainScaleMap.put(key, scale); - }); - } - } - - private String getDomainKey(String schemaName, String domainName) - { - // Domain names are returned from column metadata fully qualified and quoted, so save them that way. See #26149. - return ("public".equals(schemaName) ? domainName : "\"" + schemaName + "\".\"" + domainName + "\""); - } - - // Query any settings that may affect dialect behavior. Right now, only "standard_conforming_strings". - protected void determineSettings(DbScope scope) - { - if (getServerType().supportsSpecialMetadataQueries()) - { - Selector selector = new SqlSelector(scope, "SELECT setting FROM pg_settings WHERE name = 'standard_conforming_strings'"); - _standardConformingStrings = "on".equalsIgnoreCase(selector.getObject(String.class)); - } - } - /** * Wrap one or more INSERT statements to allow explicit specification * of values for autoincrementing columns (e.g. IDENTITY in SQL Server @@ -842,7 +738,6 @@ public boolean isEditable() @Override public ColumnMetaDataReader getColumnMetaDataReader(ResultSet rsCols, TableInfo table) { - // Retrieve and pass in the previously queried scale values for this scope. return new PostgreSqlColumnMetaDataReader(rsCols, table); } @@ -994,9 +889,9 @@ public SQLFragment isNumericExpr(SQLFragment expression) .append(") AS TEXT) ~ '^[+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)$' THEN 1 ELSE 0 END)"); } - private class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader + public static class PostgreSqlColumnMetaDataReader extends ColumnMetaDataReader { - private final TableInfo _table; + protected final TableInfo _table; public PostgreSqlColumnMetaDataReader(ResultSet rsCols, TableInfo table) { @@ -1036,39 +931,6 @@ public int getSqlType() throws SQLException return sqlType; } - @Override - public int getScale() throws SQLException - { - int sqlType = super.getSqlType(); - - return Types.DISTINCT == sqlType ? getDomainScale(getSqlTypeName()) : super.getScale(); - } - - private int getDomainScale(String domainName) throws SQLException - { - Integer scale = _domainScaleMap.get(domainName); - - if (null == scale) - { - // Some domain wasn't there when we initialized the datasource, so reload now. This will happen at bootstrap. - DbSchema schema = _table.getSchema(); - initializeUserDefinedTypes(schema.getScope()); - scale = _domainScaleMap.get(domainName); - - // If scale is still null, then we have a problem. We've seen occasional exception reports showing this, - // but haven't had the information to track it down... so log additional info. - if (null == scale) - { - String message = "Null scale for \"" + domainName + "\" in column \"" + _table.getName() + "." + getName() + "\" in schema \"" + schema.getName() + "\""; - ExceptionUtil.logExceptionToMothership(null, new Exception(message)); - assert false : message; - return 4; // Return something on production servers so schema can continue to load - } - } - - return scale; - } - @Nullable @Override public String getDefault() throws SQLException @@ -1084,17 +946,6 @@ public PkMetaDataReader getPkMetaDataReader(ResultSet rs) return new PkMetaDataReader(rs, "COLUMN_NAME", "KEY_SEQ"); } - @Override - public String getExtraInfo(SQLException e) - { - // Deadlock between two different DB connections - if ("40P01".equals(e.getSQLState())) - { - return getOtherDatabaseThreads(); - } - return null; - } - @Override public ConnectionFactory getConnectionFactory(boolean useJdbcCaching, boolean selfContained, DbScope scope, SQLFragment sql) { @@ -1280,20 +1131,6 @@ public void addAdminWarningMessages(Warnings warnings, boolean showAllWarnings) warnings.add(_adminWarning); } - @Override - public boolean isProcedureExists(DbScope scope, String schema, String name) - { - // Don't bother querying LabKey for stored procedures - return getServerType().supportsSpecialMetadataQueries() && super.isProcedureExists(scope, schema, name); - } - - @Override - public boolean shouldTest() - { - // Don't test a LabKey data source - return getServerType().shouldTest(); - } - @Override public @Nullable String getApplicationNameParameter() { @@ -1311,10 +1148,4 @@ public boolean shouldTest() { return "PostgreSQL JDBC Driver"; } - - @Override - public @NotNull String getApplicationConnectionsSql() - { - return "SELECT pid, usename, client_addr, client_hostname, xact_start, query_start, state, application_name, query FROM pg_stat_activity WHERE pid <> pg_backend_pid() AND datname = ? AND application_name = ?"; - } } diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 9fa85397ff4..0db60369654 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -485,7 +485,8 @@ protected DialectStringHandler createStringHandler() return new StandardDialectStringHandler(); } - public synchronized DialectStringHandler getStringHandler() + // Override createStringHandler() instead + public final synchronized DialectStringHandler getStringHandler() { if (null == _stringHandler) _stringHandler = createStringHandler(); diff --git a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java index 9ca0dd81a1a..2c217c50058 100644 --- a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java +++ b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java @@ -19,6 +19,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.jetbrains.annotations.NotNull; +import org.labkey.api.collections.CopyOnWriteHashMap; import org.labkey.api.data.Constraint; import org.labkey.api.data.CoreSchema; import org.labkey.api.data.DatabaseIdentifier; @@ -40,6 +41,7 @@ import org.labkey.api.data.TempTableTracker; import org.labkey.api.data.dialect.BackslashEscapingStringHandler; import org.labkey.api.data.dialect.BasePostgreSqlDialect; +import org.labkey.api.data.dialect.ColumnMetaDataReader; import org.labkey.api.data.dialect.DialectStringHandler; import org.labkey.api.data.dialect.JdbcHelper; import org.labkey.api.data.dialect.SqlDialect; @@ -47,6 +49,7 @@ import org.labkey.api.exp.PropertyType; import org.labkey.api.query.AliasManager; import org.labkey.api.util.ConfigurationException; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HtmlString; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.view.template.Warnings; @@ -58,6 +61,7 @@ import java.sql.Driver; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Types; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -90,6 +94,128 @@ abstract class PostgreSql92Dialect extends BasePostgreSqlDialect private final TempTableInClauseGenerator _tempTableInClauseGenerator = new TempTableInClauseGenerator(); private final AtomicBoolean _arraySortFunctionExists = new AtomicBoolean(false); + // Specifies if this PostgreSQL server treats backslashes in string literals as normal characters (as per the SQL + // standard) or as escape characters (old, non-standard behavior). As of PostgreSQL 9.1, the setting + // standard_conforming_strings is on by default; before 9.1, it was off by default. We check the server setting + // when we prepare a new DbScope and use this when we escape and parse string literals. + private Boolean _standardConformingStrings = Boolean.TRUE; + private PostgreSqlServerType _serverType = PostgreSqlServerType.PostgreSQL; + private final Map _domainScaleMap = new CopyOnWriteHashMap<>(); + + public boolean getStandardConformingStrings() + { + // This should always be set before prior to getting the value + if (_standardConformingStrings == null) + throw new IllegalStateException("Standard Conforming Strings are not set for " + this); + return _standardConformingStrings; + } + + @Override + protected DialectStringHandler createStringHandler() + { + // TODO: Should we look at "backslash_quote" setting instead/in addition? + if (getStandardConformingStrings()) + return super.createStringHandler(); + else + return new BackslashEscapingStringHandler(); + } + + public void setStandardConformingStrings(boolean standardConformingStrings) + { + _standardConformingStrings = standardConformingStrings; + } + + public PostgreSqlServerType getServerType() + { + return _serverType; + } + + public void setServerType(PostgreSqlServerType serverType) + { + _serverType = serverType; + } + + // When a new PostgreSQL DbScope is created, we enumerate the domains (user-defined types) in the public schema + // of the datasource, determine their "scale," and stash that information in a map associated with the DbScope. + // When the PostgreSQLColumnMetaDataReader reads metadata, it returns these scale values for all domains. + private void initializeUserDefinedTypes(DbScope scope) + { + // Skip domains query if connecting to LabKey Server - it has no user-defined types + if (getServerType().supportsSpecialMetadataQueries()) + { + Selector selector = new SqlSelector(scope, "SELECT * FROM information_schema.domains"); + selector.forEach(rs -> { + String schemaName = rs.getString("domain_schema"); + String domainName = rs.getString("domain_name"); + String dataType = rs.getString("data_type"); + int scale; + + if (dataType.startsWith("character")) + { + String maxLength = rs.getString("character_maximum_length"); + + // VARCHAR with no specific size has null maxLength... but character_octet_length seems okay + scale = Integer.parseInt(null != maxLength ? maxLength : rs.getString("character_octet_length")); + } + else + { + // Assume everything else is an integer for now. We should support more types for better external schema handling. + scale = 4; + } + + String key = getDomainKey(schemaName, domainName); + _domainScaleMap.put(key, scale); + }); + } + } + + private String getDomainKey(String schemaName, String domainName) + { + // Domain names are returned from column metadata fully qualified and quoted, so save them that way. See #26149. + return ("public".equals(schemaName) ? domainName : "\"" + schemaName + "\".\"" + domainName + "\""); + } + + @Override + public ColumnMetaDataReader getColumnMetaDataReader(ResultSet rsCols, TableInfo table) + { + // Subclass that supports PostgreSQL-specific domains / user-defined types + return new PostgreSqlColumnMetaDataReader(rsCols, table) + { + @Override + public int getScale() throws SQLException + { + int sqlType = super.getSqlType(); + + return Types.DISTINCT == sqlType ? getDomainScale(getSqlTypeName()) : super.getScale(); + } + + private int getDomainScale(String domainName) throws SQLException + { + Integer scale = _domainScaleMap.get(domainName); + + if (null == scale) + { + // Some domain wasn't there when we initialized the datasource, so reload now. This will happen at bootstrap. + DbSchema schema = _table.getSchema(); + initializeUserDefinedTypes(schema.getScope()); + scale = _domainScaleMap.get(domainName); + + // If scale is still null, then we have a problem. We've seen occasional exception reports showing this, + // but haven't had the information to track it down... so log additional info. + if (null == scale) + { + String message = "Null scale for \"" + domainName + "\" in column \"" + _table.getName() + "." + getName() + "\" in schema \"" + schema.getName() + "\""; + ExceptionUtil.logExceptionToMothership(null, new Exception(message)); + assert false : message; + return 4; // Return something on production servers so schema can continue to load + } + } + + return scale; + } + }; + } + @Override public void handleCreateDatabaseException(SQLException e) throws ServletException { @@ -136,6 +262,8 @@ public String prepare(DbScope scope) { initializeInClauseGenerator(scope); determineIfArraySortFunctionExists(scope); + initializeUserDefinedTypes(scope); + determineSettings(scope); return super.prepare(scope); } @@ -164,13 +292,42 @@ public String getProductName() return PRODUCT_NAME; } - // Query PostgreSQL-specific settings @Override + protected String getSystemTableNames() + { + return "pg_logdir_ls"; + } + + @Override + public SQLFragment getDatabaseSizeSql(String databaseName) + { + return new SQLFragment("SELECT pg_database_size(?)", databaseName); + } + + @Override + public String getExtraInfo(SQLException e) + { + // Deadlock between two different DB connections + if ("40P01".equals(e.getSQLState())) + { + return getOtherDatabaseThreads(); + } + return null; + } + + @Override + public @NotNull String getApplicationConnectionsSql() + { + return "SELECT pid, usename, client_addr, client_hostname, xact_start, query_start, state, application_name, query FROM pg_stat_activity WHERE pid <> pg_backend_pid() AND datname = ? AND application_name = ?"; + } + + // Query PostgreSQL-specific settings protected void determineSettings(DbScope scope) { if (getServerType().supportsSpecialMetadataQueries()) { - super.determineSettings(scope); + Selector selector = new SqlSelector(scope, "SELECT setting FROM pg_settings WHERE name = 'standard_conforming_strings'"); + _standardConformingStrings = "on".equalsIgnoreCase(selector.getObject(String.class)); String value = new SqlSelector(scope, "SELECT setting FROM pg_settings WHERE name = 'max_identifier_length'").getObject(String.class); try @@ -185,13 +342,17 @@ protected void determineSettings(DbScope scope) } @Override - protected DialectStringHandler createStringHandler() + public boolean isProcedureExists(DbScope scope, String schema, String name) { - // TODO: Isn't this the wrong setting? Should we be looking at the "backslash_quote" setting instead? - if (getStandardConformingStrings()) - return super.createStringHandler(); - else - return new BackslashEscapingStringHandler(); + // Don't bother querying LabKey for stored procedures + return getServerType().supportsSpecialMetadataQueries() && super.isProcedureExists(scope, schema, name); + } + + @Override + public boolean shouldTest() + { + // Don't test a LabKey data source + return getServerType().shouldTest(); } /* diff --git a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java index 382f4264746..5620c3977b9 100644 --- a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java +++ b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java @@ -27,7 +27,6 @@ import org.labkey.api.data.dialect.BasePostgreSqlDialect; import org.labkey.api.data.dialect.DatabaseNotSupportedException; import org.labkey.api.data.dialect.JdbcHelperTest; -import org.labkey.api.data.dialect.PostgreSqlServerType; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.data.dialect.SqlDialectFactory; import org.labkey.api.data.dialect.TestUpgradeCodeCounter; diff --git a/api/src/org/labkey/api/data/dialect/PostgreSqlServerType.java b/core/src/org/labkey/core/dialect/PostgreSqlServerType.java similarity index 97% rename from api/src/org/labkey/api/data/dialect/PostgreSqlServerType.java rename to core/src/org/labkey/core/dialect/PostgreSqlServerType.java index 2fd04575c68..67fabd19404 100644 --- a/api/src/org/labkey/api/data/dialect/PostgreSqlServerType.java +++ b/core/src/org/labkey/core/dialect/PostgreSqlServerType.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.labkey.api.data.dialect; +package org.labkey.core.dialect; import java.util.Map;