Skip to content

Commit d744f2f

Browse files
committed
Only use a single option variable for flat node store
This makes it impossible for the two variables to not be in sync.
1 parent 7d9f51c commit d744f2f

File tree

8 files changed

+22
-32
lines changed

8 files changed

+22
-32
lines changed

src/db-check.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ void check_db(options_t const &options)
4242
// it probably means we used a flat node store when we created this
4343
// database. Check for that and stop if it looks like we are missing
4444
// the node location store option.
45-
if (options.append && !options.flat_node_cache_enabled) {
45+
if (options.append && options.flat_node_file.empty()) {
4646
if (!has_table(db_connection, options.middle_dbschema,
4747
options.prefix + "_nodes")) {
4848
throw std::runtime_error{

src/middle-pgsql.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ void middle_pgsql_t::node_set(osmium::Node const &node)
318318
{
319319
m_cache->set(node.id(), node.location());
320320

321-
if (m_options->flat_node_cache_enabled) {
321+
if (!m_options->flat_node_file.empty()) {
322322
m_persistent_cache->set(node.id(), node.location());
323323
} else {
324324
m_db_copy.new_line(m_tables[NODE_TABLE].copy_target());
@@ -359,7 +359,7 @@ void middle_pgsql_t::node_delete(osmid_t osm_id)
359359
{
360360
assert(m_options->append);
361361

362-
if (m_options->flat_node_cache_enabled) {
362+
if (!m_options->flat_node_file.empty()) {
363363
m_persistent_cache->set(osm_id, osmium::Location{});
364364
} else {
365365
m_db_copy.new_line(m_tables[NODE_TABLE].copy_target());
@@ -623,7 +623,7 @@ void middle_pgsql_t::flush() { m_db_copy.sync(); }
623623
void middle_pgsql_t::stop(thread_pool_t &pool)
624624
{
625625
m_cache.reset();
626-
if (m_options->flat_node_cache_enabled) {
626+
if (!m_options->flat_node_file.empty()) {
627627
m_persistent_cache.reset();
628628
}
629629

@@ -774,7 +774,7 @@ middle_pgsql_t::middle_pgsql_t(options_t const *options)
774774
std::make_shared<db_copy_thread_t>(options->database_options.conninfo())),
775775
m_db_copy(m_copy_thread)
776776
{
777-
if (options->flat_node_cache_enabled) {
777+
if (!options->flat_node_file.empty()) {
778778
m_persistent_cache.reset(new node_persistent_cache{options});
779779
}
780780

@@ -789,7 +789,7 @@ middle_pgsql_t::middle_pgsql_t(options_t const *options)
789789
}
790790

791791
m_tables[NODE_TABLE] =
792-
table_desc{*options, sql_for_nodes(!options->flat_node_cache_enabled)};
792+
table_desc{*options, sql_for_nodes(options->flat_node_file.empty())};
793793
m_tables[WAY_TABLE] =
794794
table_desc{*options, sql_for_ways(has_bucket_index,
795795
options->way_node_index_id_shift)};

src/node-persistent-cache.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ osmium::Location node_persistent_cache::get(osmid_t id) const noexcept
1717

1818
node_persistent_cache::node_persistent_cache(const options_t *options)
1919
{
20-
if (!options->flat_node_file) {
20+
if (options->flat_node_file.empty()) {
2121
throw std::runtime_error{"Unable to set up persistent cache: the name "
2222
"of the flat node file was not set."};
2323
}
2424

25-
m_fname = options->flat_node_file->c_str();
25+
m_fname = options->flat_node_file.c_str();
2626
m_remove_file = options->droptemp;
2727
log_debug("Mid: loading persistent node cache from {}", m_fname);
2828

src/options.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,6 @@ options_t::options_t(int argc, char *argv[]) : options_t()
544544
droptemp = true;
545545
break;
546546
case 'F':
547-
flat_node_cache_enabled = true;
548547
flat_node_file = optarg;
549548
break;
550549
case 211:
@@ -702,7 +701,7 @@ void options_t::check_options()
702701
throw std::runtime_error{
703702
"RAM node cache can only be disable in slim mode."};
704703
}
705-
if (!flat_node_cache_enabled) {
704+
if (flat_node_file.empty()) {
706705
log_warn("RAM cache is disabled. This will likely slow down "
707706
"processing a lot.");
708707
}

src/options.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,10 @@ class options_t
114114
/// only copy rows that match an explicitly listed key
115115
bool hstore_match_only = false;
116116

117-
bool flat_node_cache_enabled = false;
118117
bool reproject_area = false;
119-
boost::optional<std::string> flat_node_file{boost::none};
118+
119+
/// Name of the flat node file used. Empty if flat node file is not enabled.
120+
std::string flat_node_file{};
120121

121122
/**
122123
* these options allow you to control the name of the

tests/common-options.hpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ class opt_t
6666

6767
opt_t &flatnodes()
6868
{
69-
m_opt.flat_node_file =
70-
boost::optional<std::string>("test_middle_flat.flat.nodes.bin");
71-
m_opt.flat_node_cache_enabled = true;
69+
m_opt.flat_node_file = "test_middle_flat.flat.nodes.bin";
7270
return *this;
7371
}
7472

tests/test-middle.cpp

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,7 @@ TEMPLATE_TEST_CASE("middle import", "", options_slim_default,
184184
options_ram_optimized, options_ram_flatnode)
185185
{
186186
options_t const options = TestType::options(db);
187-
testing::cleanup::file_t flatnode_cleaner{
188-
options.flat_node_file.get_value_or("")};
187+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
189188

190189
auto conn = db.connect();
191190
auto const num_tables =
@@ -373,8 +372,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update node", "",
373372
{
374373
options_t options = TestType::options(db);
375374

376-
testing::cleanup::file_t flatnode_cleaner{
377-
options.flat_node_file.get_value_or("")};
375+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
378376

379377
// Prepare a buffer with some nodes which we will add and change.
380378
test_buffer_t buffer;
@@ -573,8 +571,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update way", "",
573571
{
574572
options_t options = TestType::options(db);
575573

576-
testing::cleanup::file_t flatnode_cleaner{
577-
options.flat_node_file.get_value_or("")};
574+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
578575

579576
// Create some ways we'll use for the tests.
580577
test_buffer_t buffer;
@@ -730,8 +727,7 @@ TEMPLATE_TEST_CASE("middle: add way with attributes", "", options_slim_default,
730727
SECTION("With attributes") { options.extra_attributes = true; }
731728
SECTION("No attributes") { options.extra_attributes = false; }
732729

733-
testing::cleanup::file_t flatnode_cleaner{
734-
options.flat_node_file.get_value_or("")};
730+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
735731

736732
// Create some ways we'll use for the tests.
737733
test_buffer_t buffer;
@@ -821,8 +817,7 @@ TEMPLATE_TEST_CASE("middle: add, delete and update relation", "",
821817
{
822818
options_t options = TestType::options(db);
823819

824-
testing::cleanup::file_t flatnode_cleaner{
825-
options.flat_node_file.get_value_or("")};
820+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
826821

827822
// Create some relations we'll use for the tests.
828823
test_buffer_t buffer;
@@ -983,8 +978,7 @@ TEMPLATE_TEST_CASE("middle: add relation with attributes", "",
983978
SECTION("With attributes") { options.extra_attributes = true; }
984979
SECTION("No attributes") { options.extra_attributes = false; }
985980

986-
testing::cleanup::file_t flatnode_cleaner{
987-
options.flat_node_file.get_value_or("")};
981+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
988982

989983
// Create some relations we'll use for the tests.
990984
test_buffer_t buffer;
@@ -1046,8 +1040,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in way", "", options_slim_default,
10461040
{
10471041
options_t options = TestType::options(db);
10481042

1049-
testing::cleanup::file_t flatnode_cleaner{
1050-
options.flat_node_file.get_value_or("")};
1043+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
10511044

10521045
// create some nodes and ways we'll use for the tests
10531046
test_buffer_t buffer;
@@ -1187,8 +1180,7 @@ TEMPLATE_TEST_CASE("middle: change nodes in relation", "", options_slim_default,
11871180
{
11881181
options_t options = TestType::options(db);
11891182

1190-
testing::cleanup::file_t flatnode_cleaner{
1191-
options.flat_node_file.get_value_or("")};
1183+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
11921184

11931185
// create some nodes, ways, and relations we'll use for the tests
11941186
test_buffer_t buffer;

tests/test-persistent-cache.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ static void delete_location(node_persistent_cache &cache, osmid_t id)
2828
TEST_CASE("Persistent cache", "[NoDB]")
2929
{
3030
options_t const options = testing::opt_t().flatnodes();
31-
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file.get()};
31+
testing::cleanup::file_t flatnode_cleaner{options.flat_node_file};
3232

3333
// create a new cache
3434
{

0 commit comments

Comments
 (0)