connectivity/registry/firebird/org/openoffice/Office/DataAccess/Drivers.xcu | 10 ++ connectivity/source/commontools/dbmetadata.cxx | 9 ++ connectivity/source/drivers/firebird/Connection.cxx | 29 -------- connectivity/source/drivers/firebird/Connection.hxx | 8 -- connectivity/source/drivers/jdbc/JConnection.cxx | 36 ---------- connectivity/source/drivers/odbc/OConnection.cxx | 3 connectivity/source/drivers/odbc/OPreparedStatement.cxx | 19 ----- connectivity/source/inc/odbc/OConnection.hxx | 2 connectivity/source/parse/sqlnode.cxx | 5 + include/connectivity/dbmetadata.hxx | 4 + 10 files changed, 31 insertions(+), 94 deletions(-)
New commits: commit a29d97e6ddab8ec002ba9827bd5fc874117712e0 Author: Lionel Elie Mamane <[email protected]> Date: Tue Jan 2 22:49:31 2018 +0100 tdf#104986 move named parameters substitution into generic layer Previously, drivers were responsible for making the substitution themselves. In practice they all (Firebird, ODBC and JDBC) used the LibreOffice SQL parser to parse the SQL statement and do the substitution. This had a few negative consequences: * The substitition was applied to _all_ SQL commands, including queries having the "execute SQL directly" bit set. Which means that the SQL was _not_ sent to the DBMS exactly as typed by the user. Even if there was no substitution to be made, since the SQL command was always round-tripped through the parser, thus "normalising" it (which is what led to tdf#104986). * "execute SQL directly" queries "magically" behaved slightly differently depending on whether the LibreOffice SQL parser succeeded in parsing them or not. Change-Id: Ieedc643f33467435a139e2565a01e6f398af041f Reviewed-on: https://gerrit.libreoffice.org/47283 Tested-by: Jenkins <[email protected]> Reviewed-by: Lionel Elie Mamane <[email protected]> diff --git a/connectivity/registry/firebird/org/openoffice/Office/DataAccess/Drivers.xcu b/connectivity/registry/firebird/org/openoffice/Office/DataAccess/Drivers.xcu index e5ff61f28262..03896aff4412 100644 --- a/connectivity/registry/firebird/org/openoffice/Office/DataAccess/Drivers.xcu +++ b/connectivity/registry/firebird/org/openoffice/Office/DataAccess/Drivers.xcu @@ -61,6 +61,11 @@ <value>false</value> </prop> </node> + <node oor:name="ParameterNameSubstitution" oor:op="replace"> + <prop oor:name="Value" oor:type="xs:boolean"> + <value>true</value> + </prop> + </node> </node><!--Properties--> <node oor:name="Features"> </node><!--Features--> @@ -95,6 +100,11 @@ <value>false</value> </prop> </node> + <node oor:name="ParameterNameSubstitution" oor:op="replace"> + <prop oor:name="Value" oor:type="xs:boolean"> + <value>true</value> + </prop> + </node> </node> <node oor:name="Features"> </node> diff --git a/connectivity/source/commontools/dbmetadata.cxx b/connectivity/source/commontools/dbmetadata.cxx index 50abe346fba2..e40cde6c4bcd 100644 --- a/connectivity/source/commontools/dbmetadata.cxx +++ b/connectivity/source/commontools/dbmetadata.cxx @@ -301,6 +301,15 @@ namespace dbtools return doGenerate; } + bool DatabaseMetaData::shouldSubstituteParameterNames() const + { + bool doSubstitute( true ); + Any setting; + if ( lcl_getConnectionSetting( "ParameterNameSubstitution", *m_pImpl, setting ) ) + OSL_VERIFY( setting >>= doSubstitute ); + return doSubstitute; + } + bool DatabaseMetaData::isAutoIncrementPrimaryKey() const { bool is( true ); diff --git a/connectivity/source/drivers/firebird/Connection.cxx b/connectivity/source/drivers/firebird/Connection.cxx index 32e314029f7f..7d0937e3ae99 100644 --- a/connectivity/source/drivers/firebird/Connection.cxx +++ b/connectivity/source/drivers/firebird/Connection.cxx @@ -393,30 +393,6 @@ Reference< XStatement > SAL_CALL Connection::createStatement( ) return xReturn; } -OUString Connection::transformPreparedStatement(const OUString& _sSQL) -{ - OUString sSqlStatement (_sSQL); - try - { - OSQLParser aParser( m_xDriver->getContext() ); - OUString sErrorMessage; - OUString sNewSql; - OSQLParseNode* pNode = aParser.parseTree(sErrorMessage,_sSQL); - if(pNode) - { // special handling for parameters - OSQLParseNode::substituteParameterNames(pNode); - pNode->parseNodeToStr( sNewSql, this ); - delete pNode; - sSqlStatement = sNewSql; - } - } - catch(const Exception&) - { - SAL_WARN("connectivity.firebird", "failed to remove named parameters from '" << _sSQL << "'"); - } - return sSqlStatement; -} - Reference< XPreparedStatement > SAL_CALL Connection::prepareStatement( const OUString& _sSql) { @@ -428,10 +404,7 @@ Reference< XPreparedStatement > SAL_CALL Connection::prepareStatement( if(m_aTypeInfo.empty()) buildTypeInfo(); - OUString sSqlStatement (transformPreparedStatement( _sSql )); - - Reference< XPreparedStatement > xReturn = new OPreparedStatement(this, - sSqlStatement); + Reference< XPreparedStatement > xReturn = new OPreparedStatement(this, _sSql); m_aStatements.push_back(WeakReferenceHelper(xReturn)); return xReturn; diff --git a/connectivity/source/drivers/firebird/Connection.hxx b/connectivity/source/drivers/firebird/Connection.hxx index 70f5a417af42..e3c69e7e99c7 100644 --- a/connectivity/source/drivers/firebird/Connection.hxx +++ b/connectivity/source/drivers/firebird/Connection.hxx @@ -169,14 +169,6 @@ namespace connectivity void setupTransaction(); void disposeStatements(); - /** transform named parameters into unnamed parameters - @param _sSQL - The SQL statement to transform. - @return - The new statement with unnamed parameters - */ - OUString transformPreparedStatement(const OUString& _sSQL); - public: explicit Connection(FirebirdDriver* _pDriver); virtual ~Connection() override; diff --git a/connectivity/source/drivers/jdbc/JConnection.cxx b/connectivity/source/drivers/jdbc/JConnection.cxx index 9450872c9923..9ccc934255ee 100644 --- a/connectivity/source/drivers/jdbc/JConnection.cxx +++ b/connectivity/source/drivers/jdbc/JConnection.cxx @@ -266,7 +266,6 @@ java_sql_Connection::java_sql_Connection( const java_sql_Driver& _rDriver ) ,m_pDriverClassLoader() ,m_Driver_theClass(nullptr) ,m_aLogger( _rDriver.getLogger() ) - ,m_bParameterSubstitution(false) ,m_bIgnoreDriverPrivileges(true) ,m_bIgnoreCurrency(false) { @@ -464,32 +463,6 @@ Reference< XStatement > SAL_CALL java_sql_Connection::createStatement( ) return xStmt; } -OUString java_sql_Connection::transFormPreparedStatement(const OUString& _sSQL) -{ - OUString sSqlStatement = _sSQL; - if ( m_bParameterSubstitution ) - { - try - { - OSQLParser aParser( m_pDriver->getContext() ); - OUString sErrorMessage; - OUString sNewSql; - OSQLParseNode* pNode = aParser.parseTree(sErrorMessage,_sSQL); - if(pNode) - { // special handling for parameters - OSQLParseNode::substituteParameterNames(pNode); - pNode->parseNodeToStr( sNewSql, this ); - delete pNode; - sSqlStatement = sNewSql; - } - } - catch(const Exception&) - { - } - } - return sSqlStatement; -} - Reference< XPreparedStatement > SAL_CALL java_sql_Connection::prepareStatement( const OUString& sql ) { ::osl::MutexGuard aGuard( m_aMutex ); @@ -497,10 +470,8 @@ Reference< XPreparedStatement > SAL_CALL java_sql_Connection::prepareStatement( m_aLogger.log( LogLevel::FINE, STR_LOG_PREPARE_STATEMENT, sql ); SDBThreadAttach t; - OUString sSqlStatement = sql; - sSqlStatement = transFormPreparedStatement( sSqlStatement ); - java_sql_PreparedStatement* pStatement = new java_sql_PreparedStatement( t.pEnv, *this, sSqlStatement ); + java_sql_PreparedStatement* pStatement = new java_sql_PreparedStatement( t.pEnv, *this, sql ); Reference< XPreparedStatement > xReturn( pStatement ); m_aStatements.push_back(WeakReferenceHelper(xReturn)); @@ -515,10 +486,8 @@ Reference< XPreparedStatement > SAL_CALL java_sql_Connection::prepareCall( const m_aLogger.log( LogLevel::FINE, STR_LOG_PREPARE_CALL, sql ); SDBThreadAttach t; - OUString sSqlStatement = sql; - sSqlStatement = transFormPreparedStatement( sSqlStatement ); - java_sql_CallableStatement* pStatement = new java_sql_CallableStatement( t.pEnv, *this, sSqlStatement ); + java_sql_CallableStatement* pStatement = new java_sql_CallableStatement( t.pEnv, *this, sql ); Reference< XPreparedStatement > xStmt( pStatement ); m_aStatements.push_back(WeakReferenceHelper(xStmt)); @@ -784,7 +753,6 @@ bool java_sql_Connection::construct(const OUString& url, sDriverClassPath = impl_getJavaDriverClassPath_nothrow(sDriverClass); bAutoRetrievingEnabled = aSettings.getOrDefault( "IsAutoRetrievingEnabled", bAutoRetrievingEnabled ); sGeneratedValueStatement = aSettings.getOrDefault( "AutoRetrievingStatement", sGeneratedValueStatement ); - m_bParameterSubstitution = aSettings.getOrDefault( "ParameterNameSubstitution", m_bParameterSubstitution ); m_bIgnoreDriverPrivileges = aSettings.getOrDefault( "IgnoreDriverPrivileges", m_bIgnoreDriverPrivileges ); m_bIgnoreCurrency = aSettings.getOrDefault( "IgnoreCurrency", m_bIgnoreCurrency ); aSystemProperties = aSettings.getOrDefault( "SystemProperties", aSystemProperties ); diff --git a/connectivity/source/drivers/odbc/OConnection.cxx b/connectivity/source/drivers/odbc/OConnection.cxx index 47e7edefb470..3a3a8ce58b0c 100644 --- a/connectivity/source/drivers/odbc/OConnection.cxx +++ b/connectivity/source/drivers/odbc/OConnection.cxx @@ -53,7 +53,6 @@ OConnection::OConnection(const SQLHANDLE _pDriverHandle,ODBCDriver* _pDriver) ,m_bClosed(false) ,m_bUseCatalog(false) ,m_bUseOldDateFormat(false) - ,m_bParameterSubstitution(false) ,m_bIgnoreDriverPrivileges(false) ,m_bPreventGetVersionColumns(false) ,m_bReadOnly(true) @@ -200,8 +199,6 @@ SQLRETURN OConnection::Construct(const OUString& url,const Sequence< PropertyVal OSL_VERIFY( pBegin->Value >>= m_bIgnoreDriverPrivileges ); else if( pBegin->Name == "PreventGetVersionColumns") OSL_VERIFY( pBegin->Value >>= m_bPreventGetVersionColumns ); - else if( pBegin->Name == "ParameterNameSubstitution") - OSL_VERIFY( pBegin->Value >>= m_bParameterSubstitution ); else if( pBegin->Name == "IsAutoRetrievingEnabled") { bool bAutoRetrievingEnabled = false; diff --git a/connectivity/source/drivers/odbc/OPreparedStatement.cxx b/connectivity/source/drivers/odbc/OPreparedStatement.cxx index 2bcfb700281b..7c8727539e60 100644 --- a/connectivity/source/drivers/odbc/OPreparedStatement.cxx +++ b/connectivity/source/drivers/odbc/OPreparedStatement.cxx @@ -69,25 +69,6 @@ OPreparedStatement::OPreparedStatement( OConnection* _pConnection,const OUString ,m_bPrepared(false) { m_sSqlStatement = sql; - try - { - if(_pConnection->isParameterSubstitutionEnabled()) - { - OSQLParser aParser( comphelper::getComponentContext(_pConnection->getDriver()->getORB()) ); - OUString sErrorMessage; - OUString sNewSql; - std::unique_ptr<OSQLParseNode> pNode( aParser.parseTree(sErrorMessage,sql) ); - if ( pNode.get() ) - { // special handling for parameters - OSQLParseNode::substituteParameterNames(pNode.get()); - pNode->parseNodeToStr( sNewSql, _pConnection ); - m_sSqlStatement = sNewSql; - } - } - } - catch(Exception&) - { - } } void SAL_CALL OPreparedStatement::acquire() throw() diff --git a/connectivity/source/inc/odbc/OConnection.hxx b/connectivity/source/inc/odbc/OConnection.hxx index bb1f1430d48d..f9cd473b3b77 100644 --- a/connectivity/source/inc/odbc/OConnection.hxx +++ b/connectivity/source/inc/odbc/OConnection.hxx @@ -69,7 +69,6 @@ namespace connectivity bool m_bClosed; bool m_bUseCatalog; // should we use the catalog on filebased databases bool m_bUseOldDateFormat; - bool m_bParameterSubstitution; bool m_bIgnoreDriverPrivileges; bool m_bPreventGetVersionColumns; // #i60273# bool m_bReadOnly; @@ -122,7 +121,6 @@ namespace connectivity // should we use the catalog on filebased databases bool isCatalogUsed() const { return m_bUseCatalog; } - bool isParameterSubstitutionEnabled() const { return m_bParameterSubstitution; } bool isIgnoreDriverPrivilegesEnabled() const { return m_bIgnoreDriverPrivileges; } bool preventGetVersionColumns() const { return m_bPreventGetVersionColumns; } bool useOldDateFormat() const { return m_bUseOldDateFormat; } diff --git a/connectivity/source/parse/sqlnode.cxx b/connectivity/source/parse/sqlnode.cxx index 48616492773f..3c28d1ecaa1c 100644 --- a/connectivity/source/parse/sqlnode.cxx +++ b/connectivity/source/parse/sqlnode.cxx @@ -397,6 +397,10 @@ void OSQLParseNode::impl_parseNodeToString_throw(OUStringBuffer& rString, const rString.append(" "); if (nCount == 1) // ? m_aChildren[0]->impl_parseNodeToString_throw( rString, rParam, false ); + else if (rParam.bParseToSDBCLevel && rParam.aMetaData.shouldSubstituteParameterNames()) + { + rString.append("?"); + } else if (nCount == 2) // :Name { m_aChildren[0]->impl_parseNodeToString_throw( rString, rParam, false ); @@ -404,6 +408,7 @@ void OSQLParseNode::impl_parseNodeToString_throw(OUStringBuffer& rString, const } // [Name] else { + assert (nCount == 3); m_aChildren[0]->impl_parseNodeToString_throw( rString, rParam, false ); rString.append(m_aChildren[1]->m_aNodeValue); rString.append(m_aChildren[2]->m_aNodeValue); diff --git a/include/connectivity/dbmetadata.hxx b/include/connectivity/dbmetadata.hxx index ead3b9b58960..1d5b13935293 100644 --- a/include/connectivity/dbmetadata.hxx +++ b/include/connectivity/dbmetadata.hxx @@ -135,6 +135,10 @@ namespace dbtools */ bool shouldEscapeDateTime() const; + /** should named parameters (:foo, [foo]) be replaced by unnamed parameters (?) + */ + bool shouldSubstituteParameterNames() const; + /** auto increment columns should be automatically used as primary key. */ bool isAutoIncrementPrimaryKey() const; _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
