On Tuesday 01 Jan 2008 7:48:11 pm Jarosław Staniek wrote:
> Sharan Rao said the following, On 2007-12-31 22:07:
> > Reviewing the code again, is passing only FieldList object enough ?
> > The tablename can be obtained from one of the fields in the FieldList.
> > But this would also lead to calculation of the tableName from the
> > FieldList object three times in most insertRecord() functions.
> > 1. Normal calculation in insertRecord()
> > 2. in drv_beforeInsert()
> > 3. in drv_afterInsert()
>
> Sharan, thanks for the patch; it's mostly OK.
> First note, could you please change spaces to tabs in the indentation?
> (even if we plan to switch to spaces soon I hope)
>
> * Move Connection::drv_beforeInsert(), etc. to the header file for
> efficiency. You'll need to put Q_UNUSED(table);, etc. there to avoid
> warnings about unused args.
> * Check results of calling these methods, i.e.
> if (!drv_beforeInsert(...))
> return false;
> * commit cursor.cpp change (removal of my commentout) as a separate
> checkin. * In SybaseConnection::drv_beforeInsert() you could put instead:
>
> +bool KexiDB::SybaseConnection::drv_beforeInsert( const QString& table,
> FieldList& fields )
> +{
> +
> + if ( fields.autoIncrementFields()->isEmpty() )
> + return true;
> // explicit insertion into IDENTITY fields !!
> return drv_executeSQL( QString::fromLatin1( "SET IDENTITY_INSERT %1
> ON" ).arg( table ) );
> +}
>
> Same for other 3 methods.
> BTW, don't we need escape the table name?
Attached new patch with discussed changes.
Cheers!
Sharan Rao
Index: connection.cpp
===================================================================
--- connection.cpp (revision 750995)
+++ connection.cpp (working copy)
@@ -1045,9 +1045,18 @@
#define C_INS_REC(args, vals) \
bool Connection::insertRecord(KexiDB::TableSchema &tableSchema args) {\
- return executeSQL( \
- QString("INSERT INTO ") + escapeIdentifier(tableSchema.name()) + " VALUES (" + vals + ")" \
- ); \
+ if ( !drv_beforeInsert( tableSchema.name(), tableSchema ) ) \
+ return false; \
+ \
+ bool res = executeSQL( \
+ QString("INSERT INTO ") + escapeIdentifier(tableSchema.name()) \
+ + " (" + tableSchema.sqlFieldsList(m_driver) + ") VALUES (" + vals + ")" \
+ ); \
+ \
+ if ( !drv_afterInsert( tableSchema.name(),tableSchema ) ) \
+ return false; \
+ \
+ return res; \
}
#define C_INS_REC_ALL \
@@ -1078,10 +1087,15 @@
vals \
it.toFront(); \
QString tableName( (it.hasNext() && it.peekNext()->table()) ? it.next()->table()->name() : "??" ); \
- return executeSQL( \
+ if ( !drv_beforeInsert( tableName, fields ) ) \
+ return false; \
+ bool res = executeSQL( \
QString("INSERT INTO ") + escapeIdentifier(tableName) \
+ "(" + fields.sqlFieldsList(m_driver) + ") VALUES (" + value + ")" \
); \
+ if ( !drv_afterInsert( tableName, fields ) ) \
+ return false; \
+ return res; \
}
C_INS_REC_ALL
@@ -1119,7 +1133,13 @@
m_sql += ")";
// KexiDBDbg<<"******** "<< m_sql << endl;
- return executeSQL(m_sql);
+ if ( !drv_beforeInsert( tableSchema.name(), tableSchema ) )
+ return false;
+ bool res = executeSQL(m_sql);
+ if ( !drv_afterInsert( tableSchema.name(), tableSchema ) )
+ return false;
+
+ return res;
}
bool Connection::insertRecord(FieldList& fields, const QList<QVariant>& values)
@@ -1135,10 +1155,11 @@
m_sql.clear();
QList<QVariant>::ConstIterator it = values.constBegin();
// int i=0;
+ QString tableName = escapeIdentifier( flist->first()->table()->name() );
while (f && (it!=values.constEnd())) {
if (m_sql.isEmpty())
m_sql = QString("INSERT INTO ") +
- escapeIdentifier(flist->first()->table()->name()) + "(" +
+ tableName + "(" +
fields.sqlFieldsList(m_driver) + ") VALUES (";
else
m_sql += ",";
@@ -1150,7 +1171,13 @@
}
m_sql += ")";
- return executeSQL(m_sql);
+ if ( !drv_beforeInsert( tableName, fields ) )
+ return false;
+ bool res = executeSQL(m_sql);
+ if ( !drv_afterInsert( tableName, fields ) )
+ return false;
+
+ return res;
}
bool Connection::executeSQL( const QString& statement )
@@ -3306,7 +3333,17 @@
m_sql += (sqlset + " WHERE " + sqlwhere);
KexiDBDbg << " -- SQL == " << ((m_sql.length() > 400) ? (m_sql.left(400)+"[.....]") : m_sql) << endl;
- if (!executeSQL(m_sql)) {
+ // preprocessing before update
+ if ( !drv_beforeUpdate( mt->name(), query ) )
+ return false;
+
+ bool res = executeSQL( m_sql );
+
+ // postprocessing after update
+ if ( !drv_afterUpdate( mt->name(), query ) )
+ return false;
+
+ if (!res) {
setError(ERR_UPDATE_SERVER_ERROR, i18n("Row updating on the server failed."));
return false;
}
@@ -3409,8 +3446,16 @@
m_sql += (sqlcols + ") VALUES (" + sqlvals + ")");
// KexiDBDbg << " -- SQL == " << m_sql << endl;
- bool res = executeSQL(m_sql);
+ // do driver specific pre-processing
+ if ( !drv_beforeInsert( mt->name(), query) )
+ return false;
+ bool res = executeSQL(m_sql);
+
+ // do driver specific post-processing
+ if ( !drv_afterInsert( mt->name(), query) )
+ return false;
+
if (!res) {
setError(ERR_INSERT_SERVER_ERROR, i18n("Row inserting on the server failed."));
return false;
Index: connection.h
===================================================================
--- connection.h (revision 750642)
+++ connection.h (working copy)
@@ -1001,7 +1001,61 @@
*/
virtual bool drv_rollbackTransaction(TransactionData* trans);
- /*! Changes autocommiting option for established connection.
+
+ /*! Preprocessing (if any) required by drivers before execution of an
+ Insert statement.
+ Reimplement this method in your driver if there are any special processing steps to be
+ executed before an Insert statement.
+
+ \sa drv_afterInsert()
+ */
+ virtual bool drv_beforeInsert(const QString& table, FieldList& fields){
+ Q_UNUSED(table);
+ Q_UNUSED(fields);
+ return true;
+ }
+
+ /*! Postprocessing (if any) required by drivers before execution of an
+ Insert statement.
+ Reimplement this method in your driver if there are any special processing steps to be
+ executed after an Insert statement.
+
+ \sa drv_beforeInsert()
+ */
+ virtual bool drv_afterInsert(const QString& table, FieldList& fields){
+ Q_UNUSED(table);
+ Q_UNUSED(fields);
+ return true;
+ }
+
+ /*! Preprocessing required by drivers before execution of an
+ Update statement.
+ Reimplement this method in your driver if there are any special processing steps to be
+ executed before an Update statement.
+
+ \sa drv_afterUpdate()
+ */
+ virtual bool drv_beforeUpdate(const QString& table, FieldList& fields){
+ Q_UNUSED(table);
+ Q_UNUSED(fields);
+ return true;
+ }
+
+ /*! Postprocessing required by drivers before execution of an
+ Insert statement.
+ Reimplement this method in your driver if there are any special processing steps to be
+ executed after an Update statement.
+
+ \sa drv_beforeUpdate()
+ */
+ virtual bool drv_afterUpdate(const QString& table, FieldList& fields){
+ Q_UNUSED(table);
+ Q_UNUSED(fields);
+ return true;
+ }
+
+
+ /*! Changes autocommiting option for established connection.
\return true on success.
Note for driver developers: reimplement this only if your engine
Index: drivers/sybase/sybasedriver.cpp
===================================================================
--- drivers/sybase/sybasedriver.cpp (revision 750992)
+++ drivers/sybase/sybasedriver.cpp (working copy)
@@ -57,7 +57,9 @@
beh->_1ST_ROW_READ_AHEAD_REQUIRED_TO_KNOW_IF_THE_RESULT_IS_EMPTY=false;
beh->USING_DATABASE_REQUIRED_TO_CONNECT=false;
- beh->AUTO_INCREMENT_FIELD_OPTION="DEFAULT AUTOINCREMENT";
+ // for Sybase ASA this field is "DEFAULT AUTOINCREMENT"
+ // for MSSQL and Sybase ASE it's IDENTITY
+ beh->AUTO_INCREMENT_FIELD_OPTION="IDENTITY";
beh->AUTO_INCREMENT_PK_FIELD_OPTION = beh->AUTO_INCREMENT_FIELD_OPTION + " PRIMARY KEY ";
// confirm
@@ -169,12 +171,13 @@
+ QByteArray( "\"" ) ;
}
-QString SybaseDriver::addLimitTo1(const QString& sql, bool add)
+QString SybaseDriver::addLimitTo1(const QString& sql, bool add )
{
// length of "select" is 6
// eg: before: select foo from foobar
// after: select TOP 1 foo from foobar
- return add ? sql.trimmed().insert( 6 , " TOP 1 " ) : sql;
+ QString returnString = sql.trimmed().insert( 6, " TOP 1 " );
+ return add ? returnString : sql;
}
#include "sybasedriver.moc"
Index: drivers/sybase/sybaseconnection_p.cpp
===================================================================
--- drivers/sybase/sybaseconnection_p.cpp (revision 750992)
+++ drivers/sybase/sybaseconnection_p.cpp (working copy)
@@ -175,7 +175,7 @@
out<<" host = "<<hostName<<"\n";
if ( data.port == 0 )
- out<<" port = "<<2638<<"\n"; // default port to be used
+ out<<" port = "<<5000<<"\n"; // default port to be used
else
out<<" port = "<<data.port<<"\n";
@@ -241,8 +241,8 @@
if ( dbuse( dbProcess, dbName.toLatin1().data() ) == SUCCEED ) {
return true;
}
-// return false;
- return true; // for testing
+
+ return false;
}
/*! Executes the given SQL statement on the server.
Index: drivers/sybase/sybaseconnection.cpp
===================================================================
--- drivers/sybase/sybaseconnection.cpp (revision 750992)
+++ drivers/sybase/sybaseconnection.cpp (working copy)
@@ -58,11 +58,11 @@
QString serverVersionString;
- if ( !querySingleString( "Select @@servername" , version.string, 0 , false ) ) {
+ if ( !querySingleString( "Select @@servername" , version.string ) ) {
KexiDBDrvDbg << "Couldn't fetch server name" << endl;
}
- if ( !querySingleString( "Select @@version", serverVersionString , 0 , false ) ) {
+ if ( !querySingleString( "Select @@version", serverVersionString ) ) {
KexiDBDrvDbg << "Couldn't fetch server version" << endl;
}
@@ -97,16 +97,21 @@
if ( queryStringList( "Select name from master..sysdatabases", list ) )
return true;
- d->storeResult();
+// d->storeResult();
// setError(ERR_DB_SPECIFIC,mysql_error(d->mysql));
return false;
}
-bool SybaseConnection::drv_createDatabase( const QString &dbName) {
+bool SybaseConnection::drv_createDatabase( const QString &dbName)
+{
KexiDBDrvDbg << "SybaseConnection::drv_createDatabase: " << dbName << endl;
// mysql_create_db deprecated, use SQL here.
- if (drv_executeSQL("CREATE DATABASE " + driver()->escapeString(dbName)))
- return true;
+ if (drv_executeSQL("CREATE DATABASE " + dbName)) {
+ // set allow_nulls_by_default option to true
+ QString allowNullsQuery = QString( "sp_dboption %1, allow_nulls_by_default, true" ).arg( dbName );
+ if ( drv_executeSQL( allowNullsQuery.toLatin1().data() ) )
+ return true;
+ }
d->storeResult();
return false;
}
@@ -115,8 +120,9 @@
{
Q_UNUSED(cancelled);
Q_UNUSED(msgHandler);
+
//TODO is here escaping needed?
- return d->useDatabase( driver()->escapeString( dbName ) );
+ return d->useDatabase( dbName ) ;
}
bool SybaseConnection::drv_closeDatabase() {
@@ -137,7 +143,7 @@
{
int rowId;
- querySingleNumber( "Select @@IDENTITY", rowId );
+ querySingleNumber( "Select @@IDENTITY", rowId );
return ( qint64 )rowId;
}
@@ -174,7 +180,7 @@
bool SybaseConnection::drv_getTablesList( QStringList &list )
{
- return queryStringList( "Select name from sysobjects where type='U'", list );
+ return queryStringList( "Select name from sysobjects where type='U'", list );
}
PreparedStatement::Ptr SybaseConnection::prepareStatement(PreparedStatement::StatementType type,
@@ -183,4 +189,50 @@
return KSharedPtr<PreparedStatement>( new SybasePreparedStatement(type, *d, fields) );
}
+bool KexiDB::SybaseConnection::drv_beforeInsert( const QString& table, FieldList& fields )
+{
+
+ if ( fields.autoIncrementFields()->isEmpty() )
+ return true;
+
+ // explicit insertion into IDENTITY fields !!
+ return drv_executeSQL( QString( "SET IDENTITY_INSERT %1 ON" ).arg( escapeIdentifier( table ) ) );
+
+}
+
+bool KexiDB::SybaseConnection::drv_afterInsert( const QString& table, FieldList& fields )
+{
+ // should we instead just set a flag when an identity_insert has taken place and only check for that
+ // flag here ?
+
+ if ( fields.autoIncrementFields()->isEmpty() )
+ return true;
+
+ // explicit insertion into IDENTITY fields has taken place. Turn off IDENTITY_INSERT
+ return drv_executeSQL( QString( "SET IDENTITY_INSERT %1 OFF" ).arg( escapeIdentifier( table ) ) );
+
+}
+
+bool KexiDB::SybaseConnection::drv_beforeUpdate( const QString& table, FieldList& fields )
+{
+ if ( fields.autoIncrementFields()->isEmpty() )
+ return true;
+
+ // explicit update of IDENTITY fields has taken place.
+ return drv_executeSQL( QString( "SET IDENTITY_UPDATE %1 ON" ).arg( escapeIdentifier( table ) ) );
+}
+
+bool KexiDB::SybaseConnection::drv_afterUpdate( const QString& table, FieldList& fields )
+{
+ // should we instead just set a flag when an identity_update has taken place and only check for that
+ // flag here ?
+
+ if ( fields.autoIncrementFields()->isEmpty() )
+ return true;
+
+ // explicit insertion into IDENTITY fields has taken place. Turn off IDENTITY_INSERT
+ return drv_executeSQL( QString( "SET IDENTITY_UPDATE %1 OFF" ).arg( escapeIdentifier( table ) ) );
+
+}
+
#include "sybaseconnection.moc"
Index: drivers/sybase/sybasedriver.h
===================================================================
--- drivers/sybase/sybasedriver.h (revision 750992)
+++ drivers/sybase/sybasedriver.h (working copy)
@@ -48,7 +48,7 @@
virtual QByteArray drv_escapeIdentifier(const QByteArray& str) const;
virtual Connection *drv_createConnection( ConnectionData &conn_data );
virtual bool drv_isSystemFieldName( const QString& n ) const;
- virtual QString addLimitTo1(const QString& sql, bool add);
+ virtual QString addLimitTo1(const QString& sql, bool add );
private:
static const char *keywords[];
Index: drivers/sybase/sybaseconnection.h
===================================================================
--- drivers/sybase/sybaseconnection.h (revision 750992)
+++ drivers/sybase/sybaseconnection.h (working copy)
@@ -73,10 +73,17 @@
//TODO: move this somewhere to low level class (MIGRATION?)
virtual bool drv_containsTable( const QString &tableName );
+ virtual bool drv_beforeInsert( const QString& table, FieldList& fields );
+ virtual bool drv_afterInsert( const QString& table, FieldList& fields );
+
+ virtual bool drv_beforeUpdate( const QString& table, FieldList& fields );
+ virtual bool drv_afterUpdate( const QString& table, FieldList& fields );
+
SybaseConnectionInternal* d;
friend class SybaseDriver;
friend class SybaseCursor;
+
};
}
_______________________________________________
Kexi mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kexi
