joto left a comment (osm2pgsql-dev/osm2pgsql#2503)
This looks very interesting. I'll need more time for looking into this, but
here are a few things I wondered about when quickly reading though the code:
* There are a lot of `if (m_binary_types) {` in the `db-copy-mgr.hpp` code.
Have you considered using two separate implementations for instance with
virtual classes or a templated class to keep the two apart? I am not sure it is
worth it to separate this out, but it might make the code cleaner. Using
virtual inheritance might be problematic due to the resulting overhead though.
* The byteswap code in `put_be()` and `put_be_at()` uses a loop which I am not
sure the compiler can see through and simplify. Maybe it does. There is some
byteswap code in protozero and in libosmium, both libraries we use anyway, but
not easy accessible. Medium term we probably want to switch to `std::byteswap`
but that is only available in C++23.
* There is a lot of casting going on before calling `put_be()` and then inside
`put_be()` also. Sometimes it seems too complicated. But I haven't looked at
all the details. It is hard to see what's going on due to all the casts, but a
lot of them are probably necessary. Still I wonder if this could be simplified
somehow?
* Do we need the `std::optional` of the `copy_field_type`? Can't we just have
an additional field type for that?
--
Reply to this email directly or view it on GitHub:
https://github.com/osm2pgsql-dev/osm2pgsql/pull/2503#issuecomment-5845787343
You are receiving this because you are subscribed to this thread.
Message ID: <osm2pgsql-dev/osm2pgsql/pull/2503/[email protected]>_______________________________________________
Tile-serving mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/tile-serving
