Skip to content

Update string literal escaping for Redshift & Snowflake #8026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-adam merged 7 commits into from
Sep 10, 2026
Merged

Update string literal escaping for Redshift & Snowflake #8026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
29d4480
Update string literal escaping for Redshift
labkey-adam Sep 7, 2026
57cf1e0
Move more PostgreSQL-specific functionality out of base dialect: user…
labkey-adam Sep 7, 2026
3f4ff55
Override createStringHandler() instead
labkey-adam Sep 7, 2026
35909f1
Move a couple more PostgreSQL-specific methods
labkey-adam Sep 7, 2026
a3576e6
Move more PostgreSQL-specific methods
labkey-adam Sep 7, 2026
6ab4b0f
Snowflake escaping
labkey-adam Sep 7, 2026
e82e1a7
Merge remote-tracking branch 'origin/develop' into fb_literal_escaping
labkey-tchad Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions api/src/org/labkey/api/data/dialect/BackslashEscapingStringHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
179 changes: 5 additions & 174 deletions api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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<String, Integer> _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<String> getReservedWords()
{
Expand All @@ -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<ConnectionWrapper> connections, boolean terminate)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand Down Expand Up @@ -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()
{
Expand All @@ -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 = ?";
}
}
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/dialect/SqlDialect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading