From 9e2962755979c45453a7c286b447c28a78d3d5f5 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 10 Apr 2025 10:47:46 +0200 Subject: [PATCH 1/3] Add failing test for non-existent flat node file See #2315 --- tests/test-persistent-cache.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test-persistent-cache.cpp b/tests/test-persistent-cache.cpp index 0d91ad49b..dfb9c0f3f 100644 --- a/tests/test-persistent-cache.cpp +++ b/tests/test-persistent-cache.cpp @@ -107,3 +107,12 @@ TEST_CASE("Persistent cache", "[NoDB]") read_location(cache, 9934, -179.999, 89.1); } } + +TEST_CASE("Opening non-existent persistent cache should fail in append mode", "[NoDB]") +{ + std::string const flat_node_file = + "test_middle_flat.nonexistent.flat.nodes.bin"; + testing::cleanup::file_t const flatnode_cleaner{flat_node_file}; + + REQUIRE_THROWS(node_persistent_cache(flat_node_file, false)); +} From 4493319968698b193024a965217ee196229d3b1f Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 10 Apr 2025 10:41:56 +0200 Subject: [PATCH 2/3] Do not create flat node file in append mode It must already exist. Fixes #2315 --- src/middle-pgsql.cpp | 2 +- src/middle-ram.cpp | 2 +- src/node-persistent-cache.cpp | 11 ++++++++--- src/node-persistent-cache.hpp | 3 ++- tests/bdd/regression/properties.feature | 17 ++++++++++++++++- tests/test-persistent-cache.cpp | 6 +++--- 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 655617ddb..7bae3ec67 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -1248,7 +1248,7 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr thread_pool, } else { m_store_options.use_flat_node_file = true; m_persistent_cache = std::make_shared( - options->flat_node_file, options->droptemp); + options->flat_node_file, !options->append, options->droptemp); } log_debug("Mid: pgsql, cache={}", options->cache); diff --git a/src/middle-ram.cpp b/src/middle-ram.cpp index 10c095960..e670fecf3 100644 --- a/src/middle-ram.cpp +++ b/src/middle-ram.cpp @@ -80,7 +80,7 @@ middle_ram_t::middle_ram_t(std::shared_ptr thread_pool, if (!options->flat_node_file.empty()) { m_persistent_cache = std::make_shared( - options->flat_node_file, options->droptemp); + options->flat_node_file, !options->append, options->droptemp); } } diff --git a/src/node-persistent-cache.cpp b/src/node-persistent-cache.cpp index 7b5fb8453..6e07675dd 100644 --- a/src/node-persistent-cache.cpp +++ b/src/node-persistent-cache.cpp @@ -29,15 +29,20 @@ osmium::Location node_persistent_cache::get(osmid_t id) const noexcept } node_persistent_cache::node_persistent_cache(std::string file_name, - bool remove_file) + bool create_file, bool remove_file) : m_file_name(std::move(file_name)), m_remove_file(remove_file) { assert(!m_file_name.empty()); log_debug("Loading persistent node cache from '{}'.", m_file_name); - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-signed-bitwise) - m_fd = open(m_file_name.c_str(), O_RDWR | O_CREAT, 0644); + int flags = O_RDWR; // NOLINT(hicpp-signed-bitwise) + if (create_file) { + flags |= O_CREAT; // NOLINT(hicpp-signed-bitwise) + } + + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) + m_fd = open(m_file_name.c_str(), flags, 0644); if (m_fd < 0) { throw std::system_error{ errno, std::system_category(), diff --git a/src/node-persistent-cache.hpp b/src/node-persistent-cache.hpp index 933efc47e..936771723 100644 --- a/src/node-persistent-cache.hpp +++ b/src/node-persistent-cache.hpp @@ -21,7 +21,8 @@ class node_persistent_cache { public: - node_persistent_cache(std::string file_name, bool remove_file); + node_persistent_cache(std::string file_name, bool create_file, + bool remove_file); ~node_persistent_cache() noexcept; node_persistent_cache(node_persistent_cache const &) = delete; diff --git a/tests/bdd/regression/properties.feature b/tests/bdd/regression/properties.feature index 37c8bf379..e22e299d4 100644 --- a/tests/bdd/regression/properties.feature +++ b/tests/bdd/regression/properties.feature @@ -62,10 +62,25 @@ Feature: Updates to the test database with properties check | | | Not using flat node file (same as on import). | | --flat-nodes=x | | Using flat node file | | --flat-nodes=x | --flat-nodes=x | Using flat node file | - | --flat-nodes=x | --flat-nodes=y | Using the flat node file you specified | | --prefix=abc | | Using prefix 'abc' (same as on import). | + Scenario: Create, then append with non-existent flat node file + When running osm2pgsql pgsql with parameters + | --slim | + | --flat-nodes=x | + + Given the input file '000466354.osc.gz' + Then running osm2pgsql pgsql with parameters fails + | -a | + | --slim | + | --flat-nodes=y | + And the error output contains + """ + Unable to open flatnode file + """ + + Scenario: Create with different output than append When running osm2pgsql pgsql with parameters | --slim | diff --git a/tests/test-persistent-cache.cpp b/tests/test-persistent-cache.cpp index dfb9c0f3f..96cf150ac 100644 --- a/tests/test-persistent-cache.cpp +++ b/tests/test-persistent-cache.cpp @@ -43,7 +43,7 @@ TEST_CASE("Persistent cache", "[NoDB]") // create a new cache { - node_persistent_cache cache{flat_node_file, false}; + node_persistent_cache cache{flat_node_file, true, false}; // write in order write_and_read_location(&cache, 10, 10.01, -45.3); @@ -66,7 +66,7 @@ TEST_CASE("Persistent cache", "[NoDB]") // reopen the cache { - node_persistent_cache cache{flat_node_file, false}; + node_persistent_cache cache{flat_node_file, false, false}; // read all previously written locations read_location(cache, 10, 10.01, -45.3); @@ -114,5 +114,5 @@ TEST_CASE("Opening non-existent persistent cache should fail in append mode", "[ "test_middle_flat.nonexistent.flat.nodes.bin"; testing::cleanup::file_t const flatnode_cleaner{flat_node_file}; - REQUIRE_THROWS(node_persistent_cache(flat_node_file, false)); + REQUIRE_THROWS(node_persistent_cache(flat_node_file, false, false)); } From b245ed6a6ee7a60557dc05b964c2f283ca94c6f5 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Thu, 10 Apr 2025 11:18:12 +0200 Subject: [PATCH 3/3] Use binary mode files for flat node files on Windows --- src/node-persistent-cache.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/node-persistent-cache.cpp b/src/node-persistent-cache.cpp index 6e07675dd..5b918b54e 100644 --- a/src/node-persistent-cache.cpp +++ b/src/node-persistent-cache.cpp @@ -41,6 +41,10 @@ node_persistent_cache::node_persistent_cache(std::string file_name, flags |= O_CREAT; // NOLINT(hicpp-signed-bitwise) } +#ifdef _WIN32 + flags |= O_BINARY; // NOLINT(hicpp-signed-bitwise) +#endif + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg) m_fd = open(m_file_name.c_str(), flags, 0644); if (m_fd < 0) {