Merge pull request #2316 from joto/flat-node-no-create

Do not create flat node files in append mode

Author: lonvia

src/middle-pgsql.cpp

@@ -1248,7 +1248,7 @@ middle_pgsql_t::middle_pgsql_t(std::shared_ptr<thread_pool_t> thread_pool,
     } else {
         m_store_options.use_flat_node_file = true;
         m_persistent_cache = std::make_shared<node_persistent_cache>(
-            options->flat_node_file, options->droptemp);
+            options->flat_node_file, !options->append, options->droptemp);
     }
 
     log_debug("Mid: pgsql, cache={}", options->cache);

src/middle-ram.cpp

@@ -80,7 +80,7 @@ middle_ram_t::middle_ram_t(std::shared_ptr<thread_pool_t> thread_pool,
 
     if (!options->flat_node_file.empty()) {
         m_persistent_cache = std::make_shared<node_persistent_cache>(
-            options->flat_node_file, options->droptemp);
+            options->flat_node_file, !options->append, options->droptemp);
     }
 }
 

src/node-persistent-cache.cpp

@@ -29,15 +29,24 @@ 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)
+    }
+
+#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) {
         throw std::system_error{
             errno, std::system_category(),

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;

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 |

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);
@@ -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, false));
+}