github-actions[bot] commented on code in PR #69032:
URL: https://github.com/apache/doris/pull/69032#discussion_r4290141166
##########
be/src/exprs/function/geo/functions_geo.cpp:
##########
@@ -32,18 +44,288 @@
#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 == 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);
+ if (is_spatial_type(type) && *shape == nullptr) {
+ 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 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 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() == GEO_SHAPE_POINT ? lhs : rhs;
+ const auto& other = lhs.type() == GEO_SHAPE_POINT ? rhs : lhs;
+ const auto point =
+ canonical_geography_point(*assert_cast<const
GeoPoint&>(point_shape).point());
+ bool interior = false;
+ bool boundary = false;
+ if (other.type() == GEO_SHAPE_POINT) {
+ interior = point == canonical_geography_point(*assert_cast<const
GeoPoint&>(other).point());
+ } else if (other.type() == 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)),
+ S1ChordAngle::Zero()) <= 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_EQ(other.type(), 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)),
+ S1ChordAngle::Zero()) <= 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() !=
GEO_SHAPE_POINT) ||
+ (name == "st_touches" && lhs.type() != GEO_SHAPE_POINT && rhs.type()
!= GEO_SHAPE_POINT)) {
+ return Status::NotSupported("Function {} does not support this
Geography shape pair", name);
+ }
+ if (lhs.type() == GEO_SHAPE_POINT || rhs.type() == 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);
+ DCHECK(name == "st_intersects" || name == "st_disjoint");
+ *result = name == "st_intersects" ? intersects : !intersects;
+ return Status::OK();
+}
+
+static S2LatLng geography_point_coordinate(StringRef wkb) {
+ double longitude;
+ double latitude;
+ const auto status = WkbParse::point_coordinates(wkb.data, wkb.size,
&longitude, &latitude);
+ DCHECK_EQ(status, GEO_PARSE_OK);
+ // Use original coordinates: reconstructing a shared longitude from two S2
vectors
+ // can turn due north into a tiny negative bearing that wraps to almost
2*pi.
+ if (std::abs(latitude) == 90) {
+ longitude = 0;
+ } else if (longitude == 180) {
+ longitude = -180;
+ }
+ return S2LatLng::FromDegrees(latitude, longitude);
+}
+
+static bool geography_bearing(StringRef from, StringRef to, double* bearing) {
+ const auto start = geography_point_coordinate(from);
+ const auto end = geography_point_coordinate(to);
+ // Coincident or antipodal positions have no unique initial bearing.
+ if (start == end || start.GetDistance(end).radians() == M_PI) {
Review Comment:
[P2] Detect antipodes without equality on rounded haversine distance. The
newly added guard calls S2LatLng::GetDistance, whose pinned [S2 v0.10.0
implementation](https://raw.githubusercontent.com/google/s2geometry/v0.10.0/src/s2/s2latlng.cc)
uses a haversine formula with reduced precision near antipodes. For POINT(0 0)
-> POINT(179.999999 0), its intermediate rounds to 1 and the distance becomes
exactly pi, so typed ST_Azimuth returns NULL although the unique initial
bearing is pi/2. Conversely the exact antipodes POINT(0 10) -> POINT(180 -10)
produce 3.1415926237874707 and pass this guard. ST_Angle shares the same
helper. Check antipodality from the canonical original coordinate relationship
instead of comparing this approximate distance to pi; a distance tolerance
would preserve the false-NULL problem. Cover exact antipodes at varied
latitudes and nearby non-antipodes.
##########
be/src/exprs/function/geo/functions_geo.cpp:
##########
@@ -845,122 +1363,46 @@ struct StDistance {
auto null_map = ColumnUInt8::create(size, 0);
auto& null_map_data = null_map->get_data();
- if (left_const) {
- const_vector(left_column, right_column, res, null_map_data, size);
- } else if (right_const) {
- vector_const(left_column, right_column, res, null_map_data, size);
- } else {
- vector_vector(left_column, right_column, res, null_map_data, size);
- }
- block.replace_by_position(result,
- ColumnNullable::create(std::move(res),
std::move(null_map)));
- return Status::OK();
- }
-
-private:
- static bool decode_shape(const StringRef& value,
std::unique_ptr<GeoShape>& shape) {
- shape = GeoShape::from_encoded(value.data, value.size);
- return static_cast<bool>(shape);
- }
-
- static void loop_do(StringRef& lhs_value, StringRef& rhs_value,
- std::vector<std::unique_ptr<GeoShape>>& shapes,
- ColumnFloat64::MutablePtr& res, NullMap& null_map, int
row) {
- StringRef* strs[2] = {&lhs_value, &rhs_value};
- for (int i = 0; i < 2; ++i) {
- if (!decode_shape(*strs[i], shapes[i])) {
- null_map[row] = 1;
- res->insert_default();
- return;
- }
- }
- double distance = shapes[0]->Distance(shapes[1].get());
- if (UNLIKELY(distance < 0)) {
- null_map[row] = 1;
- res->insert_default();
- return;
- }
- res->insert_value(distance);
- }
-
- static void const_vector(const ColumnPtr& left_column, const ColumnPtr&
right_column,
- ColumnFloat64::MutablePtr& res, NullMap&
null_map, const size_t size) {
- const auto* left_string = assert_cast<const
ColumnString*>(left_column.get());
- const auto* right_string = assert_cast<const
ColumnString*>(right_column.get());
-
- auto lhs_value = left_string->get_data_at(0);
- std::unique_ptr<GeoShape> lhs_shape;
- if (!decode_shape(lhs_value, lhs_shape)) {
- for (int row = 0; row < size; ++row) {
- null_map[row] = 1;
- res->insert_default();
- }
- return;
- }
-
- std::unique_ptr<GeoShape> rhs_shape;
- for (int row = 0; row < size; ++row) {
- auto rhs_value = right_string->get_data_at(row);
- if (!decode_shape(rhs_value, rhs_shape)) {
- null_map[row] = 1;
+ std::unique_ptr<GeoShape> left_shape;
+ std::unique_ptr<GeoShape> right_shape;
+ bool left_decoded = false;
+ bool right_decoded = false;
+ for (size_t row = 0; row < size; ++row) {
+ // Propagate NULL before using nested placeholders; a NULL
geometry has an
+ // empty payload, not a WKB shape.
+ if (any_geo_argument_is_null(block, arguments, row)) {
+ null_map_data[row] = 1;
res->insert_default();
continue;
}
- double distance = lhs_shape->Distance(rhs_shape.get());
- if (UNLIKELY(distance < 0)) {
- null_map[row] = 1;
- res->insert_default();
- continue;
+ if (!left_const || !left_decoded) {
+ RETURN_IF_ERROR(
+
decode_calculation_shape(left_column->get_data_at(left_const ? 0 : row),
+ left_type, &left_shape,
NAME));
+ left_decoded = true;
}
- res->insert_value(distance);
- }
- }
-
- static void vector_const(const ColumnPtr& left_column, const ColumnPtr&
right_column,
- ColumnFloat64::MutablePtr& res, NullMap&
null_map, const size_t size) {
- const auto* left_string = assert_cast<const
ColumnString*>(left_column.get());
- const auto* right_string = assert_cast<const
ColumnString*>(right_column.get());
-
- auto rhs_value = right_string->get_data_at(0);
- std::unique_ptr<GeoShape> rhs_shape;
- if (!decode_shape(rhs_value, rhs_shape)) {
- for (int row = 0; row < size; ++row) {
- null_map[row] = 1;
- res->insert_default();
+ if (!right_const || !right_decoded) {
+ RETURN_IF_ERROR(
+
decode_calculation_shape(right_column->get_data_at(right_const ? 0 : row),
+ right_type, &right_shape,
NAME));
+ right_decoded = true;
}
- return;
- }
-
- std::unique_ptr<GeoShape> lhs_shape;
- for (int row = 0; row < size; ++row) {
- auto lhs_value = left_string->get_data_at(row);
- if (!decode_shape(lhs_value, lhs_shape)) {
- null_map[row] = 1;
+ if (!left_shape || !right_shape) {
+ null_map_data[row] = 1;
res->insert_default();
continue;
}
- double distance = lhs_shape->Distance(rhs_shape.get());
+ const double distance = left_shape->Distance(right_shape.get());
Review Comment:
[P2] Handle empty Geography polygons before computing distance. The new
typed overload accepts POLYGON EMPTY (`010300000000000000`), and readPolygon
constructs a non-null GeoPolygon with zero loops, so decode_calculation_shape
lets it through. Consequently ST_Distance(empty_polygon, point) returns
+infinity, while ST_Distance(empty_polygon, empty_polygon) returns
numeric_limits<double>::max(): the distance helpers never update their initial
sentinels and this path only rejects negative values. Empty Point/LineString
already fail explicitly. Reject unsupported empty calculation shapes at the
typed boundary (or define a consistent NULL result), and cover both argument
orders plus empty/empty; raw EMPTY storage/accessors should continue to work.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorSpatialWriteValidator.java:
##########
@@ -0,0 +1,119 @@
+// 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.
+
+package org.apache.doris.datasource.connector.converter;
+
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.CaseWhen;
+import org.apache.doris.nereids.trees.expressions.Cast;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.Coalesce;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.If;
+import org.apache.doris.nereids.types.DataType;
+
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * Preserves the kind and parameters of spatial connector columns before sink
coercion.
+ * A spatial value is an opaque WKB payload at execution time, so a generic
cast cannot
+ * safely convert between CRS, edge algorithms, or Geometry and Geography.
+ */
+public final class ConnectorSpatialWriteValidator {
+ private ConnectorSpatialWriteValidator() {
+ }
+
+ public static void validate(List<Column> targetColumns,
+ List<? extends NamedExpression> sourceColumns) {
+ if (targetColumns.size() != sourceColumns.size()) {
+ throw new AnalysisException("Spatial connector write target and
source columns are not aligned");
+ }
+ for (int i = 0; i < targetColumns.size(); i++) {
+ validate(targetColumns.get(i), sourceColumns.get(i));
+ }
+ }
+
+ public static void validate(Column targetColumn, Expression source) {
+ Type targetType = targetColumn.getType();
+ if (targetType instanceof ScalarType && ((ScalarType)
targetType).isSpatialType()) {
+ validateExpression(source, (ScalarType) targetType,
targetColumn.getName());
+ } else if (targetType.typeContainsSpatialType()) {
+ throw new AnalysisException("Nested spatial columns are not
supported for connector writes: "
+ + targetColumn.getName());
+ }
+ }
+
+ private static void validateExpression(Expression expression, ScalarType
targetType, String columnName) {
+ if (expression instanceof Alias) {
+ validateExpression(expression.child(0), targetType, columnName);
+ return;
+ }
+ DataType targetDataType = DataType.fromCatalogType(targetType);
+ if (expression instanceof If &&
expression.getDataType().equals(targetDataType)) {
+ If branch = (If) expression;
+ validateExpression(branch.getTrueValue(), targetType, columnName);
+ validateExpression(branch.getFalseValue(), targetType, columnName);
+ return;
+ }
+ if (expression instanceof Cast &&
expression.getDataType().equals(targetDataType)) {
+ validateExpression(expression.child(0), targetType, columnName);
+ return;
+ }
+ if (expression instanceof CaseWhen &&
expression.getDataType().equals(targetDataType)) {
+ CaseWhen caseWhen = (CaseWhen) expression;
+ caseWhen.getWhenClauses().forEach(clause ->
+ validateExpression(clause.getResult(), targetType,
columnName));
+ caseWhen.getDefaultValue().ifPresent(value ->
validateExpression(value, targetType, columnName));
+ return;
+ }
+ if (expression instanceof Coalesce &&
expression.getDataType().equals(targetDataType)) {
+ expression.children().forEach(child -> validateExpression(child,
targetType, columnName));
+ return;
+ }
+ validateType(expression.getDataType().toCatalogDataType(), targetType,
columnName);
Review Comment:
[P2] Accept typed NULL literals before checking spatial identity. For a
nullable default-GEOMETRY target, INSERT VALUES with CAST(NULL AS
GEOMETRY('EPSG:3857')) is rejected with a CRS mismatch; CAST(NULL AS INT) is
likewise rejected as nonspatial. Reduced plan: `Sink[geom:Geometry(CRS84)] ->
InlineTable[Alias(Cast(NULL,Geometry(EPSG:3857)))]`.
InsertUtils.normalizePlan's buildExprAnalyzer.visitCast folds the inner cast to
NullLiteral(Geometry(EPSG:3857)), but this call discards the literal identity
and validateType checks only its declared CRS/type. The later castValue ->
castIfNotSameType would correctly create a target-typed NULL because it checks
input.isNullLiteral first. Accept expression-level NullLiteral after unwrapping
Alias, while retaining identity checks for actual values, and cover typed
spatial/nonspatial NULLs through INSERT VALUES normalization.
##########
be/src/core/column/column_spatial.cpp:
##########
@@ -0,0 +1,294 @@
+// 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.
+
+#include "core/column/column_spatial.h"
+
+#include <crc32c/crc32c.h>
+
+#include <algorithm>
+#include <cstring>
+
+#include "common/exception.h"
+#include "core/column/columns_common.h"
+#include "exec/common/sip_hash.h"
+#include "exec/sort/sort_block.h"
+#include "util/hash_util.hpp"
+
+namespace doris {
+
+namespace {
+
+template <typename HashValue>
+void for_each_spatial_value(const ColumnSpatial& column, size_t start, size_t
end,
+ const uint8_t* null_map, HashValue&& hash_value) {
+ for (size_t row = start; row < end; ++row) {
+ if (null_map == nullptr || null_map[row] == 0) {
+ hash_value(row, column.get_data_at(row));
+ }
+ }
+}
+
+} // namespace
+
+void ColumnSpatial::update_hash_with_value(size_t n, SipHash& hash) const {
+ const auto value = get_data_at(n);
+ hash.update(reinterpret_cast<const char*>(&value.size),
sizeof(value.size));
+ hash.update(value.data, value.size);
+}
+
+void ColumnSpatial::update_hashes_with_value(uint64_t* __restrict hashes,
+ const uint8_t* __restrict
null_data) const {
+ for_each_spatial_value(*this, 0, size(), null_data, [&](size_t row,
StringRef value) {
+ hashes[row] = HashUtil::xxHash64WithSeed(value.data, value.size,
hashes[row]);
+ });
+}
+
+void ColumnSpatial::update_xxHash_with_value(size_t start, size_t end,
uint64_t& hash,
+ const uint8_t* __restrict
null_data) const {
+ for_each_spatial_value(*this, start, end, null_data, [&](size_t, StringRef
value) {
+ hash = HashUtil::xxHash64WithSeed(value.data, value.size, hash);
+ });
+}
+
+void ColumnSpatial::update_crcs_with_value(uint32_t* __restrict hashes,
PrimitiveType /* type */,
+ uint32_t rows, uint32_t /* offset
*/,
+ const uint8_t* __restrict
null_data) const {
+ DCHECK_EQ(rows, size());
+ for_each_spatial_value(*this, 0, rows, null_data, [&](size_t row,
StringRef value) {
+ hashes[row] =
+ HashUtil::zlib_crc_hash(value.data,
cast_set<uint32_t>(value.size), hashes[row]);
+ });
+}
+
+void ColumnSpatial::update_crc_with_value(size_t start, size_t end, uint32_t&
hash,
+ const uint8_t* __restrict null_data)
const {
+ for_each_spatial_value(*this, start, end, null_data, [&](size_t, StringRef
value) {
+ hash = HashUtil::zlib_crc_hash(value.data,
cast_set<uint32_t>(value.size), hash);
+ });
+}
+
+void ColumnSpatial::update_crc32c_batch(uint32_t* __restrict hashes,
+ const uint8_t* __restrict null_map)
const {
+ for_each_spatial_value(*this, 0, size(), null_map, [&](size_t row,
StringRef value) {
+ hashes[row] = crc32c_extend(hashes[row], reinterpret_cast<const
uint8_t*>(value.data),
+ value.size);
+ });
+}
+
+void ColumnSpatial::update_crc32c_single(size_t start, size_t end, uint32_t&
hash,
+ const uint8_t* __restrict null_map)
const {
+ for_each_spatial_value(*this, start, end, null_map, [&](size_t, StringRef
value) {
+ hash = crc32c_extend(hash, reinterpret_cast<const
uint8_t*>(value.data), value.size);
+ });
+}
+
+void ColumnSpatial::insert_data(const char* pos, size_t length) {
+ if (length <= StringView::kInlineSize) {
+ _data.emplace_back(pos, cast_set<uint32_t>(length));
+ } else {
+ _data.emplace_back(_arena.insert(pos, length),
cast_set<uint32_t>(length));
+ }
+}
+
+int ColumnSpatial::compare_at(size_t n, size_t m, const IColumn& rhs,
+ int /* nan_direction_hint */) const {
+ const auto& spatial = assert_cast<const ColumnSpatial&>(rhs);
+ DCHECK_EQ(_primitive_type, spatial._primitive_type);
+ return _data[n].compare(spatial._data[m]);
+}
+
+MutableColumnPtr ColumnSpatial::clone_resized(size_t size) const {
+ auto result = create(_primitive_type);
+ const size_t copied = std::min(this->size(), size);
+ for (size_t i = 0; i < copied; ++i) {
+ const auto value = get_data_at(i);
+ result->insert_data(value.data, value.size);
+ }
+ result->insert_many_defaults(size - copied);
+ return result;
+}
+
+void ColumnSpatial::insert_range_from(const IColumn& src, size_t start, size_t
length) {
+ const auto& spatial = assert_cast<const ColumnSpatial&>(src);
+ DCHECK_EQ(_primitive_type, spatial._primitive_type);
+ if (start + length > spatial.size()) {
+ throw Exception(ErrorCode::INTERNAL_ERROR,
+ "Spatial column range start = {}, length = {} is out
of bounds for {} rows",
+ start, length, spatial.size());
+ }
+ for (size_t i = 0; i < length; ++i) {
+ const auto value = spatial.get_data_at(start + i);
+ insert_data(value.data, value.size);
+ }
+}
+
+void ColumnSpatial::insert_indices_from(const IColumn& src, const uint32_t*
begin,
+ const uint32_t* end) {
+ const auto& spatial = assert_cast<const ColumnSpatial&>(src);
+ DCHECK_EQ(_primitive_type, spatial._primitive_type);
+ for (const auto* it = begin; it != end; ++it) {
+ const auto value = spatial.get_data_at(*it);
+ insert_data(value.data, value.size);
+ }
+}
+
+bool ColumnSpatial::has_enough_capacity(const IColumn& src) const {
+ const auto& spatial = assert_cast<const ColumnSpatial&>(src);
+ return _data.capacity() - _data.size() > spatial.size();
+}
+
+ColumnPtr ColumnSpatial::filter(const IColumn::Filter& filter, ssize_t
result_size_hint) const {
+ column_match_filter_size(size(), filter.size());
+ auto result = create(_primitive_type);
+ if (result_size_hint > 0) {
+ result->_data.reserve(result_size_hint);
+ }
+ for (size_t i = 0; i < size(); ++i) {
+ if (filter[i]) {
+ const auto value = get_data_at(i);
+ result->insert_data(value.data, value.size);
+ }
+ }
+ return result;
+}
+
+size_t ColumnSpatial::filter(const IColumn::Filter& filter) {
+ column_match_filter_size(size(), filter.size());
+ auto filtered = this->filter(filter, -1);
Review Comment:
[P2] Compact spatial rows without copying their WKB twice. For an
exclusively owned column, normal residual-predicate filtering reaches this
mutable overload (also through ColumnNullable::filter). Calling the const
filter first copies every retained payload into a fresh arena, then
clear()+insert_range_from copies all of those bytes back into another arena. A
filter retaining most rows of a large Polygon/MultiPolygon block therefore
performs two full surviving-payload copies and allocates a complete temporary
payload arena just to remove a few rows. Compact the existing StringViews while
retaining their arena, or transfer the filtered storage without recopying its
WKB; the mutable path should not require both copies.
--
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]
