Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STIsValid returning incorrect result for parameterized query #85

Open
mark-oppedahl opened this issue May 1, 2023 · 6 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@mark-oppedahl
Copy link

mark-oppedahl commented May 1, 2023

I'm trying to work around the lack of a STIsValid() method on SqlGeometry by handing the geometry to SQL to evaluate. But for an invalid self-intersecting polygon, the result comes back as valid if I try to pass the geometry in as a parameter. In the following, isValid1 and isValid2 should match, but they don't. Am I missing something? The SQL connection string can point to any SQL Server instance you want. Using the 1.5.0 version for System.Data.SqlClient compatibility.

using Microsoft.SqlServer.Types;
...
var wkt = "POLYGON ((90.0219727 43.0729006, 91.4282227 42.0207329, 89.9395752 42.0859935, 91.5490723 43.0648747, 90.0219727 43.0729006))";
var geo = SqlGeography.STGeomFromText(new SqlChars(new SqlString(wkt)), 4326);

using (var conn = new SqlConnection("Integrated Security=SSPI;Initial Catalog=UnitTest;Data Source=(localdb)\\MSSQLLocalDB;"))
{
    conn.Open();
    using (var cmd = conn.CreateCommand())
    {
        cmd.CommandType = CommandType.Text;
        cmd.CommandText = "SELECT @1.STIsValid(), geography::STGeomFromText(@2, 4326).STIsValid()";

        var param = cmd.CreateParameter();
        param.ParameterName = "@1";
        param.Value = geo;
        param.SqlDbType = SqlDbType.Udt;
        param.UdtTypeName = "Geography";
        cmd.Parameters.Add(param);

        param = cmd.CreateParameter();
        param.ParameterName = "@2";
        param.Value = wkt;
        cmd.Parameters.Add(param);

        using(var reader = cmd.ExecuteReader())
        {
            reader.Read();
            var isValid1 = reader.GetBoolean(0);
            var isValid2 = reader.GetBoolean(1);
        }
    }
} 
@dotMorten
Copy link
Owner

If you send in the text as a parameter (instead of as geography type) and convert to geography server-side do you get the same result?

@mark-oppedahl
Copy link
Author

That's what the second parameter in the sample is doing -- passing in the text and creating the geography server-side. It returns the correct result for STIsValid, unlike passing in the geography, which returns an incorrect result. I assume there's a performance hit to convert the geography to text client-side and back to geography server-side.

@dotMorten dotMorten added bug Something isn't working help wanted Extra attention is needed labels May 3, 2023
@mark-oppedahl
Copy link
Author

mark-oppedahl commented May 3, 2023

Some more information: It look like SQL Server's call to STIsValid returns whatever the geography object's internal _isValid is set to. If you create the geography in the sample client-side, the internal _isValid value is true. When you send the object up as a parameter and call STIsValid(), it returns true. But if you use reflection to set _isValid to false before sending the parameter, SQL returns false. SQL isn't recalculating isValid, but essentially trusting that the client set it correctly.

@mark-oppedahl
Copy link
Author

MakeValid has a similar problem--SQL doesn't modifiy the geometry to make it valid if the internal _isValid is already true.

@dotMorten
Copy link
Owner

Whaaaaat? That’s just ridiculous 🤦‍♂️🤦‍♂️🤦‍♂️
In that case I honestly don’t think there’s much I can do beyond actually implementing valid calculations which is not something I want to get into

@mark-oppedahl
Copy link
Author

mark-oppedahl commented May 3, 2023

Here's a way to pass the geometry as a parameter and force the server to reevaluate isValid:

SELECT geography::STGeomFromWKB(@1.STAsBinary(), 4236).STIsValid()

Similarly, if you want MakeValid to work, it's

SELECT geography::STGeomFromWKB(@1.STAsBinary(), 4236).MakeValid()

Yuck, but it appears to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants