API: SubdatasetInfo

[elpaso]

Fix #7261

Initial GDALSubdatasetInfo API.

[elpaso]

@rouault I'd like a general review of the API before I get through all the formats, for now the only implemented one is GPKG.

I will also probably need a list of the drivers that support subdatasets.

[coveralls]

coverage: 67.607% (-0.003%) from 67.61% when pulling e94dbb4 on elpaso:subdatasetinfo-api-func-pointers into 7f18ca5 on OSGeo:master.

[elpaso]

@rouault I agree there is a little confusion in the terminology, here is my proposal to clarify and avoid ambiguity:

The complete file name (e.g. GPKG:/path_to.gpkg:layer_name:var_name) will be named "File name" and the methods and variables will be renamed accordingly.

The parsed components will be named:

• driver prefix e.g. GPKG -> "Prefix component"
• file path: e.g. /path_to.gpkg -> "Path component"
• subdataset: e.g. layer_name -> "Subdataset component"

So, the methods will become:

GetFileName() -> returns the original complete file name e.g. GPKG:/path_to.gpkg:layer_name:var_name
GetPathComponent() -> returns the path component e.g. /path_to.gpkg
GetPrefixComponent() -> returns the prefix component e.g. GPKG
GetSubdatasetComponent() -> returns the subdataset component e.g. layer_name
ModifyPathComponent( newPathComponent) -> replaces the path component

Or, the shorter versions without Component:
GetFileName()
GetPath()
GetPrefix()
GetSubdataset()
ModifyPath(newPath)

[rouault]

GetSubdataset()

or maybe GetSuffix() ?

[elpaso]

GetSubdataset()

or maybe GetSuffix() ?

I don't follow: isn't subdataset more appropriate to describe it? Suffix is really generic and if we have variables it might not even be the last component in the descriptor.

[rouault]

I don't follow: isn't subdataset more appropriate to describe it? Suffix is really generic and if we have variables it might not even be the last component in the descriptor.

ok, let's keep GetSubdataset(). "There are only two hard things in Computer Science: cache invalidation and naming things. " :-)

[rouault]

what do you think about the opportunity of using this in the GTiffDataset::OpenDir() method to avoid duplication of parsing ? (this could apply to other drivers as well)

[elpaso]

I'll have a look, I prefer a method that does not have to open the file though, ideally this API should just manipulate strings without I/O overhead.

[rouault]

I prefer a method that does not have to open the file though, ideally this API should just manipulate strings without I/O overhead.

yes, totally agreed. I mean to update the existing Open() methods to use the new parseFileName()

[rouault]

There is a more esoteric supported variant: GTIFF_DIR:off:<dir_offset>:filename
If we reused this class for GTiffDataset::OpenDir(), we'd have to support it

[sgillies]

@rouault @elpaso how did we arrive at using : as a delimiter between dataset and subdataset name? Does this have to be extended to drivers where it doesn't exist already? I think : is a poor choice and leads to people putting quotes around dataset URLs within a GDAL filename.

[elpaso]

@rouault @elpaso how did we arrive at using : as a delimiter between dataset and subdataset name? Does this have to be extended to drivers where it doesn't exist already? I think : is a poor choice and leads to people putting quotes around dataset URLs within a GDAL filename.

I don't know where the colon separator comes from, but I totally agree that it's a poor choice, the QGIS counterpart QgsDatasourceUri is a little bit better engineered, for most datasources it uses URL encoding for the KVP and it allows to encode the opening options.

[rouault]

@rouault @elpaso how did we arrive at using : as a delimiter between dataset and subdataset name? Does this have to be extended to drivers where it doesn't exist already? I think : is a poor choice and leads to people putting quotes around dataset URLs within a GDAL filename.

@sgillies Before my involvement with the project. I can see that this syntax was already used in the initial version of the HDF4 driver back in 2002: 7c56e20797c17d2bcb3f173d652ec1fdf590d558
While this is indeed not ideal, changing the subdataset syntax in existing drivers would have backward compatibility issues. I suspect a number of people have VRT files referencing the old syntax, or some code manually composing them.
The new API itself in this PR makes no assumption on the separator, so I don't think it is a step backwards, and it should be hopefully compatible with better subsdataset syntax.
That said the replacement logic in ModifyPathComponent() is fully string based currently. if at some point a driver would URI-encode the filename, that wouldn't work. But if that occured, the method could be made virtual and implemented by the driver that would deal with the URI encoding/decoding.

@elpaso I believe to keep this PR manageable we should target the most popular file-based drivers that use subsdataset (others might follow in subsequent PR): netCDF, HDF4, HDF5, GPKG . Anything else ?
I'd be interested however to see frmts/vrt/vrtsources.cpp updated to remove the NETCDF releated hacks in favor of the new API to demonstrate its usefulness (hum I see that to remove all of them we should also tackle NITF, PDF, RASTERLITE and TILEDB)

[elpaso]

@rouault @elpaso how did we arrive at using : as a delimiter between dataset and subdataset name? Does this have to be extended to drivers where it doesn't exist already? I think : is a poor choice and leads to people putting quotes around dataset URLs within a GDAL filename.

@sgillies Before my involvement with the project. I can see that this syntax was already used in the initial version of the HDF4 driver back in 2002: 7c56e20797c17d2bcb3f173d652ec1fdf590d558 While this is indeed not ideal, changing the subdataset syntax in existing drivers would have backward compatibility issues. I suspect a number of people have VRT files referencing the old syntax, or some code manually composing them. The new API itself in this PR makes no assumption on the separator, so I don't think it is a step backwards, and it should be hopefully compatible with better subsdataset syntax. That said the replacement logic in ModifyPathComponent() is fully string based currently. if at some point a driver would URI-encode the filename, that wouldn't work. But if that occured, the method could be made virtual and implemented by the driver that would deal with the URI encoding/decoding.

Yeah.

@elpaso I believe to keep this PR manageable we should target the most popular file-based drivers that use subsdataset (others might follow in subsequent PR): netCDF, HDF4, HDF5, GPKG . Anything else ? I'd be interested however to see frmts/vrt/vrtsources.cpp updated to remove the NETCDF releated hacks in favor of the new API to demonstrate its usefulness (hum I see that to remove all of them we should also tackle NITF, PDF, RASTERLITE and TILEDB)

I have actually already implemented WMS and PostGIS raster because I wanted to test the API with a DB and a webservice. I will proceed according to your priority list.

[elpaso]

@rouault here is another iteration. I have removed the double quotes from the logic by storing both the original unmodified path (used only internally to modify the path) and I am storing isQuoted to determine if the new path should be quoted (if it is already quoted it is not quoted twice).

When returning the path it is always unquoted and unescaped.

I have added a few new test cases.

[rouault]

@sgillies Do you think we can merge this PR or do you want the RFC you mention in https://lists.osgeo.org/pipermail/gdal-dev/2023-September/057641.html to be a pre-condition for it ?

[rouault]

@elpaso Can you rebase on top of latest master and fix conflict in autotest/gdrivers/netcdf.py?

[elpaso]

@elpaso Can you rebase on top of latest master and fix conflict in autotest/gdrivers/netcdf.py?

done