Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20602 )
Change subject: WIP: port critical geospatial functions to c++ ...................................................................... Patch Set 8: (48 comments) gerrit-auto-critic failed. You can reproduce it locally using command: python3 bin/jenkins/critique-gerrit-review.py --dryrun To run it, you might need a virtual env with Python3's venv installed. http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/formatted-double.h File be/src/exprs/geo/formatted-double.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/formatted-double.h@96 PS8, Line 96: // https://github.com/Esri/geometry-api-java/blob/d9ed3598b72029c9ebde024e0e616933cff81db2/src/main/java/com/esri/core/geometry/OperatorExportToWktLocal.java#L797 line too long (164 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.h File be/src/exprs/geo/geometry-wrapper.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.h@35 PS8, Line 35: // types: https://github.com/Esri/geometry-api-java/blob/d9ed3598b72029c9ebde024e0e616933cff81db2/src/main/java/com/esri/core/geometry/Geometry.java#L49 line too long (152 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.h@36 PS8, Line 36: // serialized formats: https://github.com/Esri/geometry-api-java/blob/master/src/main/java/com/esri/core/geometry/OperatorExportToESRIShapeCursor.java#L404 line too long (155 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.cc File be/src/exprs/geo/geometry-wrapper.cc: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.cc@28 PS8, Line 28: bool GeometryWrapper::FromEsriBinary(FunctionContext* ctx, const StringVal& geom, OGCType ogcType) { line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.cc@150 PS8, Line 150: string msg = Substitute("GeometryWrapper::FromWkt: Geometry type $0 not supported.", ogcType); line too long (102 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.cc@160 PS8, Line 160: bool CoordinatesToPoints(FunctionContext* ctx, int num_coords, const DoubleVal* coords, vector<point2d>& points) { line too long (114 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geometry-wrapper.cc@174 PS8, Line 174: bool GeometryWrapper::FromCoordinates(FunctionContext* ctx, int num_coords, const DoubleVal* coords, OGCType ogcType) { line too long (119 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc File be/src/exprs/geo/geospatial-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@134 PS8, Line 134: StringVal GeospatialFunctions::st_LineString(FunctionContext* ctx, int num_coords, const DoubleVal* coords) { line too long (109 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@136 PS8, Line 136: if (!wrapper.FromCoordinates(ctx, num_coords, coords, ST_LINESTRING)) return StringVal::null(); line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@140 PS8, Line 140: StringVal GeospatialFunctions::st_MultiLineString(FunctionContext* ctx, const StringVal& wkt) { line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@153 PS8, Line 153: StringVal GeospatialFunctions::st_Polygon(FunctionContext* ctx, int num_coords, const DoubleVal* coords) { line too long (106 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@155 PS8, Line 155: if (!wrapper.FromCoordinates(ctx, num_coords, coords, ST_POLYGON)) return StringVal::null(); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@159 PS8, Line 159: StringVal GeospatialFunctions::st_MultiPolygon(FunctionContext* ctx, const StringVal& wkt) { line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@173 PS8, Line 173: StringVal GeospatialFunctions::st_MultiPoint(FunctionContext* ctx, int num_coords, const DoubleVal* coords) { line too long (109 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@175 PS8, Line 175: if (!wrapper.FromCoordinates(ctx, num_coords, coords, ST_MULTIPOINT)) return StringVal::null(); line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@244 PS8, Line 244: StringVal GeospatialFunctions::st_GeomFromText(FunctionContext* ctx, const StringVal& wkt) { line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@305 PS8, Line 305: StringVal GeospatialFunctions::st_BinenvelopeBinId(FunctionContext* ctx, const BigIntVal& bin_size, line too long (99 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@312 PS8, Line 312: StringVal GeospatialFunctions::st_BinenvelopeBinId(FunctionContext* ctx, const DoubleVal& bin_size, line too long (99 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@319 PS8, Line 319: StringVal GeospatialFunctions::st_BinenvelopeGeom(FunctionContext* ctx, const BigIntVal& bin_size, line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@333 PS8, Line 333: StringVal GeospatialFunctions::st_BinenvelopeGeom(FunctionContext* ctx, const DoubleVal& bin_size, line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@347 PS8, Line 347: StringVal GeospatialFunctions::st_BinenvelopeWkt(FunctionContext* ctx, const BigIntVal& bin_size, line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/geospatial-functions-ir.cc@358 PS8, Line 358: StringVal GeospatialFunctions::st_BinenvelopeWkt(FunctionContext* ctx, const DoubleVal& bin_size, line too long (97 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/multi-point-shape-format.cc File be/src/exprs/geo/multi-point-shape-format.cc: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/multi-point-shape-format.cc@23 PS8, Line 23: bool MultiPointShapeFormat::Read(FunctionContext* ctx, const StringVal& geom, multipoint2d& out) { line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.h File be/src/exprs/geo/poly-line-shape-format.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.h@40 PS8, Line 40: static void WritePolygonRings(const polygon2d& polygon, int point_array_offset, StringVal& result, line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.h@50 PS8, Line 50: static StringVal Write(FunctionContext* ctx, const multi_linestring2d& multi_linestring); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc File be/src/exprs/geo/poly-line-shape-format.cc: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@79 PS8, Line 79: string msg = Substitute("PolyLine has invalid part index. First: $0 last: $1 num_point $2 num_parts: $3", line too long (111 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@115 PS8, Line 115: FunctionContext* ctx, const GeometryT& geom, OGCType ogcType, int num_parts, int num_points) { line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@116 PS8, Line 116: int size = MIN_NON_POINT_SIZE + NUM_PARTS_SIZE + NUM_POINTS_SIZE + 4 * num_parts + 16 * num_points; line too long (101 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@134 PS8, Line 134: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const linestring2d& linestring) { line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@151 PS8, Line 151: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const multi_linestring2d& multi_linestring) { line too long (104 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@158 PS8, Line 158: StringVal result = WriteHeader(ctx, multi_linestring, ST_MULTILINESTRING, num_parts, num_points); line too long (99 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@171 PS8, Line 171: void IncreasePartAndPointCount(const polygon2d& polygon, int* part_count, int* point_count) { line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@179 PS8, Line 179: void PolyLineShapeFormat::WritePolygonRings(const polygon2d& polygon, int point_array_offset, StringVal& result, line too long (112 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@187 PS8, Line 187: writeToGeom<uint32_t>(*points_written, result, PARTS_ARRAY_OFFSET + *parts_written * 4); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@209 PS8, Line 209: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const multi_polygon2d& mpolygon) { line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/poly-line-shape-format.cc@221 PS8, Line 221: WritePolygonRings(polygon, point_array_offset, result, &parts_written, &points_written); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.h File be/src/exprs/geo/relation-wrapper.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.h@59 PS8, Line 59: BooleanVal EvalInner2(FunctionContext* ctx, const lhs_geometry_t& lhs, const rhs_geometry_t& rhs); line too long (100 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.cc File be/src/exprs/geo/relation-wrapper.cc: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.cc@174 PS8, Line 174: using rtree_t = boost::geometry::index::rtree<rtree_val_t, boost::geometry::index::quadratic<16>>; line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.cc@184 PS8, Line 184: bool treeAssistedIntersect(const multi_polygon2d& mpoly, const polygon2d& poly, const rtree_t& rtree) { line too long (103 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/relation-wrapper.cc@224 PS8, Line 224: BooleanVal RelationWrapper::EvalInner2(FunctionContext* ctx, const lhs_geometry_t& lhs, const rhs_geometry_t& rhs) { line too long (116 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/shape-format.h File be/src/exprs/geo/shape-format.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/shape-format.h@295 PS8, Line 295: inline void writeHeader(StringVal& res, OGCType ogc_type, const box2d& bounding_rect, uint32_t srid = 0) { line too long (106 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/utils.h File be/src/exprs/geo/utils.h: http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/utils.h@28 PS8, Line 28: // https://github.com/Esri/spatial-framework-for-hadoop/blob/7226df669cbfaaf1edbfac0461acd1af45e12b81/hive/src/main/java/com/esri/hadoop/hive/BinUtils.java#L5 line too long (158 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/utils.h@148 PS8, Line 148: inline bool RingsToPolygon(FunctionContext* ctx, vector<vector<point2d>>& rings, polygon2d& polygon) { line too long (102 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/be/src/exprs/geo/utils.h@175 PS8, Line 175: inline bool RingsToMultiPolygon(FunctionContext* ctx, vector<vector<point2d>>& rings, multi_polygon2d& mpolygon) { line too long (114 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java File fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java: http://gerrit.cloudera.org:8080/#/c/20602/8/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@270 PS8, Line 270: addNative(db, fnName, "_Binary_Binary", false, Type.BOOLEAN, Type.BINARY, Type.BINARY); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/20602/8/tests/custom_cluster/test_geospatial_library.py File tests/custom_cluster/test_geospatial_library.py: http://gerrit.cloudera.org:8080/#/c/20602/8/tests/custom_cluster/test_geospatial_library.py@51 PS8, Line 51: @ flake8: E303 too many blank lines (2) http://gerrit.cloudera.org:8080/#/c/20602/8/tests/query_test/test_geospatial_functions.py File tests/query_test/test_geospatial_functions.py: http://gerrit.cloudera.org:8080/#/c/20602/8/tests/query_test/test_geospatial_functions.py@36 PS8, Line 36: @ flake8: E303 too many blank lines (2) http://gerrit.cloudera.org:8080/#/c/20602/8/tests/query_test/test_geospatial_functions.py@37 PS8, Line 37: d flake8: F811 redefinition of unused 'add_test_dimensions' from line 28 -- To view, visit http://gerrit.cloudera.org:8080/20602 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I692ebc13617c37d61f77fffb09e872095a0fd11c Gerrit-Change-Number: 20602 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 10 Dec 2025 15:09:38 +0000 Gerrit-HasComments: Yes
