Fix setSubsetString return value on error

[uclaros]

Description

QgsOgrProvider::setSubsetString should return false on malformed sql.

Fixes #34259

[m-kuhn]

Any chance to also log an error message if this happens?

[uclaros]

QgsOgrProviderUtils::setSubsetString is just a utility method used by QgsOgrProvider::_setSubsetString that handles the error and the logging:

QGIS/src/core/providers/ogr/qgsogrprovider.cpp

Lines 2067 to 2073 in 0b4cc5a

	subsetLayerH = QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), theSQL );
	}
	if ( !subsetLayerH )
	{
	pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
	return false;
	}

then

QGIS/src/core/qgsvectordataprovider.cpp

Lines 705 to 710 in 0b4cc5a

	void QgsVectorDataProvider::pushError( const QString &msg ) const
	{
	QgsDebugMsg( msg );
	mErrors << msg;
	emit raiseError( msg );
	}

So I don't think extra logging is needed. I could add some unit test though, I just need to figure out where!...

[m-kuhn]

That sounds good.

Some tests exist already in tests/src/python/test_provider_ogr*.py, you could build on top of that

[uclaros]

Yes, but shapefile and spatialite tests are in test_provider_shapefile.py and test_provider_spatialite.py, are they not using ogrdataprovider? I think I'll put them on test_provider_ogr.py that is more generic, though currently all tests regarding setSubsetString are in each format's test file.

[uclaros]

Changed my mind and added the test on test_provider_shapefile.py since most ogr tests are there.

[elpaso]

@uclaros any chance to see it backported to 3.10 and 3.12? I don't see any potential regression unless there was some code relying on a plain wrong behavior.

[elpaso]

sorry, I meant backport to 3.10 only.

[elpaso]

@uclaros thank you!

[uclaros]

@elpaso You're welcome :)