Skip to content

Commit 36048d5

Browse files
MKleusbergMKleusberg
committed
Rework 89587a7
This partially reverts 89587a7 to fix some issues it introduced. Using the knowledge we gained in the further optimisation process, this commit now gets us the best of both worlds: good performance when executing complex queries as well as a more straightforward way to deal with the multithreaded nature of data loading. See issue #2537.
1 parent c951c1c commit 36048d5

File tree

6 files changed

+66
-47
lines changed

6 files changed

+66
-47
lines changed

‎src/RowLoader.cpp‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,14 @@ namespace {
1616
} // anon ns
1717

1818

19-
RowLoader::RowLoader (
20-
std::function<std::shared_ptr<sqlite3>(void)> db_getter_,
19+
RowLoader::RowLoader (std::function<std::shared_ptr<sqlite3>(void)> db_getter_,
2120
std::function<void(QString)> statement_logger_,
2221
std::vector<std::string> & headers_,
23-
std::vector<int>& data_types_,
2422
std::mutex & cache_mutex_,
2523
Cache & cache_data_
2624
)
2725
: db_getter(db_getter_), statement_logger(statement_logger_)
28-
, headers(headers_), data_types(data_types_)
26+
, headers(headers_)
2927
, cache_mutex(cache_mutex_), cache_data(cache_data_)
3028
, query()
3129
, countQuery()
@@ -229,18 +227,6 @@ void RowLoader::process (Task & t)
229227
auto row = t.row_begin;
230228
if(sqlite3_prepare_v2(pDb.get(), utf8Query, utf8Query.size(), &stmt, nullptr) == SQLITE_OK)
231229
{
232-
// If the cache is uninitialised, retrieve the column names and types
233-
const bool first_chunk = !cache_data.initialised();
234-
if(first_chunk)
235-
{
236-
int num_columns = sqlite3_column_count(stmt);
237-
for(int i=0;i<num_columns;++i)
238-
{
239-
headers.push_back(sqlite3_column_name(stmt, i));
240-
data_types.push_back(sqlite3_column_type(stmt, i));
241-
}
242-
}
243-
244230
while(!t.cancel && sqlite3_step(stmt) == SQLITE_ROW)
245231
{
246232
size_t num_columns = static_cast<size_t>(sqlite3_data_count(stmt));
@@ -269,8 +255,9 @@ void RowLoader::process (Task & t)
269255
// - this is the first batch of data we load for this query
270256
// - we got exactly the number of rows back we queried (which indicates there might be more rows)
271257
// If there is no need to query the row count this means the number of rows we just got is the total row count.
272-
if(first_chunk)
258+
if(!cache_data.initialised())
273259
{
260+
cache_data.setInitialised();
274261
if(row == t.row_end)
275262
triggerRowCountDetermination(t.token);
276263
else

‎src/RowLoader.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class RowLoader : public QThread
3030
std::function<std::shared_ptr<sqlite3>(void)> db_getter,
3131
std::function<void(QString)> statement_logger,
3232
std::vector<std::string> & headers,
33-
std::vector<int>& data_types,
3433
std::mutex & cache_mutex,
3534
Cache & cache_data
3635
);
@@ -71,7 +70,6 @@ class RowLoader : public QThread
7170
const std::function<std::shared_ptr<sqlite3>()> db_getter;
7271
const std::function<void(QString)> statement_logger;
7372
std::vector<std::string> & headers;
74-
std::vector<int> & data_types;
7573
std::mutex & cache_mutex;
7674
Cache & cache_data;
7775

‎src/TableBrowser.cpp‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -348,16 +348,6 @@ TableBrowser::TableBrowser(DBBrowserDB* _db, QWidget* parent) :
348348

349349
// Connect slots
350350
connect(m_model, &SqliteTableModel::finishedFetch, this, &TableBrowser::fetchedData);
351-
connect(m_model, &SqliteTableModel::columnsChanged, this, [this]() {
352-
// Apply all settings
353-
const sqlb::ObjectIdentifier tablename = currentlyBrowsedTableName();
354-
const BrowseDataTableSettings& storedData = m_settings[tablename];
355-
356-
applyModelSettings(storedData);
357-
applyViewportSettings(storedData, tablename);
358-
updateRecordsetLabel();
359-
emit updatePlot(ui->dataTable, m_model, &m_settings[tablename], true);
360-
});
361351

362352
// Load initial settings
363353
reloadSettings();
@@ -515,8 +505,10 @@ void TableBrowser::refresh()
515505
// Current table changed
516506
emit currentTableChanged(tablename);
517507

518-
// Set query which also resets the model
519-
m_model->setQuery(buildQuery(storedData, tablename));
508+
// Build query and apply settings
509+
applyModelSettings(storedData, buildQuery(storedData, tablename));
510+
applyViewportSettings(storedData, tablename);
511+
updateRecordsetLabel();
520512
}
521513

522514
void TableBrowser::clearFilters()
@@ -793,8 +785,11 @@ sqlb::Query TableBrowser::buildQuery(const BrowseDataTableSettings& storedData,
793785
return query;
794786
}
795787

796-
void TableBrowser::applyModelSettings(const BrowseDataTableSettings& storedData)
788+
void TableBrowser::applyModelSettings(const BrowseDataTableSettings& storedData, const sqlb::Query& query)
797789
{
790+
// Set query which also resets the model
791+
m_model->setQuery(query);
792+
798793
// Regular conditional formats
799794
for(auto formatIt=storedData.condFormats.cbegin(); formatIt!=storedData.condFormats.cend(); ++formatIt)
800795
m_model->setCondFormats(false, formatIt->first, formatIt->second);

‎src/TableBrowser.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ private slots:
171171
void modifyFormat(std::function<void(CondFormat&)> changeFunction);
172172

173173
sqlb::Query buildQuery(const BrowseDataTableSettings& storedData, const sqlb::ObjectIdentifier& tablename) const;
174-
void applyModelSettings(const BrowseDataTableSettings& storedData);
174+
void applyModelSettings(const BrowseDataTableSettings& storedData, const sqlb::Query& query);
175175
void applyViewportSettings(const BrowseDataTableSettings& storedData, const sqlb::ObjectIdentifier& tablename);
176176
void generateFilters();
177177
};

‎src/sqlitetablemodel.cpp‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ SqliteTableModel::SqliteTableModel(DBBrowserDB& db, QObject* parent, const QStri
3535
worker = new RowLoader(
3636
[this, force_wait](){ return m_db.get(tr("reading rows"), force_wait); },
3737
[this](QString stmt){ return m_db.logSQL(stmt, kLogMsg_App); },
38-
m_headers, m_vDataTypes, m_mutexDataCache, m_cache
38+
m_headers, m_mutexDataCache, m_cache
3939
);
4040

4141
worker->start();
@@ -67,18 +67,6 @@ void SqliteTableModel::handleFinishedFetch (int life_id, unsigned int fetched_ro
6767

6868
Q_ASSERT(fetched_row_end >= fetched_row_begin);
6969

70-
// Tell the query object about the column names
71-
m_query.setColumNames(m_headers);
72-
73-
// If the cache has been uninitialised so far we set it to initialised now and
74-
// tell the view to update the table layout.
75-
if(!m_cache.initialised())
76-
{
77-
m_cache.setInitialised();
78-
emit layoutChanged();
79-
emit columnsChanged();
80-
}
81-
8270
auto old_row_count = m_currentRowCount;
8371
auto new_row_count = std::max(old_row_count, fetched_row_begin);
8472
new_row_count = std::max(new_row_count, fetched_row_end);
@@ -140,20 +128,40 @@ void SqliteTableModel::setQuery(const sqlb::Query& query)
140128
m_query = query;
141129
m_table_of_query = m_db.getObjectByName<sqlb::Table>(query.table());
142130

131+
// The first column is the rowid column and therefore is always of type integer
132+
m_vDataTypes.emplace_back(SQLITE_INTEGER);
133+
134+
// Get the data types of all other columns as well as the column names
143135
// Set the row id columns
144136
if(m_table_of_query && m_table_of_query->fields.size()) // It is a table and parsing was OK
145137
{
146138
sqlb::StringVector rowids = m_table_of_query->rowidColumns();
147139
m_query.setRowIdColumns(rowids);
140+
141+
m_headers.push_back(sqlb::joinStringVector(rowids, ","));
142+
143+
// Store field names and affinity data types
144+
for(const sqlb::Field& fld : m_table_of_query->fields)
145+
{
146+
m_headers.push_back(fld.name());
147+
m_vDataTypes.push_back(fld.affinity());
148+
}
148149
} else {
149150
// If for one reason or another (either it's a view or we couldn't parse the table statement) we couldn't get the field
150151
// information we retrieve it from SQLite using an extra query.
151152
// NOTE: It would be nice to eventually get rid of this piece here. As soon as the grammar parser is good enough...
152153

154+
std::string sColumnQuery = "SELECT * FROM " + query.table().toString() + ";";
153155
if(m_query.rowIdColumns().empty())
154156
m_query.setRowIdColumn("_rowid_");
157+
m_headers.emplace_back("_rowid_");
158+
auto columns = getColumns(nullptr, sColumnQuery, m_vDataTypes);
159+
m_headers.insert(m_headers.end(), columns.begin(), columns.end());
155160
}
156161

162+
// Tell the query object about the column names
163+
m_query.setColumNames(m_headers);
164+
157165
// Apply new query and update view
158166
buildQuery();
159167
}
@@ -174,8 +182,16 @@ void SqliteTableModel::setQuery(const QString& sQuery, const QString& sCountQuer
174182

175183
worker->setQuery(m_sQuery, sCountQuery);
176184

185+
if(!dontClearHeaders)
186+
{
187+
auto columns = getColumns(worker->getDb(), sQuery.toStdString(), m_vDataTypes);
188+
m_headers.insert(m_headers.end(), columns.begin(), columns.end());
189+
}
190+
177191
// now fetch the first entries
178192
triggerCacheLoad(static_cast<int>(m_chunkSize / 2) - 1);
193+
194+
emit layoutChanged();
179195
}
180196

181197
int SqliteTableModel::rowCount(const QModelIndex&) const
@@ -776,6 +792,27 @@ void SqliteTableModel::removeCommentsFromQuery(QString& query)
776792
}
777793
}
778794

795+
std::vector<std::string> SqliteTableModel::getColumns(std::shared_ptr<sqlite3> pDb, const std::string& sQuery, std::vector<int>& fieldsTypes) const
796+
{
797+
if(!pDb)
798+
pDb = m_db.get(tr("retrieving list of columns"));
799+
800+
sqlite3_stmt* stmt;
801+
std::vector<std::string> listColumns;
802+
if(sqlite3_prepare_v2(pDb.get(), sQuery.c_str(), static_cast<int>(sQuery.size()), &stmt, nullptr) == SQLITE_OK)
803+
{
804+
int columns = sqlite3_column_count(stmt);
805+
for(int i = 0; i < columns; ++i)
806+
{
807+
listColumns.push_back(sqlite3_column_name(stmt, i));
808+
fieldsTypes.push_back(sqlite3_column_type(stmt, i));
809+
}
810+
}
811+
sqlite3_finalize(stmt);
812+
813+
return listColumns;
814+
}
815+
779816
void addCondFormatToMap(std::map<size_t, std::vector<CondFormat>>& mCondFormats, size_t column, const CondFormat& condFormat)
780817
{
781818
// If the condition is already present in the vector, update that entry and respect the order, since two entries with the same

‎src/sqlitetablemodel.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@ public slots:
149149
signals:
150150
void finishedFetch(int fetched_row_begin, int fetched_row_end);
151151
void finishedRowCount();
152-
void columnsChanged();
153152

154153
protected:
155154
Qt::DropActions supportedDropActions() const override;
@@ -168,6 +167,9 @@ public slots:
168167

169168
void buildQuery();
170169

170+
/// \param pDb connection to query; if null, obtains it from 'm_db'.
171+
std::vector<std::string> getColumns(std::shared_ptr<sqlite3> pDb, const std::string& sQuery, std::vector<int>& fieldsTypes) const;
172+
171173
QByteArray encode(const QByteArray& str) const;
172174
QByteArray decode(const QByteArray& str) const;
173175

0 commit comments

Comments
 (0)