Implement STGeometryType for geometry and geography types#4509
Conversation
Pull Request Test Coverage Report for Build 22112542606Warning: 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 |
|
|
||
| --Geometry | ||
| CREATE OR REPLACE FUNCTION sys.STGeometryType(geom sys.GEOMETRY) | ||
| RETURNS sys.NVARCHAR |
There was a problem hiding this comment.
Documentation says that it should be nvarchar(4000) so we should stick to that.
There was a problem hiding this comment.
done, i have fixed it
| RETURN substr(geom_type, 4); | ||
| END IF; | ||
|
|
||
| RETURN geom_type; |
There was a problem hiding this comment.
I think we should raise exception in case geom_type is not in 'ST_%' format.
There was a problem hiding this comment.
done, i have raised the exception
| --Geography | ||
|
|
||
| CREATE OR REPLACE FUNCTION sys.STGeometryType(geog sys.GEOGRAPHY) | ||
| RETURNS sys.NVARCHAR |
There was a problem hiding this comment.
Same as above, it should be nvarchar(4000)
There was a problem hiding this comment.
done ,i have fixed it
| RETURN substr(geom_type, 4); | ||
| END IF; | ||
|
|
||
| RAISE EXCEPTION 'Unexpected geometry type format: %. Expected ST_* prefix.', geom_type; |
There was a problem hiding this comment.
Why do you need this ? it can be handled by ST_GeometryType internally right ?
There was a problem hiding this comment.
Rishabh asked me to raise the exception for this
| END IF; | ||
| geom_type := sys.ST_GeometryType(geog); | ||
|
|
||
| IF geom_type LIKE 'ST_%' THEN |
There was a problem hiding this comment.
'' is a wildcard in LIKE operator. Using 'ST%' means any string of the form 'ST<any_single_character>' will be valid here. I do not think we want that. Please use something like:
geom_type LIKE 'ST\_%' ESCAPE '\'
There was a problem hiding this comment.
i have fixed this
| IF STIsValid(geog) = 0 THEN | ||
| RAISE EXCEPTION 'This operation cannot be completed because the instance is not valid'; | ||
| END IF; | ||
| geom_type := sys.ST_GeometryType(geog); |
There was a problem hiding this comment.
Don't we need to check whether it is empty or not?
There was a problem hiding this comment.
actually it is handled by the c functions
| $$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; | ||
|
|
||
| --STGeomType | ||
| CREATE OR REPLACE FUNCTION sys.STGeometryType(geom sys.GEOMETRY) |
There was a problem hiding this comment.
Same comments as previous comments
There was a problem hiding this comment.
actually it is handled by the c functions
| $$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE; | ||
|
|
||
| --STGeomType | ||
| CREATE OR REPLACE FUNCTION sys.STGeometryType(geog sys.GEOGRAPHY) |
There was a problem hiding this comment.
Why the name of the function suggests geometry whereas the input is of type geography?
There was a problem hiding this comment.
The function is named STGeometryType to match T-SQL documentation
| @@ -0,0 +1,13 @@ | |||
| USE TestGeospatialMethods3_DB | |||
There was a problem hiding this comment.
Why the test file is .txt and not .sql?
There was a problem hiding this comment.
In the future, we may add special test directives like prepst#!# thats why it is the .txt not .sql
| @@ -0,0 +1,13 @@ | |||
| USE TestGeospatialMethods3_DB | |||
|
|
|||
| USE MASTER | |||
There was a problem hiding this comment.
What's the point of creating TestGeospatialMethods3_DB 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.
| @@ -0,0 +1,32 @@ | |||
| CREATE DATABASE TestGeospatialMethods3_DB; | |||
There was a problem hiding this comment.
Same comment as above^
There was a problem hiding this comment.
i have added tables and corrosponding testcases in verify.sql file
| @@ -0,0 +1,231 @@ | |||
| SELECT geometry::STGeomFromText('POINT(1 2)', 4326).STGeometryType(); | |||
There was a problem hiding this comment.
Please add comments stating what are we testing: null input, empty input, negative test case, invalid input etc
There was a problem hiding this comment.
yes i have mark the comments in test files
b5bc2fa
into
babelfish-for-postgresql:BABEL_5_X_DEV
…for-postgresql#4509) Implemented the STGeometryType() method for both sys.GEOMETRY and sys.GEOGRAPHY types, extending the existing geospatial functionality in Babelfish. This function returns the geometry type name (Point, LineString, Polygon, etc.) with full TSQL compatibility. Changes Made Function Implementations: STGeometryType for sys.GEOMETRY - Returns geometry type name with proper validation STGeometryType for sys.GEOGRAPHY - Returns geography type name with same behavior Utilizes PostGIS functions with necessary adjustments for TSQL compatibility Strips 'ST_' prefix from PostGIS type names to match TSQL Server format Task: BABEL-6310 Signed-off by: gopalgv gopalgv@amazon.com
…for-postgresql#4509) Implemented the STGeometryType() method for both sys.GEOMETRY and sys.GEOGRAPHY types, extending the existing geospatial functionality in Babelfish. This function returns the geometry type name (Point, LineString, Polygon, etc.) with full TSQL compatibility. Changes Made Function Implementations: STGeometryType for sys.GEOMETRY - Returns geometry type name with proper validation STGeometryType for sys.GEOGRAPHY - Returns geography type name with same behavior Utilizes PostGIS functions with necessary adjustments for TSQL compatibility Strips 'ST_' prefix from PostGIS type names to match TSQL Server format Task: BABEL-6310 Signed-off by: gopalgv gopalgv@amazon.com
Description
Implement STGeometryType for Geometry and Geography Types**
Implemented the
STGeometryType()method for bothsys.GEOMETRYandsys.GEOGRAPHYtypes, extending the existing geospatial functionality in Babelfish. This function returns the geometry type name (Point, LineString, Polygon, etc.) with full TSQL compatibility.Changes Made
Function Implementations:
STGeometryTypeforsys.GEOMETRY- Returns geometry type name with proper validationSTGeometryTypeforsys.GEOGRAPHY- Returns geography type name with same behaviorKey Features:
STIsValid()before processingsys.NVARCHARtype and matches TSQL naming conventionsExamples
For STGeometryType (Geometry):
Output:
PointFor STGeometryType (Geography):
Output:
LineStringTable Operations:
Files Modified
geometry.sql- Added STGeometryType for geometry typegeography.sql- Added STGeometryType for geography typespatial_types--5.5.0--5.6.0.sql- Upgrade scriptIssues Resolved
BABEL-6330 - Implement STGeometryType for Geometry and Geography Types
Signed-off by: gopalgv gopalgv@amazon.com
Authored by: gopalgv gopalgv@amazon.com
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.