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

NTS: Additional translations #457

Closed
bricelam opened this issue Jun 5, 2018 · 35 comments
Closed

NTS: Additional translations #457

bricelam opened this issue Jun 5, 2018 · 35 comments
Labels
enhancement New feature or request

Comments

@bricelam
Copy link
Contributor

bricelam commented Jun 5, 2018

Doing my research, I found some additional translations you may want to add:

NTS PostGIS
AsBinary ST_AsBinary
Buffer ST_Buffer
ConvexHull ST_ConvexHull
Dimension ST_Dimension
EndPoint ST_EndPoint
Envelope ST_Envelope
ExteriorRing ST_ExteriorRing
GetInteriorRingN ST_InteriorRingN
GetPointN ST_PointN
IsCCW ST_IsPolygonCCW
IsRing ST_IsRing
Normalized ST_Normalize
NumInteriorRings ST_NumInteriorRing
PointOnSurface ST_PointOnSurface
SRID ST_SRID
StartPoint ST_StartPoint
ToGMLFeature ST_AsGML
WKBReader.Read ST_GeomFromWKB
WKTReader.Read ST_GeomFromText
@roji
Copy link
Member

roji commented Jun 5, 2018

Yeah, went I implemented the NTS plugin I implemented everything that was straightforward. The above were more difficult for some reason - mainly because the NetTopologySuite API didn't map out-of-the-box - so I deferred them. As people request the above we'll work on adding them.

This is a good up-for-grabs for anyone wanting to get some practice on EF Core SQL translation - I'll accept PRs for individual translations (you don't have to do everything at once).

@roji roji added enhancement New feature or request good first issue Good for newcomers labels Jun 5, 2018
@roji roji added this to the Backlog milestone Jun 10, 2018
@tematim
Copy link

tematim commented Jun 26, 2018

Hello,

I'll need these one too :
Point
ST_Equals
ST_Buffer

LineString
ST_Centroid
ST_StartPoint
ST_Equals
ST_Buffer

Polygon
ST_Centroid
ST_Multi
ST_Buffer
ST_Dump

@slavanap
Copy link
Contributor

Please don't forget about ST_GeographyFromWKT. I really need that one. Now I'm trying to use NetTopologySuite.Geometries.Geometry.DefaultFactory.CreateMultiPolygon() as a workaround.

@slavanap
Copy link
Contributor

More than that, I'm trying to migrate to PostGIS from SqlGeography and haven't found method similar to SqlGeography.ReorientObject() just yet.

@roji
Copy link
Member

roji commented Oct 27, 2018

Note, implemented the following as part of #672: ST_Dimension, ST_EndPoint, ST_Envelope, ST_ExteriorRing, ST_IsRing, ST_NumInteriorRings, ST_PointOnSurface, ST_SRID, ST_StartPoint

@EricStG
Copy link
Contributor

EricStG commented Nov 7, 2018

Are there plans to add the <-> operator to that list? while Distance would work, it doesn't use indexes

@roji
Copy link
Member

roji commented Nov 11, 2018

@EricStG am far from a PostGIS expert: should the <-> operator be used everywhere instead of ST_Distance? If so, why does ST_Distance exist and not make use of indexes? Any more precise info on this would help.

@EricStG
Copy link
Contributor

EricStG commented Nov 11, 2018

@roji I'm far from being an expert as well, but my understanding is that <-> returns values that gives you relative distances, but doesn't necessarily map to a real distance unit, that's why it only makes sense to use it for ordering.
From this section, they mention that only specific functions/operators can make use of indices, but not sure why this couldn't be applied to something like ST_Distance

@roji
Copy link
Member

roji commented Nov 12, 2018

@EricStG thanks for the extra info, although I'm still not totally clear on what should happen, and when exactly <-> should be used instead of ST_Distance().

Can you help out by contacting the PostGIS folks and asking for clarification, possibly proposing that they make their documentation clearer on this point?

@EricStG
Copy link
Contributor

EricStG commented Nov 12, 2018

To get the nearest neighbors, you would use <->
To search within a distance, you would use ST_DWithin. More info here: https://postgis.net/2013/08/26/tip_ST_DWithin/

You would use ST_Distance or ST_Distance_Sphere when you need to know the actual distance between two points. like the kilometers between two geography points, but because it's calculated for every row, performance would suffer over any significant datasets.

I'll try posting on the PostGIS mailing list to see if anyone can point me to better documentation.

@roji
Copy link
Member

roji commented Nov 13, 2018

Thanks for your help on this @EricStG, we'll wait to see what comes out.

@EricStG
Copy link
Contributor

EricStG commented Nov 13, 2018

@roji This is the answer I got: https://lists.osgeo.org/pipermail/postgis-users/2018-November/043009.html

ST_Distance existed before we had <->.
If you are using PostgreSQL 9.5+ with PostGIS 2.2, you can just stick with using <->, but note that it doesn't always use and index. <-> only uses and index when it is in the ORDER BY clause and one side is a constant geometry (which you can achieve even when you don't have a constant by using a LATERAL clause).
In the case of geography though <-> will always give the sphere distance, while ST_Distance by default gives the spheroid distance. So ST_Distance is slightly more accurate to use for geography.

@roji
Copy link
Member

roji commented Nov 18, 2018

@EricStG OK, thanks for the info... I don't think I mind assuming PostgreSQL 9.5+ with PostGIS 2.2.

However, if I'm understanding things correctly, we may want to always use <-> for geometry, but use it for geography only under ORDER BY (since it's inaccurate compared to ST_Distance()).

In any case I'd certainly write back to the PostGIS people to propose adding this important information to their docs...

@EricStG
Copy link
Contributor

EricStG commented Mar 12, 2019

Looking at the EF Core spatial data documentation, it looks like the following are still missing:

Geometry.Buffer(double, int)

Can be implemented using ST_Buffer

Geometry.InteriorPoint

Can be implemented using ST_PointOnSurface

Geometry.OgcGeometryType

Can be implemented using ST_GeometryType

Geometry.Union()

There's a comment in the code mentioning that ST_Union with a single parameter is an aggregate, but the SQLite translation uses UnaryUnion, which is supported via ST_UnaryUnion

Does this make sense? Anyone mind if I work on it?

@bricelam
Copy link
Contributor Author

bricelam commented Mar 13, 2019

Correct, Geometry.Union() translates to ST_UnaryUnion. Aggregate Union is tracked by dotnet/efcore#13278. The expression would look something like UnaryOp.Union(geometries) but the signature probably needs to change to IEnumerable first...

@roji
Copy link
Member

roji commented Mar 14, 2019

Does this make sense? Anyone mind if I work on it?

Absolutely! (it's even marked up for grabs). Looking forward to seeing a PR!

@EricStG
Copy link
Contributor

EricStG commented Feb 3, 2020

Doing some quick search in the code, it looks like these are the only missing methods based on the original list:

  • ST_IsPolygonCCW
  • ST_Normalize
  • ST_AsGML
  • ST_GeomFromWKB
  • ST_GeomFromText

I'll see if I can make those happen in the next few weeks

@roji
Copy link
Member

roji commented Feb 3, 2020

@EricStG that would be great!

@bricelam I'm not sure if we have any unimplemented spatial operations that don't have a good corresponding NetTopologySuite API. If so, should we think about a common EF.Functions thing to possibly expose those operations across providers?

@bricelam
Copy link
Contributor Author

bricelam commented Feb 5, 2020

I'd run them by the NTS team first. They seemed receptive to adding operations that made sense. Otherwise yes, EF.Functions (or maybe just Geometry extension methods) would be the next best thing.

@nookiepl
Copy link

Quick question. Operator "<->" hasnt' been implemented yet, has it? In my case the difference is huge. Order by "<->" takes around 4 ms, ST_Distance with spheroid set to true = 250 ms, set to false = 120 ms.

@roji
Copy link
Member

roji commented Mar 18, 2021

Isn't this simply because <-> always returns the 2D distance (so very fast), whereas ST_Distance returns geodesic distance for geography? In other words, the results are completely different - are you sure 2D is what you want?

@roji
Copy link
Member

roji commented Mar 18, 2021

BTW not saying it should not be supported - there are definitely situations where 2D distance is useful for geography, just asking.

@nookiepl
Copy link

Interestingly the operator returns the same result as ST_Distance with spheroid set to false. Does anyone know why then there is such a performance gap? As for my use case - precision isn't that important.

BTW. the difference (between operator/spheroid=false and spheroid=true) isn't big though. ~10km for distance around 4500 km.

@EricStG
Copy link
Contributor

EricStG commented Mar 18, 2021

In short, ST_DIstance can't use an index: https://postgis.net/workshops/postgis-intro/knn.html

@john-larson
Copy link

+1 for the implementation of ST_DWithin. It must be used instead of ST_Distance if spatial indexes are to be used. I tried using a user-defined function as a proxy for a quick work-around but it does not use the spatial index inside a user-defined function. @roji @bricelam Is there any other work-around I can use until this is implemented?

@roji
Copy link
Member

roji commented Apr 4, 2021

I took another look, and the provider already translates Geometry.IsWithinDistance to ST_DWithin since version 3.0.0 (see #740). All supported spatial translations are documented here.

I tried using a user-defined function as a proxy for a quick work-around but it does not use the spatial index inside a user-defined function.

That shouldn't be the case - whether the provider translates to ST_DWithin or whether you translate to it via a UDF shouldn't have any bearing on whether it uses a spatial index or not... Review your index and the SQL being generated to find out what's going on.

@faibistes
Copy link

faibistes commented Apr 28, 2021

I took another look, and the provider already translates Geometry.IsWithinDistance to ST_DWithin since version 3.0.0 (see #740). All supported spatial translations are documented here.

I tried using a user-defined function as a proxy for a quick work-around but it does not use the spatial index inside a user-defined function.

That shouldn't be the case - whether the provider translates to ST_DWithin or whether you translate to it via a UDF shouldn't have any bearing on whether it uses a spatial index or not... Review your index and the SQL being generated to find out what's going on.

You have to define your function as SQL (usually, a single select), not plsql, so postgresql can inline it and use indexes:

CREATE OR REPLACE FUNCTION public.fast_distance(geom1 geometry, geom2 geometry)
 RETURNS double precision
 LANGUAGE sql
 IMMUTABLE STRICT
AS $function$
	SELECT geom1<->geom2;
$function$
;

Then:

public static class Extensiones
{
    public static double FastDistance(this Geometry a,this Geometry b) => throw new InvalidOperationException("Server only");
}

And in the context:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    var gMethodInfo = typeof(Extensiones.Extensiones)
        .GetRuntimeMethod(nameof(Extensiones.Extensiones.FastDistance), new[] { typeof(Geometry), typeof(Geometry) });
    modelBuilder.HasDbFunction(gMethodInfo)
        .HasName("public.fast_distance");
}

BTW, is there any way I can write a query that translates as LATERAL JOIN? AFAIK, it's the only way to get KNN with indexes for a fairly common use case, namely, matching the closest point in table b for all points in table a

EDIT: OK, this is brilliant:

q = Ctos.Select(x => new {
     Cto = x,
     Finca = Fincas.OrderBy(y => y.Area.FastDistance(x.Area)).First()
});

translates to:

SELECT c.gid, c.geom, c.migac, c.cobertura, c.cto, c.tipo, t.gid, t.accesos, t.baftth, t.geom, t.estado, t.fuente, t.finca, t.fxliberacion, t.modifpor, t.cnucl, t.fxsenda, t.acctrac, t.uuii_norm
FROM nac.cto AS c
LEFT JOIN LATERAL (
    SELECT f.gid, f.accesos, f.baftth, f.geom, f.estado, f.fuente, f.finca, f.fxliberacion, f.modifpor, f.cnucl, f.fxsenda, f.acctrac, f.uuii_norm
    FROM nac.fincadesp AS f
    ORDER BY nac.fast_distance(f.geom, c.geom)
    LIMIT 1
) AS t ON TRUE

@roji
Copy link
Member

roji commented Apr 28, 2021

FYI I'm adding built-in translation support for the <-> operator for version 6.0.

@faibistes that's a good workaround in the meantime. For LATERAL JOINs, EF Core will automatically produce these as needed, depending on how your query is structured.

@luo007
Copy link

luo007 commented May 8, 2021

Can "ST_AsMVT" and "ST_AsMVTGeom" be supported?

@roji
Copy link
Member

roji commented May 8, 2021

@luo007 sure, translations are added based on how much people ask for them (PRs are also always welcome!). For ST_AsMVTGeom, we'd need to decide how to map the required PostGIS box2d type (which doesn't exist in NetTopologySuite).

ST_AsMVT is an aggregate function, so blocked by EF Core support: dotnet/efcore#22957

@luo007
Copy link

luo007 commented May 10, 2021

@luo007 sure, translations are added based on how much people ask for them (PRs are also always welcome!). For ST_AsMVTGeom, we'd need to decide how to map the required PostGIS box2d type (which doesn't exist in NetTopologySuite).

ST_AsMVT is an aggregate function, so blocked by EF Core support: dotnet/efcore#22957

I think the box2d type can be replaced by the geometry type in NTS temporarily, and used with ST_MakeEnvelope.
ST_AsMVT I have no idea how to implement it

@XYCaptain
Copy link

XYCaptain commented Jun 10, 2021

Can "ST_AsMVT" and "ST_AsMVTGeom" be supported?

@luo007
Hello, how do you implement these calls now? I would appreciate it if you could give me a hint.

@luo007
Copy link

luo007 commented Jun 15, 2021

可以支持“ST_AsMVT”和“ST_AsMVTGeom”吗?

@luo007
你好,你现在如何实现这些调用?如果你能给我一个提示,我将不胜感激。

ST_AsMVT Because efcore does not support custom aggregate functions, it cannot be implemented

@luo007
Copy link

luo007 commented Dec 12, 2023

Hi, I see that efcore7 now supports custom aggregate functions. ST_AsMVT Whether it is supported

@roji
Copy link
Member

roji commented Dec 12, 2023

Yes, it should now be possible to translate to ST_AsMVT.

I don't think keeping this "additional translations" issue is useful at this point, since most translations are already supported - we can track specific translations via individual issues (opened #3016 for ST_AsMVT specifically).

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
@roji roji removed this from the Backlog milestone Dec 12, 2023
@roji roji removed the good first issue Good for newcomers label Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

10 participants