From 2eac1c0382c7ba6735319c2ba957b00356349d82 Mon Sep 17 00:00:00 2001 From: Leijurv Date: Thu, 24 Sep 2026 14:45:33 -0700 Subject: [PATCH 1/2] fix reading attributes of nodes from the middle --- src/middle-pgsql.cpp | 33 ++++++++++++++--------- tests/test-middle.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 631ed95aa..454d2b146 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -276,25 +276,29 @@ void pgsql_parse_nodes(char const *string, osmium::memory::Buffer *buffer, } } +/** + * Set the attributes of an object from the result of a query with the + * columns from {attribute_columns_use} starting at the specified column. + */ template void set_attributes_on_builder(T *builder, pg_result_t const &result, int num, - int offset) + int column) { - if (!result.is_null(num, offset + 2)) { + if (!result.is_null(num, column)) { builder->set_timestamp( - std::strtoul(result.get_value(num, offset + 2), nullptr, 10)); + std::strtoul(result.get_value(num, column), nullptr, 10)); } - if (!result.is_null(num, offset + 3)) { - builder->set_version(result.get_value(num, offset + 3)); + if (!result.is_null(num, column + 1)) { + builder->set_version(result.get_value(num, column + 1)); } - if (!result.is_null(num, offset + 4)) { - builder->set_changeset(result.get_value(num, offset + 4)); + if (!result.is_null(num, column + 2)) { + builder->set_changeset(result.get_value(num, column + 2)); } - if (!result.is_null(num, offset + 5)) { - builder->set_uid(result.get_value(num, offset + 5)); + if (!result.is_null(num, column + 3)) { + builder->set_uid(result.get_value(num, column + 3)); } - if (!result.is_null(num, offset + 6)) { - builder->set_user(result.get_value(num, offset + 6)); + if (!result.is_null(num, column + 4)) { + builder->set_user(result.get_value(num, column + 4)); } } @@ -735,6 +739,7 @@ void build_node(osmid_t id, pg_result_t const &res, int res_num, int offset, (int)std::strtol(res.get_value(res_num, offset + 1), nullptr, 10)}); if (with_attributes) { + // lon, lat, tags, then the attributes set_attributes_on_builder(&builder, res, res_num, offset + 3); } pgsql_parse_json_tags(res.get_value(res_num, offset + 2), buffer, &builder); @@ -750,7 +755,8 @@ void build_way(osmid_t id, pg_result_t const &res, int res_num, int offset, builder.set_id(id); if (with_attributes) { - set_attributes_on_builder(&builder, res, res_num, offset); + // nodes, tags, then the attributes + set_attributes_on_builder(&builder, res, res_num, offset + 2); } pgsql_parse_nodes(res.get_value(res_num, offset + 0), buffer, &builder); pgsql_parse_json_tags(res.get_value(res_num, offset + 1), buffer, &builder); @@ -907,7 +913,8 @@ bool middle_query_pgsql_t::relation_get(osmid_t id, builder.set_id(id); if (m_store_options.with_attributes) { - set_attributes_on_builder(&builder, res, 0, 0); + // members, tags, then the attributes + set_attributes_on_builder(&builder, res, 0, 2); } pgsql_parse_json_members(res.get_value(0, 0), buffer, &builder); diff --git a/tests/test-middle.cpp b/tests/test-middle.cpp index 36a88b776..cfaf78495 100644 --- a/tests/test-middle.cpp +++ b/tests/test-middle.cpp @@ -570,6 +570,68 @@ bool no_way(std::shared_ptr const &mid, osmid_t id) } // anonymous namespace +TEST_CASE("middle: add node with attributes") +{ + auto thread_pool = std::make_shared(1U); + + options_t options = options_slim_default::options(db); + + SECTION("With attributes") { options.extra_attributes = true; } + SECTION("No attributes") { options.extra_attributes = false; } + + test_buffer_t buffer; + auto const &node10 = + buffer.add_node("n10 v123 c456 t2009-02-13T23:31:30Z i789 usomebody" + " x1.1 y2.2 Tamenity=bench,name=Blue"); + + auto const check = [&](std::shared_ptr const &mid) { + auto const mid_q = mid->get_query_instance(); + osmium::memory::Buffer outbuf{4096, + osmium::memory::Buffer::auto_grow::yes}; + REQUIRE(mid_q->node_get(10, &outbuf)); + auto const &node = outbuf.get(0); + + CHECK(node.id() == 10); + CHECK(node.location() == node10.location()); + if (options.extra_attributes) { + CHECK(node.timestamp() == node10.timestamp()); + CHECK(node.version() == node10.version()); + CHECK(node.changeset() == node10.changeset()); + CHECK(node.uid() == node10.uid()); + CHECK(std::strcmp(node.user(), node10.user()) == 0); + } else { + CHECK(node.version() == 0); + CHECK(node.changeset() == 0); + CHECK(node.uid() == 0); + } + REQUIRE(node.tags().size() == 2); + CHECK(std::strcmp(node.tags()["amenity"], "bench") == 0); + CHECK(std::strcmp(node.tags()["name"], "Blue") == 0); + }; + + { + auto mid = std::make_shared(thread_pool, &options); + mid->start(); + + mid->node(node10); + mid->after_nodes(); + mid->after_ways(); + mid->after_relations(); + + check(mid); + } + + // From now on use append mode to not destroy the data we just added. + options.append = true; + + { + auto mid = std::make_shared(thread_pool, &options); + mid->start(); + + check(mid); + } +} + TEMPLATE_TEST_CASE("middle: add, delete and update way", "", options_slim_default, options_flat_node_cache) { From 98430ad1ae7dbbdee24cbea3ea10d4abab7bd369 Mon Sep 17 00:00:00 2001 From: Leijurv Date: Fri, 25 Sep 2026 09:18:00 -0700 Subject: [PATCH 2/2] simplify node attributes test: drop the append-mode reread --- tests/test-middle.cpp | 69 ++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/tests/test-middle.cpp b/tests/test-middle.cpp index cfaf78495..1a42b430d 100644 --- a/tests/test-middle.cpp +++ b/tests/test-middle.cpp @@ -584,52 +584,35 @@ TEST_CASE("middle: add node with attributes") buffer.add_node("n10 v123 c456 t2009-02-13T23:31:30Z i789 usomebody" " x1.1 y2.2 Tamenity=bench,name=Blue"); - auto const check = [&](std::shared_ptr const &mid) { - auto const mid_q = mid->get_query_instance(); - osmium::memory::Buffer outbuf{4096, - osmium::memory::Buffer::auto_grow::yes}; - REQUIRE(mid_q->node_get(10, &outbuf)); - auto const &node = outbuf.get(0); - - CHECK(node.id() == 10); - CHECK(node.location() == node10.location()); - if (options.extra_attributes) { - CHECK(node.timestamp() == node10.timestamp()); - CHECK(node.version() == node10.version()); - CHECK(node.changeset() == node10.changeset()); - CHECK(node.uid() == node10.uid()); - CHECK(std::strcmp(node.user(), node10.user()) == 0); - } else { - CHECK(node.version() == 0); - CHECK(node.changeset() == 0); - CHECK(node.uid() == 0); - } - REQUIRE(node.tags().size() == 2); - CHECK(std::strcmp(node.tags()["amenity"], "bench") == 0); - CHECK(std::strcmp(node.tags()["name"], "Blue") == 0); - }; - - { - auto mid = std::make_shared(thread_pool, &options); - mid->start(); - - mid->node(node10); - mid->after_nodes(); - mid->after_ways(); - mid->after_relations(); - - check(mid); - } + auto mid = std::make_shared(thread_pool, &options); + mid->start(); - // From now on use append mode to not destroy the data we just added. - options.append = true; + mid->node(node10); + mid->after_nodes(); + mid->after_ways(); + mid->after_relations(); - { - auto mid = std::make_shared(thread_pool, &options); - mid->start(); - - check(mid); + auto const mid_q = mid->get_query_instance(); + osmium::memory::Buffer outbuf{4096, osmium::memory::Buffer::auto_grow::yes}; + REQUIRE(mid_q->node_get(10, &outbuf)); + auto const &node = outbuf.get(0); + + CHECK(node.id() == 10); + CHECK(node.location() == node10.location()); + if (options.extra_attributes) { + CHECK(node.timestamp() == node10.timestamp()); + CHECK(node.version() == node10.version()); + CHECK(node.changeset() == node10.changeset()); + CHECK(node.uid() == node10.uid()); + CHECK(std::strcmp(node.user(), node10.user()) == 0); + } else { + CHECK(node.version() == 0); + CHECK(node.changeset() == 0); + CHECK(node.uid() == 0); } + REQUIRE(node.tags().size() == 2); + CHECK(std::strcmp(node.tags()["amenity"], "bench") == 0); + CHECK(std::strcmp(node.tags()["name"], "Blue") == 0); } TEMPLATE_TEST_CASE("middle: add, delete and update way", "",