From 880c58db42192ebe9718a59aff44cf4d083f4049 Mon Sep 17 00:00:00 2001 From: Julien Nabet Date: Sun, 12 Dec 2021 11:59:53 +0100 Subject: tdf#38235: fix sort failure for Evolution Several issues to deal with: 1) The initial pb: "Error setting the source criteria. The column XXX must be visible as a column. SQL status: 01000, Error code 1000." => OEvoabResultSetMetaData::getColumnLabel was returning the nickname of the column with g_param_spec_get_nick we just want the column name to display and to use 2) SQL parsing in OCommonStatement::orderByAnalysis 2 "sub-issues": a) ENSURE_OR_THROW was testing SQL_ISRULE( pAscDesc, opt_asc_desc ) opt_asc_desc is defined in connectivity/source/parse/sqlbison.y with: opt_asc_desc: {$$ = SQL_NEW_RULE;} | SQL_TOKEN_ASC | SQL_TOKEN_DESC ; not sure if it should be kept but for DESC I had to use this: SQL_ISTOKEN(pAscDesc, DESC) b) Retrieve of ascending By default ascending is at true but then we tested the node with: if ( ( pAscDesc->count() == 1 ) && SQL_ISTOKEN( pAscDesc->getChild( 0 ), DESC ) But when we use DESC, it's directly a TOKEN so just this should suffice: bool bAscending = !SQL_ISTOKEN(pAscDesc, DESC); 3) CompareContacts wasn't taking into account bAscending since we use comparison function for g_slist_sort_with_data, I only used int nOrder = 1; // if descending sort, reverse order if (!sortCol.bAscending) nOrder = -1; and multiply the result with this to have the ad hoc order Change-Id: I3c360a5ef9cf0dc737a7ce4a138d2d2586abdca9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126698 Tested-by: Jenkins Reviewed-by: Julien Nabet (cherry picked from commit ab864bd178a44208c98a2fd1b1248df5f1db1fc9) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126967 Reviewed-by: Christian Lohmaier --- connectivity/source/drivers/evoab2/NResultSet.cxx | 14 +++++++++----- connectivity/source/drivers/evoab2/NResultSetMetaData.cxx | 10 +--------- connectivity/source/drivers/evoab2/NStatement.cxx | 14 +++++++------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/connectivity/source/drivers/evoab2/NResultSet.cxx b/connectivity/source/drivers/evoab2/NResultSet.cxx index 6df2d806ab3c..e6a43dab7687 100644 --- a/connectivity/source/drivers/evoab2/NResultSet.cxx +++ b/connectivity/source/drivers/evoab2/NResultSet.cxx @@ -308,6 +308,10 @@ static int CompareContacts( gconstpointer _lhs, gconstpointer _rhs, gpointer _us for ( const auto& sortCol : rCompData.rSortOrder ) { sal_Int32 nField = sortCol.nField; + int nOrder = 1; + // if descending sort, reverse order + if (!sortCol.bAscending) + nOrder = -1; GType eFieldType = evoab::getGFieldType( nField ); bool success = getValue( lhs, nField, eFieldType, &aLhsValue, bLhsNull ) @@ -317,9 +321,9 @@ static int CompareContacts( gconstpointer _lhs, gconstpointer _rhs, gpointer _us return 0; if ( bLhsNull && !bRhsNull ) - return -1; + return -1 * nOrder; if ( !bLhsNull && bRhsNull ) - return 1; + return 1 * nOrder; if ( bLhsNull && bRhsNull ) continue; @@ -329,16 +333,16 @@ static int CompareContacts( gconstpointer _lhs, gconstpointer _rhs, gpointer _us sRhs = valueToOUString( aRhsValue ); sal_Int32 nCompResult = rCompData.aIntlWrapper.getCaseCollator()->compareString( sLhs, sRhs ); if ( nCompResult != 0 ) - return nCompResult; + return nCompResult * nOrder; continue; } bLhs = valueToBool( aLhsValue ); bRhs = valueToBool( aRhsValue ); if ( bLhs && !bRhs ) - return -1; + return -1 * nOrder; if ( !bLhs && bRhs ) - return 1; + return 1 * nOrder; continue; } diff --git a/connectivity/source/drivers/evoab2/NResultSetMetaData.cxx b/connectivity/source/drivers/evoab2/NResultSetMetaData.cxx index a980c2067041..a2ce922ff193 100644 --- a/connectivity/source/drivers/evoab2/NResultSetMetaData.cxx +++ b/connectivity/source/drivers/evoab2/NResultSetMetaData.cxx @@ -101,15 +101,7 @@ OUString SAL_CALL OEvoabResultSetMetaData::getColumnTypeName( sal_Int32 nColumnN OUString SAL_CALL OEvoabResultSetMetaData::getColumnLabel( sal_Int32 nColumnNum ) { - sal_uInt32 nField = m_aEvoabFields[nColumnNum - 1]; - const ColumnProperty *pSpecs = getField(nField); - GParamSpec *pSpec = pSpecs->pField; - OUString aLabel; - - if( pSpec ) - aLabel = OStringToOUString( g_param_spec_get_nick( pSpec ), - RTL_TEXTENCODING_UTF8 ); - return aLabel; + return getColumnName(nColumnNum); } OUString SAL_CALL OEvoabResultSetMetaData::getColumnServiceName( sal_Int32 /*nColumnNum*/ ) diff --git a/connectivity/source/drivers/evoab2/NStatement.cxx b/connectivity/source/drivers/evoab2/NStatement.cxx index 542ee1de8d45..1f3e21a2ef90 100644 --- a/connectivity/source/drivers/evoab2/NStatement.cxx +++ b/connectivity/source/drivers/evoab2/NStatement.cxx @@ -268,8 +268,11 @@ void OCommonStatement::orderByAnalysis( const OSQLParseNode* _pOrderByClause, So ENSURE_OR_THROW( ( pColumnRef != nullptr ) && ( pAscDesc != nullptr ) - && SQL_ISRULE( pAscDesc, opt_asc_desc ) - && ( pAscDesc->count() < 2 ), + && ( pAscDesc->isLeaf() ) + && ( SQL_ISRULE( pAscDesc, opt_asc_desc ) + || SQL_ISTOKEN(pAscDesc, ASC) + || SQL_ISTOKEN(pAscDesc, DESC) + ), "ordering_spec structure error" ); // column name -> column field @@ -278,11 +281,7 @@ void OCommonStatement::orderByAnalysis( const OSQLParseNode* _pOrderByClause, So const OUString sColumnName( impl_getColumnRefColumnName_throw( *pColumnRef ) ); guint nField = evoab::findEvoabField( sColumnName ); // ascending/descending? - bool bAscending = true; - if ( ( pAscDesc->count() == 1 ) - && SQL_ISTOKEN( pAscDesc->getChild( 0 ), DESC ) - ) - bAscending = false; + bool bAscending = !SQL_ISTOKEN(pAscDesc, DESC); _out_rSort.push_back( FieldSort( nField, bAscending ) ); } @@ -506,6 +505,7 @@ void OCommonStatement::parseSql( const OUString& sql, QueryData& _out_rQueryData pOrderByClause->showParseTree( sTreeDebug ); SAL_INFO( "connectivity.evoab2", "found order-by tree:\n" << sTreeDebug ); #endif + orderByAnalysis( pOrderByClause, _out_rQueryData.aSortOrder ); } -- cgit