Leave Ordinates.None in PostGisWriter to allow detection

[roji]

PostGisWriter contains CheckOrdinates(), which looks at the given geometry object to check which ordinates it contains; this is done only if HandleOrdinates is set to None (i.e. the user hasn't explicitly requested specific ordinates).

However, the HandleOrdinates property ORs XY into the given value, so when the default constructor sets the property to None it gets translated to XY. This means that CheckOrdinates() never gets called.

This changes the HandleOrdinates property to only OR XY into the given value if it is different from None.

[airbreather]

Hmm. So what you're saying is that this line is actually setting the value to Ordinates.XY, which means that the public Write methods will ignore Z and M entirely, because this check will never call CheckOrdinates.

Subtle... if only we had tests that actually covered that logic 🤦‍♂️. That explains why #4 was needed: the entire method was dead code, so any bugs in there would be masked by the one that this fixes.

This is related to npgsql/npgsql#2255, right?

[roji]

Hmm. So what you're saying is that this line is actually setting the value to Ordinates.XY, which means that the public Write methods will ignore Z and M entirely, because this check will never call CheckOrdinates.

Not exactly... If you set HandleOrdinates to XYZ from the outside (after the constructor), it gets set correctly and the behavior is ok. The problem is that it's impossible to set HandleOrdinates to None, which should mean that the writer should simply infer the ordinates from the objects it is given (sending sometimes XY, sometimes XYZ based on the init).

This auto-detection, which seems to be the desirable default behavior, is implemented by CheckOrdinates(), which never gets called because it is impossible to set HandleOrdinates to None (since the property setter always adds XY).

[roji]

Re testing, Npgsql actually has a few tests which it executes against PostgreSQL. We definitely could have some tests in NetTopologySuite for basic stuff, although without actually connecting to PostgreSQL they won't go very far...

[roji]

I'll be happy to help write tests though.

[roji]

Please hold off reviewing or merging, I've opened a more general discussion on ordinates handling in #8.

[roji]

Superceded by #9