OIiveirra commented on code in PR #69032:
URL: https://github.com/apache/doris/pull/69032#discussion_r4336105824
##########
be/src/exprs/function/geo/functions_geo.cpp:
##########
@@ -32,18 +45,308 @@
#include "core/column/column_array.h"
#include "core/column/column_execute_util.h"
#include "core/column/column_nullable.h"
+#include "core/column/column_varbinary.h"
#include "core/data_type/data_type_array.h"
#include "core/data_type/data_type_nullable.h"
#include "core/data_type/data_type_number.h"
+#include "core/data_type/data_type_spatial.h"
#include "core/data_type/data_type_string.h"
+#include "core/data_type/data_type_varbinary.h"
#include "core/data_type/define_primitive_type.h"
#include "core/string_ref.h"
#include "exprs/function/geo/geo_common.h"
#include "exprs/function/geo/geo_types.h"
+#include "exprs/function/geo/wkb_parse.h"
#include "exprs/function/simple_function_factory.h"
+#include "exprs/function/string_hex_util.h"
namespace doris {
+static bool is_spatial_type(const DataTypePtr& type) {
+ const auto primitive_type = remove_nullable(type)->get_primitive_type();
+ return primitive_type == TYPE_GEOMETRY || primitive_type == TYPE_GEOGRAPHY;
+}
+
+static Status validate_geography_semantics(const DataTypePtr& type, const
char* function_name) {
+ const auto& nested_type = remove_nullable(type);
+ if (!is_spatial_type(nested_type)) {
+ return Status::OK();
+ }
+
+ const auto* spatial_type = dynamic_cast<const
DataTypeSpatial*>(nested_type.get());
+ DCHECK(spatial_type != nullptr);
+ if (spatial_type->get_primitive_type() == TYPE_GEOGRAPHY &&
+ spatial_type->crs() == "OGC:CRS84" && spatial_type->algorithm() ==
"spherical") {
+ return Status::OK();
+ }
+ return Status::NotSupported(
+ "Function {} requires GEOGRAPHY(OGC:CRS84, spherical) for spatial
inputs",
+ function_name);
+}
+
+static std::unique_ptr<GeoShape> decode_geo_shape(StringRef value, const
DataTypePtr& type) {
+ if (!is_spatial_type(type)) {
+ return GeoShape::from_encoded(value.data, value.size);
+ }
+
+ GeoParseStatus status;
+ auto shape = GeoShape::from_wkb_bytes(value.data, value.size, status);
+ return status == GeoParseStatus::GEO_PARSE_OK ? std::move(shape) : nullptr;
+}
+
+static bool any_geo_argument_is_null(const Block& block, const ColumnNumbers&
arguments,
+ size_t row) {
+ return std::ranges::any_of(arguments, [&](auto argument) {
+ return block.get_by_position(argument).column->is_null_at(row);
+ });
+}
+
+static Status decode_calculation_shape(StringRef value, const DataTypePtr&
type,
+ std::unique_ptr<GeoShape>* shape,
+ const char* function_name) {
+ *shape = decode_geo_shape(value, type);
+ // POLYGON EMPTY decodes successfully, but distance helpers retain
infinity/max sentinels.
+ // Reject it for typed calculations just like empty points/lines, leaving
raw WKB untouched.
+ if (is_spatial_type(type) &&
+ (*shape == nullptr ||
+ ((*shape)->type() == GeoShapeType::GEO_SHAPE_POLYGON &&
+ assert_cast<const GeoPolygon&>(**shape).polygon()->num_loops() ==
0))) {
+ return Status::NotSupported("Function {} does not support this
Geography WKB shape",
+ function_name);
+ }
+ return Status::OK();
+}
+
+// Calculations use canonical positions; raw spatial accessors still return
the original WKB.
+static S2Point canonical_geography_point(const S2Point& point) {
+ const auto coordinate = S2LatLng(point);
+ const double latitude = coordinate.lat().radians();
+ // All longitudes at a pole and +/-180 at the date line denote the same
position.
+ if (std::abs(latitude) == M_PI_2) {
+ return {0, 0, latitude > 0 ? 1.0 : -1.0};
+ }
+ if (coordinate.lng().radians() == M_PI) {
+ // Flip only sin(longitude): rebuilding latitude can change its last
bit.
+ return {point.x(), -point.y(), point.z()};
+ }
+ return point;
+}
+
+static void geography_shape_index(const GeoShape& shape, MutableS2ShapeIndex*
index) {
+ switch (shape.type()) {
+ case GeoShapeType::GEO_SHAPE_LINE_STRING: {
+ const auto* line = assert_cast<const GeoLine&>(shape).polyline();
+ std::vector<S2Point> vertices;
+ for (int i = 0; i < line->num_vertices(); ++i) {
+ vertices.push_back(canonical_geography_point(line->vertex(i)));
+ }
+ index->Add(std::make_unique<S2LaxPolylineShape>(vertices));
+ break;
+ }
+ case GeoShapeType::GEO_SHAPE_POLYGON: {
+ const auto* polygon = assert_cast<const GeoPolygon&>(shape).polygon();
+ std::vector<S2LaxPolygonShape::Loop> loops;
+ for (int i = 0; i < polygon->num_loops(); ++i) {
+ auto& vertices = loops.emplace_back();
+ const auto* loop = polygon->loop(i);
+ for (int j = 0; j < loop->num_vertices(); ++j) {
+
vertices.push_back(canonical_geography_point(loop->oriented_vertex(j)));
+ }
+ }
+ index->Add(std::make_unique<S2LaxPolygonShape>(loops));
+ break;
+ }
+ default:
+ // decode_calculation_shape admits only nonempty
Point/LineString/Polygon WKB.
+ throw Exception(ErrorCode::INTERNAL_ERROR, "Unexpected Geography
calculation shape");
+ }
+}
+
+static bool geography_point_relation(std::string_view name, const GeoShape&
lhs,
+ const GeoShape& rhs) {
+ const auto& point_shape = lhs.type() == GeoShapeType::GEO_SHAPE_POINT ?
lhs : rhs;
+ const auto& other = lhs.type() == GeoShapeType::GEO_SHAPE_POINT ? rhs :
lhs;
+ const auto point =
+ canonical_geography_point(*assert_cast<const
GeoPoint&>(point_shape).point());
+ bool interior = false;
+ bool boundary = false;
+ // S2LatLng::ToPoint has up to 1.5*epsilon error, plus degrees-to-radians
+ // rounding. Allow only conversion-scale error for the point and edge
endpoints:
+ // POINT(10 45.5) is on LINESTRING(10 45,10 46), but their rounded vectors
+ // are not exactly coplanar. This is about 1e-8 metres, not a user snap
radius.
+ const auto conversion_error =
+ S1ChordAngle(S1Angle::Radians(8 *
std::numeric_limits<double>::epsilon()));
+ if (other.type() == GeoShapeType::GEO_SHAPE_POINT) {
+ interior = point == canonical_geography_point(*assert_cast<const
GeoPoint&>(other).point());
+ } else if (other.type() == GeoShapeType::GEO_SHAPE_LINE_STRING) {
+ const auto* line = assert_cast<const GeoLine&>(other).polyline();
+ bool member = false;
+ // S2 Boolean operations symbolically move points off edge interiors.
Use its
+ // unperturbed distance predicate on every real geodesic edge instead.
+ for (int i = 0; i + 1 < line->num_vertices(); ++i) {
+ member = member ||
+ s2pred::CompareEdgeDistance(point,
canonical_geography_point(line->vertex(i)),
+
canonical_geography_point(line->vertex(i + 1)),
+ conversion_error) <= 0;
+ }
+ const auto first = canonical_geography_point(line->vertex(0));
+ const auto last =
canonical_geography_point(line->vertex(line->num_vertices() - 1));
+ boundary = first != last && (point == first || point == last);
+ interior = member && !boundary;
+ } else {
+ DCHECK(other.type() == GeoShapeType::GEO_SHAPE_POLYGON);
+ const auto* polygon = assert_cast<const GeoPolygon&>(other).polygon();
+ // Polygon rings have closing edges; open polylines above do not.
+ for (int i = 0; i < polygon->num_loops(); ++i) {
+ const auto* loop = polygon->loop(i);
+ for (int j = 0; j < loop->num_vertices(); ++j) {
+ boundary = boundary || s2pred::CompareEdgeDistance(
+ point,
canonical_geography_point(loop->vertex(j)),
+
canonical_geography_point(loop->vertex(j + 1)),
+ conversion_error) <= 0;
+ }
+ }
+ interior = !boundary && polygon->Contains(point);
+ }
+ if (name == "st_intersects") {
+ return interior || boundary;
+ }
+ if (name == "st_disjoint") {
+ return !interior && !boundary;
+ }
+ if (name == "st_touches") {
+ return boundary;
+ }
+ return interior;
+}
+
+static Status geography_relation(std::string_view name, const GeoShape& lhs,
const GeoShape& rhs,
+ bool* result) {
+ const auto& container = name == "st_within" ? rhs : lhs;
+ const auto& value = name == "st_within" ? lhs : rhs;
+ // Legacy base contains() returns false even for p within itself. Complex
containment
+ // and touches pairs need interior/boundary semantics that are not
implemented here.
+ if (((name == "st_contains" || name == "st_within") &&
+ value.type() != GeoShapeType::GEO_SHAPE_POINT) ||
+ (name == "st_touches" && lhs.type() != GeoShapeType::GEO_SHAPE_POINT &&
+ rhs.type() != GeoShapeType::GEO_SHAPE_POINT)) {
+ return Status::NotSupported("Function {} does not support this
Geography shape pair", name);
+ }
+ if (lhs.type() == GeoShapeType::GEO_SHAPE_POINT ||
+ rhs.type() == GeoShapeType::GEO_SHAPE_POINT) {
+ *result = geography_point_relation(name, lhs, rhs);
+ return Status::OK();
+ }
+ MutableS2ShapeIndex left;
+ MutableS2ShapeIndex right;
+ geography_shape_index(container, &left);
+ geography_shape_index(value, &right);
+ S2BooleanOperation::Options options;
+ options.set_polygon_model(S2BooleanOperation::PolygonModel::CLOSED);
+ options.set_polyline_model(S2BooleanOperation::PolylineModel::CLOSED);
+ options.set_polyline_loops_have_boundaries(false);
+ const bool intersects = S2BooleanOperation::Intersects(left, right,
options);
Review Comment:
Fixed in
[3adc81f7c02](https://github.com/apache/doris/commit/3adc81f7c0257593542dd51893c5936a7773455e).
Nonpoint Geography intersections now use the same conversion-scale error bound
as point relations. Added overlapping meridian lines, nearby disjoint controls
and both operand orders in BE and real Iceberg regressions; BE UT 126/126 and
Iceberg generation/verification passed locally.
##########
be/src/core/data_type/define_primitive_type.h:
##########
@@ -75,7 +75,9 @@ enum PrimitiveType : PrimitiveNative {
TYPE_VARBINARY, /* 41, varbinary */
TYPE_TIMESTAMPTZ, /* 42, timestamptz */
TYPE_TIMESTAMP_NS, /* 43, signed Int64 epoch nanoseconds
*/
- TYPE_UUID /* 44, uuid */
+ TYPE_UUID, /* 44, uuid */
+ TYPE_GEOMETRY, /* 45, Iceberg geometry */
Review Comment:
Fixed in
[3adc81f7c02](https://github.com/apache/doris/commit/3adc81f7c0257593542dd51893c5936a7773455e).
LEAST/GREATEST now reject original spatial arguments in FE before coercion.
Added both-kind, distinct-slot and nullable coverage, including real Iceberg
columns; ordinary key + spatial value support remains intact. FE UT 72/72 and
Iceberg generation/verification passed locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
