Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#1100 closed defect (fixed)

QuantumGIS no like pretty names

Reported by: robe Owned by: pramsey
Priority: blocker Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

Okay as I feared, we'll have issues with these pretty names.

Loaded up my new pretty database and told my 1.6 QGIS to just look in geometry_columns. Showed the list with types in upper case.

When I went to select a table to build a query it said:

tiger.faces has a geometry type, MultiPolygon which Quantum GIS does not currently support.

Pops up the same message for both constraint and typmod defined tables.

FWIW Paul — I setup the structure so I only need to change the geometry_columns view/postgis_constraint_type functions back to the old uppercase names. So instead of prettifying constraint defined types, I can just uglify the typmod based geometry column types.

The postgis_type_name function is indifferent which name it is given and will return which type name you choose based on the use_new_name optional arg.

Alternatively we could just have QGIS fix, but given people's squimishness with upgrades on client software and also that we have much less control over other third party controls, I'd recommend swithcing back to old names for geometry_columns.

Attachments (1)

UpperPostgis.patch (627 bytes ) - added by strk 13 years ago.
patch for the 1.7 branch of qgis

Download all attachments as: .zip

Change History (13)

comment:1 by strk, 13 years ago

Looking at qgis source code it also appears to be using 'geometrytype', which is likely gone in 2.0 isn't it ?

The code is in src/providers/postgres/qgspostgresprovider.cpp

Tickets for qgis should be filed here: http://hub.qgis.org/projects/quantum-gis

It should use the same authentication means as this trac, so you should already have an account.

comment:2 by robe, 13 years ago

Nope geometrytype is staying. It's not deprecated because it returns something different from ST_GeometryType. So doesn't follow the deprecation rule any func that does the same as ST_func.

comment:3 by robe, 13 years ago

Hey strk, when PostgreSQL finally comes out with synonyms like Oracle and SQL Server have, we can bring back all those functions to save you typing :)

I just don't want them cluttering my function space. They can clutter the synonym space all they want.

comment:4 by strk, 13 years ago

Qgis 1.7 seems to be using geometrytype too. Have you tried that version ?

comment:5 by robe, 13 years ago

It should use GeometryType. We use GeometryType in a bunch of our functions. To reiterate — GeometryType is not deprecated or going away. ST_GeometryType I never use because who wants to go around putting ST_ in front of all your geometry types? That's just dumb. Unfortunately that one is mandated by SQL/MM so can't get rid of it.

comment:6 by robe, 13 years ago

I haven't had a chance to try QGIS 1.7 — was about to install when got side tracked.

comment:7 by robe, 13 years ago

okay tried QGIS 1.7. Same issue doesn't like the casing when i choose to only look in geometry_columns. Though works fine if I change my view. Like I said, I'm not so worried about QGIS as I am worried about other tools or people who have a lot of clients using QGIS and its a pain to get them to upgrade to a newer.

So I still go with the - We should upper case things at the very least. Paul — would that work for you?

You can keep your PointZ, PointZM — but in geometry_columns it has to be uppercase POINTZ, POINTZM. 3D support is fairly new in most apps anyway — so not too big of a deal to get them used to POINTZ, POINTZM vs. POINT

comment:8 by strk, 13 years ago

As incredible as it may seem:

19:03 < strk> jef: geometry_columns is being changed to a view. current trunk 
              can't be seen by qgis 1.7 due to geometry_columns.type bein 
              MixedCase
19:03 < strk> we're discussingi about reverting such change, but might be a 
              good idea to use upper(type) for geometry_columns like it's done 
              with geography_columns
19:03 < strk> I've a quick patch, if you want to test it
19:03 -!- GitHub100 [~GitHub100@sh1-ext.rs.github.com] has joined #qgis
19:03 < GitHub100> [Quantum-GIS] jef-n pushed 3 new commits to master: 
                   536:http://bit.ly/mPr4h5
19:03 < GitHub100> [Quantum-GIS/master] add support for mixed case geometry 
                   types of PostGIS 2.0 - Juergen E. Fischer

by strk, 13 years ago

Attachment: UpperPostgis.patch added

patch for the 1.7 branch of qgis

comment:9 by strk, 13 years ago

So, qgis trunk is patched (altought probably too much, in that also the output of geometrytype is being passed to upper). FOr 1.7 branch we should likely wait some more, I attach a patch here.

See also the openjump casein #1106 (seems to be a different beast)

comment:10 by robe, 13 years ago

Resolution: fixed
Status: newclosed

at r7621 - Okay made geometry_columns.type fields uppercase and stripped off the Z and ZM flags at end so now its fully backward compatible except not being able to update.

I can now see my tiger tables in quantum, but my zoom to extent still doesn't seem to work unless I do a build filter first. I'm not absolutely sure that was working before — could be my dataset.

comment:11 by strk, 13 years ago

About "not bein able to update" I think it'd be nice to have a rule on INSERT or UPDATE raising a WARNING. It'd be helpful to find client code still not being updated.

comment:12 by robe, 13 years ago

I don't think I can raise WARNINGs in SQL. I could be wrong since I've never tried.

Of course in 9.1 we can do that since we can put real triggers behind views :)

Note: See TracTickets for help on using tickets.