From d9442d2a7bf0808b9322562acd1d0042ae2cd127 Mon Sep 17 00:00:00 2001 From: Sumit Kumar Date: Fri, 7 Apr 2023 00:00:03 +0530 Subject: [PATCH] feat: Oracle Integration: Add support for Prepared Statements (#21744) ## Description - This PR introduces support for prepared statements to the Oracle integration that is currently behind a feature flag. - Fixed a bug with PL/SQL cmd due to semicolon. - Fixed a bug with reading Array type. - The following tasks will be taken up separately and will not be part of this PR. They will be resolved before lifting the feature flag: - DB structure and query templates - Error messages infra - JUnit TC - The following data types were tested: `char, varchar, array, int, float, double, raw, timestamp, timestamp_tz, interval` . The following cmd was used to test the feature: ``` select * from TYPESTEST4 where c_varchar2={{'varchar2'}} and c_nvarchar2={{'nvarchar2'}} and c_number={{1}} and c_float={{11.22}} and c_date={{'2002-10-03'}} and c_binary_float={{11.22}} and c_binary_double={{11.22}} and c_timestamp=TO_TIMESTAMP({{'01-01-1997 09:26:50.124'}}) and c_timestamp_tz=TO_UTC_TIMESTAMP_TZ({{"1997-01-01T09:26:56.66+02:00"}}) and c_interval_day=NUMTODSINTERVAL({{1}}, {{'HOUR'}}) and c_char={{'char '}} and c_raw=utl_raw.cast_to_raw({{'raw'}}) ``` - `JSON` type could not be tested because it was only recently introduced in version 21c. However, the Oracle test instance credentials available on Notion are for 19c. Tracking it [here](https://github.com/appsmithorg/appsmith/issues/20796). Fixes #20533 ## Type of change - New feature (non-breaking change which adds functionality) ## How Has This Been Tested? - Manual ## How to Test ``` (1) Create a Table with various data types like: create table typestest4 ( c_varchar2 varchar2(20), c_nvarchar2 nvarchar2(20), c_number number, c_float float, c_date date, c_binary_float binary_float, c_binary_double binary_double, c_timestamp timestamp, c_timestamp_tz timestamp with time zone, c_timestamp_ltz timestamp with local time zone, c_interval_year interval year to month, c_interval_day interval day to second, c_raw raw(256), c_rowid rowid, c_urowid urowid, c_char char(256), c_nchar nchar(256), c_clob clob, c_nclob nclob, c_blob blob ) (2) Insert data into the table: insert into TYPESTEST4 values ( 'varchar2', 'nvarchar2', 1, 11.22, '03-OCT-02', 11.22, 11.22, TIMESTAMP'1997-01-01 09:26:50.124', TIMESTAMP'1997-01-01 09:26:56.66 +02:00', TIMESTAMP'1999-04-05 8:00:00 US/Pacific', INTERVAL '1' YEAR(3), INTERVAL '1' HOUR, utl_raw.cast_to_raw('raw'), '000001F8.0001.0006', '000001F8.0001.0006', 'char', 'nchar', 'clob', 'nclob', utl_raw.cast_to_raw('raw') ) (3) Run the following select cmd with prepared statement toggle on: select * from TYPESTEST4 where c_varchar2={{'varchar2'}} and c_nvarchar2={{'nvarchar2'}} and c_number={{1}} and c_float={{11.22}} and c_date={{'2002-10-03'}} and c_binary_float={{11.22}} and c_binary_double={{11.22}} and c_timestamp=TO_TIMESTAMP({{'01-01-1997 09:26:50.124'}}) and c_timestamp_tz=TO_UTC_TIMESTAMP_TZ({{"1997-01-01T09:26:56.66+02:00"}}) and c_interval_day=NUMTODSINTERVAL({{1}}, {{'HOUR'}}) and c_char={{'char '}} and c_raw=utl_raw.cast_to_raw({{'raw'}}) ``` ### Test Plan > Add Testsmith test cases links that relate to this PR ### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) ## Checklist: ### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag ### QA activity: - [ ] Test plan has been approved by relevant developers - [ ] Test plan has been peer reviewed by QA - [ ] Cypress test cases have been added and approved by either SDET or manual QA - [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA - [ ] Added Test Plan Approved label after reveiwing all Cypress test --- .../com/external/plugins/OraclePlugin.java | 115 +++++++++++++++++- .../plugins/utils/OracleExecuteUtils.java | 83 +++++++++++-- .../utils/OracleSpecificDataTypes.java | 53 ++++++++ 3 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleSpecificDataTypes.java diff --git a/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java index 720fe223f4..e0558904bc 100644 --- a/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java +++ b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/OraclePlugin.java @@ -1,9 +1,11 @@ package com.external.plugins; +import com.appsmith.external.constants.DataType; import com.appsmith.external.dtos.ExecuteActionDTO; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException; +import com.appsmith.external.helpers.DataTypeServiceUtils; import com.appsmith.external.helpers.MustacheHelper; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionExecutionRequest; @@ -11,15 +13,20 @@ import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.DatasourceStructure; import com.appsmith.external.models.MustacheBindingToken; +import com.appsmith.external.models.Param; import com.appsmith.external.models.PsParameterDTO; import com.appsmith.external.models.RequestParamDTO; import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.external.plugins.SmartSubstitutionInterface; import com.external.plugins.utils.OracleDatasourceUtils; +import com.external.plugins.utils.OracleSpecificDataTypes; import com.zaxxer.hikari.HikariDataSource; import com.zaxxer.hikari.HikariPoolMXBean; +import com.zaxxer.hikari.pool.HikariProxyConnection; import lombok.extern.slf4j.Slf4j; +import oracle.jdbc.OraclePreparedStatement; +import org.apache.commons.io.IOUtils; import org.pf4j.Extension; import org.pf4j.PluginWrapper; import org.springframework.util.CollectionUtils; @@ -27,12 +34,19 @@ import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import java.io.IOException; +import java.math.BigDecimal; import java.sql.Connection; +import java.sql.Date; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; +import java.sql.Types; import java.time.Duration; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -56,6 +70,7 @@ import static com.external.plugins.utils.OracleDatasourceUtils.JDBC_DRIVER; import static com.external.plugins.utils.OracleDatasourceUtils.createConnectionPool; import static com.external.plugins.utils.OracleDatasourceUtils.getConnectionFromConnectionPool; import static com.external.plugins.utils.OracleExecuteUtils.closeConnectionPostExecution; +import static com.external.plugins.utils.OracleExecuteUtils.isPLSQL; import static com.external.plugins.utils.OracleExecuteUtils.populateRowsAndColumns; import static com.external.plugins.utils.OracleExecuteUtils.removeSemicolonFromQuery; import static java.lang.Boolean.FALSE; @@ -135,7 +150,16 @@ public class OraclePlugin 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); - updatedQuery = removeSemicolonFromQuery(updatedQuery); + /** + * PL/SQL cmds have a block structure of the following format: DECLARE...BEGIN...EXCEPTION...END + * Ref: https://blogs.oracle.com/connect/post/building-with-blocks + * + * Oracle supports semicolon as a delimiter with PL/SQL syntax but not with normal SQL. + * Ref: https://forums.oracle.com/ords/apexds/post/why-semicolon-not-allowed-in-jdbc-oracle-0099 + */ + if (!isPLSQL(updatedQuery)) { + updatedQuery = removeSemicolonFromQuery(updatedQuery); + } setDataValueSafelyInFormData(formData, BODY, updatedQuery); return executeCommon(connection, datasourceConfiguration, actionConfiguration, TRUE, mustacheKeysInOrder, executeActionDTO); @@ -208,8 +232,7 @@ public class OraclePlugin extends BasePlugin { preparedQuery = (PreparedStatement) smartSubstitutionOfBindings(preparedQuery, mustacheValuesInOrder, executeActionDTO.getParams(), - parameters, - connectionFromPool); + parameters); IntStream.range(0, parameters.size()) .forEachOrdered(i -> @@ -268,9 +291,7 @@ public class OraclePlugin extends BasePlugin { result.setRequest(request); return result; }) - .timeout(Duration.ofMillis(actionConfiguration.getTimeoutInMillisecond())) .subscribeOn(scheduler); - } @Override @@ -290,5 +311,89 @@ public class OraclePlugin extends BasePlugin { return messages; } + + @Override + public Object substituteValueInInput(int index, + String binding, + String value, + Object input, + List> insertedParams, + Object... args) throws AppsmithPluginException { + + PreparedStatement preparedStatement = (PreparedStatement) input; + Param param = (Param) args[0]; + DataType valueType; + valueType = DataTypeServiceUtils.getAppsmithType(param.getClientDataType(), value, + OracleSpecificDataTypes.pluginSpecificTypes).type(); + Map.Entry parameter = new AbstractMap.SimpleEntry<>(value, valueType.toString()); + insertedParams.add(parameter); + + try { + switch (valueType) { + case NULL: { + preparedStatement.setNull(index, Types.NULL); + break; + } + case BINARY: { + preparedStatement.setBinaryStream(index, IOUtils.toInputStream(value)); + break; + } + case BYTES: { + preparedStatement.setBytes(index, value.getBytes("UTF-8")); + break; + } + case INTEGER: { + preparedStatement.setInt(index, Integer.parseInt(value)); + break; + } + case LONG: { + preparedStatement.setLong(index, Long.parseLong(value)); + break; + } + case FLOAT: + case DOUBLE: { + preparedStatement.setBigDecimal(index, new BigDecimal(String.valueOf(value))); + break; + } + case BOOLEAN: { + preparedStatement.setBoolean(index, Boolean.parseBoolean(value)); + break; + } + case DATE: { + preparedStatement.setDate(index, Date.valueOf(value)); + break; + } + case TIME: { + preparedStatement.setTime(index, Time.valueOf(value)); + break; + } + case TIMESTAMP: { + preparedStatement.setTimestamp(index, Timestamp.valueOf(value)); + break; + } + case STRING: { + /* same as the next case */ + } + case JSON_OBJECT: { + preparedStatement.setString(index, value); + break; + } + default: + break; + } + + } catch (SQLException | IllegalArgumentException | IOException e) { + if ((e instanceof SQLException) && e.getMessage().contains("The column index is out of range:")) { + // In case the parameter being set is out of range, then this must be getting + // set in the commented part of + // the query. Ignore the exception + } else { + throw new AppsmithPluginException(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, + e.getMessage()); + } + } + + return preparedStatement; + } } } diff --git a/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleExecuteUtils.java b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleExecuteUtils.java index 1cedc44b0a..aa6c22663f 100644 --- a/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleExecuteUtils.java +++ b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleExecuteUtils.java @@ -1,6 +1,8 @@ package com.external.plugins.utils; +import com.appsmith.external.constants.DataType; import com.appsmith.external.plugins.SmartSubstitutionInterface; +import oracle.jdbc.OracleArray; import oracle.sql.Datum; import org.apache.commons.lang.ObjectUtils; @@ -16,6 +18,7 @@ import java.time.format.DateTimeFormatter; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import static com.appsmith.external.helpers.PluginUtils.getColumnsListForJdbcPlugin; import static java.lang.Boolean.FALSE; @@ -26,6 +29,30 @@ public class OracleExecuteUtils implements SmartSubstitutionInterface { public static final String TIMESTAMPTZ_TYPE_NAME = "TIMESTAMP WITH TIME ZONE"; public static final String INTERVAL_TYPE_NAME = "interval"; public static final String AFFECTED_ROWS_KEY = "affectedRows"; + 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"; + + /** + * Every PL/SQL block must have `BEGIN` and `END` keywords to define the block. Apart from these they could also + * have the following two optional keywords: `DECLARE` and `EXCEPTION`. The following regex is meant to check for + * the presence of any one of these keywords to indicate the usage of PL/SQL block. + * Please note that we convert the query into lowercase before regex match. Also, this regex would not match any + * of the keywords enclosed within single or double quotes e.g. 'declare' or "declare" + * + * Quoting from official Oracle documentation: + * " A PL/SQL block is defined by the keywords DECLARE, BEGIN, EXCEPTION, and END. These keywords partition the + * block into a declarative part, an executable part, and an exception-handling part. Only the executable part is + * required. " + * Ref: https://docs.oracle.com/cd/B14117_01/appdev.101/b10807/13_elems003.htm#:~:text=A%20PL%2FSQL%20block%20is,the%20executable%20part%20is%20required. + */ + private static final String PLSQL_MATCH_REGEX = "(\\bdeclare\\b(\\s))|(\\bbegin\\b(\\s))|(\\bend\\b(\\s|;))|(\\bexception\\b(\\s))"; + private static final Pattern PL_SQL_MATCH_PATTERN = Pattern.compile(PLSQL_MATCH_REGEX); public static void closeConnectionPostExecution(ResultSet resultSet, Statement statement, PreparedStatement preparedQuery, Connection connectionFromPool) { @@ -75,6 +102,23 @@ public class OracleExecuteUtils implements SmartSubstitutionInterface { return query.replaceAll(";", ""); } + /** + * PL/SQL cmds have a block structure of the following format: DECLARE...BEGIN...EXCEPTION...END + * Ref: https://blogs.oracle.com/connect/post/building-with-blocks + * + * Oracle supports semicolon as a delimiter with PL/SQL syntax but not with normal SQL. + * Ref: https://forums.oracle.com/ords/apexds/post/why-semicolon-not-allowed-in-jdbc-oracle-0099 + */ + public static boolean isPLSQL(String query) { + /** + * Please don't use Java's String.matches(...) function here because it doesn't behave like normal regex + * match. It returns true only if the entire string matches the regex as opposed to finding a substring + * matching the pattern. + * Ref: https://stackoverflow.com/questions/8923398/regex-doesnt-work-in-string-matches + */ + return PL_SQL_MATCH_PATTERN.matcher(query.toLowerCase()).find(); + } + public static void populateRowsAndColumns(List> rowsList, List columnsList, ResultSet resultSet, Boolean isResultSet, Boolean preparedStatement, Statement statement, PreparedStatement preparedQuery) throws SQLException { @@ -119,16 +163,15 @@ public class OracleExecuteUtils implements SmartSubstitutionInterface { } else if (INTERVAL_TYPE_NAME.equalsIgnoreCase(typeName)) { value = resultSet.getObject(i).toString(); - } else { + } else if (resultSet.getObject(i) instanceof OracleArray) { + value = ((OracleArray)resultSet.getObject(i)).getArray(); + } + else { value = resultSet.getObject(i); /** - * Any type that JDBC does not understand gets mapped to PGobject. PGobject has - * two attributes: type and value. Hence, when PGobject gets serialized, it gets - * converted into a JSON like {"type":"citext", "value":"someText"}. Since we are - * only interested in the value and not the type, it makes sense to extract out - * the value as a string. - * Reference: https://jdbc.postgresql.org/documentation/publicapi/org/oracleql/util/PGobject.html + * 'Datum' class is the root of Oracle native datatype hierarchy. + * Ref: https://docs.oracle.com/cd/A97329_03/web.902/q20224/oracle/sql/Datum.html */ if (value instanceof Datum) { value = new String(((Datum) value).getBytes()); @@ -142,4 +185,30 @@ public class OracleExecuteUtils implements SmartSubstitutionInterface { } } } + + public static String toOraclePrimitiveTypeName(DataType type) { + switch (type) { + case LONG: + return INT8; + case INTEGER: + return INT4; + case FLOAT: + return DECIMAL; + case STRING: + return VARCHAR; + case BOOLEAN: + return BOOL; + case DATE: + return DATE; + case TIME: + return TIME; + case DOUBLE: + return FLOAT8; + case ARRAY: + throw new IllegalArgumentException("Array of Array datatype is not supported."); + default: + throw new IllegalArgumentException( + "Unable to map the computed data type to primitive Postgresql type"); + } + } } diff --git a/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleSpecificDataTypes.java b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleSpecificDataTypes.java new file mode 100644 index 0000000000..6cbd379866 --- /dev/null +++ b/app/server/appsmith-plugins/oraclePlugin/src/main/java/com/external/plugins/utils/OracleSpecificDataTypes.java @@ -0,0 +1,53 @@ +package com.external.plugins.utils; + +import com.appsmith.external.datatypes.AppsmithType; +import com.appsmith.external.datatypes.ArrayType; +import com.appsmith.external.datatypes.BigDecimalType; +import com.appsmith.external.datatypes.BooleanType; +import com.appsmith.external.datatypes.ClientDataType; +import com.appsmith.external.datatypes.DateType; +import com.appsmith.external.datatypes.DoubleType; +import com.appsmith.external.datatypes.IntegerType; +import com.appsmith.external.datatypes.JsonObjectType; +import com.appsmith.external.datatypes.LongType; +import com.appsmith.external.datatypes.NullArrayType; +import com.appsmith.external.datatypes.NullType; +import com.appsmith.external.datatypes.StringType; +import com.appsmith.external.datatypes.TimeType; +import com.appsmith.external.datatypes.TimestampType; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class OracleSpecificDataTypes { + public final static Map> pluginSpecificTypes = new HashMap<>(); + + static { + pluginSpecificTypes.put(ClientDataType.NULL, List.of(new NullType())); + + pluginSpecificTypes.put(ClientDataType.ARRAY, List.of(new NullArrayType(), new ArrayType())); + + pluginSpecificTypes.put(ClientDataType.BOOLEAN, List.of(new BooleanType())); + + pluginSpecificTypes.put(ClientDataType.NUMBER, List.of( + new IntegerType(), + new LongType(), + new DoubleType(), + new BigDecimalType() + )); + + /* + JsonObjectType is the preferred server-side data type when the client-side data type is of type OBJECT. + Fallback server-side data type for client-side OBJECT type is String. + */ + pluginSpecificTypes.put(ClientDataType.OBJECT, List.of(new JsonObjectType())); + + pluginSpecificTypes.put(ClientDataType.STRING, List.of( + new TimeType(), + new DateType(), + new TimestampType(), + new StringType() + )); + } +}