Improvement CORE-4529: Allow to use index when GROUP BY on field whic…

…h has DESCENDING index

src/jrd/Optimizer.cpp

@@ -687,12 +687,12 @@ void OptimizerRetrieval::analyzeNavigation()
 		const index_desc::idx_repeat* idx_tail = idx->idx_rpt;
 		const index_desc::idx_repeat* const idx_end = idx_tail + idx->idx_count;
 		NestConst<ValueExprNode>* ptr = sort->expressions.begin();
-		const bool* descending = sort->descending.begin();
-		const int* nullOrder = sort->nullOrder.begin();
+		const SortDirection* direction = sort->direction.begin();
+		const NullsPlacement* nullOrder = sort->nullOrder.begin();
 
 		for (const NestConst<ValueExprNode>* const end = sort->expressions.end();
 			 ptr != end;
-			 ++ptr, ++descending, ++nullOrder, ++idx_tail)
+			 ++ptr, ++direction, ++nullOrder, ++idx_tail)
 		{
 			ValueExprNode* const node = *ptr;
 			FieldNode* fieldNode;
@@ -727,10 +727,10 @@ void OptimizerRetrieval::analyzeNavigation()
 				}
 			}
 
-			if ((*descending && !(idx->idx_flags & idx_descending)) ||
-				(!*descending && (idx->idx_flags & idx_descending)) ||
-				((*nullOrder == rse_nulls_first && *descending) ||
-				 (*nullOrder == rse_nulls_last && !*descending)))
+			if ((*direction == ORDER_DESC && !(idx->idx_flags & idx_descending)) ||
+				(*direction == ORDER_ASC && (idx->idx_flags & idx_descending)) ||
+				((*nullOrder == NULLS_FIRST && *direction == ORDER_DESC) ||
+				 (*nullOrder == NULLS_LAST && *direction == ORDER_ASC)))
 			{
 				usableIndex = false;
 				break;

src/jrd/RecordSourceNodes.cpp

@@ -121,7 +121,7 @@ SortNode* SortNode::copy(thread_db* tdbb, NodeCopier& copier) const
 	for (const NestConst<ValueExprNode>* i = expressions.begin(); i != expressions.end(); ++i)
 		newSort->expressions.add(copier.copy(tdbb, *i));
 
-	newSort->descending = descending;
+	newSort->direction = direction;
 	newSort->nullOrder = nullOrder;
 
 	return newSort;
@@ -1616,10 +1616,12 @@ RecordSource* AggregateSourceNode::generate(thread_db* tdbb, OptimizerBlk* opt,
 			FB_NEW_POOL(*tdbb->getDefaultPool()) SortNode(*tdbb->getDefaultPool());
 
 		aggregate->expressions.add(aggNode->arg);
-		// in the max case, flag the sort as descending
-		aggregate->descending.add(aggNode->aggInfo.blr == blr_agg_max);
+		// For MAX(), mark the sort as descending. For MIN(), mark as ascending.
+		const SortDirection direction =
+			(aggNode->aggInfo.blr == blr_agg_max) ? ORDER_DESC : ORDER_ASC;
+		aggregate->direction.add(direction);
 		// 10-Aug-2004. Nickolay Samofatov - Unneeded nulls seem to be skipped somehow.
-		aggregate->nullOrder.add(rse_nulls_default);
+		aggregate->nullOrder.add(NULLS_DEFAULT);
 
 		rse->flags |= RseNode::FLAG_OPT_FIRST_ROWS;
 	}

src/jrd/RecordSourceNodes.h

@@ -53,7 +53,7 @@ class SortNode : public Firebird::PermanentStorage, public Printable
 		: PermanentStorage(pool),
 		  unique(false),
 		  expressions(pool),
-		  descending(pool),
+		  direction(pool),
 		  nullOrder(pool)
 	{
 	}
@@ -71,19 +71,22 @@ class SortNode : public Firebird::PermanentStorage, public Printable
 	bool computable(CompilerScratch* csb, StreamType stream, bool allowOnlyCurrentStream);
 	void findDependentFromStreams(const OptimizerRetrieval* optRet, SortedStreamList* streamList);
 
-	int getEffectiveNullOrder(unsigned index) const
+	NullsPlacement getEffectiveNullOrder(unsigned index) const
 	{
-		if (descending[index])
-			return nullOrder[index] == rse_nulls_default ? rse_nulls_last : nullOrder[index];
-		else
-			return nullOrder[index] == rse_nulls_default ? rse_nulls_first : nullOrder[index];
+		if (direction[index] == ORDER_ASC)
+			return (nullOrder[index] == NULLS_DEFAULT) ? NULLS_FIRST : nullOrder[index];
+		else if (direction[index] == ORDER_DESC)
+			return (nullOrder[index] == NULLS_DEFAULT) ? NULLS_LAST : nullOrder[index];
+
+		fb_assert(false);
+		return NULLS_DEFAULT;
 	}
 
 public:
-	bool unique;						// sorts using unique key - for distinct and group by
+	bool unique;						// sort uses unique key - for DISTINCT and GROUP BY
 	NestValueArray expressions;			// sort expressions
-	Firebird::Array<bool> descending;	// true = descending / false = ascending
-	Firebird::Array<int> nullOrder;		// rse_nulls_*
+	Firebird::Array<SortDirection> direction;	// rse_order_*
+	Firebird::Array<NullsPlacement> nullOrder;	// rse_nulls_*
 };
 
 class MapNode : public Firebird::PermanentStorage, public Printable

src/jrd/exe.h

@@ -86,10 +86,11 @@ class MessageNode;
 class PlanNode;
 class RecordSource;
 
+// Direction for each column in sort order
+enum SortDirection { ORDER_ANY, ORDER_ASC, ORDER_DESC };
+
 // Types of nulls placement for each column in sort order
-const int rse_nulls_default	= 0;
-const int rse_nulls_first	= 1;
-const int rse_nulls_last	= 2;
+enum NullsPlacement { NULLS_DEFAULT, NULLS_FIRST, NULLS_LAST };
 
 
 // Aggregate Sort Block (for DISTINCT aggregates)

src/jrd/opt.cpp

@@ -2491,12 +2491,12 @@ SortedStream* OPT_gen_sort(thread_db* tdbb, CompilerScratch* csb, const StreamLi
 
 	SortedStream::SortMap::Item* map_item = map->items.getBuffer(items);
 	sort_key_def* sort_key = map->keyItems.getBuffer(2 * sort->expressions.getCount());
-	int* nullOrder = sort->nullOrder.begin();
-	const bool* descending = sort->descending.begin();
+	const SortDirection* direction = sort->direction.begin();
+	const NullsPlacement* nullOrder = sort->nullOrder.begin();
 
 	for (NestConst<ValueExprNode>* node_ptr = sort->expressions.begin();
 		 node_ptr != end_node;
-		 ++node_ptr, ++nullOrder, ++descending, ++map_item)
+		 ++node_ptr, ++nullOrder, ++direction, ++map_item)
 	{
 		// Pick up sort key expression.
 
@@ -2531,7 +2531,7 @@ SortedStream* OPT_gen_sort(thread_db* tdbb, CompilerScratch* csb, const StreamLi
 		sort_key->skd_flags = SKD_ascending;
 
 		// Have SQL-compliant nulls ordering for ODS11+
-		if ((*nullOrder == rse_nulls_default && !*descending) || *nullOrder == rse_nulls_first)
+		if ((*nullOrder == NULLS_DEFAULT && *direction != ORDER_DESC) || *nullOrder == NULLS_FIRST)
 			sort_key->skd_flags |= SKD_descending;
 
 		prev_key = sort_key++;
@@ -2541,7 +2541,7 @@ SortedStream* OPT_gen_sort(thread_db* tdbb, CompilerScratch* csb, const StreamLi
 		sort_key->setSkdLength(sort_dtypes[desc->dsc_dtype], desc->dsc_length);
 		sort_key->setSkdOffset(&sort_key[-1], desc);
 		sort_key->skd_flags = SKD_ascending;
-		if (*descending)
+		if (*direction == ORDER_DESC)
 			sort_key->skd_flags |= SKD_descending;
 
 		if (!sort_key->skd_dtype)
@@ -2921,8 +2921,8 @@ static bool gen_equi_join(thread_db* tdbb, OptimizerBlk* opt, RiverList& org_riv
 			for (selected_class = selected_classes.begin();
 				 selected_class != selected_classes.end(); ++selected_class)
 			{
-				key->descending.add(false);	// Ascending sort
-				key->nullOrder.add(rse_nulls_default);	// Default nulls placement
+				key->direction.add(ORDER_ASC);	// Ascending sort
+				key->nullOrder.add(NULLS_DEFAULT);	// Default nulls placement
 				key->expressions.add((*selected_class)[number]);
 			}
 
@@ -3499,14 +3499,14 @@ static void set_direction(SortNode* fromClause, SortNode* toClause)
 	const size_t fromCount = fromClause->expressions.getCount();
 
 	fb_assert(fromCount <= toClause->expressions.getCount());
-	fb_assert(fromCount == fromClause->descending.getCount() &&
+	fb_assert(fromCount == fromClause->direction.getCount() &&
 		fromCount == fromClause->nullOrder.getCount());
-	fb_assert(toClause->expressions.getCount() == toClause->descending.getCount() &&
+	fb_assert(toClause->expressions.getCount() == toClause->direction.getCount() &&
 		toClause->expressions.getCount() == toClause->nullOrder.getCount());
 
 	for (FB_SIZE_T i = 0; i < fromCount; ++i)
 	{
-		toClause->descending[i] = fromClause->descending[i];
+		toClause->direction[i] = fromClause->direction[i];
 		toClause->nullOrder[i] = fromClause->nullOrder[i];
 	}
 }

src/jrd/par.cpp

@@ -1437,8 +1437,8 @@ SortNode* PAR_sort_internal(thread_db* tdbb, CompilerScratch* csb, bool allClaus
 		*tdbb->getDefaultPool());
 
 	NestConst<ValueExprNode>* ptr = sort->expressions.getBuffer(count);
-	bool* ptr2 = sort->descending.getBuffer(count);
-	int* ptr3 = sort->nullOrder.getBuffer(count);
+	SortDirection* ptr2 = sort->direction.getBuffer(count);
+	NullsPlacement* ptr3 = sort->nullOrder.getBuffer(count);
 
 	while (count-- > 0)
 	{
@@ -1449,25 +1449,25 @@ SortNode* PAR_sort_internal(thread_db* tdbb, CompilerScratch* csb, bool allClaus
 			switch (code)
 			{
 				case blr_nullsfirst:
-					*ptr3++ = rse_nulls_first;
+					*ptr3++ = NULLS_FIRST;
 					code = csb->csb_blr_reader.getByte();
 					break;
 
 				case blr_nullslast:
-					*ptr3++ = rse_nulls_last;
+					*ptr3++ = NULLS_LAST;
 					code = csb->csb_blr_reader.getByte();
 					break;
 
 				default:
-					*ptr3++ = rse_nulls_default;
+					*ptr3++ = NULLS_DEFAULT;
 			}
 
-			*ptr2++ = (code == blr_descending);
+			*ptr2++ = (code == blr_descending) ? ORDER_DESC : ORDER_ASC;
 		}
 		else
 		{
-			*ptr2++ = false;	// ascending
-			*ptr3++ = rse_nulls_default;
+			*ptr2++ = ORDER_ANY;
+			*ptr3++ = NULLS_DEFAULT;
 		}
 
 		*ptr++ = PAR_parse_value(tdbb, csb);

src/jrd/recsrc/AggregatedStream.cpp

@@ -308,17 +308,18 @@ int BaseAggWinStream<ThisType, NextType>::lookForChange(thread_db* tdbb, jrd_req
 		 ptrValue != endValue;
 		 ++ptrValue)
 	{
-		int direction = 1;
-		int nullDirection = 1;
+		int sortDirection = 1;
+		int nullsPlacement = 1;
 
 		if (sort)
 		{
 			unsigned index = ptrValue - group->begin();
 
-			if (sort->descending[index])
-				direction = -1;
+			if (sort->direction[index] == ORDER_DESC)
+				sortDirection = -1;
 
-			nullDirection = (sort->getEffectiveNullOrder(index) == rse_nulls_first ? 1 : -1);
+			if (sort->getEffectiveNullOrder(index) == NULLS_LAST)
+				nullsPlacement = -1;
 		}
 
 		const ValueExprNode* from = *ptrValue;
@@ -330,12 +331,12 @@ int BaseAggWinStream<ThisType, NextType>::lookForChange(thread_db* tdbb, jrd_req
 		if (request->req_flags & req_null)
 		{
 			if (vtemp->vlu_desc.dsc_address)
-				return -1 * nullDirection;
+				return -1 * nullsPlacement;
 		}
 		else if (!vtemp->vlu_desc.dsc_address)
-			return 1 * nullDirection;
+			return 1 * nullsPlacement;
 		else if ((n = MOV_compare(tdbb, desc, &vtemp->vlu_desc)) != 0)
-			return n * direction;
+			return n * sortDirection;
 	}
 
 	return 0;

src/jrd/recsrc/WindowedStream.cpp

@@ -271,18 +271,18 @@ WindowedStream::WindowedStream(thread_db* tdbb, CompilerScratch* csb,
 		{
 			if (window->order)
 			{
-				Array<int>::iterator nullIt = window->order->nullOrder.begin();
+				Array<NullsPlacement>::iterator nullIt = window->order->nullOrder.begin();
 
-				for (Array<bool>::iterator descIt = window->order->descending.begin();
-					 descIt != window->order->descending.end();
+				for (Array<SortDirection>::iterator descIt = window->order->direction.begin();
+					 descIt != window->order->direction.end();
 					 ++descIt, ++nullIt)
 				{
 					*descIt = !*descIt;
 
-					if (*nullIt == rse_nulls_first)
-						*nullIt = rse_nulls_last;
-					else if (*nullIt == rse_nulls_last)
-						*nullIt = rse_nulls_first;
+					if (*nullIt == NULLS_FIRST)
+						*nullIt = NULLS_LAST;
+					else if (*nullIt == NULLS_LAST)
+						*nullIt = NULLS_FIRST;
 				}
 			}
 
@@ -305,13 +305,13 @@ WindowedStream::WindowedStream(thread_db* tdbb, CompilerScratch* csb,
 		{
 			windowOrder = FB_NEW_POOL(csb->csb_pool) SortNode(csb->csb_pool);
 			windowOrder->expressions.join(window->group->expressions);
-			windowOrder->descending.join(window->group->descending);
+			windowOrder->direction.join(window->group->direction);
 			windowOrder->nullOrder.join(window->group->nullOrder);
 
 			if (window->order)
 			{
 				windowOrder->expressions.join(window->order->expressions);
-				windowOrder->descending.join(window->order->descending);
+				windowOrder->direction.join(window->order->direction);
 				windowOrder->nullOrder.join(window->order->nullOrder);
 			}
 		}
@@ -478,7 +478,7 @@ WindowedStream::WindowStream::WindowStream(thread_db* tdbb, CompilerScratch* csb
 			{
 				int direction = frame->bound == WindowClause::Frame::Bound::FOLLOWING ? 1 : -1;
 
-				if (m_order->descending[0])
+				if (m_order->direction[0] == ORDER_DESC)
 					direction *= -1;
 
 				m_arithNodes[i] = FB_NEW_POOL(csb->csb_pool) ArithmeticNode(csb->csb_pool,
@@ -953,7 +953,7 @@ SINT64 WindowedStream::WindowStream::locateFrameRange(thread_db* tdbb, jrd_req*
 	{
 		int direction = (frame->bound == WindowClause::Frame::Bound::FOLLOWING ? 1 : -1);
 
-		if (m_order->descending[0])
+		if (m_order->direction[0] == ORDER_DESC)
 			direction *= -1;
 
 		cacheValues(tdbb, request, &m_order->expressions, impure->orderValues,