Skip to content

Implemetation of parse geography and geometry datatype#4518

Merged
shardgupta merged 18 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
Gopalverma062:BABEL_6310_PARSE
Mar 1, 2026
Merged

Implemetation of parse geography and geometry datatype#4518
shardgupta merged 18 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
Gopalverma062:BABEL_6310_PARSE

Conversation

@Gopalverma062
Copy link
Contributor

@Gopalverma062 Gopalverma062 commented Feb 3, 2026

Description

Implemented Parse function support for GeoSpatial TSQL data types that was previously unsupported in Babelfish. The Parse function has been added for both geometry and geography data types.

Changes include:

Function Implementation:

  • Implemented geometry::Parse() and geography::Parse() functions by utilizing PostGIS parsing capabilities with necessary adjustments to ensure TSQL compatibility.

Parser Updates:

  • Added Parse function definitions to the TSQL parser to recognize these new static methods.
  • Ensured proper syntax handling for Parse function, including NVARCHAR parameter types and return values.

Test Cases:

  • Created comprehensive test cases for Parse function, covering various scenarios including basic parsing, empty geometries, NULL handling, whitespace tolerance, case insensitivity, and error conditions.

Error Handling:

  • Implemented appropriate error handling for invalid geometry strings and malformed inputs.

Extending existing functionalities:

  • Added support for parsing empty instances in both geometry and geography datatypes
  • Enhanced CAST operations compatibility with Parse results
  • Added hierarchyid::Parse() support for completeness

Testing:

  • All new Parse functions pass the added test cases.
  • Existing functionality remains unaffected.

Examples:

For geometry::Parse

DECLARE @geomText NVARCHAR(MAX);
SET @geomText = 'POINT(1.0 2.0)';
SELECT geometry::Parse(@geomText).STAsText() AS ParsedGeometry;
GO

Output:

ParsedGeometry
--------------
POINT(1 2)

For geography::Parse

DECLARE @geogText NVARCHAR(MAX);
SET @geogText = 'LINESTRING(0 0, 1 1, 2 2)';
SELECT geography::Parse(@geogText).STAsText() AS ParsedGeography;
GO

Output:

ParsedGeography
---------------
LINESTRING(0 0,1 1,2 2)

For Empty Geometry Parsing

DECLARE @emptyPoint NVARCHAR(MAX);
SET @emptyPoint = 'POINT EMPTY';
SELECT geometry::Parse(@emptyPoint).STAsText() AS ParsedGeometry;
GO

Output:

ParsedGeometry
--------------
POINT EMPTY

Issues Resolved

Task: 6339

Signed-off by: Gopalgv gopalgv@amazon.com

Authored by: Gopalgv gopalgv@amazon.com


Check List

  • Commits are signed per the DCO using --signoff

By 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.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@coveralls
Copy link
Collaborator

coveralls commented Feb 3, 2026

Pull Request Test Coverage Report for Build 22102330700

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

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 944 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.003%) to 76.996%

Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tsql/src/hooks.c 1 85.82%
contrib/babelfishpg_tds/src/backend/tds/tdscomm.c 2 76.23%
contrib/babelfishpg_tds/src/backend/tds/tdsresponse.c 2 80.9%
contrib/babelfishpg_tsql/src/session.c 4 96.62%
contrib/babelfishpg_tds/src/backend/tds/tdslogin.c 31 77.42%
contrib/babelfishpg_tsql/src/collation.c 32 80.88%
contrib/babelfishpg_tsql/src/pl_handler.c 215 90.82%
contrib/babelfishpg_tsql/src/pl_exec.c 657 44.52%
Totals Coverage Status
Change from base Build 21625550326: 0.003%
Covered Lines: 52722
Relevant Lines: 68474

💛 - Coveralls

RETURNS sys.GEOMETRY
AS $$
BEGIN
IF UPPER(geometry_tagged_text::text COLLATE "default") = 'NULL' THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to cast it to text first? Can't we directly compare the nvarchar?

RETURNS sys.GEOMETRY
AS $$
BEGIN
IF UPPER(geometry_tagged_text COLLATE "default") = 'NULL' THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parse function should be implemented similar to stgeomfromtext. Please verify.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i follow the same structure in Parse as a STGeomfromtext

INSERT INTO TestGeospatialParse_GeometryTable3 (ID, GeomColumn) VALUES (1, geometry::Point(3.0, 4.0, 4326));
~~ROW COUNT: 1~~

INSERT INTO TestGeospatialParse_GeometryTable3 (ID, GeomColumn) VALUES (2, geometry::STGeomFromText('LINESTRING(0 0, 1 1, 2 2)', 4326));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests should be like geometry::Parse('LINESTRING(0 0, 1 1, 2 2)', 4326)); . Need to update all the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done i have fixed it

INSERT INTO TestGeospatialParse_GeographyTable3 (ID, GeogColumn) VALUES (1, geography::Point(3.0, 4.0, 4326));
~~ROW COUNT: 1~~

INSERT INTO TestGeospatialParse_GeographyTable3 (ID, GeogColumn) VALUES (2, geography::STGeomFromText('LINESTRING(0 0, 1 1, 2 2)', 4326));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need more coverage like: Linestring with XY, XYM, XYZ, XYZM points

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done i have addded the more table and testcases which covering XY, XYM, XYZ, XYZM

CREATE VIEW TestGeospatialMethods_empty_ValFromGeomTemp AS SELECT location.STIsEmpty() FROM TestGeospatialMethods_SPATIALPOINTGEOM_dttemp ORDER BY location.STX;

CREATE VIEW TestGeospatialMethods_empty_TextFromGeomTemp AS SELECT location.STIsEmpty() AS Dimension FROM TestGeospatialMethods_SPATIALPOINTGEOG_dttemp ORDER BY location.Lat;
CREATE VIEW TestGeospatialMethods_empty_TextFromGeomTemp AS SELECT location.STIsEmpty() AS Dimension FROM TestGeospatialMethods_SPATIALPOINTGEOG_dttemp ORDER BY location.Lat;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove changes in Test-spatial-functions-2 files.


DECLARE @geogText NVARCHAR(MAX);
SET @geogText = 'POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))';
SELECT geography::Parse(@geogText).STAsText() AS ParsedGeography;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test Parse function alone without STAsText

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have added the testcase that test parse alone without STAsText

Function sys.geography(sys.bpchar)
Function sys.geography(sys.geography,integer,boolean)
Function sys.geography(text,integer,boolean)
Function sys.geography__parse(sys.nvarchar)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this by creating a dependency for Parse function eg. CREATE VIEW

RETURNS sys.GEOGRAPHY
AS $$
BEGIN
IF UPPER(geography_tagged_text COLLATE "default") = 'NULL' THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COLLATE "default" should not be used here. use DATABASE_DEFAULT instead. "default" is a postgres collation, not babelfish collation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i have fixed it

RAISE EXCEPTION 'parse error - invalid geometry';
END IF;

RETURN sys.geogfromtext_helper(geography_tagged_text::text, 4326);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to cast geography_tagged_text to text again? sys.geogfromtext_helper should be doing that implicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have fixed it

RETURN NULL;
END IF;
-- Reject Z/ZM dimension qualifier
IF geography_tagged_text COLLATE "default" ~* '\s+ZM?\s*\(' THEN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

$$ LANGUAGE plpgsql IMMUTABLE STRICT PARALLEL SAFE;

--Parse
CREATE OR REPLACE FUNCTION sys.Geometry__Parse(geometry_tagged_text sys.NVARCHAR)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above


hierarchyid_static_method
: GETROOT
| PARSE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we remove from here?

Copy link
Contributor Author

@Gopalverma062 Gopalverma062 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PARSE was incorrectly categorized under hierarchyid_static_method. After discussing with Rohit i ,removed it from here

@@ -0,0 +1,382 @@
--Parse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comments stating what are we testing for ease of reviewing. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@shardgupta shardgupta merged commit e04abbc into babelfish-for-postgresql:BABEL_5_X_DEV Mar 1, 2026
68 of 70 checks passed
Gopalverma062 added a commit to Gopalverma062/babelfish_extensions that referenced this pull request Mar 19, 2026
…-postgresql#4518)

Implemented Parse function support for GeoSpatial TSQL data types that was previously unsupported in Babelfish. The Parse function has been added for both geometry and geography data types.

Changes include:

Implemented geometry::Parse() and geography::Parse() functions by utilizing PostGIS parsing capabilities with necessary adjustments to ensure TSQL compatibility. Added Parse function definitions to the TSQL parser to recognize these new static methods. Ensured proper syntax handling for Parse function, including NVARCHAR parameter types and return values.

Examples:

DECLARE @geomText NVARCHAR(MAX);
SET @geomText = 'POINT(1.0 2.0)';
SELECT geometry::Parse(@geomText).STAsText() AS ParsedGeometry;
GO

ParsedGeometry
--------------
POINT(1 2)


DECLARE @geogText NVARCHAR(MAX);
SET @geogText = 'LINESTRING(0 0, 1 1, 2 2)';
SELECT geography::Parse(@geogText).STAsText() AS ParsedGeography;
GO

ParsedGeography
---------------
LINESTRING(0 0,1 1,2 2)

Task: BABEL-6310

Signed-off by: Gopalgv gopalgv@amazon.com
Gopalverma062 added a commit to Gopalverma062/babelfish_extensions that referenced this pull request Mar 20, 2026
…-postgresql#4518)

Implemented Parse function support for GeoSpatial TSQL data types that was previously unsupported in Babelfish. The Parse function has been added for both geometry and geography data types.

Changes include:

Implemented geometry::Parse() and geography::Parse() functions by utilizing PostGIS parsing capabilities with necessary adjustments to ensure TSQL compatibility. Added Parse function definitions to the TSQL parser to recognize these new static methods. Ensured proper syntax handling for Parse function, including NVARCHAR parameter types and return values.

Examples:

DECLARE @geomText NVARCHAR(MAX);
SET @geomText = 'POINT(1.0 2.0)';
SELECT geometry::Parse(@geomText).STAsText() AS ParsedGeometry;
GO

ParsedGeometry
--------------
POINT(1 2)


DECLARE @geogText NVARCHAR(MAX);
SET @geogText = 'LINESTRING(0 0, 1 1, 2 2)';
SELECT geography::Parse(@geogText).STAsText() AS ParsedGeography;
GO

ParsedGeography
---------------
LINESTRING(0 0,1 1,2 2)

Task: BABEL-6310

Signed-off by: Gopalgv gopalgv@amazon.com
rishabhtanwar29 pushed a commit that referenced this pull request Mar 20, 2026
Implemented Parse function support for GeoSpatial TSQL data types that was previously unsupported in Babelfish. The Parse function has been added for both geometry and geography data types.

Changes include:

Implemented geometry::Parse() and geography::Parse() functions by utilizing PostGIS parsing capabilities with necessary adjustments to ensure TSQL compatibility. Added Parse function definitions to the TSQL parser to recognize these new static methods. Ensured proper syntax handling for Parse function, including NVARCHAR parameter types and return values.

Examples:

DECLARE @geomText NVARCHAR(MAX);
SET @geomText = 'POINT(1.0 2.0)';
SELECT geometry::Parse(@geomText).STAsText() AS ParsedGeometry;
GO

ParsedGeometry
--------------
POINT(1 2)


DECLARE @geogText NVARCHAR(MAX);
SET @geogText = 'LINESTRING(0 0, 1 1, 2 2)';
SELECT geography::Parse(@geogText).STAsText() AS ParsedGeography;
GO

ParsedGeography
---------------
LINESTRING(0 0,1 1,2 2)

Task: BABEL-6310
Signed-off by: Gopalgv gopalgv@amazon.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants