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 <ci@libreoffice.org>
Reviewed-by: Lionel Elie Mamane <lionel@mamane.lu>

10 files changed, 31 insertions, 94 deletions

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;