Opened 11 years ago

Closed 9 years ago

#286 closed task (fixed)

Change "no SRID" SRID to 0

Reported by: pramsey Owned by: pramsey
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc: rouault

Description

This breaking change will put is into synch with ISO SQL/MM.

Change History (17)

comment:1 Changed 9 years ago by pramsey

Here it is, r7912

comment:2 Changed 9 years ago by strk

Knon breakages so far seem to be all due to a mix of explicit and implicit unknown srid. One is in our testsuite (topology) and another I got a report for is in qgis.

If we threat all SRID<=0 all the same way there should be much less problems. To me it's fine to map all to -1 for backward compatibility, to 0 for sql/mm compatibility, to NULL for strk-mind-compatibility.

But we do need to threat them all the same, IMHO (translate in input to a common srid).

comment:3 Changed 9 years ago by strk

As of r7963 we do threat SRID<=0 all the same (as UNKNOWN).

Returning UNKNOWN srid as 0 (rather than -1) may still have backward compatibility issues with code checking values against a literal -1 to mean UNKNOWN. Note that some literal -1 values may also be in dataspace (geometry_columns.srid, topology.topology.srid).

comment:4 Changed 9 years ago by robe

I thought we hadn't decided to do this -- return 0. I'm happy but I thought everyone else was against this because of fear of backward compatibility issues

comment:5 Changed 9 years ago by strk

Yes, I know. I was just pointing out the known breackages (after the <=0 is unknown change). Seems time to start with the "which value is the official unknown value" discussion on list...

comment:6 Changed 9 years ago by rouault

Cc: rouault added

Hi,

I've tried to read discussions on postgis-devel and could not see if returning 0 for unknown SRID was definitely approved. I see that this has been implemented in SVN, but is there still a possibility for it to be reverted to -1 ?

I have a patch ready for OGR Postgres driver, mainly to avoid the warning that -1 is not the official value for unknown SRID. The autotest suite didn't show any sign of regression though.

Even

comment:7 Changed 9 years ago by strk

Even, I feel much more confident in this change to stay after r8366. In any case the idea for users is to _omit_ the SRID when unknown, rather than using a literal 0 or -1. If that's possible it's much better to do that. Please let us know if it's not possible in your case.

comment:8 Changed 9 years ago by rouault

When composing the HEX EWKB, the code of the OGR PG driver already omits the SRID if it is unknown. So thinks look good I think

comment:9 Changed 9 years ago by strk

I guess we still need a function to convert tables having enforce_srid=-1 constraints to call this done.

comment:10 Changed 9 years ago by robe

I can handle this one if you want. This will be a pre-dump script correct taht will search all teh psotgis geometry tables that have this constraint and drop them.

comment:11 Changed 9 years ago by strk

Well I wasn't really thinking about a pre-dump thing as the postgis_Restore.pl can also do that itself. I was more thinking about people periodically running the minor_upgrade against 2.0 since a long time and thus having some of those checks in place which they'd like to fix w/out a dump/postgis_restore round.

Beside, we'll have to decide on a policy. The current postgis_restore.pl doesn't drop the constraint but rather updates it to check for srid=0 rather than srid=-1. Is that wrong ?

comment:12 Changed 9 years ago by robe

Yes I would prefer it just drop it.

comment:13 Changed 9 years ago by robe

Okay so this function should be part of minor upgrade. Makes sense. I forgot people might have data in the db already that violates.

comment:14 Changed 9 years ago by strk

With r8467 the postgis_restore.pl also takes care of fixing SRID in topology.topology, taking it from geometry_columns, which is supposedly the most correct as it looks at actual typmod or constraints.

Question: if we do drop the constraints for SRID=0, what would geometry_columns contain for such typmod-less tables with no SRID constraint ? Also, is there a pointer to a mailing list discussion about this ? I'd like to know more about the rationale for NOT enforcing the unknown srid...

comment:15 Changed 9 years ago by robe

strk -- geometry_columns just puts in 0 for SRID if the SRID has a constraint of unknown or its unspecified. The main reason I don't want us to bother putting in a constraint is our system can't distinguish between unknown and a mixed bag of geometries. So why bother making the distinction in constraint and god forbid we need to change unknown later -- enforching unknown will just bite us. I think its more important to just stare people from using unknown unless they absolutely need to. Putting so much significance on it is not going to help that cause. It's all academic.

Getting back to distinction if you have a geometry defined like geometry(POLYGON)

Typmod doesn't care what you put in for srid as long as all items are polygons so why should constraint care? Its like varchar with no length qualifier -- it can be as long as you want (or what can be held)

comment:16 Changed 9 years ago by strk

Should "populate_geometry_columns" do the drop of enforce_srid_ checks enforcing -1 ?

comment:17 Changed 9 years ago by strk

Resolution: fixed
Status: newclosed

We're pretty done here. Other tasks (like convertion functions) will be worth a new ticket.

Note: See TracTickets for help on using tickets.