Skip to content

Commit eaff092

Browse files
lonvialonvia
authored
Merge pull request #1966 from joto/no-attrs-in-tags
Refactor middle: Pull special osm_* tags out of the tag list
2 parents 48c9813 + 8cb0847 commit eaff092

5 files changed

Lines changed: 192 additions & 94 deletions

File tree

‎src/middle-pgsql.cpp‎

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,54 @@ static char const *decode_to_delimiter(char const *src, std::string *dst)
156156
}
157157

158158
namespace {
159+
160+
/**
161+
* Go through tags returned from a middle table, get the attributes from the
162+
* "osm_*" tags and add them to the builder.
163+
*/
164+
template <typename T>
165+
void pgsql_get_attr_from_tags(char const *string, T *builder)
166+
{
167+
if (*string++ != '{') {
168+
return;
169+
}
170+
171+
std::string key;
172+
std::string val;
173+
std::string user;
174+
while (*string != '}') {
175+
string = decode_to_delimiter(string, &key);
176+
// String points to the comma */
177+
++string;
178+
string = decode_to_delimiter(string, &val);
179+
180+
if (key == "osm_version") {
181+
builder->set_version(val.c_str());
182+
} else if (key == "osm_timestamp") {
183+
builder->set_timestamp(osmium::Timestamp{val});
184+
} else if (key == "osm_changeset") {
185+
builder->set_changeset(val.c_str());
186+
} else if (key == "osm_uid") {
187+
builder->set_uid(val.c_str());
188+
} else if (key == "osm_user") {
189+
user = val;
190+
}
191+
192+
// String points to the comma or closing '}' */
193+
if (*string == ',') {
194+
++string;
195+
}
196+
}
197+
198+
// Must be done at the end, because after that call the builder might
199+
// become invalid due to buffer resizing.
200+
builder->set_user(user);
201+
}
202+
203+
/**
204+
* Go through tags returned from a middle table, get all tags except the
205+
* pseudo-tags containing attributes and add them to the builder.
206+
*/
159207
template <typename T>
160208
void pgsql_parse_tags(char const *string, osmium::memory::Buffer *buffer,
161209
T *obuilder)
@@ -173,7 +221,12 @@ void pgsql_parse_tags(char const *string, osmium::memory::Buffer *buffer,
173221
// String points to the comma */
174222
++string;
175223
string = decode_to_delimiter(string, &val);
176-
builder.add_tag(key, val);
224+
225+
if (key != "osm_version" && key != "osm_timestamp" &&
226+
key != "osm_changeset" && key != "osm_uid" && key != "osm_user") {
227+
builder.add_tag(key, val);
228+
}
229+
177230
// String points to the comma or closing '}' */
178231
if (*string == ',') {
179232
++string;
@@ -443,6 +496,20 @@ void middle_pgsql_t::way_set(osmium::Way const &way)
443496
m_db_copy.finish_line();
444497
}
445498

499+
/**
500+
* Build way in buffer from database results.
501+
*/
502+
static void build_way(osmid_t id, pg_result_t const &res, int res_num,
503+
int offset, osmium::memory::Buffer *buffer)
504+
{
505+
osmium::builder::WayBuilder builder{*buffer};
506+
builder.set_id(id);
507+
508+
pgsql_get_attr_from_tags(res.get_value(res_num, offset + 1), &builder);
509+
pgsql_parse_nodes(res.get_value(res_num, offset + 0), buffer, &builder);
510+
pgsql_parse_tags(res.get_value(res_num, offset + 1), buffer, &builder);
511+
}
512+
446513
bool middle_query_pgsql_t::way_get(osmid_t id,
447514
osmium::memory::Buffer *buffer) const
448515
{
@@ -454,13 +521,7 @@ bool middle_query_pgsql_t::way_get(osmid_t id,
454521
return false;
455522
}
456523

457-
{
458-
osmium::builder::WayBuilder builder{*buffer};
459-
builder.set_id(id);
460-
461-
pgsql_parse_nodes(res.get_value(0, 0), buffer, &builder);
462-
pgsql_parse_tags(res.get_value(0, 1), buffer, &builder);
463-
}
524+
build_way(id, res, 0, 0, buffer);
464525

465526
buffer->commit();
466527

@@ -506,12 +567,7 @@ middle_query_pgsql_t::rel_members_get(osmium::Relation const &rel,
506567
// back to the list of ways given by the caller
507568
for (int j = 0; j < res.num_tuples(); ++j) {
508569
if (member.ref() == wayidspg[static_cast<std::size_t>(j)]) {
509-
osmium::builder::WayBuilder builder{*buffer};
510-
builder.set_id(member.ref());
511-
512-
pgsql_parse_nodes(res.get_value(j, 1), buffer, &builder);
513-
pgsql_parse_tags(res.get_value(j, 2), buffer, &builder);
514-
570+
build_way(member.ref(), res, j, 1, buffer);
515571
++members_found;
516572
break;
517573
}
@@ -588,6 +644,7 @@ bool middle_query_pgsql_t::relation_get(osmid_t id,
588644
osmium::builder::RelationBuilder builder{*buffer};
589645
builder.set_id(id);
590646

647+
pgsql_get_attr_from_tags(res.get_value(0, 1), &builder);
591648
pgsql_parse_members(res.get_value(0, 0), buffer, &builder);
592649
pgsql_parse_tags(res.get_value(0, 1), buffer, &builder);
593650
}

‎src/output-flex.cpp‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -153,63 +153,19 @@ static void push_osm_object_to_lua_stack(lua_State *lua_state,
153153
if (with_attributes) {
154154
if (object.version() != 0U) {
155155
luaX_add_table_int(lua_state, "version", object.version());
156-
} else {
157-
// This is a workaround, because the middle will give us the
158-
// attributes as pseudo-tags.
159-
char const *const val = object.tags()["osm_version"];
160-
if (val) {
161-
luaX_add_table_int(lua_state, "version",
162-
osmium::string_to_object_version(val));
163-
}
164156
}
165-
166157
if (object.timestamp().valid()) {
167158
luaX_add_table_int(lua_state, "timestamp",
168159
object.timestamp().seconds_since_epoch());
169-
} else {
170-
// This is a workaround, because the middle will give us the
171-
// attributes as pseudo-tags.
172-
char const *const val = object.tags()["osm_timestamp"];
173-
if (val) {
174-
auto const timestamp = osmium::Timestamp{val};
175-
luaX_add_table_int(lua_state, "timestamp",
176-
timestamp.seconds_since_epoch());
177-
}
178160
}
179-
180161
if (object.changeset() != 0U) {
181162
luaX_add_table_int(lua_state, "changeset", object.changeset());
182-
} else {
183-
char const *const val = object.tags()["osm_changeset"];
184-
// This is a workaround, because the middle will give us the
185-
// attributes as pseudo-tags.
186-
if (val) {
187-
luaX_add_table_int(lua_state, "changeset",
188-
osmium::string_to_changeset_id(val));
189-
}
190163
}
191-
192164
if (object.uid() != 0U) {
193165
luaX_add_table_int(lua_state, "uid", object.uid());
194-
} else {
195-
// This is a workaround, because the middle will give us the
196-
// attributes as pseudo-tags.
197-
char const *const val = object.tags()["osm_uid"];
198-
if (val) {
199-
luaX_add_table_int(lua_state, "uid",
200-
osmium::string_to_uid(val));
201-
}
202166
}
203-
204167
if (object.user()[0] != '\0') {
205168
luaX_add_table_str(lua_state, "user", object.user());
206-
} else {
207-
// This is a workaround, because the middle will give us the
208-
// attributes as pseudo-tags.
209-
char const *const val = object.tags()["osm_user"];
210-
if (val) {
211-
luaX_add_table_str(lua_state, "user", val);
212-
}
213169
}
214170
}
215171

‎tests/bdd/flex/extra-attributes.feature‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
Feature: Tests for including extra attributes
22

33
Background:
4-
Given the grid
5-
| 11 | 12 |
6-
| 10 | |
7-
And the OSM data
8-
"""
9-
w20 v1 dV c31 t2020-01-12T12:34:56Z i17 utest Thighway=primary Nn10,n11,n12
10-
"""
11-
And the lua style
4+
Given the lua style
125
"""
136
local attr_table = osm2pgsql.define_table{
147
name = 'osm2pgsql_test_attr',
@@ -32,12 +25,19 @@ Feature: Tests for including extra attributes
3225
"""
3326

3427
Scenario: Importing data without extra attributes
28+
Given the grid
29+
| 11 | 12 |
30+
| 10 | |
31+
And the OSM data
32+
"""
33+
w20 v1 dV c31 t2020-01-12T12:34:56Z i17 utest Thighway=primary Nn10,n11,n12
34+
"""
3535
When running osm2pgsql flex with parameters
3636
| --slim |
3737

3838
Then table osm2pgsql_test_attr contains
39-
| type | way_id | tags->'highway' | version | changeset | timestamp | uid | "user" |
40-
| way | 20 | primary | NULL | NULL | NULL | NULL | NULL |
39+
| type | way_id | tags->'highway' | tags->'osm_version' | version | changeset | timestamp | uid | "user" |
40+
| way | 20 | primary | NULL | NULL | NULL | NULL | NULL | NULL |
4141

4242
Given the grid
4343
| | |
@@ -46,17 +46,24 @@ Feature: Tests for including extra attributes
4646
| --slim | --append |
4747

4848
Then table osm2pgsql_test_attr contains
49-
| type | way_id | tags->'highway' | version | changeset | timestamp | uid | "user" |
50-
| way | 20 | primary | NULL | NULL | NULL | NULL | NULL |
49+
| type | way_id | tags->'highway' | tags->'osm_version' | version | changeset | timestamp | uid | "user" |
50+
| way | 20 | primary | NULL | NULL | NULL | NULL | NULL | NULL |
5151

5252

5353
Scenario: Importing data with extra attributes
54+
Given the grid
55+
| 11 | 12 |
56+
| 10 | |
57+
And the OSM data
58+
"""
59+
w20 v1 dV c31 t2020-01-12T12:34:56Z i17 utest Thighway=primary Nn10,n11,n12
60+
"""
5461
When running osm2pgsql flex with parameters
5562
| --slim | -x |
5663

5764
Then table osm2pgsql_test_attr contains
58-
| type | way_id | tags->'highway' | version | changeset | timestamp | uid | "user" |
59-
| way | 20 | primary | 1 | 31 | 1578832496 | 17 | test |
65+
| type | way_id | tags->'highway' | tags->'osm_version' | version | changeset | timestamp | uid | "user" |
66+
| way | 20 | primary | NULL | 1 | 31 | 1578832496 | 17 | test |
6067

6168
Given the grid
6269
| | |
@@ -65,6 +72,6 @@ Feature: Tests for including extra attributes
6572
| --slim | --append | -x |
6673

6774
Then table osm2pgsql_test_attr contains
68-
| type | way_id | tags->'highway' | version | changeset | timestamp | uid | "user" |
69-
| way | 20 | primary | 1 | 31 | 1578832496 | 17 | test |
75+
| type | way_id | tags->'highway' | tags->'osm_version' | version | changeset | timestamp | uid | "user" |
76+
| way | 20 | primary | NULL | 1 | 31 | 1578832496 | 17 | test |
7077

‎tests/bdd/regression/extra-attributes.feature‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
Feature: Tests for including extra attributes
2+
3+
Scenario: Importing data without extra attributes
4+
Given the grid
5+
| 11 | 12 |
6+
| 10 | |
7+
And the OSM data
8+
"""
9+
w20 v1 dV c31 t2020-01-12T12:34:56Z i17 utest Thighway=primary Nn10,n11,n12
10+
"""
11+
When running osm2pgsql pgsql with parameters
12+
| --slim | -j |
13+
14+
Then table planet_osm_roads contains
15+
| osm_id | tags->'highway' | tags->'osm_version' | tags->'osm_changeset' |
16+
| 20 | primary | NULL | NULL |
17+
18+
Given the grid
19+
| | |
20+
| | 10 |
21+
When running osm2pgsql pgsql with parameters
22+
| --slim | -j | --append |
23+
24+
Then table planet_osm_roads contains
25+
| osm_id | tags->'highway' | tags->'osm_version' | tags->'osm_changeset' |
26+
| 20 | primary | NULL | NULL |
27+
28+
29+
Scenario: Importing data with extra attributes
30+
Given the grid
31+
| 11 | 12 |
32+
| 10 | |
33+
And the OSM data
34+
"""
35+
w20 v1 dV c31 t2020-01-12T12:34:56Z i17 utest Thighway=primary Nn10,n11,n12
36+
"""
37+
When running osm2pgsql pgsql with parameters
38+
| --slim | -j | -x |
39+
40+
Then table planet_osm_roads contains
41+
| osm_id | tags->'highway' | tags->'osm_version' | tags->'osm_changeset' |
42+
| 20 | primary | 1 | 31 |
43+
44+
Given the grid
45+
| | |
46+
| | 10 |
47+
When running osm2pgsql pgsql with parameters
48+
| --slim | -j | --append | -x |
49+
50+
Then table planet_osm_roads contains
51+
| osm_id | tags->'highway' | tags->'osm_version' | tags->'osm_changeset' |
52+
| 20 | primary | 1 | 31 |
53+

0 commit comments

Comments
 (0)