[c++] Column abstraction: SOMAGeometryColumn, part 3
#3427
Conversation
XanthosXanthopoulos
changed the title
SOMAGeometryColumn, part 3SOMAGeometryColumn, part 3
There was a problem hiding this comment.
@XanthosXanthopoulos thanks for working on this! The split-up makes this PR much easier to review.
|
|
||
| // Create a range object and reuse if for all dimensions | ||
| std::vector<std::pair<double_t, double_t>> range(1); | ||
| size_t dimensionality = dimensions.size() / 2; |
There was a problem hiding this comment.
Please explain the magic number 2 here
| transformed_points = _transform_points( | ||
| std::any_cast<std::span<const std::vector<double_t>>>(points)); | ||
|
|
||
| auto domain_limits = _limits(ctx, *query->schema()); |
There was a problem hiding this comment.
Core has current domain and domain. SOMA has domain and maxdomain.
Saying "domain" is ambiguous.
I've been consistent to say "core_domain", "core_current_domain"/"current_domain", "soma_domain", "soma_maxdomain". Please do the same here and make the variable name indicate whether this domain variable is intended to be core domain or soma domain (which are not the same).
| } | ||
|
|
||
| void SOMAGeometryColumn::_set_current_domain_slot( | ||
| NDRectangle& rectangle, std::span<const std::any> domain) const { |
There was a problem hiding this comment.
As above: we must not use the plain variable name domain without qualification, as it is ambiguous and error-prone.
|
|
||
| void SOMAGeometryColumn::_set_current_domain_slot( | ||
| NDRectangle& rectangle, std::span<const std::any> domain) const { | ||
| if (2 * domain.size() != dimensions.size()) { |
There was a problem hiding this comment.
Please explain the magic number 2 here.
|
|
||
| std::pair<bool, std::string> SOMAGeometryColumn::_can_set_current_domain_slot( | ||
| std::optional<NDRectangle>& rectangle, | ||
| std::span<const std::any> new_domain) const { |
There was a problem hiding this comment.
| std::span<const std::any> new_domain) const { | |
| std::span<const std::any> new_current_domain) const { |
| std::span<const std::any> new_domain) const { | ||
| if (new_domain.size() != dimensions.size() / 2) { | ||
| throw TileDBSOMAError(std::format( | ||
| "[SOMADimension][_can_set_current_domain_slot] Expected domain " |
There was a problem hiding this comment.
| "[SOMADimension][_can_set_current_domain_slot] Expected domain " | |
| "[SOMADimension][_can_set_current_domain_slot] Expected current_domain " |
| const SOMAContext& ctx, const ArraySchema& schema) const { | ||
| std::vector<std::pair<double_t, double_t>> limits; | ||
|
|
||
| for (size_t i = 0; i < dimensions.size() / 2; ++i) { |
There was a problem hiding this comment.
Please explain the magic number 2 here
| ranges) const { | ||
| if (ranges.size() != 1) { | ||
| throw TileDBSOMAError( | ||
| "Multi ranges are not supported for geometry dimension"); |
There was a problem hiding this comment.
| "Multi ranges are not supported for geometry dimension"); | |
| "Multiranges are not supported for geometry dimension"); |
| const std::span<const std::vector<double_t>>& points) const { | ||
| if (points.size() != 1) { | ||
| throw TileDBSOMAError( | ||
| "Multi points are not supported for geometry dimension"); |
There was a problem hiding this comment.
| "Multi points are not supported for geometry dimension"); | |
| "Multipoints are not supported for geometry dimension"); |
| @@ -49,9 +49,6 @@ | |||
| #include "soma/column_buffer.h" | |||
| #include "soma/soma_array.h" | |||
| #include "soma/soma_collection.h" | |||
| #include "soma/soma_column.h" | |||
There was a problem hiding this comment.
I don't understand why these lines are being deleted on this PR
There was a problem hiding this comment.
They are intended to be used internally (within SOMAArray and subclasses) so exporting them seemed unnecessary. In any case if the usage pattern changes, we can export them again or do it preemptively now, I have no opinion on that.
…nt domain checks, replace vector with span when selecting points
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-geometry-column
branch
from
fc53e6e to
497224a
Compare
This PR introduces the
SOMAGeometryColumnconcrete class that implement a spatial index WKB (Well-Known Binary) column. A set of internal TileDB dimensions are used to provide the index mechanics as well as a TileDB Attribute to hold the geometry data encoded in a WKB blob.