[gdal-dev] Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[gdal-dev] Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Even Rouault-2
Hi,

This is a call for discussion on revisiting the existing RFC 31 - OGR 64bit
Integer Fields and FIDs, initiated by Frank in 2010 and that I've extended.

http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64

Below the summary :
"""
This RFC addresses steps to upgrade OGR to support 64bit integer fields and
feature ids. Many feature data formats support wide integers, and the
inability to transform these through OGR causes increasing numbers of
problems.
"""

A first discussion took place at http://lists.osgeo.org/pipermail/gdal-
dev/2010-November/thread.html#26883. My main remarks at that time were about
compatibility issues, but as this would go in GDAL 2.0, they are likely less
critical.

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Dmitry Baryshnikov-2
Hi Even,

The RFC looks good. One note. May be in GDAL 2 we can use GetFID instead
GetFID64? Because we can allow some incompatibility between GDAL 1.x and
GDAL 2.x.

Best regards,
     Dmitry

23.01.2015 18:17, Even Rouault пишет:

> Hi,
>
> This is a call for discussion on revisiting the existing RFC 31 - OGR 64bit
> Integer Fields and FIDs, initiated by Frank in 2010 and that I've extended.
>
> http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64
>
> Below the summary :
> """
> This RFC addresses steps to upgrade OGR to support 64bit integer fields and
> feature ids. Many feature data formats support wide integers, and the
> inability to transform these through OGR causes increasing numbers of
> problems.
> """
>
> A first discussion took place at http://lists.osgeo.org/pipermail/gdal-
> dev/2010-November/thread.html#26883. My main remarks at that time were about
> compatibility issues, but as this would go in GDAL 2.0, they are likely less
> critical.
>
> Even
>

_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Even Rouault-2
In reply to this post by Even Rouault-2
If people need more time to review, tell me otherwise I'll call for a vote on
it tomorrow.

> Hi,
>
> This is a call for discussion on revisiting the existing RFC 31 - OGR 64bit
> Integer Fields and FIDs, initiated by Frank in 2010 and that I've extended.
>
> http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64
>
> Below the summary :
> """
> This RFC addresses steps to upgrade OGR to support 64bit integer fields and
> feature ids. Many feature data formats support wide integers, and the
> inability to transform these through OGR causes increasing numbers of
> problems.
> """
>
> A first discussion took place at http://lists.osgeo.org/pipermail/gdal-
> dev/2010-November/thread.html#26883. My main remarks at that time were
> about compatibility issues, but as this would go in GDAL 2.0, they are
> likely less critical.
>
> Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Daniel Morissette
In reply to this post by Dmitry Baryshnikov-2
Hi Even,

I like the RFC, and just like Dimitri I was wondering if instead of
GetFID64(), GetFeatureCount64(), etc, we could not simply keep the
current method names since 2.0 is the time to break the API anyway.

Also note that in the list of driver upgrades, you could add potential
enhancement to the support for MITAB "TAB Seamless Tables" which had a
potential FID overflow problem on very large tables with 32 bit ids. The
driver would have caught and reported an error in this case, but that
limitation can be solved with 64 bit ids if/when the code is upgraded to
use them. Look for TABSeamless::EncodeFeatureId() and related methods in
mitab_tabseamless.cpp.

Daniel


On 2015-01-24 8:07 AM, Dmitriy Baryshnikov wrote:

> Hi Even,
>
> The RFC looks good. One note. May be in GDAL 2 we can use GetFID instead
> GetFID64? Because we can allow some incompatibility between GDAL 1.x and
> GDAL 2.x.
>
> Best regards,
>      Dmitry
>
> 23.01.2015 18:17, Even Rouault пишет:
>> Hi,
>>
>> This is a call for discussion on revisiting the existing RFC 31 - OGR
>> 64bit
>> Integer Fields and FIDs, initiated by Frank in 2010 and that I've
>> extended.
>>
>> http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64
>>
>> Below the summary :
>> """
>> This RFC addresses steps to upgrade OGR to support 64bit integer
>> fields and
>> feature ids. Many feature data formats support wide integers, and the
>> inability to transform these through OGR causes increasing numbers of
>> problems.
>> """
>>
>> A first discussion took place at http://lists.osgeo.org/pipermail/gdal-
>> dev/2010-November/thread.html#26883. My main remarks at that time were
>> about
>> compatibility issues, but as this would go in GDAL 2.0, they are
>> likely less
>> critical.
>>
>> Even
>>
>
> _______________________________________________
> gdal-dev mailing list
> [hidden email]
> http://lists.osgeo.org/mailman/listinfo/gdal-dev


--
Daniel Morissette
T: +1 418-696-5056 #201
http://www.mapgears.com/
Provider of Professional MapServer Support since 2000
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Even Rouault-2
Le mardi 27 janvier 2015 18:17:44, Daniel Morissette a écrit :
> Hi Even,
>
> I like the RFC, and just like Dimitri I was wondering if instead of
> GetFID64(), GetFeatureCount64(), etc, we could not simply keep the
> current method names since 2.0 is the time to break the API anyway.

Hi Daniel,

Not sure if you saw my answer to Dmitriy as it ended up into a separate thread
due to dumb email agent. Reposting it inline

"""Dmitriy,

The main reason for the new symbol is that if we change the return type it
might cause crashing problems that are not detected at compile time if GetFID
is used in a printf like function. For example

printf("%ld %s", GetFID (), str)

Even
"""

I've taken that approach from Frank's initial draft and that seamed a valid
concern. That said, if you guys think that changing the return type is better,
I've no strong mind.

>
> Also note that in the list of driver upgrades, you could add potential
> enhancement to the support for MITAB "TAB Seamless Tables" which had a
> potential FID overflow problem on very large tables with 32 bit ids. The
> driver would have caught and reported an error in this case, but that
> limitation can be solved with 64 bit ids if/when the code is upgraded to
> use them. Look for TABSeamless::EncodeFeatureId() and related methods in
> mitab_tabseamless.cpp.

OK thanks for the hint, I'll have a look at this. Do you have samples datasets
that would trigger that issue ?

Even

>
> Daniel
>
> On 2015-01-24 8:07 AM, Dmitriy Baryshnikov wrote:
> > Hi Even,
> >
> > The RFC looks good. One note. May be in GDAL 2 we can use GetFID instead
> > GetFID64? Because we can allow some incompatibility between GDAL 1.x and
> > GDAL 2.x.
> >
> > Best regards,
> >
> >      Dmitry
> >
> > 23.01.2015 18:17, Even Rouault пишет:
> >> Hi,
> >>
> >> This is a call for discussion on revisiting the existing RFC 31 - OGR
> >> 64bit
> >> Integer Fields and FIDs, initiated by Frank in 2010 and that I've
> >> extended.
> >>
> >> http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64
> >>
> >> Below the summary :
> >> """
> >> This RFC addresses steps to upgrade OGR to support 64bit integer
> >> fields and
> >> feature ids. Many feature data formats support wide integers, and the
> >> inability to transform these through OGR causes increasing numbers of
> >> problems.
> >> """
> >>
> >> A first discussion took place at http://lists.osgeo.org/pipermail/gdal-
> >> dev/2010-November/thread.html#26883. My main remarks at that time were
> >> about
> >> compatibility issues, but as this would go in GDAL 2.0, they are
> >> likely less
> >> critical.
> >>
> >> Even
> >
> > _______________________________________________
> > gdal-dev mailing list
> > [hidden email]
> > http://lists.osgeo.org/mailman/listinfo/gdal-dev

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

jratike80
In reply to this post by Even Rouault-2
Even Rouault <even.rouault <at> spatialys.com> writes:

 
> Hi,
>
> This is a call for discussion on revisiting the existing RFC 31 - OGR 64bit
> Integer Fields and FIDs, initiated by Frank in 2010 and that I've extended.
>
> http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64


+1

Looks like a huge work that you have done with everything listed in
https://github.com/rouault/gdal2/compare/rfc31_64bit.


-Jukka Rahkonen-

_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Daniel Morissette
In reply to this post by Even Rouault-2
Hi Even,

Replies in-line below

On 2015-01-27 12:47 PM, Even Rouault wrote:
>
> Not sure if you saw my answer to Dmitriy as it ended up into a separate thread
> due to dumb email agent. Reposting it inline
>

No, I'm sorry it seems that I had missed it.


> """Dmitriy,
>
> The main reason for the new symbol is that if we change the return type it
> might cause crashing problems that are not detected at compile time if GetFID
> is used in a printf like function. For example
>
> printf("%ld %s", GetFID (), str)
>
> Even
> """
>

This may not be true on all platforms, but at least with GCC, we get
warnings in a case like this, e.g.


#include <stdio.h>
#include <stdint.h>

int main()
{
   int64_t id=10;

   printf ("%d %s\n", id, "test");
   return 0;
}


$ gcc -Wall ttt.c -o ttt
ttt.c: In function 'main':
ttt.c:8: warning: format '%ld' expects type 'long int', but argument 2
has type 'int64_t'



> I've taken that approach from Frank's initial draft and that seamed a valid
> concern. That said, if you guys think that changing the return type is better,
> I've no strong mind.
>

I don't feel that strongly about it. I was mostly wondering if there
would be a way to avoid the *64() suffixes.

>>
>> Also note that in the list of driver upgrades, you could add potential
>> enhancement to the support for MITAB "TAB Seamless Tables" which had a
>> potential FID overflow problem on very large tables with 32 bit ids. The
>> driver would have caught and reported an error in this case, but that
>> limitation can be solved with 64 bit ids if/when the code is upgraded to
>> use them. Look for TABSeamless::EncodeFeatureId() and related methods in
>> mitab_tabseamless.cpp.
>
> OK thanks for the hint, I'll have a look at this. Do you have samples datasets
> that would trigger that issue ?
>

No. I don't think I've ever seen or heard of a seamless table large
enough to trigger the issue.


--
Daniel Morissette
T: +1 418-696-5056 #201
http://www.mapgears.com/
Provider of Professional MapServer Support since 2000
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Even Rouault-2
Daniel,

>
> This may not be true on all platforms, but at least with GCC, we get
> warnings in a case like this, e.g.
>
>
> #include <stdio.h>
> #include <stdint.h>
>
> int main()
> {
>    int64_t id=10;
>
>    printf ("%d %s\n", id, "test");
>    return 0;
> }
>
>
> $ gcc -Wall ttt.c -o ttt
> ttt.c: In function 'main':
> ttt.c:8: warning: format '%ld' expects type 'long int', but argument 2
> has type 'int64_t'
>
> > I've taken that approach from Frank's initial draft and that seamed a
> > valid concern. That said, if you guys think that changing the return
> > type is better, I've no strong mind.
>
> I don't feel that strongly about it. I was mostly wondering if there
> would be a way to avoid the *64() suffixes.

Yes GCC nicely warns. I don't think MSVC does, but oh well people building on
Windows are used to self-inflicted pain ;-)

So I've renamed GetFID64() -> GetFID() and GetFeatureCount64() -> GetFeature()
in latest commit :
https://github.com/rouault/gdal2/commit/40eb13bb9e2a4c86df9d3cf4d030ab14d67749d5

>
> >> Also note that in the list of driver upgrades, you could add potential
> >> enhancement to the support for MITAB "TAB Seamless Tables" which had a
> >> potential FID overflow problem on very large tables with 32 bit ids. The
> >> driver would have caught and reported an error in this case, but that
> >> limitation can be solved with 64 bit ids if/when the code is upgraded to
> >> use them. Look for TABSeamless::EncodeFeatureId() and related methods in
> >> mitab_tabseamless.cpp.
> >

I've made changes in TABSeamless and now builds a 64 bit FID  with the upper
32bits being for the index table and the lower 32bits for the feature within
the table.
Looking at the code, I've manually forged a simple seamless table and added it
in autotest.
https://github.com/rouault/gdal2/commit/a922228eff8d0b07918db520de142154e8454f6a

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: Call for discusson on RFC 31 - OGR 64bit Integer Fields and FIDs

Daniel Morissette
Merci!

Daniel

On 2015-01-28 7:01 AM, Even Rouault wrote:

> Daniel,
>
>>
>> This may not be true on all platforms, but at least with GCC, we get
>> warnings in a case like this, e.g.
>>
>>
>> #include <stdio.h>
>> #include <stdint.h>
>>
>> int main()
>> {
>>     int64_t id=10;
>>
>>     printf ("%d %s\n", id, "test");
>>     return 0;
>> }
>>
>>
>> $ gcc -Wall ttt.c -o ttt
>> ttt.c: In function 'main':
>> ttt.c:8: warning: format '%ld' expects type 'long int', but argument 2
>> has type 'int64_t'
>>
>>> I've taken that approach from Frank's initial draft and that seamed a
>>> valid concern. That said, if you guys think that changing the return
>>> type is better, I've no strong mind.
>>
>> I don't feel that strongly about it. I was mostly wondering if there
>> would be a way to avoid the *64() suffixes.
>
> Yes GCC nicely warns. I don't think MSVC does, but oh well people building on
> Windows are used to self-inflicted pain ;-)
>
> So I've renamed GetFID64() -> GetFID() and GetFeatureCount64() -> GetFeature()
> in latest commit :
> https://github.com/rouault/gdal2/commit/40eb13bb9e2a4c86df9d3cf4d030ab14d67749d5
>
>>
>>>> Also note that in the list of driver upgrades, you could add potential
>>>> enhancement to the support for MITAB "TAB Seamless Tables" which had a
>>>> potential FID overflow problem on very large tables with 32 bit ids. The
>>>> driver would have caught and reported an error in this case, but that
>>>> limitation can be solved with 64 bit ids if/when the code is upgraded to
>>>> use them. Look for TABSeamless::EncodeFeatureId() and related methods in
>>>> mitab_tabseamless.cpp.
>>>
>
> I've made changes in TABSeamless and now builds a 64 bit FID  with the upper
> 32bits being for the index table and the lower 32bits for the feature within
> the table.
> Looking at the code, I've manually forged a simple seamless table and added it
> in autotest.
> https://github.com/rouault/gdal2/commit/a922228eff8d0b07918db520de142154e8454f6a
>


--
Daniel Morissette
T: +1 418-696-5056 #201
http://www.mapgears.com/
Provider of Professional MapServer Support since 2000
_______________________________________________
gdal-dev mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/gdal-dev