Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[expressions] Treat all datetime values as UTC when comparing
While QDateTime has innate handling of timezones, we don't
expose these ANYWHERE in QGIS. So to avoid confusion where
seemingly equal datetime values give unexpected results
(due to different hidden timezones), we force all datetime
comparisons to treat all datetime values as having the same time zone

We should revisit this when/if we start exposing time zone
handling on a user level
  • Loading branch information
nyalldawson committed May 9, 2020
1 parent 9dfa715 commit 16fd1a8
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/core/expression/qgsexpressionnodeimpl.cpp
Expand Up @@ -405,10 +405,18 @@ QVariant QgsExpressionNodeBinaryOperator::evalNode( QgsExpression *parent, const
}
else if ( ( vL.type() == QVariant::DateTime && vR.type() == QVariant::DateTime ) )
{
const QDateTime dL = QgsExpressionUtils::getDateTimeValue( vL, parent );
QDateTime dL = QgsExpressionUtils::getDateTimeValue( vL, parent );
ENSURE_NO_EVAL_ERROR;
const QDateTime dR = QgsExpressionUtils::getDateTimeValue( vR, parent );
QDateTime dR = QgsExpressionUtils::getDateTimeValue( vR, parent );
ENSURE_NO_EVAL_ERROR;

// while QDateTime has innate handling of timezones, we don't expose these ANYWHERE
// in QGIS. So to avoid confusion where seemingly equal datetime values give unexpected
// results (due to different hidden timezones), we force all datetime comparisons to treat
// all datetime values as having the same time zone
dL.setTimeSpec( Qt::UTC );
dR.setTimeSpec( Qt::UTC );

return compare( dR.msecsTo( dL ) ) ? TVL_True : TVL_False;
}
else if ( ( vL.type() == QVariant::Date && vR.type() == QVariant::Date ) )
Expand Down
24 changes: 24 additions & 0 deletions tests/src/core/testqgsexpression.cpp
Expand Up @@ -640,6 +640,30 @@ class TestQgsExpression: public QObject
QTest::newRow( "to_interval('1 minute') = to_interval('20 days')" ) << "to_interval('1 minute') = to_interval('20 days')" << false << QVariant( 0 );
QTest::newRow( "to_interval('1 minute') != to_interval('20 days')" ) << "to_interval('1 minute') != to_interval('20 days')" << false << QVariant( 1 );
QTest::newRow( "to_interval('1 minute') = to_interval('60 seconds')" ) << "to_interval('1 minute') = to_interval('60 seconds')" << false << QVariant( 1 );
QTest::newRow( "make_date(2010,9,8) < make_date(2010,9,9)" ) << "make_date(2010,9,8) < make_date(2010,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_date(2010,9,8) > make_date(2010,9,9)" ) << "make_date(2010,9,8) > make_date(2010,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_date(2010,9,9) > make_date(2010,9,8)" ) << "make_date(2010,9,9) > make_date(2010,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_date(2010,9,9) < make_date(2010,9,8)" ) << "make_date(2010,9,9) < make_date(2010,9,8)" << false << QVariant( 0 );
QTest::newRow( "make_date(2010,9,8) = make_date(2010,9,8)" ) << "make_date(2010,9,8) = make_date(2010,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_date(2010,9,8) = make_date(2010,9,9)" ) << "make_date(2010,9,8) = make_date(2010,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_date(2010,9,8) != make_date(2010,9,9)" ) << "make_date(2010,9,8) != make_date(2010,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_date(2010,9,8) != make_date(2010,9,8)" ) << "make_date(2010,9,8) != make_date(2010,9,8)" << false << QVariant( 0 );
QTest::newRow( "make_time(12,9,8) < make_time(12,9,9)" ) << "make_time(12,9,8) < make_time(12,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_time(12,9,8) > make_time(12,9,9)" ) << "make_time(12,9,8) > make_time(12,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_time(12,9,9) > make_time(12,9,8)" ) << "make_time(12,9,9) > make_time(12,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_time(12,9,9) < make_time(12,9,8)" ) << "make_time(12,9,9) < make_time(12,9,8)" << false << QVariant( 0 );
QTest::newRow( "make_time(12,9,8) = make_time(12,9,8)" ) << "make_time(12,9,8) = make_time(12,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_time(12,9,8) = make_time(12,9,9)" ) << "make_time(12,9,8) = make_time(12,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_time(12,9,8) != make_time(12,9,9)" ) << "make_time(12,9,8) != make_time(12,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_time(12,9,8) != make_time(12,9,8)" ) << "make_time(12,9,8) != make_time(12,9,8)" << false << QVariant( 0 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) < make_datetime(2012,3,4,12,9,9)" ) << "make_datetime(2012,3,4,12,9,8) < make_datetime(2012,3,4,12,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) > make_datetime(2012,3,4,12,9,9)" ) << "make_datetime(2012,3,4,12,9,8) > make_datetime(2012,3,4,12,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_datetime(2012,3,4,12,9,9) > make_datetime(2012,3,4,12,9,8)" ) << "make_datetime(2012,3,4,12,9,9) > make_datetime(2012,3,4,12,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_datetime(2012,3,4,12,9,9) < make_datetime(2012,3,4,12,9,8)" ) << "make_datetime(2012,3,4,12,9,9) < make_datetime(2012,3,4,12,9,8)" << false << QVariant( 0 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) = make_datetime(2012,3,4,12,9,8)" ) << "make_datetime(2012,3,4,12,9,8) = make_datetime(2012,3,4,12,9,8)" << false << QVariant( 1 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) = make_datetime(2012,3,4,12,9,9)" ) << "make_datetime(2012,3,4,12,9,8) = make_datetime(2012,3,4,12,9,9)" << false << QVariant( 0 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) != make_datetime(2012,3,4,12,9,9)" ) << "make_datetime(2012,3,4,12,9,8) != make_datetime(2012,3,4,12,9,9)" << false << QVariant( 1 );
QTest::newRow( "make_datetime(2012,3,4,12,9,8) != make_datetime(2012,3,4,12,9,8)" ) << "make_datetime(2012,3,4,12,9,8) != make_datetime(2012,3,4,12,9,8)" << false << QVariant( 0 );

// is, is not
QTest::newRow( "is null,null" ) << "null is null" << false << QVariant( 1 );
Expand Down

0 comments on commit 16fd1a8

Please sign in to comment.