Conversation
|
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:
|
|
Thank you for the review!
|
|
I apologize I have to correct my above comment, the paragraph beginning Long explanation (click to expand)The exact scenario, where this PR causes a different result than master, is when a float64 is sent to Postgres and its mantissa takes the following form:
That's the implicit leading 1 bit, then 23 irrelevant bits, then a one, then 28 zeros. The odds of this happening to a uniformly random bits would be 1 in 2^29, which is why I (approximately) claimed as such above. I also "verified" it by ensuring that the fp64->text and text->fp32 was consistent across all polygons in the planet: Note that I can't check it on Unfortunately, that was misleading, because PostGIS computes areas differently to Lua (which uses boost). That st_area has the following mantissa bits: But, for this particular way, Lua gives us Note exactly 28 trailing zeros indeed. And, oh no, it does indeed round differently, we ended up precisely on the boundary between two float32s, we get The explanation here is that we have way_area as Due to this loss of precision in Boost, this rare case of 28 trailing zeros happens to about 1 in 2,000 ways (rather than 1 in 500,000,000 ways as in theory / as in PostGIS). Here's a histogram of how many trailing bits of the mantissa are zero, across all polygons in Delaware.
The highlighted column alone is what has the difference in rounding (any more or fewer trailing zeros rounds equally). And even in that column it's not certain the rounding is different, it depends on further details on the float64 to string converter. Bottom line: ~0.001% of polygons have their way_area change by 1/2^24 (0.00000596%) To fix this and exactly match master, we could check for this case on every fp64 we send to postgres, and fall back to |
|
Nobody should have code that depends on tiny changes in float values, that shouldn't be a problem. In practice the only typical use of floats I am aware of is for polygon areas and there it is only used to not render very small ones or something like that. Not worth thinking about the details. |

Fixes #2286
This PR switches the Postgres COPY for the middle tables and most flex tables from the textual format to the binary format. This is a moderate win in decreasing CPU time that is otherwise spent on int-to-text-to-int, float-to-text-to-float, WKB hex, etc.
Details:
db_target_descr_tknows the binary versions of its column types, if presentdb_copy_mgr_twill seamlessly switch to binary format for the same calls (add_column,new_array, etc)int8[]is supported, to make this possible.Changes to behavior:
real, currently text mode implicitly does fp64 -> text -> fp32. This causes rounding to happen twice. Binary mode will now round fp64 -> fp32 directly. Of course, one could reimplement the double rounding behavior, but I don't think this is good behavior to preserve. The effect is at most 1 ULP of difference, and it only happens when a fp64 landed exactly halfway between two fp32 values, which is extremely unlikely. A potential place where this could theoretically arise is inway_area- but in practice, there are none, in the full current planet, that have any difference here, and there probably never will be, as less than 1 in a billion doubles are like this. Out of range values like inf remain an error as in text mode.Testing:
test-db-copy-mgr.cpp--slim -xand then updated with an .osc. (this is what exposed Fix reading attributes of nodes from the middle #2501). All table contents identical to master.Performance:
--slim--slim --drop--slimSo as we can see Postgres spends about 14% less CPU to ingest. The overall win to wall clock time is currently not so much because we are bottlenecked by the single threaded Lua processing time. The benefit from this PR is larger when combined with upcoming PRs... (teaser post credit scene 😺) (EDIT: this was teasing #2504)