Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pasting unsaved changes as temporary scratch layers #60474

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
867fada
Unset attribute values are always compatible with all field types
nyalldawson Feb 6, 2025
e4b238a
Use QgsUnsetValueAttribute for unset fields when creating new features
nyalldawson Feb 6, 2025
3a340b9
Handle QgsUnsetAttributeValue in text edit widget wrapper
nyalldawson Feb 6, 2025
85bd85a
Add provider conformance tests for adding/editing features with unset…
nyalldawson Feb 7, 2025
b626ea5
[memory] Ignore unset attributes when calling changeAttributeValues
nyalldawson Feb 7, 2025
dfe7122
[ogr] Ignore unset attributes when calling changeAttributeValues
nyalldawson Feb 7, 2025
753f219
[spatialite] Ignore unset attributes when adding, changing attributes
nyalldawson Feb 7, 2025
ef5e5f8
[pyprovider] Ignore unset attributes when changing attribute values
nyalldawson Feb 7, 2025
d029731
Add Python hash for QgsUnsetAttributeValue
nyalldawson Feb 7, 2025
caff949
Remove dead code
nyalldawson Feb 7, 2025
de69c47
[postgres] Ignore unset attributes when changing attribute values, ad…
nyalldawson Feb 7, 2025
0170c12
[hana] Ignore unset attributes when changing attribute values, adding…
nyalldawson Feb 7, 2025
0fe4d91
[oracle] Ignore unset attributes when changing attribute values, addi…
nyalldawson Feb 7, 2025
ae7aa26
[mssql] Ignore unset attributes when changing attribute values, addin…
nyalldawson Feb 7, 2025
1c7bb39
Override test to handle different source table definition
nyalldawson Feb 9, 2025
ec849af
Fix handling of unset attributes when adding features to postgres
nyalldawson Feb 9, 2025
5fc13f9
Update test
nyalldawson Feb 9, 2025
b7223cf
[oracle] Fix handling of type null variants
nyalldawson Feb 11, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/PyQt6/core/__init__.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ def _date_range_repr(self):
QgsDateTimeRange.__repr__ = _datetime_range_repr
QgsDateRange.__repr__ = _date_range_repr


QgsProperty.__bool__ = lambda self: self.propertyType() != Qgis.PropertyType.Invalid
QgsOptionalExpression.__bool__ = lambda self: self.enabled()
QgsUnsetAttributeValue.__hash__ = lambda self: 2178310

# add some __repr__ methods to processing classes
def _processing_source_repr(self):
Expand Down
2 changes: 1 addition & 1 deletion python/core/__init__.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ QgsDateRange.__repr__ = _date_range_repr

QgsProperty.__bool__ = lambda self: self.propertyType() != Qgis.PropertyType.Invalid
QgsOptionalExpression.__bool__ = lambda self: self.enabled()

QgsUnsetAttributeValue.__hash__ = lambda self: 2178310

# add some __repr__ methods to processing classes
def _processing_source_repr(self):
Expand Down
3 changes: 3 additions & 0 deletions src/core/providers/memory/qgsmemoryprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,9 @@ bool QgsMemoryProvider::changeAttributeValues( const QgsChangedAttributesMap &at
continue;

QVariant attrValue = it2.value();
if ( attrValue.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

// Check attribute conversion
const bool conversionError { ! QgsVariantUtils::isNull( attrValue )
&& ! mFields.at( it2.key() ).convertCompatible( attrValue, &errorMessage ) };
Expand Down
5 changes: 4 additions & 1 deletion src/core/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2763,7 +2763,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
{
useUpdate = false;
}
if ( useUpdate )
if ( useUpdate && val.userType() != qMetaTypeId<QgsUnsetAttributeValue >() )
{
QString sql = QStringLiteral( "UPDATE %1 SET %2 = %3" )
.arg( QString::fromUtf8( QgsOgrProviderUtils::quotedIdentifier( mOgrLayer->name(), mGDALDriverName ) ) )
Expand Down Expand Up @@ -2838,6 +2838,9 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
for ( QgsAttributeMap::const_iterator it2 = attr.begin(); it2 != attr.end(); ++it2 )
{
int f = it2.key();
if ( it2->userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( mFirstFieldIsFid )
{
if ( f == 0 )
Expand Down
6 changes: 6 additions & 0 deletions src/core/qgsfield.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "qgsapplication.h"
#include "qgsreferencedgeometry.h"
#include "qgsvariantutils.h"
#include "qgsunsetattributevalue.h"

#include <QDataStream>
#include <QIcon>
Expand Down Expand Up @@ -482,6 +483,11 @@ bool QgsField::convertCompatible( QVariant &v, QString *errorMessage ) const
return true;
}

if ( v.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
{
return true;
}

if ( d->type == QMetaType::Type::Int && v.toInt() != v.toLongLong() )
{
v = QgsVariantUtils::createNullVariant( d->type );
Expand Down
2 changes: 1 addition & 1 deletion src/core/vector/qgsvectorlayerutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ QgsFeatureList QgsVectorLayerUtils::createFeatures( const QgsVectorLayer *layer,
QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
if ( !providerDefault.isEmpty() )
{
v = providerDefault;
v = QgsUnsetAttributeValue( providerDefault );
checkUnique = false;
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/gui/editorwidgets/qgstexteditwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ QVariant QgsTextEditWrapper::value() const

if ( !QgsVariantUtils::isNull( defaultValue() ) && v == defaultValue().toString() )
{
return defaultValue();
return QVariant::fromValue( QgsUnsetAttributeValue( defaultValue().toString() ) );
}

QVariant res( v );
Expand Down Expand Up @@ -302,6 +302,10 @@ void QgsTextEditWrapper::setWidgetValue( const QVariant &val )
{
v = QgsApplication::nullRepresentation();
}
else if ( val.userType() == qMetaTypeId<QgsUnsetAttributeValue>() )
{
v = defaultValue().toString();
}
else
{
v = field().displayString( val );
Expand Down
14 changes: 13 additions & 1 deletion src/providers/hana/qgshanaprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,11 @@ bool QgsHanaProvider::addFeatures( QgsFeatureList &flist, Flags flags )
const int fieldIndex = fieldIds[i];
const AttributeField &field = mAttributeFields.at( fieldIndex );
QVariant attrValue = fieldIndex < attrs.length() ? attrs.at( fieldIndex ) : QgsVariantUtils::createNullVariant( QMetaType::Type::LongLong );

// no default value clause handling supported in this provider, best we can do for now is set to NULL
if ( attrValue.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
attrValue = QVariant();

if ( pkFields[i] )
{
hasIdValue = hasIdValue || !attrValue.isNull();
Expand Down Expand Up @@ -1175,6 +1180,9 @@ bool QgsHanaProvider::changeAttributeValues( const QgsChangedAttributesMap &attr
if ( field.name.isEmpty() || field.isAutoIncrement )
continue;

if ( it2->userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

pkChanged = pkChanged || mPrimaryKeyAttrs.contains( fieldIndex );
auto qType = mFields.at( fieldIndex ).type();
if ( field.type == QgsHanaDataType::Geometry && qType == QMetaType::Type::QString )
Expand Down Expand Up @@ -1202,7 +1210,11 @@ bool QgsHanaProvider::changeAttributeValues( const QgsChangedAttributesMap &attr
if ( field.name.isEmpty() || field.isAutoIncrement )
continue;

setStatementValue( stmtUpdate, paramIndex, field, *attrIt );
const QVariant attrValue = *attrIt;
if ( attrValue.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

setStatementValue( stmtUpdate, paramIndex, field, attrValue );
++paramIndex;
}

Expand Down
12 changes: 12 additions & 0 deletions src/providers/mssql/qgsmssqlprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,9 @@ bool QgsMssqlProvider::addFeatures( QgsFeatureList &flist, Flags flags )
if ( fld.name().isEmpty() )
continue; // invalid

if ( attrs.at( i ).userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( mDefaultValues.contains( i ) && mDefaultValues.value( i ) == attrs.at( i ).toString() )
continue; // skip fields having default values

Expand Down Expand Up @@ -1300,6 +1303,9 @@ bool QgsMssqlProvider::addFeatures( QgsFeatureList &flist, Flags flags )
if ( fld.name().isEmpty() )
continue; // invalid

if ( attrs.at( i ).userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( mDefaultValues.contains( i ) && mDefaultValues.value( i ) == attrs.at( i ).toString() )
continue; // skip fields having default values

Expand Down Expand Up @@ -1554,6 +1560,9 @@ bool QgsMssqlProvider::changeAttributeValues( const QgsChangedAttributesMap &att
if ( fld.name().isEmpty() )
continue; // invalid

if ( it2.value().userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( mComputedColumns.contains( fld.name() ) )
continue; // skip computed columns because they are done server side.

Expand Down Expand Up @@ -1593,6 +1602,9 @@ bool QgsMssqlProvider::changeAttributeValues( const QgsChangedAttributesMap &att
if ( fld.name().isEmpty() )
continue; // invalid

if ( it2.value().userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( mComputedColumns.contains( fld.name() ) )
continue; // skip computed columns because they are done server side.

Expand Down
14 changes: 13 additions & 1 deletion src/providers/oracle/qgsoracleprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1394,10 +1394,18 @@ bool QgsOracleProvider::addFeatures( QgsFeatureList &flist, QgsFeatureSink::Flag
QVariant value = attributevec.value( fieldId[i], QVariant() );

QgsField fld = field( fieldId[i] );
if ( ( value.isNull() && mPrimaryKeyAttrs.contains( i ) && !defaultValues.at( i ).isEmpty() ) || ( value.toString() == defaultValues[i] ) )
if ( ( QgsVariantUtils::isNull( value ) && mPrimaryKeyAttrs.contains( i ) && !defaultValues.at( i ).isEmpty() )
|| ( value.toString() == defaultValues[i] )
|| value.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
{
value = evaluateDefaultExpression( defaultValues[i], fld.type() );
}
else if ( QgsVariantUtils::isNull( value ) )
{
// don't use typed null variants, always use invalid variants. Otherwise the connection
// may incorrectly try to coerce a null value to the variant type
value = QVariant();
}
features->setAttribute( fieldId[i], value );

QgsDebugMsgLevel( QStringLiteral( "addBindValue: %1" ).arg( value.toString() ), 4 );
Expand Down Expand Up @@ -1854,6 +1862,10 @@ bool QgsOracleProvider::changeAttributeValues( const QgsChangedAttributesMap &at
QgsLogger::warning( tr( "Changing the value of GENERATED field %1 is not allowed." ).arg( fld.name() ) );
continue;
}
if ( siter.value().userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
{
continue;
}

pkChanged = pkChanged || mPrimaryKeyAttrs.contains( siter.key() );

Expand Down
43 changes: 26 additions & 17 deletions src/providers/postgres/qgspostgresprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2287,7 +2287,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
QVariant v2 = attrs2.value( idx, QgsVariantUtils::createNullVariant( QMetaType::Type::Int ) );
// a PK field with a sequence val is auto populate by QGIS with this default
// we are only interested in non default values
if ( !QgsVariantUtils::isNull( v2 ) && v2.toString() != defaultValue )
if ( !QgsVariantUtils::isNull( v2 ) && v2.toString() != defaultValue && v2.userType() != qMetaTypeId< QgsUnsetAttributeValue >() )
{
foundNonEmptyPK = true;
break;
Expand All @@ -2298,7 +2298,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )

if ( !skipSinglePKField )
{
for ( int idx : mPrimaryKeyAttrs )
for ( int idx : std::as_const( mPrimaryKeyAttrs ) )
{
if ( mIdentityFields[idx] == 'a' )
overrideIdentity = true;
Expand Down Expand Up @@ -2327,7 +2327,6 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
continue;

QString fieldname = mAttributeFields.at( idx ).name();

if ( !mGeneratedValues.value( idx, QString() ).isEmpty() )
{
QgsDebugMsgLevel( QStringLiteral( "Skipping field %1 (idx %2) which is GENERATED." ).arg( fieldname, QString::number( idx ) ), 2 );
Expand Down Expand Up @@ -2469,7 +2468,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
QVariant value = attrIdx < attrs.length() ? attrs.at( attrIdx ) : QgsVariantUtils::createNullVariant( QMetaType::Type::Int );

QString v;
if ( QgsVariantUtils::isNull( value ) )
if ( QgsVariantUtils::isNull( value ) || value.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
{
QgsField fld = field( attrIdx );
v = paramValue( defaultValues[i], defaultValues[i] );
Expand Down Expand Up @@ -2949,6 +2948,10 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
{
try
{
const QVariant attributeValue = siter.value();
if ( attributeValue.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

QgsField fld = field( siter.key() );

pkChanged = pkChanged || mPrimaryKeyAttrs.contains( siter.key() );
Expand All @@ -2965,39 +2968,39 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
delim = ',';

QString defVal = defaultValueClause( siter.key() );
if ( qgsVariantEqual( *siter, defVal ) )
if ( qgsVariantEqual( attributeValue, defVal ) )
{
sql += defVal.isNull() ? "NULL" : defVal;
}
else if ( fld.typeName() == QLatin1String( "geometry" ) )
{
QString val = geomAttrToString( siter.value(), connectionRO() );
QString val = geomAttrToString( attributeValue, connectionRO() );

sql += QStringLiteral( "%1(%2)" )
.arg( connectionRO()->majorVersion() < 2 ? "geomfromewkt" : "st_geomfromewkt", quotedValue( val ) );
}
else if ( fld.typeName() == QLatin1String( "geography" ) )
{
sql += QStringLiteral( "st_geographyfromtext(%1)" )
.arg( quotedValue( siter->toString() ) );
.arg( quotedValue( attributeValue.toString() ) );
}
else if ( fld.typeName() == QLatin1String( "jsonb" ) )
{
sql += QStringLiteral( "%1::jsonb" )
.arg( quotedJsonValue( siter.value() ) );
.arg( quotedJsonValue( attributeValue ) );
}
else if ( fld.typeName() == QLatin1String( "json" ) )
{
sql += QStringLiteral( "%1::json" )
.arg( quotedJsonValue( siter.value() ) );
.arg( quotedJsonValue( attributeValue ) );
}
else if ( fld.typeName() == QLatin1String( "bytea" ) )
{
sql += quotedByteaValue( siter.value() );
sql += quotedByteaValue( attributeValue );
}
else
{
sql += quotedValue( *siter );
sql += quotedValue( attributeValue );
}
}
catch ( PGFieldNotFound )
Expand Down Expand Up @@ -3328,39 +3331,45 @@ bool QgsPostgresProvider::changeFeatures( const QgsChangedAttributesMap &attr_ma
continue;
}

const QVariant value = siter.value();
if ( value.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
{
continue;
}

numChangedFields++;

sql += delim + QStringLiteral( "%1=" ).arg( quotedIdentifier( fld.name() ) );
delim = ',';

if ( fld.typeName() == QLatin1String( "geometry" ) )
{
QString val = geomAttrToString( siter.value(), connectionRO() );
QString val = geomAttrToString( value, connectionRO() );
sql += QStringLiteral( "%1(%2)" )
.arg( connectionRO()->majorVersion() < 2 ? "geomfromewkt" : "st_geomfromewkt", quotedValue( val ) );
}
else if ( fld.typeName() == QLatin1String( "geography" ) )
{
sql += QStringLiteral( "st_geographyfromtext(%1)" )
.arg( quotedValue( siter->toString() ) );
.arg( quotedValue( value.toString() ) );
}
else if ( fld.typeName() == QLatin1String( "jsonb" ) )
{
sql += QStringLiteral( "%1::jsonb" )
.arg( quotedJsonValue( siter.value() ) );
.arg( quotedJsonValue( value ) );
}
else if ( fld.typeName() == QLatin1String( "json" ) )
{
sql += QStringLiteral( "%1::json" )
.arg( quotedJsonValue( siter.value() ) );
.arg( quotedJsonValue( value ) );
}
else if ( fld.typeName() == QLatin1String( "bytea" ) )
{
sql += quotedByteaValue( siter.value() );
sql += quotedByteaValue( value );
}
else
{
sql += quotedValue( *siter );
sql += quotedValue( value );
}
}
catch ( PGFieldNotFound )
Expand Down
7 changes: 6 additions & 1 deletion src/providers/spatialite/qgsspatialiteprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4131,7 +4131,10 @@ bool QgsSpatiaLiteProvider::addFeatures( QgsFeatureList &flist, Flags flags )

for ( int i = 0; i < attributevec.count(); ++i )
{
if ( mDefaultValues.contains( i ) && ( mDefaultValues.value( i ) == attributevec.at( i ).toString() || !attributevec.at( i ).isValid() ) )
if (
( mDefaultValues.contains( i ) && ( mDefaultValues.value( i ) == attributevec.at( i ).toString() || !attributevec.at( i ).isValid() ) )
|| ( attributevec.at( i ).userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
)
{
defaultIndexes.push_back( i );
continue;
Expand Down Expand Up @@ -4597,6 +4600,8 @@ bool QgsSpatiaLiteProvider::changeAttributeValues( const QgsChangedAttributesMap
{
QgsField fld = field( siter.key() );
const QVariant &val = siter.value();
if ( val.userType() == qMetaTypeId< QgsUnsetAttributeValue >() )
continue;

if ( !first )
sql += ',';
Expand Down
7 changes: 7 additions & 0 deletions tests/src/core/testqgsfield.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "qgsapplication.h"
#include "qgstest.h"
#include "qgsreferencedgeometry.h"
#include "qgsunsetattributevalue.h"

class TestQgsField : public QObject
{
Expand Down Expand Up @@ -603,6 +604,9 @@ void TestQgsField::convertCompatible()
QVERIFY( stringField.convertCompatible( nullDouble ) );
QCOMPARE( static_cast<QMetaType::Type>( nullDouble.userType() ), QMetaType::Type::QString );
QVERIFY( nullDouble.isNull() );
QVariant unsetValue = QgsUnsetAttributeValue( QStringLiteral( "Autonumber" ) );
QVERIFY( stringField.convertCompatible( unsetValue ) );
QCOMPARE( static_cast<QMetaType::Type>( unsetValue.userType() ), qMetaTypeId< QgsUnsetAttributeValue >() );

//test double
const QgsField doubleField( QStringLiteral( "double" ), QMetaType::Type::Double, QStringLiteral( "double" ) );
Expand Down Expand Up @@ -636,6 +640,9 @@ void TestQgsField::convertCompatible()
QVERIFY( doubleField.convertCompatible( nullDouble ) );
QCOMPARE( static_cast<QMetaType::Type>( nullDouble.userType() ), QMetaType::Type::Double );
QVERIFY( nullDouble.isNull() );
unsetValue = QgsUnsetAttributeValue( QStringLiteral( "Autonumber" ) );
QVERIFY( doubleField.convertCompatible( unsetValue ) );
QCOMPARE( static_cast<QMetaType::Type>( unsetValue.userType() ), qMetaTypeId< QgsUnsetAttributeValue >() );

//test special rules

Expand Down
Loading