[OSGeo-Standards] Comments about OGC 12-128r1 Version: 0.2.0 -OpenGIS(R) GeoPackage Implementation Specification

Sat Jan 12 14:52:05 PST 2013

Thanks, Even -

Much appreciated

Carl Reed

-----Original Message----- 
From: Even Rouault
Sent: Saturday, January 12, 2013 1:31 PM
To: requests at lists.opengeospatial.org ; standards at lists.osgeo.org
Subject: [OSGeo-Standards] Comments about OGC 12-128r1 Version: 
0.2.0 -OpenGIS(R) GeoPackage Implementation Specification

PART A

1. Evaluator:
    Even Rouault, even.rouault at mines-paris dot org
    GDAL/OGR contributor

2. Submission:
    OGC 12-128r1 Version: 0.2.0 - OpenGIS® GeoPackage Implementation
    Specification

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    General

3. Criticality:
    Minor

4. Comments/justifications for changes:

    It could perhaps be made more clear in the preface/introduction/scope 
that
the  specification contain 2 different things :
        - storage requirements that make possible the exchange of data
        - and various SQL operations that can be used on the data stored in 
a
geopackage, but are optionnal to just read or write the data.

    The Spatialite reference implementation, or any other implementation 
with
equivalent SQL functions, is in no way necesserary to be able to use the
vector information in a GeoPackage. What you need is to support the 
Spatialite
geometry blob format. This is for example done in the GDAL/OGR library.

    Linked to that, I'm uneasy with the term "container" as it is used in 
the
document. It makes sense  in REQ 1 to define it as a file, but later
requirements mention properties that cannot be supported by a container in
itself, but rather by a software implementation that provides capabilities.
For example REQ 4 "The GeoPackage container shall provide a “C” language 
CLI":
a .geopackage file doesn't provide a C language CLI, it is libsqlite3 that
provides that. Perhaps different terms could be used to separate the format
specification, and the services that a software implementation handling
.geopackage should offer.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "7 - Table diagram"

3. Criticality:
    Editorial

4. Comments/justifications for changes:

    The max_y field of table geopackage_contents should be of type REAL, not
BLOB.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 9

2. Implementation Specification Section number:
    Paragraph "6.2 - Reference Implementation"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    Is SQLite 3.7 really necessary to define the storage file format ?
    According to http://www.sqlite.org/formatchng.html , the last file 
format
change was in SQLite 3.6.0. Many enterprise Linux distributions ship with
SQLite 3.6.X (e.g. SQLite 3.6.20 on RHEL 6).

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 10

2. Implementation Specification Section number:
    Paragraph "6.2 - Reference Implementation"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    What's the usefulness of redefining the header of a SQLite3 database ? 
This
is implied by REQ 9, and there are so many other requirements that should be
written to specify the interoperability with a SQLite3 database, if you 
don't
use the "SQLite3 reference implementation". This might be just an
informational note however, to help writing a preliminary check that the 
file
is a good candidate for being a geopackage container.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "8.2 - Geopackage Contents Table"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    There are inconsistencies between the default values in "Table  –
Geopackage Contents Table or View Definition" and "Table 2 –
geopackage_contents Table Definition SQL" for columns "identifier" and
"description". I haven't verified, but I doubt that a default value makes 
sense
for identifier if it is supposed to be UNIQUE. And for description, table 1
mentions "", whereas table 2 mentions "none".

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "9.4 - Feature Tables"

3. Criticality:
    Major

4. Comments/justifications for changes:

    This paragraph can lead the implementator of another implementation in
error, since you get the false impression that the "gb" storage type is WKB,
whereas it is in fact later explained (page 46) to be the spatialite blob
format. I think this should be more clearly stated right here.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 25

2. Implementation Specification Section number:
    Paragraph "9.4 - Feature Tables"

3. Criticality:
    Major

4. Comments/justifications for changes:

    It is unfortunate that spatial index is mentionned, but not specified !
Different implementations of GeoPackage could have incompatible spatial 
index.
Why not just specifying the implementation done in Spatialite ?, i.e. based 
on
the SQLite3 R-Tree extension.

    If that was done, it should be mentionned in REQ 9 that the SQLite3
implementation to use must have R-Tree support, which is an optional
configuration to enable at build time.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 26 and REQ 27

2. Implementation Specification Section number:
    Paragraph "9.4 - Feature Tables"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    I fail to see why the various quantities (1 degree bounding box, 1000
features, etc...) are mentionned. REQ 27 could be just written "Spatial
queries in a GeoPackage shall provide the same results with and without a
spatial index." And REQ 28 should be just removed.
    You can certainly find artificial cases of data where the spatial index 
will
lead to slower request times.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 44

2. Implementation Specification Section number:
    Paragraph "9.6 - Reference Implementation"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    Why not just specifying Spatialite V4 as the minimum version ? The 
various
hacks given at page 46 to turn a Spatialite V3 into a pseudo V4 are just 
ugly
and lead to confusion. They could be mentionned on non-official documents, 
such
as the Spatialite wiki, but it is awkward to see them mentionned in a formal
document, like that one.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    REQ 47

2. Implementation Specification Section number:
    Paragraph "9.6 - Reference Implementation"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    It would be worth mentionning that the "Compressed geometries" described
in http://www.gaia-gis.it/gaia-sins/BLOB-Geometry.html are not valid for a
baseline GeoPackage.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "10.2 Raster Columns"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    I'd suggest making the compr_qual_factor and georectification columns
nullable, or have another default value meaning "unknown". If you convert an
existing set of tiled JPEGs into a GeoPackage, you generally don't know the
quality associated (it is certainly different of 100 %, but is it 25% or 75%
??). Same for georectification. Those columns provide non essential
information.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "10.7 Rasters or Tiles Table Metadata"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    Actually, in RasterLite (current state of public sources at least), the
raster metadata table is suffixed by _metadata, not _rt_metadata.

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "10.8.10 gpkgBboxToTiles SQL Function"

3. Criticality:
    Major

4. Comments/justifications for changes:

    I'm not sure how gpkgBboxToTiles() can be implemented as a SQLite
function. To the best of my knowledge, and according to
http://www.sqlite.org/c3ref/create_function.html, custom SQLite functions 
can
return a single value (string, integer, real, text, blob), potentially
operating as aggregates on several source rows, but not generating several
rows. What is the "set of integers" return value mentionned here : a string 
of
ids, separated by commas or spaces ?

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    -

3. Criticality:
    Major

4. Comments/justifications for changes:

    In the context of a raster pyramid whose more resoluted level is e.g. a
world region, but that has pyramids up to the "whole world" zoom level, my
understanding is that the bounding box recorded into geopackage_contents 
would
be -180,-90,180,90. Not particularly useful. It would be good to have a 
direct
way of knowing the extent of the more resoluted data, without having to run 
a
query on the _rt_metadata table, like "SELECT MIN(min_x), MIN(min_y),
MAX(max_x), MAX(max_y) FROM xxx_rt_metadata", which can be slow. 
Furthermore,
if the edge tiles have padding, you would get that padding in the returned
extent, which might be undesirable.
    In the tile_matrix_metadata table, this could perhaps be implemented by
adding min_x, min_y, max_x, max_y columns, which would represent the extent 
of
the significant area (that is to say without any potential padding).

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "10.2 Raster Columns"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    In the raster_columns table, it might be appropriate to have a
"band_count" column, that would indicate the maximal number of raster bands
(GDAL terminology, or also called image channels) among all the tiles /
rasters associated with the table with the raster column.
    For example, 1 for grey-level raster, 3 for RGB images, 4 for  RGBA
images. This would be usefull for example when converting a GeoPackage into 
a
old-fashioned GIS format, like GeoTIFF (PNG8 without transparency would be
considered as 3 bands after color table expansion, and with transparency as 
4
bands).

-----------------------------------------------------------------------------------

PART B

1. Requirement:
    -

2. Implementation Specification Section number:
    Paragraph "13.2 Manifest Content Model and Table Definition"

3. Criticality:
    Minor

4. Comments/justifications for changes:

    Is the default value proposed for the manifest column (empty string)
considered as a valid value that will comply with REQ 78 (I think not) ?
_______________________________________________
Standards mailing list
Standards at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/standards 