search

maplibre / martin

Blazing fast and lightweight PostGIS, MBtiles and PMtiles tile server, tile generation, and mbtiles tooling.
https://martin.maplibre.org
Apache License 2.0
2.29k stars 215 forks source link

id column not included in properties #466

Closed iferencik closed 1 year ago

iferencik commented 2 years ago

I have noticed following behavior with martin:

the data

geodata=> \d poverty 
                                            Table "zambia.poverty"
         Column          |            Type             | Collation | Nullable |            Default             
-------------------------+-----------------------------+-----------+----------+--------------------------------
 id                      | integer                     |           | not null | nextval('zp_id_seq'::regclass)
 country_name            | character varying(6)        |           |          | 
 province                | character varying(254)      |           |          | 
 district                | character varying(254)      |           |          | 
 constituency            | character varying(254)      |           |          | 
 poverty                 | double precision            |           |          | 
 absolute_number_of_poor | double precision            |           |          | 
 geom                    | geometry(MultiPolygon,3857) |           |          | 
Indexes:
    "zp_pkey" PRIMARY KEY, btree (id)
    "zp_geom_geom_idx" gist (geom)

the config

table_sources:
  zambia.poverty:
    id: zambia.poverty
    schema: zambia
    table: poverty
    bounds: [21.99937075303238, -18.07947347592123, 33.705705741754244, -8.224359676087436]
    srid: 3857
    geometry_column: geom
    id_column: id
    extent: 4096
    buffer: 64
    geometry_type: MULTIPOLYGON
    clip_geometry: true
    properties:
      id: int4
      poverty: float8
      district: varchar
      province: varchar
      constituency: varchar
      country_name: varchar
      absolute_number_of_poor: float8

the index.json image

the featureID and available props (first 10)


1 {'absolute_number_of_poor': 22101.0, 'district': 'Mpika', 'poverty': 0.82, 'province': 'Muchinga', 'constituency': 'Mfuwe', 'country_name': 'Zambia'}
2 {'absolute_number_of_poor': 36685.0, 'district': 'Kawambwa', 'poverty': 0.77, 'province': 'Luapula', 'constituency': 'Kawambwa', 'country_name': 'Zambia'}
4 {'absolute_number_of_poor': 53678.0, 'district': 'Solwezi', 'poverty': 0.62, 'province': 'North Western', 'constituency': 'Solwezi West', 'country_name': 'Zambia'}
5 {'absolute_number_of_poor': 175504.0, 'district': 'Kapiri-Mposhi', 'poverty': 0.68, 'province': 'Central', 'constituency': 'Kapiri Mposhi', 'country_name': 'Zambia'}
56 {'absolute_number_of_poor': 76280.0, 'district': 'Katete', 'poverty': 0.79, 'province': 'Eastern', 'constituency': 'Mkaika', 'country_name': 'Zambia'}
3 {'absolute_number_of_poor': 70049.0, 'district': 'Nyimba', 'poverty': 0.78, 'province': 'Eastern', 'constituency': 'Nyimba', 'country_name': 'Zambia'}
6 {'absolute_number_of_poor': 64100.0, 'district': 'Lufwanyama', 'poverty': 0.81, 'province': 'Copperbelt', 'constituency': 'Lufwanyama', 'country_name': 'Zambia'}
79 {'absolute_number_of_poor': 55507.0, 'district': 'Lusaka', 'poverty': 0.19, 'province': 'Lusaka', 'constituency': 'Matero', 'country_name': 'Zambia'}
80 {'absolute_number_of_poor': 73578.0, 'district': 'Lusaka', 'poverty': 0.2, 'province': 'Lusaka', 'constituency': 'Kanyama', 'country_name': 'Zambia'}
7 {'absolute_number_of_poor': 41139.0, 'district': 'Kalabo', 'poverty': 0.89, 'province': 'Western', 'constituency': 'Sikongo', 'country_name': 'Zambia'}
8 {'absolute_number_of_poor': 52477.0, 'district': 'Kafue', 'poverty': 0.43, 'province': 'Lusaka', 'constituency': 'Kafue', 'country_name': 'Zambia'}

as you can see the id is not in the list of props.

is this a feature or a bug?

thanks

nyurik commented 2 years ago

ID shouldn't be in the list of properties. MVT has a separate ID field, not part of the list of properties. See https://github.com/mapbox/vector-tile-spec/tree/master/2.1

iferencik commented 2 years ago

@nyurik

"A feature MAY contain an id field. If a feature has an id field, the value of the id SHOULD be unique among the features of the parent layer."

I was aware a feature can have an id

id_column: id

I was not aware that if I specify a column to provide the id for every feature, the very same column can not be included in the properties.

for what it is worth, pg_tileserv includes the id into the props:

1 {'iso3cd': 'IDN', 'isoadm': 'IDN', 'area_sqkm': 1902569.0, 'perimeter_km': 53606.18, 'country_name': 'Indonesia', 'continent': 'ASI', 'admin_type': 'State', 'id': 1}
2 {'iso3cd': 'AUS', 'isoadm': 'AUS', 'area_sqkm': 7717236.07, 'perimeter_km': 34569.05, 'country_name': 'Australia', 'continent': 'OCE', 'admin_type': 'State', 'id': 2}
3 {'iso3cd': 'COD', 'isoadm': 'COD', 'area_sqkm': 2346038.07, 'perimeter_km': 9347.76, 'country_name': 'Democratic Republic of the Congo', 'continent': 'AFR', 'admin_type': 'State', 'id': 3}
4 {'iso3cd': 'STP', 'isoadm': 'STP', 'area_sqkm': 1010.5, 'perimeter_km': 183.86, 'country_name': 'Sao Tome and Principe', 'continent': 'AFR', 'admin_type': 'State', 'id': 4}
5 {'iso3cd': 'LBY', 'isoadm': 'LBY', 'area_sqkm': 1620580.25, 'perimeter_km': 6799.54, 'country_name': 'Libya', 'continent': 'AFR', 'admin_type': 'State', 'id': 5}
6 {'iso3cd': 'UGA', 'isoadm': 'UGA', 'area_sqkm': 243249.82, 'perimeter_km': 2355.99, 'country_name': 'Uganda', 'continent': 'AFR', 'admin_type': 'State', 'id': 6}
7 {'iso3cd': 'MNG', 'isoadm': 'MNG', 'area_sqkm': 1563435.45, 'perimeter_km': 10614.46, 'country_name': 'Mongolia', 'continent': 'ASI', 'admin_type': 'State', 'id': 7}
8 {'iso3cd': 'RUS', 'isoadm': 'RUS', 'area_sqkm': 16945856.07, 'perimeter_km': 241245.57, 'country_name': 'Russian Federation', 'continent': 'EUR', 'admin_type': 'State', 'id': 8}
9 {'iso3cd': 'PLW', 'isoadm': 'PLW', 'area_sqkm': 468.88, 'perimeter_km': 291.7, 'country_name': 'Palau', 'continent': 'OCE', 'admin_type': 'State', 'id': 9}
10 {'iso3cd': 'SOM', 'isoadm': 'SOM', 'area_sqkm': 637467.92, 'perimeter_km': 5533.02, 'country_name': 'Somalia', 'continent': 'AFR', 'admin_type': 'State', 'id': 10}
11 {'iso3cd': 'ECU', 'isoadm': 'ECU', 'area_sqkm': 258451.4, 'perimeter_km': 4075.07, 'country_name': 'Ecuador', 'continent': 'AME', 'admin_type': 'State', 'id': 11}

thanks for clarifying this

iferencik commented 2 years ago

@nyurik i removed the id column from props list in the cfg and it does not work

[2022-10-27T21:23:08Z INFO  martin] Starting Martin v0.6.0-rc.1
[2022-10-27T21:23:08Z INFO  martin] Using /etc/martin/config.yaml
[2022-10-27T21:23:08Z INFO  martin::pg::db] Connecting to database
[2022-10-27T21:23:08Z INFO  martin::pg::db] Loaded "admin.admin0" table source with "geom" column (MULTIPOLYGON, SRID=3857)
[2022-10-27T21:23:08Z INFO  martin::pg::db] Loaded "zambia.poverty" table source with "geom" column (MULTIPOLYGON, SRID=3857)
[2022-10-27T21:23:08Z INFO  martin] Martin has been started on 0.0.0.0:3000.
[2022-10-27T21:24:08Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:09Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:18Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:18Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:18Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:18Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:19Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:19Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:19Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:19Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:24:23Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:25:16Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:25:16Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:25:16Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type
[2022-10-27T21:25:16Z ERROR martin::srv::server] Can't get composite source tile: db error: ERROR: mvt_agg_transfn: Could not find column 'id' of integer type

if i put it back, it works, but then the index.json advertises the layer has id col in it's props while in reality it does not

So i think there is something fishy here, i do not think it is correct to advertise a property in the index.json and that property then does not exist in the props.

This is important for me as I use the index.json to prepare UI (what props a layer has, type of those props) and the I can see there is a prop "id" in the list and when i select it the layer becomes all black (maplibre)

To me this looks like a bug. What is you take on that?

nyurik commented 2 years ago

Thx for all the research on this! So at the end of the day, Martin calls ST_AsMVT, which has a feature_id_name parameter:

feature_id_name is the name of the Feature ID column in the row data. If NULL or negative the Feature ID is not set. The first column matching name and valid type (smallint, integer, bigint) will be used as Feature ID, and any subsequent column will be added as a property. JSON properties are not supported.

In other words, my understanding is that given a set of columns, if any of them match the feature_id_name, that column is only used for the ID, but not included in the properties. If that same column must be duplicated as a property too, the query must output two columns with the same content - possibly with the same column name.

nyurik commented 2 years ago

P.S. Note that this has nothing to do with how the catalog is being generated (all those *.json requests). There might be a bug in that format, and I am all for including a fix for that in the upcoming release.

nyurik commented 2 years ago

Ok, so after some research, it seems there is an issue with Martin's configuration and output. Let's start with this config file (using the test database):

---
connection_string: 'postgres://postgres@localhost:5432/db'
danger_accept_invalid_certs: false
default_srid: 4326
keep_alive: 75
listen_addresses: 0.0.0.0:3000
pool_size: 20
worker_processes: 8
table_sources:
  public.points1:
    id: public.points1
    schema: public
    table: points1
    minzoom: 0
    maxzoom: 30
    bounds: [-180.0, -90.0, 180.0, 90.0]
    id_column: gid
    geometry_column: geom
    srid: 4326
    extent: 4096
    buffer: 64
    clip_geom: true
    geometry_type: GEOMETRY
    properties:
      gid: int4

Note that it has both id_column: gid and properties: { gid: int4 } are set. Martin won't work without both being present, but this a bug -- it should be possible to have one without the other. This command shows features like this one:

curl localhost:3000/public.points1/0/0/0.pbf | ./tests/vtzero-show /dev/stdin
...
  feature: 8086
    id: 9536
    geomtype: point
    geometry:
      POINT(3618,883)
    properties:

Note the empty properties and the presence of the id field. It is not possible to remove the gid property in the config - Martin shows an error.

Proposed fix

iferencik commented 2 years ago

@nyurik

thanks so much for looking into this. Also thanks to Jin for the following up with you on this as well.

Your solutions is correct and straightforward and me or Jin might look into it. But I am am reluctant to promise this because I have never coded in rust and I do not wish to introduce more bugs.

You are also right about the wastefulness of including the FID in the props, but I think there could be some edge cases where this might be desirable and your solution supports that.

nyurik commented 2 years ago

@iferencik don't worry about bugs - that's the whole idea of Rust -- it won't let bugs in, and I will review it as well :)

nyurik commented 1 year ago

@iferencik and @JinIgarashi the big sources rewrite has been merged to main - feel free to submit a pull request for a fix :)

nyurik commented 1 year ago

This is being fixed as part of #380 -- already works, but hasn't been merged yet

JinIgarashi commented 1 year ago

Thanks for fixing this issue!