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

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>

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 );

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;

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;

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 );

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;

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()

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; }

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);

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;