fix handling of subqueries in query design

 - don't remove parentheses around subqueries
   (without the parentheses, it is not valid SQL)
   * when saving a Field (name value) typed by the user interactively
   * when parsing SQL and constructing the initial Query Design view

 - automatically add the necessary parentheses
   when a SELECT statement is entered as column name

Also:

In code saving a Field (name value) typed by the user interactively,
factorise some common code

Assorted minor fixes (typos in comments, etc)

Change-Id: I3843258323c903cba23238b0730ec4eb5875f792
Reviewed-on: https://gerrit.libreoffice.org/1827
Reviewed-by: Miklos Vajna <vmiklos@suse.cz>
Tested-by: Miklos Vajna <vmiklos@suse.cz>

3 files changed, 51 insertions, 36 deletions

diff --git a/dbaccess/source/ui/querydesign/QueryDesignView.cxx b/dbaccess/source/ui/querydesign/QueryDesignView.cxx
index 3c386dbad5c0..826415295c6c 100644
--- a/dbaccess/source/ui/querydesign/QueryDesignView.cxx
+++ b/dbaccess/source/ui/querydesign/QueryDesignView.cxx
@@ -2198,7 +2198,8 @@ namespace
                     pColumnRef = pColumnRef->getChild(0);
                     OTableFieldDescRef aInfo = new OTableFieldDesc();
 
-                    if (    pColumnRef->count() == 3 &&
+                    if (    pColumnRef->getKnownRuleID() != OSQLParseNode::subquery &&
+                            pColumnRef->count() == 3 &&
                             SQL_ISPUNCTUATION(pColumnRef->getChild(0),"(") &&
                             SQL_ISPUNCTUATION(pColumnRef->getChild(2),")")
                         )
diff --git a/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx b/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
index 554454972b11..e85cb29b4c0a 100644
--- a/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
+++ b/dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx
@@ -649,7 +649,7 @@ sal_Bool OSelectionBrowseBox::fillColumnRef(const ::rtl::OUString& _sColumnName,
     return bError;
 }
 // -----------------------------------------------------------------------------
-sal_Bool OSelectionBrowseBox::saveField(const String& _sFieldName,OTableFieldDescRef& _pEntry,sal_Bool& _bListAction)
+sal_Bool OSelectionBrowseBox::saveField(String& _sFieldName ,OTableFieldDescRef& _pEntry, sal_Bool& _bListAction)
 {
     sal_Bool bError = sal_False;
 
@@ -680,50 +680,63 @@ sal_Bool OSelectionBrowseBox::saveField(const String& _sFieldName,OTableFieldDes
     // we have to look which entries  we should quote
 
     const ::rtl::OUString sFieldAlias = _pEntry->GetFieldAlias();
-    size_t nPass = 4;
     ::connectivity::OSQLParser& rParser( rController.getParser() );
+    {
+        // automatically add parentheses around subqueries
+        OSQLParseNode *pParseNode = NULL;
+        OUString devnull;
+        pParseNode = rParser.parseTree( devnull, _sFieldName, true );
+        if (pParseNode == NULL)
+            pParseNode = rParser.parseTree( devnull, _sFieldName, false );
+        if (pParseNode != NULL && SQL_ISRULE(pParseNode, select_statement))
+            _sFieldName = ::rtl::OUString("(") + _sFieldName + ")";
+    }
+
     OSQLParseNode* pParseNode = NULL;
-    // 4 passes in trying to interprete the field name
-    // - don't quote the field name, parse internationally
-    // - don't quote the field name, parse en-US
-    // - quote the field name, parse internationally
-    // - quote the field name, parse en-US
-    do
-    {
-        bool bQuote = ( nPass <= 2 );
-        bool bInternational = ( nPass % 2 ) == 0;
-
-        ::rtl::OUString sSql;
-        if ( bQuote )
-            sSql += ::dbtools::quoteName( xMetaData->getIdentifierQuoteString(), _sFieldName );
-        else
-            sSql += _sFieldName;
+    {
+        // 4 passes in trying to interprete the field name
+        // - don't quote the field name, parse internationally
+        // - don't quote the field name, parse en-US
+        // - quote the field name, parse internationally
+        // - quote the field name, parse en-US
+        size_t nPass = 4;
+        ::rtl::OUString sQuotedFullFieldName(::dbtools::quoteName( xMetaData->getIdentifierQuoteString(), _sFieldName ));
+        ::rtl::OUString sFullFieldName(_sFieldName);
 
         if  ( _pEntry->isAggreateFunction() )
         {
             OSL_ENSURE(!_pEntry->GetFunction().isEmpty(),"Functionname darf hier nicht leer sein! ;-(");
-            ::rtl::OUStringBuffer aTmpStr2( _pEntry->GetFunction());
-            aTmpStr2.appendAscii("(");
-            aTmpStr2.append(sSql);
-            aTmpStr2.appendAscii(")");
-            sSql = aTmpStr2.makeStringAndClear();
+            sQuotedFullFieldName = _pEntry->GetFunction() + "(" + sQuotedFullFieldName + ")";
+            sFullFieldName = _pEntry->GetFunction() + "(" + sFullFieldName + ")";
         }
 
-        sSql = ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("SELECT ")) + sSql;
-        if ( !sFieldAlias.isEmpty() )
-        { // always quote the alias name there canbe no function in it
-            sSql += ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(" "));
-            sSql += ::dbtools::quoteName( xMetaData->getIdentifierQuoteString(), sFieldAlias );
-        }
-        sSql += ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(" FROM x"));
+        do
+        {
+            bool bQuote = ( nPass <= 2 );
+            bool bInternational = ( nPass % 2 ) == 0;
 
-        pParseNode = rParser.parseTree( sErrorMsg, sSql, bInternational );
+            ::rtl::OUString sSql;
+            if ( bQuote )
+                sSql += sQuotedFullFieldName;
+            else
+                sSql += sFullFieldName;
+
+            sSql = ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("SELECT ")) + sSql;
+            if ( !sFieldAlias.isEmpty() )
+            { // always quote the alias name: there cannot be a function in it
+                sSql += ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(" "));
+                sSql += ::dbtools::quoteName( xMetaData->getIdentifierQuoteString(), sFieldAlias );
+            }
+            sSql += ::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(" FROM x"));
+
+            pParseNode = rParser.parseTree( sErrorMsg, sSql, bInternational );
+        }
+        while ( ( pParseNode == NULL ) && ( --nPass > 0 ) );
     }
-    while ( ( pParseNode == NULL ) && ( --nPass > 0 ) );
 
     if ( pParseNode == NULL )
     {
-        // something different which we have to check (may be a select statement)
+        // something different which we have to check
         String sErrorMessage( ModuleRes( STR_QRY_COLUMN_NOT_FOUND ) );
         sErrorMessage.SearchAndReplaceAscii("$name$",_sFieldName);
         OSQLWarningBox( this, sErrorMessage ).Execute();
@@ -737,8 +750,8 @@ sal_Bool OSelectionBrowseBox::saveField(const String& _sFieldName,OTableFieldDes
     {
         _pEntry->SetField(_sFieldName);
         clearEntryFunctionField(_sFieldName,_pEntry,_bListAction,_pEntry->GetColumnId());
-    } // travel through the select column parse node
-    else
+    }
+    else // travel through the select column parse node
     {
         ::comphelper::UStringMixEqual bCase(xMetaData->supportsMixedCaseQuotedIdentifiers());
 
@@ -777,6 +790,7 @@ sal_Bool OSelectionBrowseBox::saveField(const String& _sFieldName,OTableFieldDes
 
             ::connectivity::OSQLParseNode* pColumnRef = pChild->getChild(0);
             if (
+                    pColumnRef->getKnownRuleID() != OSQLParseNode::subquery &&
                     pColumnRef->count() == 3 &&
                     SQL_ISPUNCTUATION(pColumnRef->getChild(0),"(") &&
                     SQL_ISPUNCTUATION(pColumnRef->getChild(2),")")
diff --git a/dbaccess/source/ui/querydesign/SelectionBrowseBox.hxx b/dbaccess/source/ui/querydesign/SelectionBrowseBox.hxx
index b3b2c9815214..55bf2703bc16 100644
--- a/dbaccess/source/ui/querydesign/SelectionBrowseBox.hxx
+++ b/dbaccess/source/ui/querydesign/SelectionBrowseBox.hxx
@@ -246,7 +246,7 @@ namespace dbaui
             @return
                 <TRUE/> if an error occurred otherwise <FALSE/>
         */
-        sal_Bool        saveField(const String& _sFieldName,OTableFieldDescRef& _pEntry,sal_Bool& _bListAction);
+        sal_Bool        saveField(String& _sFieldName, OTableFieldDescRef& _pEntry, sal_Bool& _bListAction);
 
         /** sets the table window at the _pEntry
             @param  _pEntry