Skip to content

MakeValid implementation for geometry and geography#4513

Merged
shardgupta merged 12 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
Gopalverma062:BABEL_6310_MAKEVALID
Mar 9, 2026
Merged

MakeValid implementation for geometry and geography#4513
shardgupta merged 12 commits intobabelfish-for-postgresql:BABEL_5_X_DEVfrom
Gopalverma062:BABEL_6310_MAKEVALID

Conversation

@Gopalverma062
Copy link
Copy Markdown
Contributor

@Gopalverma062 Gopalverma062 commented Feb 2, 2026

Implement MakeValid GeoSpatial TSQL Function

Implemented the MakeValid GeoSpatial TSQL function that was previously unsupported in Babelfish.


What's Changed

Function Implementation

  • Added sys.MakeValid(geom sys.GEOMETRY) function in geometry.sql
  • Added sys.MakeValid(geog sys.GEOGRAPHY) function in geography.sql
  • Delegates to respective helper functions for processing invalid geometries
  • Functions are marked as IMMUTABLE STRICT PARALLEL SAFE for optimal performance

Error Handling

  • Returns NULL for NULL input instances (handled by STRICT)
  • Returns original geometry/geography for empty instances
  • Returns original geometry/geography for already valid instances
  • Applies correction logic for invalid instances

Examples

Point (Already Valid)

DECLARE @point geometry;
SET @point = geometry::STGeomFromText('POINT(10 20)', 0);
SELECT @point.MakeValid().STAsText() AS result, @point.MakeValid().STIsValid() AS is_valid;
-- Output: POINT (10 20), 1

Invalid Self-Intersecting Polygon

DECLARE @polygon geometry;
SET @polygon = geometry::STGeomFromText('POLYGON((0 0, 10 10, 10 0, 0 10, 0 0))', 0);
SELECT @polygon.STIsValid() AS before_valid, @polygon.MakeValid().STIsValid() AS after_valid;
-- Output: 0, 1

Geography Instance

DECLARE @geog geography;
SET @geog = geography::STGeomFromText('POINT(-122.349 47.651)', 4326);
SELECT @geog.MakeValid().STAsText() AS result, @geog.MakeValid().STIsValid() AS is_valid;
-- Output: POINT (-122.349 47.651), 1

Empty Instance

DECLARE @empty geometry;
SET @empty = geometry::STGeomFromText('POINT EMPTY', 0);
SELECT @empty.MakeValid().STAsText() AS result;
-- Output: POINT EMPTY

Issues Resolved

Task: BABEL-6331


Test Coverage

  • Use case based
  • Boundary conditions
  • Arbitrary inputs
  • Negative test cases

Checklist

  • Commits are signed per the DCO using --signoff
  • Contribution is under Apache 2.0 and PostgreSQL licenses

Signed-off-by: Gopal Verma [email protected]

Authored-by: Gopal Verma [email protected]


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.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Feb 2, 2026

Pull Request Test Coverage Report for Build 22111839764

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.
  • 1081 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.01%) to 77.001%

Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tsql/src/hooks.c 1 85.82%
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_tds/src/include/tds_request.h 36 76.22%
contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c 103 87.44%
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 21506975134: 0.01%
Covered Lines: 52726
Relevant Lines: 68474

💛 - Coveralls

INSERT INTO TestSpatialFunc3_MakeValidGeomTemp (ID, GeomColumn, Description) VALUES (2, geometry::STGeomFromText('POINT(30 40)', 4326), 'Valid Point with SRID');
~~ROW COUNT: 1~~

INSERT INTO TestSpatialFunc3_MakeValidGeomTemp (ID, GeomColumn, Description) VALUES (3, geometry::STGeomFromText('LINESTRING(0 0, 10 10, 20 20)', 0), 'Valid LineString');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to add more tests for invalid linestring and invalid polygon with MakeValid function. Most cases cover only valid instances.

Copy link
Copy Markdown
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

DROP TABLE TestGeospatialMethods_SPATIALPOINTGEOG_dttemp

DROP DATABASE TestGeospatialMethods_DB
DROP DATABASE TestGeospatialMethods_DB
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove this, unnecessary change.

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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above, we don't need to change any Test-spatial-functions-2 file

M_DOUBLE_QUOTE: ["] M ["] {pltsql_quoted_identifier == true}?;
M_SQBRACKET: '[' M ']';
MANUAL: M A N U A L;
MAKEVALID: 'MakeValid';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why aren't we using M A K E V A L I D?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed ,Updated to M A K E V A L I D for case-insensitive matching

@@ -0,0 +1,65 @@
CREATE DATABASE TestSpatialFunc3_DB;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the point of creating TestSpatialFunc3_DB db? All the objects are created inside master db

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed I've now created the test tables inside TestSpatialFunc3_DB and added the corresponding tests in the verify file.

@@ -0,0 +1,334 @@
--MakeValid
Copy link
Copy Markdown
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 testing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done Added comments explaining what each test case is validating.

@shardgupta shardgupta merged commit 1ff559d into babelfish-for-postgresql:BABEL_5_X_DEV Mar 9, 2026
47 checks passed
Gopalverma062 added a commit to Gopalverma062/babelfish_extensions that referenced this pull request Mar 19, 2026
…stgresql#4513)

Implemented the MakeValid GeoSpatial TSQL function that was previously unsupported in Babelfish.

Added sys.MakeValid(geom sys.GEOMETRY) function in geometry.sql
Added sys.MakeValid(geog sys.GEOGRAPHY) function in geography.sql

Delegates to respective helper functions for processing invalid geometries. Functions are marked as IMMUTABLE STRICT PARALLEL SAFE for optimal performance.

Task: BABEL-6310

Signed-off-by: Gopal Verma <[email protected]>
Gopalverma062 added a commit to Gopalverma062/babelfish_extensions that referenced this pull request Mar 20, 2026
…stgresql#4513)

Implemented the MakeValid GeoSpatial TSQL function that was previously unsupported in Babelfish.

Added sys.MakeValid(geom sys.GEOMETRY) function in geometry.sql
Added sys.MakeValid(geog sys.GEOGRAPHY) function in geography.sql

Delegates to respective helper functions for processing invalid geometries. Functions are marked as IMMUTABLE STRICT PARALLEL SAFE for optimal performance.

Task: BABEL-6310

Signed-off-by: Gopal Verma <[email protected]>
rishabhtanwar29 pushed a commit that referenced this pull request Mar 23, 2026
Implemented the MakeValid GeoSpatial TSQL function that was previously unsupported in Babelfish.

Added sys.MakeValid(geom sys.GEOMETRY) function in geometry.sql
Added sys.MakeValid(geog sys.GEOGRAPHY) function in geography.sql

Delegates to respective helper functions for processing invalid geometries. Functions are marked as IMMUTABLE STRICT PARALLEL SAFE for optimal performance.

Task: BABEL-6310
Signed-off-by: Gopal Verma <[email protected]>
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