Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[temporal] Fix filter generation for vector temporal layers
When features have a temporal "range", we should never treat
that range as inclusive of the end (or we end up with features
appearing in multiple date ranges)

Completes fixes begun by @rduivenvoorde in #40989

Fixes #38468
  • Loading branch information
nyalldawson authored and github-actions[bot] committed Jun 9, 2021
1 parent 127b605 commit 01ebc06
Show file tree
Hide file tree
Showing 4 changed files with 539 additions and 43 deletions.
Expand Up @@ -328,7 +328,7 @@ occur before or within the map's temporal range should be rendered).
.. seealso:: :py:func:`accumulateFeatures`
%End

QString createFilterString( const QgsVectorLayerTemporalContext &context, const QgsDateTimeRange &range ) const;
QString createFilterString( QgsVectorLayerTemporalContext context, const QgsDateTimeRange &range ) const;
%Docstring
Creates a QGIS expression filter string for filtering features within
the specified ``context`` to those within the specified time ``range``.
Expand Down
26 changes: 10 additions & 16 deletions src/core/qgsvectorlayertemporalproperties.cpp
Expand Up @@ -411,7 +411,7 @@ QString dateTimeExpressionLiteral( const QDateTime &datetime )
.arg( datetime.time().second() + datetime.time().msec() / 1000.0 );
}

QString QgsVectorLayerTemporalProperties::createFilterString( const QgsVectorLayerTemporalContext &, const QgsDateTimeRange &range ) const
QString QgsVectorLayerTemporalProperties::createFilterString( QgsVectorLayerTemporalContext, const QgsDateTimeRange &range ) const
{
if ( !isActive() )
return QString();
Expand Down Expand Up @@ -441,9 +441,9 @@ QString QgsVectorLayerTemporalProperties::createFilterString( const QgsVectorLay
else
{
// Working with features with events with a duration, so taking this duration into account (+ QgsInterval( -mFixedDuration, mDurationUnit ) ))
// Then we are NOT taking the range.includeBeginning() and range.includeEnd() into account (deliberately, see #38468)
return QStringLiteral( "(%1 > %2 AND %1 < %3) OR %1 IS NULL" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
return QStringLiteral( "(%1 > %2 AND %1 %3 %4) OR %1 IS NULL" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
dateTimeExpressionLiteral( range.begin() + QgsInterval( -mFixedDuration, mDurationUnit ) ),
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ) );
}
}
Expand Down Expand Up @@ -496,37 +496,33 @@ QString QgsVectorLayerTemporalProperties::createFilterString( const QgsVectorLay
case QgsUnitTypes::TemporalUnknownUnit:
return QString();
}
return QStringLiteral( "(%1 %2 %3 OR %1 IS NULL) AND ((%1 + %4 %5 %6) OR %7 IS NULL)" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
return QStringLiteral( "(%1 %2 %3 OR %1 IS NULL) AND ((%1 + %4 > %5) OR %6 IS NULL)" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ),
intervalExpression,
range.includeBeginning() ? QStringLiteral( ">=" ) : QStringLiteral( ">" ),
dateTimeExpressionLiteral( range.begin() ),
QgsExpression::quotedColumnRef( mDurationFieldName ) );
break;
}

case ModeFeatureDateTimeStartAndEndFromFields:
{
if ( !mStartFieldName.isEmpty() && !mEndFieldName.isEmpty() )
{
return QStringLiteral( "(%1 %2 %3 OR %1 IS NULL) AND (%4 %5 %6 OR %4 IS NULL)" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
return QStringLiteral( "(%1 %2 %3 OR %1 IS NULL) AND (%4 > %5 OR %4 IS NULL)" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ),
QgsExpression::quotedColumnRef( mEndFieldName ),
range.includeBeginning() ? QStringLiteral( ">=" ) : QStringLiteral( ">" ),
dateTimeExpressionLiteral( range.begin() ) );
}
else if ( !mStartFieldName.isEmpty() )
{
return QStringLiteral( "%1 %2 %3 OR %1 IS NULL" ).arg( QgsExpression::quotedColumnRef( mStartFieldName ),
range.includeBeginning() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ) );
}
else if ( !mEndFieldName.isEmpty() )
{
return QStringLiteral( "%1 %2 %3 OR %1 IS NULL" ).arg( QgsExpression::quotedColumnRef( mEndFieldName ),
range.includeBeginning() ? QStringLiteral( ">=" ) : QStringLiteral( ">" ),
return QStringLiteral( "%1 > %2 OR %1 IS NULL" ).arg( QgsExpression::quotedColumnRef( mEndFieldName ),
dateTimeExpressionLiteral( range.begin() ) );
}
break;
Expand All @@ -536,23 +532,21 @@ QString QgsVectorLayerTemporalProperties::createFilterString( const QgsVectorLay
{
if ( !mStartExpression.isEmpty() && !mEndExpression.isEmpty() )
{
return QStringLiteral( "((%1) %2 %3) AND ((%4) %5 %6)" ).arg( mStartExpression,
return QStringLiteral( "((%1) %2 %3) AND ((%4) > %5)" ).arg( mStartExpression,
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ),
mEndExpression,
range.includeBeginning() ? QStringLiteral( ">=" ) : QStringLiteral( ">" ),
dateTimeExpressionLiteral( range.begin() ) );
}
else if ( !mStartExpression.isEmpty() )
{
return QStringLiteral( "(%1) %2 %3" ).arg( mStartExpression,
range.includeBeginning() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
range.includeEnd() ? QStringLiteral( "<=" ) : QStringLiteral( "<" ),
dateTimeExpressionLiteral( range.end() ) );
}
else if ( !mEndExpression.isEmpty() )
{
return QStringLiteral( "(%1) %2 %3" ).arg( mEndExpression,
range.includeBeginning() ? QStringLiteral( ">=" ) : QStringLiteral( ">" ),
return QStringLiteral( "(%1) > %2" ).arg( mEndExpression,
dateTimeExpressionLiteral( range.begin() ) );
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsvectorlayertemporalproperties.h
Expand Up @@ -325,7 +325,7 @@ class CORE_EXPORT QgsVectorLayerTemporalProperties : public QgsMapLayerTemporalP
* isVisibleInTemporalRange() when testing whether features from a layer set to the
* ModeFixedTemporalRange should ALL be filtered out.
*/
QString createFilterString( const QgsVectorLayerTemporalContext &context, const QgsDateTimeRange &range ) const;
QString createFilterString( QgsVectorLayerTemporalContext context, const QgsDateTimeRange &range ) const;

/**
* Attempts to setup the temporal properties by scanning a set of \a fields
Expand Down

0 comments on commit 01ebc06

Please sign in to comment.