From b8eb2f1aa53bf851d1f25de2c1dd59bd8981fa2d Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Thu, 15 Jul 2021 19:33:32 +0530 Subject: [PATCH] Adding support for explicit data type casting in postgres prepared statement (#5842) * Adding support for explicit data type casting in postgres prepared statement * Added text and int psql data types for support in explicit typecasting * Documenting the code --- .../com/appsmith/external/models/Param.java | 2 + .../com/external/plugins/PostgresPlugin.java | 50 +++++-- .../plugins/utils/PostgresDataTypeUtils.java | 138 ++++++++++++++++++ .../external/plugins/PostgresPluginTest.java | 69 ++++++++- 4 files changed, 242 insertions(+), 17 deletions(-) create mode 100644 app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/utils/PostgresDataTypeUtils.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java index 5c4bb9234e..d8fe508471 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Param.java @@ -9,6 +9,8 @@ import lombok.ToString; @Getter @Setter @ToString +@AllArgsConstructor +@NoArgsConstructor public class Param { String key; diff --git a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java index 5ece6dbaf1..eb8ce46a95 100644 --- a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java +++ b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java @@ -15,8 +15,8 @@ import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.DatasourceStructure; import com.appsmith.external.models.DatasourceTestResult; import com.appsmith.external.models.Endpoint; -import com.appsmith.external.models.PsParameterDTO; import com.appsmith.external.models.Property; +import com.appsmith.external.models.PsParameterDTO; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.models.SSLDetails; import com.appsmith.external.plugins.BasePlugin; @@ -25,8 +25,8 @@ import com.appsmith.external.plugins.SmartSubstitutionInterface; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; import com.zaxxer.hikari.HikariPoolMXBean; -import com.zaxxer.hikari.pool.HikariProxyConnection; import com.zaxxer.hikari.pool.HikariPool.PoolInitializationException; +import com.zaxxer.hikari.pool.HikariProxyConnection; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.ObjectUtils; import org.pf4j.Extension; @@ -70,6 +70,15 @@ import static com.appsmith.external.helpers.MustacheHelper.replaceQuestionMarkWi import static com.appsmith.external.helpers.PluginUtils.getColumnsListForJdbcPlugin; import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns; import static com.appsmith.external.helpers.PluginUtils.getPSParamLabel; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.BOOL; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DATE; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DECIMAL; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.FLOAT8; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.INT4; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.INT8; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.TIME; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.VARCHAR; +import static com.external.plugins.utils.PostgresDataTypeUtils.extractExplicitCasting; import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; @@ -201,7 +210,7 @@ public class PostgresPlugin extends BasePlugin { // In case of non prepared statement, simply do binding replacement and execute if (FALSE.equals(isPreparedStatement)) { prepareConfigurationsForExecution(executeActionDTO, actionConfiguration, datasourceConfiguration); - return executeCommon(connection, datasourceConfiguration, actionConfiguration, FALSE, null, null); + return executeCommon(connection, datasourceConfiguration, actionConfiguration, FALSE, null, null, null); } // Prepared Statement @@ -210,8 +219,10 @@ public class PostgresPlugin extends BasePlugin { List mustacheKeysInOrder = MustacheHelper.extractMustacheKeysInOrder(query); // Replace all the bindings with a ? as expected in a prepared statement. String updatedQuery = MustacheHelper.replaceMustacheWithQuestionMark(query, mustacheKeysInOrder); + List explicitCastDataTypes = extractExplicitCasting(updatedQuery); actionConfiguration.setBody(updatedQuery); - return executeCommon(connection, datasourceConfiguration, actionConfiguration, TRUE, mustacheKeysInOrder, executeActionDTO); + return executeCommon(connection, datasourceConfiguration, actionConfiguration, TRUE, + mustacheKeysInOrder, executeActionDTO, explicitCastDataTypes); } private Mono executeCommon(HikariDataSource connection, @@ -219,7 +230,8 @@ public class PostgresPlugin extends BasePlugin { ActionConfiguration actionConfiguration, Boolean preparedStatement, List mustacheValuesInOrder, - ExecuteActionDTO executeActionDTO) { + ExecuteActionDTO executeActionDTO, + List explicitCastDataTypes) { final Map requestData = new HashMap<>(); requestData.put("preparedStatement", TRUE.equals(preparedStatement) ? true : false); @@ -277,7 +289,8 @@ public class PostgresPlugin extends BasePlugin { mustacheValuesInOrder, executeActionDTO.getParams(), parameters, - connectionFromPool); + connectionFromPool, + explicitCastDataTypes); IntStream.range(0, parameters.size()) .forEachOrdered(i -> @@ -759,7 +772,14 @@ public class PostgresPlugin extends BasePlugin { PreparedStatement preparedStatement = (PreparedStatement) input; HikariProxyConnection connection = (HikariProxyConnection) args[0]; - DataType valueType = DataTypeStringUtils.stringToKnownDataTypeConverter(value); + List explicitCastDataTypes = (List) args[1]; + DataType valueType; + // If explicitly cast, set the user specified data type + if (explicitCastDataTypes != null && explicitCastDataTypes.get(index - 1) != null) { + valueType = explicitCastDataTypes.get(index - 1); + } else { + valueType = DataTypeStringUtils.stringToKnownDataTypeConverter(value); + } Map.Entry parameter = new SimpleEntry<>(value, valueType.toString()); insertedParams.add(parameter); @@ -851,21 +871,21 @@ public class PostgresPlugin extends BasePlugin { private static String toPostgresqlPrimitiveTypeName(DataType type) { switch (type) { case LONG: - return "int8"; + return INT8; case INTEGER: - return "int4"; + return INT4; case FLOAT: - return "decimal"; + return DECIMAL; case STRING: - return "varchar"; + return VARCHAR; case BOOLEAN: - return "bool"; + return BOOL; case DATE: - return "date"; + return DATE; case TIME: - return "time"; + return TIME; case DOUBLE: - return "float8"; + return FLOAT8; case ARRAY: throw new IllegalArgumentException("Array of Array datatype is not supported."); default: diff --git a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/utils/PostgresDataTypeUtils.java b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/utils/PostgresDataTypeUtils.java new file mode 100644 index 0000000000..0d29ff0930 --- /dev/null +++ b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/utils/PostgresDataTypeUtils.java @@ -0,0 +1,138 @@ +package com.external.plugins.utils; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static com.appsmith.external.constants.DataType.BOOLEAN; +import static com.appsmith.external.constants.DataType.DOUBLE; +import static com.appsmith.external.constants.DataType.FLOAT; +import static com.appsmith.external.constants.DataType.INTEGER; +import static com.appsmith.external.constants.DataType.LONG; +import static com.appsmith.external.constants.DataType.STRING; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.BOOL; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DATE; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.DECIMAL; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.FLOAT8; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.INT; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.INT4; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.INT8; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.TEXT; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.TIME; +import static com.external.plugins.utils.PostgresDataTypeUtils.DataType.VARCHAR; + +public class PostgresDataTypeUtils { + + /** + * questionWithCast will match the following sample strings in a query + * - "?" + * - "?::text" + * + * Capturing only the words post "::" so that the explict data type to which the parameter must be cast can be read + * and ignoring the group "::" from getting captured by using regex "?:" which ignores the subsequent string + */ + private static String questionWithCast = "\\?(?:::)*([a-zA-Z]+)*"; + private static Pattern questionWithCastPattern = Pattern.compile(questionWithCast); + + public static DataType dataType = new DataType(); + + public static class DataType { + /** + * Declare all the explicitly castable postgresql types below. These would be automatically added to the + * dataTypes set automatically. + * + * !!! WARNING !!! + * When adding a new data type to support for explicit casting, please ensure to add an entry in the Map + * dataTypeMapper which maps the postgres data types to Appsmith data types. + */ + public static final String INT8 = "int8"; + public static final String INT4 = "int4"; + public static final String DECIMAL = "decimal"; + public static final String VARCHAR = "varchar"; + public static final String BOOL = "bool"; + public static final String DATE = "date"; + public static final String TIME = "time"; + public static final String FLOAT8 = "float8"; + public static final String TEXT = "text"; + public static final String INT = "int"; + + public Set dataTypes = null; + + public Set getDataTypes() { + // if data types hasn't been initialized, read and set all the supported data types for postgres + if (dataTypes == null || dataTypes.isEmpty()) { + dataTypes = new HashSet<>(); + + Field[] fields = this.getClass().getDeclaredFields(); + + for (Field field : fields) { + if (field.getType().equals(String.class)) { // if it is a String field + try { + dataTypes.add(field.get(dataType)); + } catch (IllegalArgumentException | IllegalAccessException e) { + // We weren't able to read the value of the field. Ignore this field and continue + // Still print the stack trace for posterity. + e.printStackTrace(); + } + } + } + } + // We are assured that data types has been set. + return dataTypes; + } + } + + // Stores the mapping between postgres data types and appsmith data types + public static Map dataTypeMapper; + + private static Map getDataTypeMapper() { + if (dataTypeMapper == null) { + dataTypeMapper = new HashMap(); + dataTypeMapper.put(INT8, LONG); + dataTypeMapper.put(INT4, INTEGER); + dataTypeMapper.put(DECIMAL, FLOAT); + dataTypeMapper.put(VARCHAR, STRING); + dataTypeMapper.put(BOOL, BOOLEAN); + dataTypeMapper.put(DATE, com.appsmith.external.constants.DataType.DATE); + dataTypeMapper.put(TIME, com.appsmith.external.constants.DataType.TIME); + dataTypeMapper.put(FLOAT8, DOUBLE); + dataTypeMapper.put(TEXT, STRING); + dataTypeMapper.put(INT, INTEGER); + + // Must ensure that all the declared postgres data types have a mapping to appsmith data types + assert(dataTypeMapper.size() == dataType.getDataTypes().size()); + } + + return dataTypeMapper; + } + + public static List extractExplicitCasting(String query) { + Matcher matcher = questionWithCastPattern.matcher(query); + List inputDataTypes = new ArrayList<>(); + + while (matcher.find()) { + String prospectiveDataType = matcher.group(1); + + if (prospectiveDataType != null) { + String dataTypeFromInput = prospectiveDataType.trim().toLowerCase(); + if (dataType.getDataTypes().contains(dataTypeFromInput)) { + com.appsmith.external.constants.DataType appsmithDataType + = (com.appsmith.external.constants.DataType) getDataTypeMapper().get(dataTypeFromInput); + inputDataTypes.add(appsmithDataType); + continue; + } + } + // Either no external casting exists or unsupported data type is being used. Do not use external casting for this + // and instead default to implicit type casting (default behaviour) by setting the entry to null. + inputDataTypes.add(null); + } + + return inputDataTypes; + } +} diff --git a/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java b/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java index a496b727dc..a6f3d34e6b 100644 --- a/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java +++ b/app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java @@ -9,9 +9,9 @@ import com.appsmith.external.models.DBAuth; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.DatasourceStructure; import com.appsmith.external.models.Endpoint; -import com.appsmith.external.models.PsParameterDTO; import com.appsmith.external.models.Param; import com.appsmith.external.models.Property; +import com.appsmith.external.models.PsParameterDTO; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.models.SSLDetails; import com.fasterxml.jackson.databind.JsonNode; @@ -1117,6 +1117,71 @@ public class PostgresPluginTest { } @Test + public void testPreparedStatementWithExplicitTypeCasting() { + DatasourceConfiguration dsConfig = createDatasourceConfiguration(); + + ActionConfiguration actionConfiguration = new ActionConfiguration(); + + String query = "INSERT INTO users (id, username, password, email, dob) VALUES ({{id}}, {{firstName}}::varchar, {{lastName}}, {{email}}, {{date}}::date)"; + actionConfiguration.setBody(query); + + List pluginSpecifiedTemplates = new ArrayList<>(); + pluginSpecifiedTemplates.add(new Property("preparedStatement", "true")); + actionConfiguration.setPluginSpecifiedTemplates(pluginSpecifiedTemplates); + + ExecuteActionDTO executeActionDTO = new ExecuteActionDTO(); + List params = new ArrayList<>(); + params.add(new Param("id", "10")); + params.add(new Param("firstName", "1001")); + params.add(new Param("lastName", "LastName")); + params.add(new Param("email", "email@email.com")); + params.add(new Param("date", "2018-12-31")); + executeActionDTO.setParams(params); + + Mono connectionCreateMono = pluginExecutor.datasourceCreate(dsConfig).cache(); + + Mono resultMono = connectionCreateMono + .flatMap(pool -> pluginExecutor.executeParameterized(pool, executeActionDTO, dsConfig, actionConfiguration)); + + StepVerifier.create(resultMono) + .assertNext(result -> { + + assertTrue(result.getIsExecutionSuccess()); + final JsonNode node = ((ArrayNode) result.getBody()).get(0); + assertEquals(node.get("affectedRows").asText(), "1"); + + List requestParams = (List) result.getRequest().getRequestParams(); + RequestParamDTO requestParamDTO = requestParams.get(0); + Map substitutedParams = requestParamDTO.getSubstitutedParams(); + for (Map.Entry substitutedParam : substitutedParams.entrySet()) { + PsParameterDTO psParameter = (PsParameterDTO) substitutedParam.getValue(); + switch (psParameter.getValue()) { + case "10" : + assertEquals(psParameter.getType(), "INTEGER"); + break; + case "1001" : + + case "LastName" : + + case "email@email.com" : + assertEquals(psParameter.getType(), "STRING"); + break; + case "2018-12-31" : + assertEquals(psParameter.getType(), "DATE"); + break; + } + } + + }) + .verifyComplete(); + + // Delete the newly added row to not affect any other test case + actionConfiguration.setBody("DELETE FROM users WHERE id = 10"); + connectionCreateMono + .flatMap(pool -> pluginExecutor.executeParameterized(pool, executeActionDTO, dsConfig, actionConfiguration)).block(); + + } + public void testReadOnlyMode() { DatasourceConfiguration dsConfig = createDatasourceConfiguration(); dsConfig.getConnection().setMode(com.appsmith.external.models.Connection.Mode.READ_ONLY); @@ -1138,4 +1203,4 @@ public class PostgresPluginTest { }) .verifyComplete(); } -} \ No newline at end of file +}