georust / geozero

Zero-Copy reading and writing of geospatial data.
Apache License 2.0
321 stars 30 forks source link

Ewkt support srid=0 #210

Closed kkk25641463 closed 1 month ago

kkk25641463 commented 2 months ago

I think it's nessary to support srid=0 with ewkt format. SRID of 0 doesn't technically exist, it just means no SRID or maintain format consistency

See also

michaelkirk commented 1 month ago

I'm not really familiar with this. Is it possible to provide a test that shows an example of the value of this change?

nyurik commented 1 month ago

hmm, technically this is correct - no SRID and SRID=0 are the same... I don't see a problem, but we do need a test case

michaelkirk commented 1 month ago

What I don't understand is, if they are the same, what's the problem with the existing approach that writes SRID=0?

nyurik commented 1 month ago

useless info? We don't want to have two different cases for identical state, so ideally srid=nnn should only be used for non-zero case?

kkk25641463 commented 1 month ago

Below is the test case. I don't know which test case can satisfy more scenarios.

PostGis:

SQL:  SELECT ST_ASEWKT('SRID=0;LINESTRING(0.75 0.75, -10 20)')
OUTPUT: LINESTRING(0.75 0.75,-10 20)

SnowFlake:

SQL: SELECT ST_ASEWKT('SRID=0;LINESTRING(0.75 0.75, -10 20)')
OUTPUT: SRID=0;LINESTRING(0.75 0.75,-10 20) 
nyurik commented 1 month ago

So I am a bit confused now - why do we want to explicitly differentiate between None and SRID=0? I would prefer to treat them the same and not print SRID=0 at all... Unless of course EWKT requires the SRID to always be set, in which case we must output None as SRID=0

nyurik commented 1 month ago

It seems EWKT supports all the standard WKT cases, e.g. SELECT ST_GeomFromEWKT('POINT(-71.064544 42.28787)'); works just fine - so it is really up to us. I guess for EWKT case we should do the EWKT-way of always specifying SRID:

        if self.first_header && self.dialect == WktDialect::Ewkt {
            self.first_header = false;
            self.out.write_all(format!("SRID={};", srid.unwrap_or_default()).as_bytes())?
        }
kkk25641463 commented 1 month ago

Understood, thank you for your analysis.