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.

Sign up for GitHub

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

Jump to bottom

Make primary key requirements less restrictive in HANA #41136

Merged
merged 2 commits into from Jan 25, 2021
Merged

Make primary key requirements less restrictive in HANA #41136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5706fa7
Make primary key requirements less restrictive in HANA
mrylov Jan 22, 2021
23bfee4
Update src/providers/hana/qgshanaconnection.cpp
stefanuhrig Jan 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/providers/hana/qgshanaconnection.cpp
View file
Open in desktop
Expand Up @@ -760,6 +760,12 @@ QgsHanaResultSetRef QgsHanaConnection::getColumns( const QString &schemaName, co
}
}

bool QgsHanaConnection::isTable( const QString &schemaName, const QString &tableName )
{
QString sql = QStringLiteral( "SELECT COUNT(*) FROM SYS.TABLES WHERE SCHEMA_NAME = ? AND TABLE_NAME = ?" );
return executeCountQuery( sql, {schemaName, tableName } ) == 1;
}

PreparedStatementRef QgsHanaConnection::createPreparedStatement( const QString &sql, const QVariantList &args )
{
PreparedStatementRef stmt = mConnection->prepareStatement( QgsHanaUtils::toUtf16( sql ) );
Expand Down
1 change: 1 addition & 0 deletions src/providers/hana/qgshanaconnection.h
View file
Open in desktop
Expand Up @@ -71,6 +71,7 @@ class QgsHanaConnection : public QObject
QString getColumnDataType( const QString &schemaName, const QString &tableName, const QString &columnName );
int getColumnSrid( const QString &schemaName, const QString &tableName, const QString &columnName );
QgsHanaResultSetRef getColumns( const QString &schemaName, const QString &tableName, const QString &fieldName );
bool isTable( const QString &schemaName, const QString &tableName );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool isTable( const QString &schemaName, const QString &tableName );
bool isTable( const QString &schemaName, const QString &tableName ) const;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we cannot change this to const because this method calls (indirectly via another non-const method) odbc::Connection::createStatment(), which is not const.

Actually, the question how const-correctness should look like in a database connector is quite interesting. Intuitively, one might expect that methods that don't change database objects are const. However, from a C++-language point of view, const-correctness relates to the state of C++objects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should look at it from a pure c++ perspective. And use mutable to hide away implementation details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a closer look at the QgsHanaConnection class. Currently, no method is const. For consistency, I'd keep the new method as not const as well.

It's not quite clear to me, which methods should be const in this class. Most of them require the creation of a statement, which is a modification of the mConnection member.

Is it okay to keep it non-const for the time being and to come up with a criteria for making methods const in this class @m-kuhn? We could then do the constness changes in a separate follow-up pull request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


static QgsHanaConnection *createConnection( const QgsDataSourceUri &uri );
static QgsHanaConnection *createConnection( const QgsDataSourceUri &uri, bool *canceled );
Expand Down
2 changes: 1 addition & 1 deletion src/providers/hana/qgshanaprimarykeys.cpp
View file
Open in desktop
Expand Up @@ -155,7 +155,7 @@ QPair<QgsHanaPrimaryKeyType, QList<int>> QgsHanaPrimaryKeyUtils::determinePrimar
if ( !attrs.isEmpty() )
keyType = ( attrs.size() == 1 ) ? getPrimaryKeyType( fields.at( attrs[0] ) ) : PktFidMap;
else
QgsMessageLog::logMessage( QObject::tr( "Keys for view/query undefined." ), QObject::tr( "SAP HANA" ) );
QgsMessageLog::logMessage( QObject::tr( "Keys for view/query undefined. Some functionality might not be available." ), QObject::tr( "SAP HANA" ) );

return qMakePair( keyType, attrs );
}
Expand Down
24 changes: 12 additions & 12 deletions src/providers/hana/qgshanaprovider.cpp
View file
Open in desktop
Expand Up @@ -336,8 +336,6 @@ QgsHanaProvider::QgsHanaProvider(
mDatabaseVersion = QgsHanaUtils::toHANAVersion( conn->getDatabaseVersion() );
readGeometryType();
readAttributeFields();
if ( !mIsQuery && mPrimaryKeyType == PktUnknown )
return;
readSrsInformation();
readMetadata();

Expand Down Expand Up @@ -1403,21 +1401,26 @@ void QgsHanaProvider::readSrsInformation()

void QgsHanaProvider::determinePrimaryKey()
{
QPair<QgsHanaPrimaryKeyType, QList<int>> primaryKey;
if ( !mIsQuery )
{
QgsHanaConnectionRef conn( mUri );
QStringList layerPrimaryKey = conn->getLayerPrimaryeKey( mSchemaName, mTableName );
auto primaryKey = QgsHanaPrimaryKeyUtils::determinePrimaryKeyFromColumns( layerPrimaryKey, mAttributeFields );
mPrimaryKeyType = primaryKey.first;
mPrimaryKeyAttrs = primaryKey.second;
if ( conn->isTable( mSchemaName, mTableName ) )
{
QStringList layerPrimaryKey = conn->getLayerPrimaryeKey( mSchemaName, mTableName );
primaryKey = QgsHanaPrimaryKeyUtils::determinePrimaryKeyFromColumns( layerPrimaryKey, mAttributeFields );
}
else
primaryKey = QgsHanaPrimaryKeyUtils::determinePrimaryKeyFromUriKeyColumn( mUri.keyColumn(), mAttributeFields );
}
else
{
auto primaryKey = QgsHanaPrimaryKeyUtils::determinePrimaryKeyFromUriKeyColumn( mUri.keyColumn(), mAttributeFields );
mPrimaryKeyType = primaryKey.first;
mPrimaryKeyAttrs = primaryKey.second;
primaryKey = QgsHanaPrimaryKeyUtils::determinePrimaryKeyFromUriKeyColumn( mUri.keyColumn(), mAttributeFields );
}

mPrimaryKeyType = primaryKey.first;
mPrimaryKeyAttrs = primaryKey.second;

if ( mPrimaryKeyAttrs.size() == 1 )
{
//primary keys are unique, not null
Expand All @@ -1426,9 +1429,6 @@ void QgsHanaProvider::determinePrimaryKey()
constraints.setConstraint( QgsFieldConstraints::ConstraintNotNull, QgsFieldConstraints::ConstraintOriginProvider );
mAttributeFields[ mPrimaryKeyAttrs[0] ].setConstraints( constraints );
}

if ( !mIsQuery )
mValid = mPrimaryKeyType != PktUnknown;
}

long QgsHanaProvider::getFeatureCount( const QString &whereClause ) const
Expand Down