Skip to content

Commit 565aa31

Browse files
committed
Refactor db_target_descr_t: Always need schema on construction
We now always construct this fully with schema, name, and id column. This way we make sure all data is there and the schema is not defaulted.
1 parent 27aa584 commit 565aa31

File tree

7 files changed

+26
-30
lines changed

7 files changed

+26
-30
lines changed

src/db-copy.hpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* For a full list of authors see the git log.
1111
*/
1212

13+
#include <cassert>
1314
#include <condition_variable>
1415
#include <deque>
1516
#include <future>
@@ -29,7 +30,7 @@
2930
struct db_target_descr_t
3031
{
3132
/// Schema of the target table.
32-
std::string schema{"public"};
33+
std::string schema;
3334
/// Name of the target table for the copy operation.
3435
std::string name;
3536
/// Name of id column used when deleting objects.
@@ -46,11 +47,15 @@ struct db_target_descr_t
4647
name == other.name && rows == other.rows);
4748
}
4849

49-
db_target_descr_t() = default;
50+
db_target_descr_t() = delete;
5051

51-
db_target_descr_t(std::string n, std::string i, std::string r = {})
52-
: name(std::move(n)), id(std::move(i)), rows(std::move(r))
53-
{}
52+
db_target_descr_t(std::string s, std::string n, std::string i,
53+
std::string r = {})
54+
: schema(std::move(s)), name(std::move(n)), id(std::move(i)),
55+
rows(std::move(r))
56+
{
57+
assert(!schema.empty());
58+
}
5459
};
5560

5661
/**

src/flex-table.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,11 +250,10 @@ class table_connection_t
250250
std::shared_ptr<db_copy_thread_t> const &copy_thread)
251251
: m_proj(reprojection::create_projection(table->srid())), m_table(table),
252252
m_target(std::make_shared<db_target_descr_t>(
253-
table->name(), table->id_column_names(),
253+
table->schema(), table->name(), table->id_column_names(),
254254
table->build_sql_column_list())),
255255
m_copy_mgr(copy_thread), m_db_connection(nullptr)
256256
{
257-
m_target->schema = table->schema();
258257
}
259258

260259
void connect(std::string const &conninfo);

src/gazetteer-style.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,10 @@ class gazetteer_copy_mgr_t : public db_copy_mgr_t<db_deleter_place_t>
8686
explicit gazetteer_copy_mgr_t(
8787
std::shared_ptr<db_copy_thread_t> const &processor)
8888
: db_copy_mgr_t<db_deleter_place_t>(processor),
89-
m_table(std::make_shared<db_target_descr_t>("place", "place_id"))
90-
{}
89+
m_table(
90+
std::make_shared<db_target_descr_t>("public", "place", "place_id"))
91+
{
92+
}
9193

9294
void prepare() { new_line(m_table); }
9395

src/middle-pgsql.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,9 @@ middle_pgsql_t::table_desc::table_desc(options_t const &options,
132132
table_sql const &ts)
133133
: m_create_table(build_sql(options, ts.create_table)),
134134
m_prepare_queries(build_sql(options, ts.prepare_queries)),
135-
m_copy_target(std::make_shared<db_target_descr_t>())
135+
m_copy_target(std::make_shared<db_target_descr_t>(
136+
options.middle_dbschema, build_sql(options, ts.name), "id"))
136137
{
137-
m_copy_target->name = build_sql(options, ts.name);
138-
m_copy_target->schema = options.middle_dbschema;
139-
m_copy_target->id = "id";
140-
141138
if (options.with_forward_dependencies) {
142139
m_create_fw_dep_indexes = build_sql(options, ts.create_fw_dep_indexes);
143140
}
@@ -1324,9 +1321,8 @@ void middle_pgsql_t::write_users_table()
13241321
{
13251322
log_info("Writing {} entries to table '{}'...", m_users.size(),
13261323
m_users_table.name());
1327-
auto const users_table =
1328-
std::make_shared<db_target_descr_t>(m_users_table.name(), "id");
1329-
users_table->schema = m_users_table.schema();
1324+
auto const users_table = std::make_shared<db_target_descr_t>(
1325+
m_users_table.schema(), m_users_table.name(), "id");
13301326

13311327
for (auto const &[id, name] : m_users) {
13321328
m_db_copy.new_line(users_table);

src/table.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,11 @@ table_t::table_t(std::string const &name, std::string type, columns_t columns,
3030
hstore_column hstore_mode,
3131
std::shared_ptr<db_copy_thread_t> const &copy_thread,
3232
std::string const &schema)
33-
: m_target(std::make_shared<db_target_descr_t>(name.c_str(), "osm_id")),
33+
: m_target(std::make_shared<db_target_descr_t>(schema, name, "osm_id")),
3434
m_type(std::move(type)), m_srid(fmt::to_string(srid)), m_append(append),
3535
m_hstore_mode(hstore_mode), m_columns(std::move(columns)),
3636
m_hstore_columns(std::move(hstore_columns)), m_copy(copy_thread)
3737
{
38-
assert(!schema.empty());
39-
m_target->schema = schema;
40-
4138
// if we dont have any columns
4239
if (m_columns.empty() && m_hstore_mode != hstore_column::all) {
4340
throw fmt_error("No columns provided for table {}.", name);

tests/test-db-copy-mgr.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ static std::shared_ptr<db_target_descr_t> setup_table(std::string const &cols)
2727
conn.exec("CREATE TABLE test_copy_mgr (id int8{}{})",
2828
cols.empty() ? "" : ",", cols);
2929

30-
auto table = std::make_shared<db_target_descr_t>();
31-
table->name = "test_copy_mgr";
32-
table->id = "id";
30+
auto table =
31+
std::make_shared<db_target_descr_t>("public", "test_copy_mgr", "id");
3332

3433
return table;
3534
}

tests/test-db-copy-thread.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ TEST_CASE("db_copy_thread_t with db_deleter_by_id_t")
2727
conn.exec("DROP TABLE IF EXISTS test_copy_thread");
2828
conn.exec("CREATE TABLE test_copy_thread (id int8)");
2929

30-
auto const table = std::make_shared<db_target_descr_t>();
31-
table->name = "test_copy_thread";
32-
table->id = "id";
30+
auto const table =
31+
std::make_shared<db_target_descr_t>("public", "test_copy_thread", "id");
3332

3433
db_copy_thread_t t(db.conninfo());
3534
using cmd_copy_t = db_cmd_copy_delete_t<db_deleter_by_id_t>;
@@ -157,9 +156,8 @@ TEST_CASE("db_copy_thread_t with db_deleter_place_t")
157156
"osm_id bigint,"
158157
"class text)");
159158

160-
auto table = std::make_shared<db_target_descr_t>();
161-
table->name = "test_copy_thread";
162-
table->id = "place_id";
159+
auto table = std::make_shared<db_target_descr_t>(
160+
"public", "test_copy_thread", "place_id");
163161

164162
db_copy_thread_t t(db.conninfo());
165163
using cmd_copy_t = db_cmd_copy_delete_t<db_deleter_place_t>;

0 commit comments

Comments
 (0)