Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-21 Thread Even Rouault
On mardi 21 mars 2017 11:07:18 CET Mark Johnson wrote: > > That will *not* work for other formats that can have multiple geometry > > fields, like GML. > > Could you send me such a GML to test this? Sure you'll find attached such a .gml and the corresponding .xsd > Have you ever seen or have a

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-21 Thread Mark Johnson
> > That will *not* work for other formats that can have multiple geometry > fields, like GML. Could you send me such a GML to test this? Have you ever seen or have a GML with a duplicate layer-name? but it would probably be cleaner to have a dedicated URI parameter for the > geometry column

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-21 Thread Even Rouault
> To solve the issue of several geometry fields we also need a (optional) > > > parameter with the geometry column name. > > subLayers() will send the layer name in the 'table_name(field_name)' format > > It was only with gdal 1 that GDALDatasetGetLayerByName could not use that > format. > That

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-21 Thread Even Rouault
On mardi 21 mars 2017 01:04:44 CET Mark Johnson wrote: > > So if we want to address both we need the layer_id and layer_name. I'd say > > when you build a OGR URI (mostly in QgisApp::askUserForOGRSublayers()), > > then look at the uniqueness of the layer names. If there's layer name > > unicity,

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > So if we want to address both we need the layer_id and layer_name. I'd say > when you build a OGR URI (mostly in QgisApp::askUserForOGRSublayers()), > then look at the uniqueness of the layer names. If there's layer name > unicity, then use only the layer name. Otherwise fallback to uniquely

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Ok - I find this thread very confusing. Does this acknowledgement mean > > that you WILL remove these ifdefs from the PR? > > Yes, indeed. > >> Here's the thing: I *want* your work to land in QGIS. It's important > > stuff, and needs to be addressed. But in its current form it won't be > >

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Nyall Dawson
On 21 March 2017 at 09:23, Mark Johnson wrote: >> Yet I still see lots of code in that PR like: >> >> if defined(GDAL_VERSION_NUM) && GDAL_VERSION_NUM >= >> GDAL_COMPUTE_VERSION(2,0,0) >> >> That's what I'm referring to. All this conditional code needs removal >> (unless

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Yet I still see lots of code in that PR like: > > if defined(GDAL_VERSION_NUM) && GDAL_VERSION_NUM >= > GDAL_COMPUTE_VERSION(2,0,0) > > That's what I'm referring to. All this conditional code needs removal > (unless it's testing for something in GDAL > 2.0 ). Mosty in the static functions -

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Nyall Dawson
On 21 March 2017 at 09:03, Mark Johnson wrote: >> Great - looking forward to seeing you push this to the PR! It'll make >> the remaining changes much easier to review. > > > Thats the way it was done when submitted.. Yet I still see lots of code in that PR like: if

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Great - looking forward to seeing you push this to the PR! It'll make > the remaining changes much easier to review. Thats the way it was done when submitted.. Mark ___ Qgis-developer mailing list Qgis-developer@lists.osgeo.org List info:

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > The layer name can be stored in subLayers() for conveniency, since it has >> usually a short life. And for subLayers() / askUserForOGRSublayers() make >> the layer id what is used. > > subLayers is not used when read from a project, only when a new source is added. > >> Not sure to what you

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Nyall Dawson
On 21 March 2017 at 08:45, Mark Johnson wrote: >> >> 1. No extra code will be allowed in master to handle GDAL < 2.0. QGIS >> 3.0 has a hard GDAL >= 2.0 requirement, and no code will be accepted >> to work around this requirement. > > > The original code did many tasks

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > > 1. No extra code will be allowed in master to handle GDAL < 2.0. QGIS > 3.0 has a hard GDAL >= 2.0 requirement, and no code will be accepted > to work around this requirement. The original code did many tasks being coded multiple times - these common tasks are written once in a extra

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Even Rouault
> The proposed solution deals with problem in QgsOgrProvider::subLayers() > - searches for duplicate names > -- an extra parameter tells QgsOgrProvider if the given id or layername > should be used Perhaps we can avoid that extra parameter, and have the layer name prioritary over the layer id

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Isn't this about PR #3697 - which is opened against 2.18? If #3697 is > abandoned, can I close to clean up the PR queue? Since the problem is not resolved, no. The present QGIS 3.0 cannot table with more than geometry. - at that time there was no QGIS 3 I changed that to 3.0 this morning.

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
There are different issues you've raised and that we might want handle or > not: > > 1) make sure that a URI points to the same layer if the datasource evolves > (new layer) > > 2) be able to deal with the corner cases of layers with same name > > > > 1) involves that we cannot always rely on the

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Nyall Dawson
On 21 March 2017 at 08:13, Mark Johnson wrote: >> Please keep in mind that's there's some non-negotiables here: > > >> One way or another this matter needs to be resolved before an initial >> release of QGIS 3. > > > This topic is only about QGIS 3.0 only. Isn't this

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Please keep in mind that's there's some non-negotiables here: > One way or another this matter needs to be resolved before an initial > release of QGIS 3. This topic is only about QGIS 3.0 only. Mark ___ Qgis-developer mailing list

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Even Rouault
On lundi 20 mars 2017 22:39:20 CET Mark Johnson wrote: > > In the provider we could decide to allocate layer ids diffently than OGR . > > > >> Example if you have a dataset with > > > > - layer1 (geom1, geom2) > > > > - layer2 (geom) > > > >> Then you would map that to : > > -

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Nyall Dawson
On 21 March 2017 at 05:39, Mark Johnson wrote: >>> we only support GDAL >= 2.0 in master > and it is clear that if QGIS is called with a gdal 1.* version the > Application will crash with a Lookup error? Hi Mark! Please keep in mind that's there's some non-negotiables

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > Application will crash with a Lookup error? Yes. And that's perfectly fine. That's only an issue that can happen for us >> developers playing > > with different versions, and we know how to solve those issues. > > The proposed solution for this constellation for-sees - at application start:

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
> > In the provider we could decide to allocate layer ids diffently than OGR . > > >> Example if you have a dataset with > > - layer1 (geom1, geom2) > > - layer2 (geom) > > >> Then you would map that to : > > - qgis_ogr_provider_layer_id = 1: layer1, geom1 > > - qgis_ogr_provider_layer_id = 2:

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Even Rouault
On lundi 20 mars 2017 20:39:22 CET Mark Johnson wrote: > >> I see no use of OGR_F_GetGeomFieldRef() so it seems > > that you select among the various different geometry fields by relying on > the > "layer_name(geometry_field_name)" syntax > > That is caused by the fact that the initial list >

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
>> I see no use of OGR_F_GetGeomFieldRef() so it seems that you select among the various different geometry fields by relying on the "layer_name(geometry_field_name)" syntax That is caused by the fact that the initial list (QgsOgrProvider::subLayers()) returns only the

Re: [Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Even Rouault
On lundi 20 mars 2017 11:19:57 CET Mark Johnson wrote: > https://github.com/qgis/QGIS/pull/3697 Mark, Most of my comments in https://github.com/qgis/QGIS/pull/3697#issuecomment-257366410 and https://github.com/qgis/QGIS/pull/3697#issuecomment-257571410 still hold true and I'd really like see

[Qgis-developer] Corrections of QgsOgrProvider implementaion of GDAL 2.0

2017-03-20 Thread Mark Johnson
What is the status of the proper QgsOgrProvider implementaion of GDAL 2.0, as described at this offered solution: https://github.com/qgis/QGIS/pull/3697 And was first reported on the 31.03.2015: https://hub.qgis.org/issues/12479 One way or another this matter needs to be resolved before an