fix: sorting by string with OPTION scroll

Related issue #3928

Author: sanikolaev

src/sorterscroll.cpp

@@ -64,6 +64,7 @@ class ScrollSorter_T : public ISphMatchSorter
 	CSphVector<CSphRowitem>			m_dStatic;
 	CSphMatchComparatorState		m_tState;
 	ScrollSettings_t				m_tScroll;
+	CSphVector<BYTE*>				m_dAllocatedPtrs; // Track allocated pointer values directly
 
 	FORCE_INLINE bool PushMatch ( const CSphMatch & tEntry );
 	void	SetupRefMatch();
@@ -122,8 +123,10 @@ void ScrollSorter_T<COMP>::SetupRefMatch()
 	const ISphSchema * pSchema = m_pSorter->GetSchema();
 	assert(pSchema);
 
+	// Free old data pointer attributes before resetting
 	FreeDataPtrAttrs();
-	m_tRefMatch.Reset ( pSchema->GetRowSize() );
+	
+	m_tRefMatch.Reset ( pSchema->GetDynamicSize() );
 	m_dStatic.Resize ( pSchema->GetStaticSize() );
 	m_tRefMatch.m_pStatic = m_dStatic.Begin();
 
@@ -137,11 +140,32 @@ void ScrollSorter_T<COMP>::SetupRefMatch()
 		}
 
 		auto pAttr = pSchema->GetAttr ( i.m_sSortAttr.cstr() );
+		if ( !pAttr )
+			continue;
+
+		// If the scroll token has a string type and the schema attribute is SPH_ATTR_STRING,
+		// use the remapped STRINGPTR version (used for sorting)
+		if ( i.m_eType==SPH_ATTR_STRINGPTR && pAttr->m_eAttrType==SPH_ATTR_STRING )
+		{
+			CSphString sRemappedName;
+			sRemappedName.SetSprintf ( "@int_attr_%s", i.m_sSortAttr.cstr() );
+			auto pRemapped = pSchema->GetAttr ( sRemappedName.cstr() );
+			if ( pRemapped && pRemapped->m_eAttrType==SPH_ATTR_STRINGPTR )
+				pAttr = pRemapped;
+		}
+
 		auto pRowData = pAttr->m_tLocator.m_bDynamic ? m_tRefMatch.m_pDynamic : m_dStatic.Begin();
 		switch ( i.m_eType )
 		{
 		case SPH_ATTR_STRINGPTR:
-			sphSetRowAttr ( pRowData, pAttr->m_tLocator, (SphAttr_t)sphPackPtrAttr ( { (const BYTE*)i.m_sValue.cstr(), i.m_sValue.Length() } ) );
+			// After remapping, pAttr should be SPH_ATTR_STRINGPTR, but check to be safe
+			if ( pAttr->m_eAttrType==SPH_ATTR_STRINGPTR )
+			{
+				BYTE * pPacked = sphPackPtrAttr ( { (const BYTE*)i.m_sValue.cstr(), i.m_sValue.Length() } );
+				sphSetRowAttr ( pRowData, pAttr->m_tLocator, (SphAttr_t)pPacked );
+				// Track the allocated pointer value directly
+				m_dAllocatedPtrs.Add(pPacked);
+			}
 			break;
 
 		case SPH_ATTR_FLOAT:
@@ -158,25 +182,16 @@ void ScrollSorter_T<COMP>::SetupRefMatch()
 template <typename COMP>
 void ScrollSorter_T<COMP>::FreeDataPtrAttrs()
 {
-	if ( !m_tRefMatch.m_pDynamic )
-		return;
-
-	const ISphSchema * pSchema = m_pSorter->GetSchema();
-	assert(pSchema);
-
-	for ( auto & i : m_tScroll.m_dAttrs )
+	// Free all pointers we allocated directly
+	// We store the pointer values themselves, not the attributes, so we don't need
+	// to access m_tRefMatch which might have been reset
+	for ( auto pPacked : m_dAllocatedPtrs )
 	{
-		if ( i.m_sSortAttr=="weight()" )
-			continue;
-
-		const CSphColumnInfo * pAttr = pSchema->GetAttr ( i.m_sSortAttr.cstr() );
-		assert(pAttr);
-		if ( sphIsDataPtrAttr ( pAttr->m_eAttrType ) )
-		{
-			auto pData = (BYTE *)m_tRefMatch.GetAttr ( pAttr->m_tLocator );
-			sphDeallocatePacked(pData);
-		}
+		if ( pPacked )
+			sphDeallocatePacked(pPacked);
 	}
+	
+	m_dAllocatedPtrs.Resize(0);
 }
 
 /////////////////////////////////////////////////////////////////////
@@ -218,9 +233,22 @@ static bool CanCreateScrollSorter ( bool bMulti, const ISphSchema & tSchema, con
 		if ( !pAttr )
 			return false;
 
+		ESphAttr eCheckType = pAttr->m_eAttrType;
+		
+		// If the scroll token has a string type and the schema attribute is SPH_ATTR_STRING,
+		// check if there's a remapped STRINGPTR version (used for sorting)
+		if ( i.m_eType==SPH_ATTR_STRINGPTR && eCheckType==SPH_ATTR_STRING )
+		{
+			CSphString sRemappedName;
+			sRemappedName.SetSprintf ( "@int_attr_%s", i.m_sSortAttr.cstr() );
+			auto pRemapped = tSchema.GetAttr ( sRemappedName.cstr() );
+			if ( pRemapped && pRemapped->m_eAttrType==SPH_ATTR_STRINGPTR )
+				eCheckType = SPH_ATTR_STRINGPTR;
+		}
+
 		bool bSupported = false;
 		for ( auto eType : dSupportedTypes )
-			if ( pAttr->m_eAttrType==eType )
+			if ( eCheckType==eType )
 			{
 				bSupported = true;
 				break;