[gdal-dev] ElasticSearch Driver

[Even Rouault]

Le mardi 24 janvier 2012 21:27:03, Adam Estrada a écrit :
> All,
> 
> I have been using ElasticSearch as a way to perform some advanced analysis
> on feature attribute data. From my work with ElasticSearch came the
> ogr2elasticsearch GDAL driver. The code can be found here.
> http://code.google.com/p/ogr2elasticsearch/
> 
> Every time I rebuild GDAL I have to add this driver back to the build path
> so that I can take advantage of it which is unfavorable. What would it take
> to get this driver included in to trunk GDAL?

* Make sure that it works with the json-c already in 
ogr/ogrsf_frmts/geojson/jsonc
* Write a drv_elasticsearch.html page (but I see you have the material for 
that on the google project). Perhaps make it clearer that it is a write-only 
driver. Out of curiousity, any plan to add a read operation (if that makes 
sense. I've never used elasticsearch). Also mentions that the geometry written 
in ElasticSearch is the center of the bounding box of the geometry (for linear 
or polygonal geometries).
* in OGRElasticDataSource constructor, I suspect bUseExtension is from another 
driver and should be removed. Same for bWriteHeaderAndFooter. And removed from 
the header file too
* in OGRElasticDataSource::Create() :
	- you should make a copy of the return of CPLGetConfigOption() for 
psWriteMap and psMetaFile. The reason is that if their values is changed later 
the pointer might become invalid. By the way, the standard hungarian notation 
for char* is psz. Ah and you should make sure they are initialized in the 
constructor and freed in the destructor of the class.
        - actually, I see that psMetafile is only used in Create(). So why make 
it a member variable of the class ?
        - there's a missing fclose(fp)
        - psMapping (which should be pszMapping) is never freed apparently ?
* in ogr_elastic.h : 
*in OGRElasticLayer definition :
	- nTotalFeatureCount should likely be removed. It is null-initialized, 
incremented in CreateFeature() but never used elsewhere
	- poFeature should be removed. It is null-initialized in the constructor 
and deleted in the destructor, but never initialized to something else.
* in OGRElasticLayer::~OGRElasticLayer(), poFeatureDefn->Release() should be 
uncommented, otherwise you're leaking it
* in OGRElasticLayer::BuildMap(), the comment // The attribute's were freed 
from the deletion of the map object and the nullifying of pAttributes are 
strange. I can see pAttributes being initialized in CreateField(), but never 
destroyed.
* in OGRElasticLayer::CreateFeature(), :
     - you dereference poFeature->GetGeometryRef(), assuming that it is always 
non NULL. Which might not be always the case...
     - in the loop iterating over field, you should likely test IsFieldSet(i) 
before calling GetFieldAs...() in the case the field is unset/null.
     - GetFieldAsInteger(), GetFieldAsDouble() and GetFieldAsInteger() can 
accept the index also as an argument : this will make the code shorter (and 
also a bit faster)

And after that and checking that it still works, attach a patch in Trac ;-)

> 
> Thanks in advance,
> Adam