OIiveirra commented on code in PR #69032: URL: https://github.com/apache/doris/pull/69032#discussion_r4292526865
########## be/src/core/data_type/data_type_spatial.h: ########## @@ -0,0 +1,70 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <string> + +#include "core/column/column_spatial.h" +#include "core/data_type/data_type.h" + +namespace doris { + +// The physical payload is WKB, but this DataType deliberately remains distinct +// from VARBINARY so that the external-format reader can enforce spatial semantics. +class DataTypeSpatial final : public IDataType { +public: + DataTypeSpatial(PrimitiveType primitive_type, std::string crs = "OGC:CRS84", + std::string algorithm = ""); + + const std::string get_family_name() const override { + return _primitive_type == TYPE_GEOMETRY ? "Geometry" : "Geography"; + } + PrimitiveType get_primitive_type() const override { return _primitive_type; } Review Comment: Fixed in [f46d29ce2da0](https://github.com/apache/doris/commit/f46d29ce2da092941b1cf3af2bede4267980149d). Single Geometry/Geography keys now use the existing serialized-key path, which calls the actual ColumnSpatial serializer and preserves nullable keys instead of casting spatial storage to ColumnString. Added one-key GROUP BY and DISTINCT for both kinds, plus equality and NULL-safe joins through the shared dispatcher. Targeted ASAN tests and the generated/verified SQL regressions pass. ########## 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 [f46d29ce2da0](https://github.com/apache/doris/commit/f46d29ce2da092941b1cf3af2bede4267980149d). CASE now dispatches both spatial kinds to ColumnSpatial and its normal insert_from materialization, copying WKB into the result arena rather than treating StringViews as fixed-width SIMD values. Coverage includes two WHEN clauses with distinct columns, nullable and constant branches, and existing numeric/date CASE controls. Targeted ASAN tests and SQL regressions pass. ########## be/src/core/column/column_spatial.h: ########## @@ -0,0 +1,136 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <glog/logging.h> +#include <pdqsort.h> + +#include <cstddef> + +#include "core/arena.h" +#include "core/assert_cast.h" +#include "core/column/column.h" +#include "core/data_type/define_primitive_type.h" +#include "core/string_view.h" + +namespace doris { + +// Stores the raw OGC WKB payload for GEOMETRY and GEOGRAPHY. The primitive type is +// part of the column identity so spatial values cannot be substituted for VARBINARY. +class ColumnSpatial final : public COWHelper<IColumn, ColumnSpatial> { +private: + using Self = ColumnSpatial; + friend class COWHelper<IColumn, ColumnSpatial>; + template <bool positive> + struct less; + +public: + using Container = PaddedPODArray<doris::StringView>; + +private: + explicit ColumnSpatial(PrimitiveType primitive_type) : _primitive_type(primitive_type) { + DCHECK(primitive_type == TYPE_GEOMETRY || primitive_type == TYPE_GEOGRAPHY); + } + ColumnSpatial(const ColumnSpatial& src) : _primitive_type(src._primitive_type) { + _data.reserve(src._data.size()); + for (const auto& value : src._data) { + insert_data(value.data(), value.size()); + } + } + +public: + std::string get_name() const override { return "ColumnSpatial"; } + PrimitiveType get_primitive_type() const { return _primitive_type; } + size_t size() const override { return _data.size(); } + const Container& get_data() const { return _data; } + void resize(size_t n) override { _data.resize(n); } Review Comment: Fixed in [f46d29ce2da0](https://github.com/apache/doris/commit/f46d29ce2da092941b1cf3af2bede4267980149d). ColumnSpatial::erase now compacts only the retained StringViews while keeping arena ownership, following the existing row-range erasure contract. Eight operator cases cover Geometry/Geography, nullable/nonnullable inputs and FIRST_VALUE/LAST_VALUE over 300 input blocks; they verify distinct per-row payloads and assert that trimming actually occurred. The SQL regression also uses 300 rows with batch_size=1 and compares both window results to their expected values. ########## be/src/exprs/function/geo/geo_types.cpp: ########## @@ -428,6 +428,19 @@ std::unique_ptr<GeoShape> GeoShape::from_wkb(const char* data, size_t size, return shape; } +std::unique_ptr<GeoShape> GeoShape::from_wkb_bytes(const char* data, size_t size, + GeoParseStatus& status) { + static constexpr char HEX[] = "0123456789ABCDEF"; + std::string hex; + hex.reserve(size * 2); + for (size_t i = 0; i < size; ++i) { + const auto byte = static_cast<unsigned char>(data[i]); + hex.push_back(HEX[byte >> 4]); + hex.push_back(HEX[byte & 0x0F]); + } + return from_wkb(hex.data(), hex.size(), status); Review Comment: Fixed in [f46d29ce2da0](https://github.com/apache/doris/commit/f46d29ce2da092941b1cf3af2bede4267980149d). Typed calculation inputs now go directly through a byte-oriented WkbParse entry that borrows the input buffer and reuses initialize_context/readGeometry. This removes the bytes-to-hex-to-stream-to-bytes/vector round trip; the legacy STRING hex entry remains unchanged. Tests cover both byte orders, Point/LineString/Polygon/EMPTY, known rejected inputs, ownership after overwriting the input buffer, and a 4096-point LineString. Existing geo/WKB tests and STRING GIS regressions pass. No performance percentage is claimed. -- 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]
