github-actions[bot] commented on code in PR #69032:
URL: https://github.com/apache/doris/pull/69032#discussion_r4291902789
##########
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:
[P2] Support spatial result types in CASE materialization. A direct `SELECT
CASE WHEN id = 1 THEN g1 WHEN id = 2 THEN g2 ELSE g3 END FROM spatial_table`,
where g1/g2/g3 are distinct columns with the same spatial type and metadata,
passes FE common-type analysis. The two-WHEN expression survives `CaseWhenToIf`
and reaches `VCaseExpr::_execute_update_result`, whose primitive-type switch
has neither GEOMETRY nor GEOGRAPHY and throws `argument_type Geometry not
supported` (likewise Geography). Add spatial result materialization using the
actual ColumnSpatial representation, including nullable/constant branches. The
current CASE regression only exercises metadata-mismatch rejection, and a
single WHEN can rewrite to IF and mask this missing path.
##########
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:
[P2] Add hash-key dispatch for a single spatial column. `SELECT
ST_AsText(geom), COUNT(*) FROM geometry_values GROUP BY geom` passes FE
analysis, but aggregate initialization calls `get_hash_key_type` with exactly
one DataTypeSpatial. This type inherits `have_maximum_size_of_value() ==
false`, and the single-variable-key branch recognizes only string/ARRAY/JSONB
before throwing `meet invalid type, type=Geometry` (likewise Geography).
`SELECT DISTINCT geom` reaches the same path. The new regression groups by both
geom and geog, which instead takes the multi-key serialized fallback and misses
this failure. Add spatial-aware single-key dispatch with matching column
representation and nullable handling, and cover both one-key GROUP BY and
DISTINCT.
##########
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:
[P2] Parse typed calculation inputs directly from WKB bytes. Every typed
Geography calculation reaches this function with raw WKB, but it expands an
N-byte payload into 2N hex characters and then calls `from_wkb`, which copies
those characters into a stringstream. `WkbParse::read_hex` decodes them into
another N-byte stream, and `read` copies that into an N-byte vector before
constructing the S2 shape. Thus ST_Length or spatial relations over large
LineString/Polygon columns repeatedly allocate at least 6N temporary payload
bytes and stream-convert every byte just to recover the original input. Add a
byte-oriented shape parser using the existing byte-context/readGeometry
machinery and keep the legacy hex entry point for STRING callers.
##########
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:
[P2] Implement erasure for spatial window arguments. `FIRST_VALUE(shape)
OVER (ORDER BY id ROWS BETWEEN 1 PRECEDING AND CURRENT ROW)` is admitted
through `FirstOrLastValue`'s AnyDataType signature. Once enough input batches
advance the analytic trim frontier (more than 257 block boundaries),
`AnalyticSinkLocalState::_remove_unused_rows` calls `erase(0, remove_rows)` on
its aggregate input columns. This class inherits `IColumn::erase`, which throws
`Method erase is not supported for ColumnSpatial`; nullable inputs reach it
through `ColumnNullable::erase` too. Implement row-range erasure while
retaining valid arena ownership, and cover FIRST_VALUE/LAST_VALUE with enough
batches to trigger trimming.
--
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]
