Add support for geography columns

[rshk]

This addresses #1503

Tests seem to pass, at least as far as detecting the table is concerned.
I feel like this could use some more tests to test the actual tile generation, I could probably use some pointers on the best way to go about that.
I'm fairly new to Rust (and this project), so please be kind 😛

It seems to be doing a good job creating tiles for a few NOAA layers imported in a postgis database and rendered in maplibre:

[rshk]

As a side note, I'm getting this error when running git push:

% just clippy
cargo clippy --workspace --all-targets -- -D warnings
    Checking martin v0.15.0 (/home/samu/Projects/martin/martin)
error: unknown lint: `clippy::ref_option`
  --> martin/src/pg/utils.rs:16:10
   |
16 | #[expect(clippy::ref_option)]
   |          ^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::ref_option_ref`
   |
   = note: `-D unknown-lints` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(unknown_lints)]`

error: could not compile `martin` (lib test) due to 1 previous error
error: Recipe `clippy` failed on line 319 with exit code 101
Exit status: 101                                                                           

[CommanderStorm]

I'm getting this error

This is because your rust version is a bit too old for this lint.
Consider updating with rustup update

We had to allow said lint because otherwise CI failed..

[rshk]

Nice, that did the trick, thanks.

[nyurik]

btw, we may want to disable pre commit hooks - it seems it is more of a nuisance than help

[nyurik]

also, we could improve the just clippy target - if it fails, it can also print "make sure you have the latest clippy installed"

[nyurik]

Nice! One possible issue - while this approach works, and does not affect existing cases, I am not sure this is the right path for the geography types -- essentially you are converting all geographies to geometries, loosing some information. This may result in bugs reported by the geography-using people of incorrect visualizations... We may need some tests that are "geography-specific" (like polygons that cross the anti-meridian, or something else similarly non-trivial?).

[nyurik]

need?

[rshk]

That's how we could distinguish between tables using a geometry vs geography column.
I initially thought we might need it, but the approach of simply casting everything to ::geometry seems to be working well, so this can be removed unless we want to expose this information to the code for some other reason.

[nyurik]

is geography metadata here substantially different from geometry to require a separate section? not a biggie, and not a blocker, but if possible - we should have just one consolidated sub-table here

[rshk]

There are a few substantials differences here: geography columns are defined in the geography_columns table, and the name of the column containing the name of the spatial column is f_geography_column. I agree that the duplication here is not ideal, but I couldn't think of a better alternative to cover both use cases.

Perhaps one thing we could do is move the JOINs to the UNION'd table, at the cost of adding yet another "with query" block? Not sure which of the two would be more preferable.

[nyurik]

we usually prefer to have the same type of elements in a table - if you mix points and linestrings, what would be the type of the MVT data? Not a blocker, but for a test we may want a few more tables like this .... although perhaps one mixed table is also good to test the results

[rshk]

I copied this from the table_source test case, which is doing the same for geometries.
I agree that we can probably use a few more tests for the geography columns.
Any recommendations on the best approach for this? I could just copy/paste all the tests we have for geometries to run them on geographies as well, but that feels like a lot of duplication, so I was wondering if there could be a more clever way?

[rshk]

Nice! One possible issue - while this approach works, and does not affect existing cases, I am not sure this is the right path for the geography types -- essentially you are converting all geographies to geometries, loosing some information. This may result in bugs reported by the geography-using people of incorrect visualizations... We may need some tests that are "geography-specific" (like polygons that cross the anti-meridian, or something else similarly non-trivial?).

I don't think converting geographies to geometries would cause any issue: the main difference between geometry and geography column is how the two are treated internally by postgis, but from a user perspective the data returned in both cases should be equivalent. It's also going to be converted to carthesian (EPSG:3857) coordinates anyways, so it can be encoded in a vector tile.

I haven't tested geometries crossing the anti-meridian yet (going to do it now) but I assume they should work as well as their geometry counterparts. AFAIK it's the map renderer (maplibre) that needs to know how to draw the "shortest path" between two points; the vector tile only contains a list of points, but no other information about the path between them.

Not sure if there's a convenient way to write an automated test for that, or they can only be verified visually?
Maybe we can add a few geography objects to the demo, that can be used for verifying that.

The only place I think casting to geometry on the fly can negatively affect performance is the calc_bounds() function, since st_extent() is unlikely going to be able to use the index; #1220 seems to provide a good solution for that though, so this shouldn't be an issue once that is merged.

[nyurik]

i think this is ok to merge. I would still like a few more test cases, but that could be done separately