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 2:

(57 comments)

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h
File be/src/exprs/geo/common.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@50
PS2, Line 50: // see 
https://github.com/Esri/spatial-framework-for-hadoop/blob/7226df669cbfaaf1edbfac0461acd1af45e12b81/hive/src/main/java/com/esri/hadoop/hive/GeometryUtils.java#L21
line too long (168 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@53
PS2, Line 53:           ST_POINT = 1,
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@54
PS2, Line 54:           ST_LINESTRING = 2,
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@55
PS2, Line 55:           ST_POLYGON = 3,
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@56
PS2, Line 56:           ST_MULTIPOINT = 4,
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@57
PS2, Line 57:           ST_MULTILINESTRING = 5,
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/common.h@58
PS2, Line 58:           ST_MULTIPOLYGON = 6
tab used for whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h
File be/src/exprs/geo/formatted-double.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/formatted-double.h@96
PS2, 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/2/be/src/exprs/geo/geometry-wrapper.h
File be/src/exprs/geo/geometry-wrapper.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.h@35
PS2, 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/2/be/src/exprs/geo/geometry-wrapper.h@36
PS2, 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/2/be/src/exprs/geo/geometry-wrapper.cc
File be/src/exprs/geo/geometry-wrapper.cc:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.cc@28
PS2, Line 28: bool GeometryWrapper::FromEsriBinary(FunctionContext* ctx, const 
StringVal& geom, OGCType ogcType) {
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geometry-wrapper.cc@150
PS2, 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/2/be/src/exprs/geo/geometry-wrapper.cc@160
PS2, 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/2/be/src/exprs/geo/geometry-wrapper.cc@174
PS2, 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/2/be/src/exprs/geo/geospatial-functions-ir.cc
File be/src/exprs/geo/geospatial-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@131
PS2, Line 131: StringVal GeospatialFunctions::st_LineString(FunctionContext* 
ctx, int num_coords, const DoubleVal* coords) {
line too long (109 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@133
PS2, Line 133:   if (!wrapper.FromCoordinates(ctx, num_coords, coords, 
ST_LINESTRING)) return StringVal::null();
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@137
PS2, Line 137: StringVal 
GeospatialFunctions::st_MultiLineString(FunctionContext* ctx, const StringVal& 
wkt) {
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@150
PS2, Line 150: StringVal GeospatialFunctions::st_Polygon(FunctionContext* ctx, 
int num_coords, const DoubleVal* coords) {
line too long (106 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@152
PS2, Line 152:   if (!wrapper.FromCoordinates(ctx, num_coords, coords, 
ST_POLYGON)) return StringVal::null();
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@156
PS2, Line 156: StringVal GeospatialFunctions::st_MultiPolygon(FunctionContext* 
ctx, const StringVal& wkt) {
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@170
PS2, Line 170: StringVal GeospatialFunctions::st_MultiPoint(FunctionContext* 
ctx, int num_coords, const DoubleVal* coords) {
line too long (109 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@172
PS2, Line 172:   if (!wrapper.FromCoordinates(ctx, num_coords, coords, 
ST_MULTIPOINT)) return StringVal::null();
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@181
PS2, Line 181:   //   
https://github.com/apache/hive/blob/9eeab40173479c74b6fbf6657c3472b81ce4efcd/ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_EnvIntersects.java#L63
line too long (156 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@325
PS2, Line 325: StringVal GeospatialFunctions::st_GeomFromText(FunctionContext* 
ctx, const StringVal& wkt) {
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@386
PS2, Line 386: StringVal 
GeospatialFunctions::st_BinenvelopeBinId(FunctionContext* ctx, const BigIntVal& 
bin_size,
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@393
PS2, Line 393: StringVal 
GeospatialFunctions::st_BinenvelopeBinId(FunctionContext* ctx, const DoubleVal& 
bin_size,
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@400
PS2, Line 400: StringVal 
GeospatialFunctions::st_BinenvelopeGeom(FunctionContext* ctx, const BigIntVal& 
bin_size,
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@414
PS2, Line 414: StringVal 
GeospatialFunctions::st_BinenvelopeGeom(FunctionContext* ctx, const DoubleVal& 
bin_size,
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@428
PS2, Line 428: StringVal 
GeospatialFunctions::st_BinenvelopeWkt(FunctionContext* ctx, const BigIntVal& 
bin_size,
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/geospatial-functions-ir.cc@439
PS2, Line 439: StringVal 
GeospatialFunctions::st_BinenvelopeWkt(FunctionContext* ctx, const DoubleVal& 
bin_size,
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/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/2/be/src/exprs/geo/multi-point-shape-format.cc@23
PS2, Line 23: bool MultiPointShapeFormat::Read(FunctionContext* ctx, const 
StringVal& geom, multipoint2d& out) {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/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/2/be/src/exprs/geo/poly-line-shape-format.h@40
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.h@50
PS2, Line 50:   static StringVal Write(FunctionContext* ctx, const 
multi_linestring2d& multi_linestring);
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/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/2/be/src/exprs/geo/poly-line-shape-format.cc@79
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@115
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@116
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@134
PS2, Line 134: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const 
linestring2d& linestring) {
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/poly-line-shape-format.cc@151
PS2, Line 151: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const 
multi_linestring2d& multi_linestring) {
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/poly-line-shape-format.cc@158
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@171
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@179
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@187
PS2, 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/2/be/src/exprs/geo/poly-line-shape-format.cc@209
PS2, Line 209: StringVal PolyLineShapeFormat::Write(FunctionContext* ctx, const 
multi_polygon2d& mpolygon) {
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/poly-line-shape-format.cc@221
PS2, Line 221:     WritePolygonRings(polygon, point_array_offset, result, 
&parts_written, &points_written);
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/relation-wrapper.h
File be/src/exprs/geo/relation-wrapper.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/relation-wrapper.h@59
PS2, 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/2/be/src/exprs/geo/relation-wrapper.cc
File be/src/exprs/geo/relation-wrapper.cc:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/relation-wrapper.cc@174
PS2, 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/2/be/src/exprs/geo/relation-wrapper.cc@184
PS2, 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/2/be/src/exprs/geo/relation-wrapper.cc@224
PS2, 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/2/be/src/exprs/geo/shape-format.h
File be/src/exprs/geo/shape-format.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/shape-format.h@45
PS2, Line 45: // See 
https://github.com/Esri/geometry-api-java/blob/d9ed3598b72029c9ebde024e0e616933cff81db2/src/main/java/com/esri/core/geometry/ShapeType.java#L27
line too long (150 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/shape-format.h@177
PS2, Line 177: 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/2/be/src/exprs/geo/utils.h
File be/src/exprs/geo/utils.h:

http://gerrit.cloudera.org:8080/#/c/20602/2/be/src/exprs/geo/utils.h@28
PS2, 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/2/be/src/exprs/geo/utils.h@148
PS2, 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/2/be/src/exprs/geo/utils.h@175
PS2, 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/2/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/2/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@62
PS2, Line 62:   private final static Logger LOG = 
LoggerFactory.getLogger(HiveEsriGeospatialBuiltins.class);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@103
PS2, Line 103:         new ST_MinX(), new ST_MinY(), new ST_Point(),
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/20602/2/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveEsriGeospatialBuiltins.java@266
PS2, Line 266:     addNative(db, fnName, "_Binary_Binary", false, Type.BOOLEAN, 
Type.BINARY, Type.BINARY);
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/20602/2/tests/custom_cluster/test_geospatial_library.py
File tests/custom_cluster/test_geospatial_library.py:

http://gerrit.cloudera.org:8080/#/c/20602/2/tests/custom_cluster/test_geospatial_library.py@61
PS2, Line 61:
flake8: W391 blank line at end of file



--
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: 2
Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Oct 2023 14:52:23 +0000
Gerrit-HasComments: Yes

Reply via email to