Merge pull request #59440 from elpaso/bugfix-gh43992-sqlite-expressio…

…n-collapse

Collapse OR nodes into IN

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,140 @@ 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;
+          }
+          else if ( ( dynamic_cast<QgsExpressionNodeColumnRef *>( op->opRight() ) && dynamic_cast<QgsExpressionNodeLiteral *>( op->opLeft() ) ) )
+          {
+            const QString fieldName = op->opRight()->dump();
+            if ( !orValuesMap.contains( fieldName ) )
+            {
+              orFieldNames.append( fieldName );
+              orValuesMap.insert( fieldName, QgsExpressionNode::NodeList() );
+            }
+            orValuesMap[fieldName].append( op->opLeft()->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() )
+    {
+
+      std::unique_ptr<QgsExpressionNode> currentNode;
+      for ( const auto &fieldName : std::as_const( orFieldNames ) )
+      {
+        auto orValuesIt = orValuesMap.find( fieldName );
+        if ( orValuesIt.value().count() == 1 )
+        {
+          std::unique_ptr<QgsExpressionNodeBinaryOperator> eqNode = std::make_unique<QgsExpressionNodeBinaryOperator>( boEQ, new QgsExpressionNodeColumnRef( fieldName ), orValuesIt.value().at( 0 )->clone() );
+          if ( currentNode )
+          {
+            currentNode = std::make_unique<QgsExpressionNodeBinaryOperator>( boOr, currentNode.release(), eqNode.release() );
+          }
+          else
+          {
+            currentNode = std::move( eqNode );
+          }
+        }
+        else
+        {
+          std::unique_ptr<QgsExpressionNodeInOperator> inNode = std::make_unique<QgsExpressionNodeInOperator>( new QgsExpressionNodeColumnRef( fieldName ), orValuesIt.value().clone() );
+          if ( currentNode )
+          {
+            currentNode = std::make_unique<QgsExpressionNodeBinaryOperator>( boOr, currentNode.release(), inNode.release() );
+          }
+          else
+          {
+            currentNode = std::move( inNode );
+          }
+        }
+      }
+
+
+      if ( currentNode )
+      {
+        mCompiledSimplifiedNode = std::move( currentNode );
+      }
+    }
+
+  }
+
   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,7 @@ void QgsRelationEditorWidget::updateUiSingleEdit()
 
     updateButtons();
   }
+
 }
 
 void QgsRelationEditorWidget::updateUiMultiEdit()

tests/src/core/testqgsexpression.cpp

@@ -5589,6 +5589,41 @@ 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'" );
+      QTest::newRow( "simple mixed order" ) << QStringLiteral( "field = 'value' OR 'value2' = field OR field = 'value3'" ) << QStringLiteral( "field  IN ('value', 'value2', '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,30 @@ void TestQgsRelationEditorWidget::testMultiEditNM()
 
 }
 
+void TestQgsRelationEditorWidget::testFeatureRequest()
+{
+
+  // Init a relation editor widget
+  QgsRelationEditorWidget relationEditorWidget( QVariantMap(),
+      new QWidget() );
+  relationEditorWidget.setRelations( *mRelation1N, *mRelationNM );
+
+  QVERIFY( !relationEditorWidget.multiEditModeActive() );
+
+  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.mDualView->masterModel()->request().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