"gdal vector filter": add --where and --fields; add "gdal vector select" #11794
Conversation
rouault
added
funded through GSP
rouault
force-pushed
the
gdal_vector_filter
branch
from
0671c17 to
7d406c7
Compare
apps/gdalalg_vector_filter.cpp
Outdated
| private: | ||
| bool m_bIsOK = true; | ||
| OGRLayer *const m_poSrcLayer; | ||
| OGRFeatureDefn *const m_poFeatureDefn = nullptr; |
There was a problem hiding this comment.
can we use a unique pointer here or is it already using the self made share ptr with reference/dereference?
There was a problem hiding this comment.
- m_poSrcLayer transformed into a reference
- for m_poFeatureDefn, we can't do much about that since OGRFeatureDefn includes its own ref counting mechanism. Changing that (or studying if we can...) is I believe in the todo list of @dbaston. I've some doubts we can do that in the GDAL 3.x series
|
I would consider putting |
rouault
force-pushed
the
gdal_vector_filter
branch
from
85e0ee0 to
8c1d084
Compare
rouault
changed the title
…ter on result layer
…r on result layer
rouault
force-pushed
the
gdal_vector_filter
branch
from
8c1d084 to
5a62208
Compare
done: "gdal vector select" added |
rouault
force-pushed
the
gdal_vector_filter
branch
from
5a62208 to
25a8487
Compare
|
Does |
rouault
force-pushed
the
gdal_vector_filter
branch
from
25a8487 to
a3dbe8d
Compare
I've added an --exclude switch |
rouault
force-pushed
the
gdal_vector_filter
branch
from
a3dbe8d to
faccd03
Compare
| A field is only selected once, even if mentioned several times in the list. | ||
|
|
||
| Geometry fields can also be specified in the list. If the source layer has | ||
| no explicit name for the geometry field, ``_ogr_geometry_`` must be used to |
There was a problem hiding this comment.
I think needing to include _ogr_geometry_ explicitly might be surprising for users. I can imagine including it by default unless an --exclude-geom argument is passed. Would be curious what others think.
There was a problem hiding this comment.
FWIW, the R sf package behaves this way (some discussion at r-spatial/sf#121).
There was a problem hiding this comment.
I think needing to include
_ogr_geometry_explicitly might be surprising for users. I can imagine including it by default unless an--exclude-geomargument is passed
that's a good question. I'm not sure either. One thing that is certain is that the current difference in behavior between OGRSQL where "SELECT" automatically selects the geometry field (unless excluded thanks to your recent work on EXCLUDE) compared to all other SQL dialects where the geometry field must be explicitly selected surprises users a lot.
| m_poFeatureDefn->Dereference(); | ||
| } | ||
|
|
||
| bool IncludeFields(const std::vector<std::string> &selectedFields, |
There was a problem hiding this comment.
The only thought I have on the implementation is whether the processing could be handled by creating an in-memory VRT that could either be materialized or not, depending on the output format. I don't think anything about this interface precludes that, so I could look into it later on as part of #10066.
There was a problem hiding this comment.
Same comment could apply to #11827
There was a problem hiding this comment.
Good question. Regarding VRT'ization of all of this, I believe both GDAL and OGR VRT will have to be extended to support accepting a datasource and a pipeline string, rather than trying to extend the capabilities of the VRT drivers themselves to support all algorithms, which would involve duplicating all the code.
rouault
force-pushed
the
gdal_vector_filter
branch
from
faccd03 to
d884bda
Compare
rouault
force-pushed
the
gdal_vector_filter
branch
from
d884bda to
32d4ef0
Compare
No description provided.