Skip to content

Commit

Permalink
fixed sort property depends on the properties order at the aggs node;…
Browse files Browse the repository at this point in the history
… added cases to test 334; fixed #2364
  • Loading branch information
tomatolog committed Jul 11, 2024
1 parent e7d72c8 commit f55d215
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 94 deletions.
201 changes: 109 additions & 92 deletions src/sphinxjsonquery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3625,6 +3625,87 @@ static bool ParseAggrComposite ( const JsonObj_c & tBucket, JsonAggr_t & tAggr,
return true;
}

static bool ParseAggsNode ( const JsonObj_c & tBucket, const JsonObj_c & tJsonItem, bool bRoot, JsonAggr_t & tItem, CSphString & sError )
{
if ( !tBucket.IsObj() )
{
sError.SetSprintf ( R"("aggs" bucket '%s' should be an object)", tItem.m_sBucketName.cstr() );
return false;
}

if ( !StrEq ( tBucket.Name(), "composite" ) && !tBucket.FetchStrItem ( tItem.m_sCol, "field", sError, false ) )
return false;

tBucket.FetchIntItem ( tItem.m_iSize, "size", sError, true );
int iShardSize = 0;
tBucket.FetchIntItem ( iShardSize, "shard_size", sError, true );
tItem.m_iSize = Max ( tItem.m_iSize, iShardSize ); // FIXME!!! use (size * 1.5 + 10) for shard size
tItem.m_eAggrFunc = GetAggrFunc ( tBucket, !bRoot );
switch ( tItem.m_eAggrFunc )
{
case Aggr_e::DATE_HISTOGRAM:
if ( !ParseAggrDateHistogram ( tBucket, tItem, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, 1000 ); // set max_matches to min\max / interval
break;

case Aggr_e::HISTOGRAM:
if ( !ParseAggrHistogram ( tBucket, tItem, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, 1000 ); // set max_matches to min\max / interval
break;

case Aggr_e::RANGE:
if ( !ParseAggrRange ( tBucket, tItem, false, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, tItem.m_tRange.GetLength() + 1 ); // set max_matches to buckets count + _all bucket
break;

case Aggr_e::DATE_RANGE:
if ( !ParseAggrRange ( tBucket, tItem, true, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, tItem.m_tDateRange.GetLength() + 1 ); // set max_matches to buckets count + _all bucket
break;
case Aggr_e::COMPOSITE:
if ( !ParseAggrComposite ( tJsonItem, tItem, sError ) )
return false;
break;
case Aggr_e::MIN:
case Aggr_e::MAX:
case Aggr_e::SUM:
case Aggr_e::AVG:
tItem.m_iSize = 1;
break;

default: break;
}

return true;
}

static bool ParseAggsNodeSort ( const JsonObj_c & tJsonItem, bool bOrder, JsonAggr_t & tItem, CSphString & sError )
{
if ( !( tJsonItem.IsArray() || tJsonItem.IsObj() ) )
{
sError.SetSprintf ( "\"%s\" property value should be an array or an object", ( bOrder ? "order" : "sort" ) );
return false;
}

bool bGotWeight = false;
JsonQuery_c tTmpQuery;
tTmpQuery.m_sSortBy = "";
tTmpQuery.m_eSort = SPH_SORT_RELEVANCE;

// FIXME!!! reports warnings for geodist sort
CSphString sWarning;

if ( !ParseSort ( tJsonItem, tTmpQuery, bGotWeight, sError, sWarning ) )
return false;

tItem.m_sSort = tTmpQuery.m_sSortBy;
return true;
}

static bool AddSubAggregate ( const JsonObj_c & tAggs, bool bRoot, CSphVector<JsonAggr_t> & dParentItems, CSphString & sError )
{
if ( bRoot && tAggs.begin().Empty() )
Expand All @@ -3635,9 +3716,6 @@ static bool AddSubAggregate ( const JsonObj_c & tAggs, bool bRoot, CSphVector<Js
return true;
}

CSphString sWarning;
JsonQuery_c tTmpQuery;

for ( const auto & tJsonItem : tAggs )
{
if ( !tJsonItem.IsObj() )
Expand All @@ -3649,102 +3727,41 @@ static bool AddSubAggregate ( const JsonObj_c & tAggs, bool bRoot, CSphVector<Js
JsonAggr_t tItem;
tItem.m_sBucketName = tJsonItem.Name();

JsonObj_c tBucket = tJsonItem.begin();

if ( StrEq ( tBucket.Name(), "aggs" ) && tBucket!=tJsonItem.end() )
{
if ( !AddSubAggregate ( tBucket, false, tItem.m_dNested, sError ) )
return false;
++tBucket;
}

if ( tBucket==tJsonItem.end() )
for ( const auto & tAggsItem : tJsonItem )
{
sError.SetSprintf ( R"("aggs" bucket '%s' with only nested items)", tItem.m_sBucketName.cstr() );
return false;
}

if ( !tBucket.IsObj() )
{
sError.SetSprintf ( R"("aggs" bucket '%s' should be an object)", tItem.m_sBucketName.cstr() );
return false;
}

if ( !StrEq ( tBucket.Name(), "composite" ) && !tBucket.FetchStrItem ( tItem.m_sCol, "field", sError, false ) )
return false;

//tBucket.FetchStrItem ( tItem.m_sExpr, "calendar_interval", sError, true );
tBucket.FetchIntItem ( tItem.m_iSize, "size", sError, true );
int iShardSize = 0;
tBucket.FetchIntItem ( iShardSize, "shard_size", sError, true );
tItem.m_iSize = Max ( tItem.m_iSize, iShardSize ); // FIXME!!! use (size * 1.5 + 10) for shard size
tItem.m_eAggrFunc = GetAggrFunc ( tBucket, !bRoot );
switch ( tItem.m_eAggrFunc )
{
case Aggr_e::DATE_HISTOGRAM:
if ( !ParseAggrDateHistogram ( tBucket, tItem, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, 1000 ); // set max_matches to min\max / interval
break;

case Aggr_e::HISTOGRAM:
if ( !ParseAggrHistogram ( tBucket, tItem, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, 1000 ); // set max_matches to min\max / interval
break;

case Aggr_e::RANGE:
if ( !ParseAggrRange ( tBucket, tItem, false, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, tItem.m_tRange.GetLength() + 1 ); // set max_matches to buckets count + _all bucket
break;

case Aggr_e::DATE_RANGE:
if ( !ParseAggrRange ( tBucket, tItem, true, sError ) )
return false;
tItem.m_iSize = Max ( tItem.m_iSize, tItem.m_tDateRange.GetLength() + 1 ); // set max_matches to buckets count + _all bucket
break;
case Aggr_e::COMPOSITE:
if ( !ParseAggrComposite ( tJsonItem, tItem, sError ) )
return false;
break;
case Aggr_e::MIN:
case Aggr_e::MAX:
case Aggr_e::SUM:
case Aggr_e::AVG:
tItem.m_iSize = 1;

default: break;
}
// could be a sort object at the aggs item or order object at the bucket
if ( strcmp ( tAggsItem.Name(), "sort" )==0 )
{
if ( !ParseAggsNodeSort ( tAggsItem, false, tItem, sError ) )
return false;

// could be a sort object at the aggs item or order object at the bucket
bool bOrder = false;
JsonObj_c tSort = tJsonItem.GetItem("sort");
if ( !tSort && tBucket.HasItem ( "order" ) )
{
tSort = tBucket.GetItem("order");
bOrder = true;
}
if ( tSort && !( tSort.IsArray() || tSort.IsObj() ) )
{
sError.SetSprintf ( "\"%s\" property value should be an array or an object", ( bOrder ? "order" : "sort" ) );
return false;
}
if ( tSort )
{
bool bGotWeight = false;
tTmpQuery.m_sSortBy = "";
tTmpQuery.m_eSort = SPH_SORT_RELEVANCE;
// FIXME!!! reports warnings for geodist sort
if ( !ParseSort ( tSort, tTmpQuery, bGotWeight, sError, sWarning ) )
return false;
} else
{
if ( StrEq ( tAggsItem.Name(), "aggs" ) || tAggsItem.HasItem ( "aggs" ) )
{
sError = R"(nested "aggs" is not supported)";
return false;
}
if ( tAggsItem==tAggsItem.end() )
{
sError.SetSprintf ( R"("aggs" bucket '%s' with only nested items)", tAggsItem.Name() );
return false;
}
if ( !ParseAggsNode ( tAggsItem, tJsonItem, bRoot, tItem, sError ) )
return false;

tItem.m_sSort = tTmpQuery.m_sSortBy;
// bucket could have its own order item
if ( tAggsItem.HasItem ( "order" ) )
{
if ( !ParseAggsNodeSort ( tAggsItem.GetItem("order"), true, tItem, sError ) )
return false;
}
}
}

if ( tItem.m_eAggrFunc==Aggr_e::NONE && !bRoot )
{
sError.SetSprintf ( R"(bucket '%s' without aggregate items, item type is '%s')", tItem.m_sBucketName.cstr(), tBucket.Name() );
sError.SetSprintf ( R"(bucket '%s' without aggregate items)", tItem.m_sBucketName.cstr() );
return false;
}

Expand Down
Loading

0 comments on commit f55d215

Please sign in to comment.