Collapse OR nodes into IN

Fixes #43992 but also implements
an optimization of OR chained nodes
by turning them into IN conditions
when possible.

On behalf of: Faunalia

src/core/expression/qgsexpressionnode.cpp

@@ -97,4 +97,3 @@ void QgsExpressionNode::cloneTo( QgsExpressionNode *target ) const
   target->parserFirstColumn = parserFirstColumn;
   target->parserFirstLine = parserFirstLine;
 }
-

src/core/expression/qgsexpressionnodeimpl.cpp

@@ -694,6 +694,122 @@ QgsExpressionNode::NodeType QgsExpressionNodeBinaryOperator::nodeType() const
 
 bool QgsExpressionNodeBinaryOperator::prepareNode( QgsExpression *parent, const QgsExpressionContext *context )
 {
+
+  // if this is an OR, try to collapse the OR expression into an IN node
+  if ( mOp == boOr )
+  {
+
+    // First step: flatten OR chain and collect values
+    QMap<QString, QgsExpressionNode::NodeList> orValuesMap;
+    QList<QString> orFieldNames;
+
+    // Get a list of all the OR and IN nodes chained together
+    std::function<bool ( QgsExpressionNode * )> visitOrNodes = [&visitOrNodes, &orValuesMap, &orFieldNames]( QgsExpressionNode * node ) -> bool
+    {
+      if ( QgsExpressionNodeBinaryOperator *op = dynamic_cast<QgsExpressionNodeBinaryOperator *>( node ) )
+      {
+        if ( op->op() != boOr && op->op() != boEQ )
+        {
+          return false;
+        }
+
+        if ( op->op() == boEQ )
+        {
+          // If left is a column ref and right is a literal, collect
+          if ( dynamic_cast<QgsExpressionNodeColumnRef *>( op->opLeft() ) && dynamic_cast<QgsExpressionNodeLiteral *>( op->opRight() ) )
+          {
+            const QString fieldName = op->opLeft()->dump();
+            if ( !orValuesMap.contains( fieldName ) )
+            {
+              orFieldNames.append( fieldName );
+              orValuesMap.insert( fieldName, QgsExpressionNode::NodeList() );
+            }
+            orValuesMap[fieldName].append( op->opRight()->clone() );
+            return true;
+          }
+          return false;
+        }
+
+        if ( visitOrNodes( op->opLeft() ) && visitOrNodes( op->opRight() ) )
+        {
+          return true;
+        }
+
+      }
+      else if ( QgsExpressionNodeInOperator *inOp = dynamic_cast<QgsExpressionNodeInOperator *>( node ) )
+      {
+        if ( inOp->node()->nodeType() != QgsExpressionNode::ntColumnRef )
+        {
+          return false;
+        }
+
+        const QString fieldName = inOp->node()->dump();
+
+        // Check if all nodes are literals
+        const auto nodes = inOp->list()->list();
+        for ( const auto &valueNode : std::as_const( nodes ) )
+        {
+          if ( valueNode->nodeType() != QgsExpressionNode::ntLiteral )
+          {
+            return false;
+          }
+        }
+
+        if ( !orValuesMap.contains( fieldName ) )
+        {
+          orFieldNames.append( fieldName );
+          orValuesMap.insert( fieldName, *inOp->list()->clone() );
+        }
+        else
+        {
+          for ( const auto &valueNode : std::as_const( nodes ) )
+          {
+            orValuesMap[fieldName].append( valueNode->clone() );
+          }
+        }
+
+        return true;
+      }
+      return false;
+    };
+
+
+    // Second step: build the OR chain of IN operators
+    if ( visitOrNodes( this ) && ! orValuesMap.empty() )
+    {
+
+      // Recursively build the OR chain of IN operators
+      std::function<QgsExpressionNode* ( QList<QString>::const_iterator )> buildOrChain = [&buildOrChain, &orValuesMap, &orFieldNames]( QList<QString>::const_iterator fieldNameit ) -> QgsExpressionNode *
+      {
+
+        QgsExpressionNode *currentNode;
+        auto orValuesIt = orValuesMap.find( *fieldNameit );
+
+        if ( orValuesIt.value().count() == 1 )
+        {
+          currentNode = new QgsExpressionNodeBinaryOperator( boEQ, new QgsExpressionNodeColumnRef( orValuesIt.key() ), orValuesIt.value().at( 0 )->clone() );
+        }
+        else
+        {
+          currentNode = new QgsExpressionNodeInOperator( new QgsExpressionNodeColumnRef( orValuesIt.key() ), orValuesIt.value().clone() );
+        }
+
+        if ( fieldNameit + 1 == orFieldNames.cend() )
+        {
+          return currentNode;
+        }
+        else
+        {
+          QgsExpressionNode *nextNode = buildOrChain( fieldNameit + 1 );
+          return new QgsExpressionNodeBinaryOperator( boOr, currentNode, nextNode );
+        }
+      };
+
+      mCompiledSimplifiedNode.reset( buildOrChain( orFieldNames.cbegin() ) );
+    }
+
+  }
+
   bool resL = mOpLeft->prepare( parent, context );
   bool resR = mOpRight->prepare( parent, context );
   return resL && resR;

src/gui/qgsrelationeditorwidget.cpp

@@ -626,8 +626,10 @@ void QgsRelationEditorWidget::updateUiSingleEdit()
   QgsVectorLayer *layer = nullptr;
   if ( mNmRelation.isValid() )
   {
-    QgsFeatureIterator it = mRelation.referencingLayer()->getFeatures( request );
     QgsFeature fet;
+    QgsFeatureRequest nmRequest;
+
+    QgsFeatureIterator it = mRelation.referencingLayer()->getFeatures( request );
     QStringList filters;
 
     while ( it.nextFeature( fet ) )
@@ -636,8 +638,15 @@ void QgsRelationEditorWidget::updateUiSingleEdit()
       filters << filter.prepend( '(' ).append( ')' );
     }
 
-    QgsFeatureRequest nmRequest;
-    nmRequest.setFilterExpression( filters.join( QLatin1String( " OR " ) ) );
+    QString reducedExpression;
+    if ( QgsExpression::attemptReduceToInClause( filters, reducedExpression ) )
+    {
+      nmRequest.setFilterExpression( reducedExpression );
+    }
+    else
+    {
+      nmRequest.setFilterExpression( filters.join( QStringLiteral( " OR " ) ) );
+    }
 
     request = nmRequest;
     layer = mNmRelation.referencedLayer();
@@ -666,6 +675,8 @@ void QgsRelationEditorWidget::updateUiSingleEdit()
 
     updateButtons();
   }
+
+  mFeatureRequest = request;
 }
 
 void QgsRelationEditorWidget::updateUiMultiEdit()

src/gui/qgsrelationeditorwidget.h

@@ -267,6 +267,9 @@ class GUI_EXPORT QgsRelationEditorWidget : public QgsAbstractRelationEditorWidge
     bool mAllowAddChildFeatureWithNoGeometry = true;
     QString mFilterExpression;
 
+    // Mainly used for testing purposes
+    QgsFeatureRequest mFeatureRequest;
+
     QList<QTreeWidgetItem *> mMultiEditPreviousSelectedItems;
     QgsFeatureIds mMultiEdit1NJustAddedIds;
 

tests/src/core/testqgsexpression.cpp

@@ -5589,6 +5589,40 @@ class TestQgsExpression: public QObject
       }
     }
 
+    void testCollapseOrValues_data()
+    {
+      QTest::addColumn<QString>( "expression" );
+      QTest::addColumn<QString>( "expected" );
+
+
+      QTest::newRow( "simple" ) << QStringLiteral( "field = 'value' OR field = 'value2'" ) << QStringLiteral( "field  IN ('value', 'value2')" );
+      QTest::newRow( "simple 2" ) << QStringLiteral( "\"field\" = 'value' OR field = 'value2' OR field = 'value3'" ) << QStringLiteral( "field  IN ('value', 'value2', 'value3')" );
+      QTest::newRow( "simple3 mixed" ) << QStringLiteral( "field = 'value' OR field = 'value2' OR field2 = 'value3'" ) << QStringLiteral( "field  IN ('value', 'value2') OR field2 = 'value3'" );
+      QTest::newRow( "simple3 mixed 2" ) << QStringLiteral( "field2 = 'value3' OR field = 'value1' OR field = 'value2'" ) << QStringLiteral( "field2 = 'value3' OR field  IN ('value1', 'value2')" );
+      QTest::newRow( "simple3 mixed 3" ) << QStringLiteral( "field = 'value1' OR field2 = 'value3' OR field = 'value2'" ) << QStringLiteral( "field  IN ('value1', 'value2') OR field2 = 'value3'" );
+
+      // test with IN
+      QTest::newRow( "simple IN" ) << QStringLiteral( "field IN ('value', 'value2') OR field = 'value3'" ) << QStringLiteral( "field  IN ('value', 'value2', 'value3')" );
+      QTest::newRow( "simple IN 2" ) << QStringLiteral( "field = 'value' OR field IN ('value2', 'value3')" ) << QStringLiteral( "field  IN ('value', 'value2', 'value3')" );
+
+      // test cases that should not trigger reduction
+      QTest::newRow( "no reduction 1" ) << QStringLiteral( "field = 'value' OR field2 = 'value2'" ) << QStringLiteral( "field = 'value' OR field2 = 'value2'" );
+      // this could theoretically be reduced, but we don't currently support this
+      QTest::newRow( "no reduction 2" ) << QStringLiteral( "field = 'value' OR field != 'value2' OR field = 'value3'" ) << QStringLiteral( "field = 'value' OR field <> 'value2' OR field = 'value3'" );
+
+    }
+
+    void testCollapseOrValues()
+    {
+      QFETCH( QString, expression );
+      QFETCH( QString, expected );
+
+      QgsExpression exp( expression );
+      QgsExpressionContext context;
+      exp.prepare( &context );
+      QCOMPARE( exp.rootNode()->effectiveNode()->dump(), expected );
+    }
+
     void testPrecomputedNodesWithIntrospectionFunctions()
     {
       QgsFields fields;

tests/src/gui/testqgsrelationeditorwidget.cpp

@@ -41,6 +41,7 @@ class TestQgsRelationEditorWidget : public QObject
 
     void testMultiEdit1N();
     void testMultiEditNM();
+    void testFeatureRequest();
     void testUpdateUi();
 
   private:
@@ -159,7 +160,7 @@ void TestQgsRelationEditorWidget::init()
   mLayerJoin->commitChanges();
 
   QgsFeature jft3( mLayerJoin->fields() );
-  jft3.setAttribute( QStringLiteral( "pk" ), 102 );
+  jft3.setAttribute( QStringLiteral( "pk" ), 103 );
   jft3.setAttribute( QStringLiteral( "fk_layer1" ), 0 );
   jft3.setAttribute( QStringLiteral( "fk_layer2" ), 11 );
   mLayerJoin->startEditing();
@@ -285,6 +286,31 @@ void TestQgsRelationEditorWidget::testMultiEditNM()
 
 }
 
+void TestQgsRelationEditorWidget::testFeatureRequest()
+{
+
+  // Init a relation editor widget
+  QgsRelationEditorWidget relationEditorWidget( QVariantMap(),
+      new QWidget() );
+  relationEditorWidget.setRelations( *mRelation1N, *mRelationNM );
+
+  QVERIFY( !relationEditorWidget.multiEditModeActive() );
+
+  QgsFeatureIds featureIds;
+  QgsFeatureIterator featureIterator = mLayer1->getFeatures();
+  QgsFeature feature;
+  featureIterator.nextFeature( feature );
+  relationEditorWidget.setFeature( feature );
+
+  QgsAttributeEditorContext context;
+  QgsTrackedVectorLayerTools tools;
+  context.setVectorLayerTools( &tools );
+  relationEditorWidget.setEditorContext( context );
+
+  relationEditorWidget.updateUiSingleEdit();
+  QCOMPARE( relationEditorWidget.mFeatureRequest.filterExpression()->expression(), QStringLiteral( "\"pk\" IN (10,11)" ) );
+}
+
 void TestQgsRelationEditorWidget::testUpdateUi()
 {
   // Test that we don't recreate all the widget when only the request have been updated