OIiveirra commented on code in PR #67907:
URL: https://github.com/apache/doris/pull/67907#discussion_r4001719934
##########
be/src/exprs/function/geo/functions_geo.cpp:
##########
@@ -32,15 +33,128 @@
#include "core/column/column_nullable.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/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/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 != nullptr && 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,
+ GeoParseStatus* parse_status
= nullptr) {
+ 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);
+ if (parse_status != nullptr) {
+ *parse_status = status;
+ }
+ return status == GEO_PARSE_OK ? std::move(shape) : nullptr;
+}
+
+static bool has_unsupported_spatial_wkb_metadata(StringRef value) {
+ if (value.size < 5) {
+ return false;
+ }
+
+ const auto byte_order = static_cast<uint8_t>(value.data[0]);
+ if (byte_order != 0 && byte_order != 1) {
+ return false;
+ }
+
+ const auto byte_at = [&value](size_t offset) {
+ return static_cast<uint8_t>(value.data[offset]);
+ };
+ const uint32_t type = byte_order == 1 ? static_cast<uint32_t>(byte_at(1)) |
+
(static_cast<uint32_t>(byte_at(2)) << 8) |
+
(static_cast<uint32_t>(byte_at(3)) << 16) |
+
(static_cast<uint32_t>(byte_at(4)) << 24)
+ : (static_cast<uint32_t>(byte_at(1))
<< 24) |
+
(static_cast<uint32_t>(byte_at(2)) << 16) |
+
(static_cast<uint32_t>(byte_at(3)) << 8) |
+
static_cast<uint32_t>(byte_at(4));
+
+ constexpr uint32_t ewkb_z_flag = 0x80000000;
+ constexpr uint32_t ewkb_m_flag = 0x40000000;
+ constexpr uint32_t ewkb_srid_flag = 0x20000000;
+ constexpr uint32_t ewkb_metadata_flags = ewkb_z_flag | ewkb_m_flag |
ewkb_srid_flag;
+ if ((type & ewkb_metadata_flags) != 0) {
+ return true;
+ }
+
+ return type >= 1000 && type < 4000;
+}
+
+static bool decode_wkb_hex(StringRef value, std::string* wkb) {
+ const char* data = value.data;
+ size_t size = value.size;
+ if (size >= 2 && ((data[0] == '0' && data[1] == 'x') || (data[0] == '\\'
&& data[1] == 'x'))) {
+ data += 2;
+ size -= 2;
+ }
+ if (size == 0 || (size & 1) != 0) {
+ return false;
+ }
+ wkb->resize(size / 2);
+ return string_hex::hex_decode(data, size, wkb->data()) == size / 2;
+}
+
+Status validate_spatial_wkb_inputs(const Block& block, const ColumnNumbers&
arguments) {
+ for (const auto argument : arguments) {
+ const auto& column = block.get_by_position(argument).column;
+ const auto& type = block.get_data_type(argument);
+ if (!is_spatial_type(type)) {
+ continue;
+ }
+ for (size_t row = 0; row < column->size(); ++row) {
+ if (column->is_null_at(row)) {
+ continue;
+ }
+ const auto value = column->get_data_at(row);
+ if (has_unsupported_spatial_wkb_metadata(value)) {
+ return Status::NotSupported(
+ "WKB dimensions or embedded SRID are not supported for
spatial inputs at "
+ "row {}",
+ row);
+ }
+ GeoParseStatus parse_status;
+ if (decode_geo_shape(value, type, &parse_status) == nullptr) {
Review Comment:
Thanks for the review. We agree that the current validation path can decode
the same WKB more than once, and that a ColumnConst may be validated once per
logical row. This does not affect correctness, but eliminating it cleanly
requires sharing decoded shapes across the validation and execution paths and
handling constant-column semantics in the function framework. To keep this PR
focused on the correctness and compatibility of Iceberg V3 spatial read/write
support, we will track that as a follow-up performance optimization, with
benchmarks and a focused design for cache lifetime and invalid-input behavior.
--
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]
