Supported PostGIS Function: STNumPoints for Geospatial datatypes#4510
Conversation
Pull Request Test Coverage Report for Build 22109069228Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
|
||
| geom_type := ST_GeometryType(geom); | ||
|
|
||
| IF geom_type = 'ST_Point' THEN |
There was a problem hiding this comment.
Does this mean Postgis is not able to handle empty geometry and Point?
There was a problem hiding this comment.
done, i have fixed it
|
|
||
| IF geom_type = 'ST_Point' THEN | ||
| RETURN 1; | ||
| ELSIF geom_type IN ('ST_LineString', 'ST_Polygon', 'ST_MultiLineString', 'ST_MultiPolygon') THEN |
There was a problem hiding this comment.
Is this condition because postgis only supports these geometries or is it that we support these for now? If later than IMO we should instead directly call STNumPoints_helper for all geometry types as otherwise we would need to update this function each time we support a new geometry type.
There was a problem hiding this comment.
done i have fixed it
There was a problem hiding this comment.
IMO, we should create a new set of test files now, something like Test-spatial-functions-3-vu-*
There was a problem hiding this comment.
done i have created
| AS $$ | ||
| BEGIN | ||
| IF sys.STIsValid(geog) = 0 THEN | ||
| RAISE EXCEPTION 'The geography instance is not valid. Use MakeValid() to convert to valid geography.'; |
There was a problem hiding this comment.
The error message is not same as in geography.sql. Please check!
There was a problem hiding this comment.
done i have fixed it
| Function sys.stintersects_helper(sys.geometry,sys.geometry) | ||
| Function sys.stisclosed_helper(sys.geography) | ||
| Function sys.stisclosed_helper(sys.geometry) | ||
| Function sys.stnumpoints_helper(sys.geography) |
There was a problem hiding this comment.
We can remove this by creating a dependency for STNumPoints function eg. CREATE VIEW. Please verify this for all functions.
| BEGIN | ||
| IF sys.STIsValid(geog) = 0 THEN | ||
| RAISE EXCEPTION 'The geography instance is not valid'; | ||
| ELSE |
There was a problem hiding this comment.
Don't we need to check whether geography is empty or not?
There was a problem hiding this comment.
actually it is handled by the c functions
| IF STIsValid(geom) = 0 THEN | ||
| RAISE EXCEPTION 'The geometry instance is not valid'; | ||
| ELSE | ||
| RETURN sys.STNumPoints_helper(geom); |
There was a problem hiding this comment.
Don't we need to check whether geometry is empty or not?
There was a problem hiding this comment.
actually it is handled by the c functions
| @@ -0,0 +1,42 @@ | |||
| CREATE DATABASE TestSTNumPoints_DB; | |||
There was a problem hiding this comment.
Why is the test file .txt and not .sql?
There was a problem hiding this comment.
This allows us to add JDBC test directives (like prepst#!# for prepared statement testing)
|
|
||
| USE TestSTNumPoints_DB; | ||
|
|
||
| USE MASTER |
There was a problem hiding this comment.
What's the point of creating TestSTNumPoints_DB? We are simply creating the db and not using it. All the objects are created inside master db. So what's the point?
There was a problem hiding this comment.
fixed The database now contains the test tables and corresponding tests have been added in the verify file.
| @@ -0,0 +1,161 @@ | |||
| --STNumPoints() | |||
There was a problem hiding this comment.
Please add some comments on the test cases as to what we are testing. For eg: invalid input, null input, empty input, negative tests etc.
There was a problem hiding this comment.
Done Added comments
| @@ -0,0 +1,13 @@ | |||
| USE TestSTNumPoints_DB | |||
|
|
|||
| USE MASTER | |||
There was a problem hiding this comment.
Again, what's the point of creating TestSTNumPoints_DB?
There was a problem hiding this comment.
fixed The database now contains the test tables and corresponding tests have been added in the verify file.
4c4798e
into
babelfish-for-postgresql:BABEL_5_X_DEV
…elfish-for-postgresql#4510) Implemented the STNumPoints GeoSpatial TSQL function that was previously unsupported in Babelfish. BABEL-6329 --------- Co-authored-by: Gopal Verma <[email protected]>
…elfish-for-postgresql#4510) Implemented the STNumPoints GeoSpatial TSQL function that was previously unsupported in Babelfish. BABEL-6329 --------- Co-authored-by: Gopal Verma <[email protected]>
…elfish-for-postgresql#4510) Implemented the STNumPoints GeoSpatial TSQL function that was previously unsupported in Babelfish. BABEL-6329 --------- Co-authored-by: Gopal Verma <[email protected]>
…) (#4676) Implemented the STNumPoints GeoSpatial TSQL function that was previously unsupported in Babelfish. ## What's Changed ### Function Implementation - Added `sys.STNumPoints(geog sys.GEOGRAPHY)` function in `geography.sql` - Delegates to `sys.STNumPoints_geography_helper()` for valid, non-empty instances - Function is marked as `IMMUTABLE STRICT PARALLEL SAFE` for optimal performance ### Error Handling - Returns exception for invalid geography instances - Returns 0 for empty geography instances - Returns actual point count for valid instances --- ## Function Behavior | Input | Output | |-------|--------| | Valid geography instance | Integer count of points | | Empty geography instance | 0 | | Invalid geography instance | Exception | --- ## Examples **Point** DECLARE @point geography; SET @point = geography::STPointFromText('POINT(-122.34900 47.65100)', 4326); SELECT @point.STNumPoints() AS NumPoints; -- Output: 1 **LineString** DECLARE @line geography; SET @line = geography::STGeomFromText('LINESTRING(-122.360 47.656, -122.343 47.656, -122.310 47.690)', 4326); SELECT @line.STNumPoints() AS NumPoints; -- Output: 3 **Polygon** DECLARE @polygon geography; SET @polygon = geography::STGeomFromText('POLYGON((-122.358 47.653, -122.348 47.649, -122.348 47.658, -122.358 47.658, -122.358 47.653))', 4326); SELECT @polygon.STNumPoints() AS NumPoints; -- Output: 5 **Empty Instance** DECLARE @empty geography; SET @empty = geography::STGeomFromText('POINT EMPTY', 4326); SELECT @empty.STNumPoints() AS NumPoints; -- Output: 0 Task: BABEL-6329 Signed-off by: Gopal verma <[email protected]>
Implement STNumPoints GeoSpatial SQL Function
Implemented the STNumPoints GeoSpatial TSQL function that was previously unsupported in Babelfish.
What's Changed
Function Implementation
sys.STNumPoints(geog sys.GEOGRAPHY)function ingeography.sqlsys.STNumPoints_geography_helper()for valid, non-empty instancesIMMUTABLE STRICT PARALLEL SAFEfor optimal performanceError Handling
Function Behavior
Examples
Point
LineString
Polygon
Empty Instance
Issues Resolved
Task: BABEL-6329
Signed-off by: Gopal verma [email protected]
Authored by: Gopal verma [email protected]
Test Coverage
Checklist
--signoffBy submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.