npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.56k stars 225 forks source link

NTS: Additional translations #457

Closed bricelam closed 11 months ago

bricelam commented 6 years ago

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 commented 6 years ago

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

tematim commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

roji commented 6 years ago

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 commented 6 years ago

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

roji commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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

EricStG commented 6 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

Correct, Geometry.Union() translates to ST_UnaryUnion. Aggregate Union is tracked by https://github.com/aspnet/EntityFrameworkCore/issues/13278. The expression would look something like UnaryOp.Union(geometries) but the signature probably needs to change to IEnumerable first...

roji commented 5 years ago

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 commented 4 years ago

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

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

roji commented 4 years ago

@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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

nookiepl commented 3 years ago

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 commented 3 years ago

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

john-larson commented 3 years ago

+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 commented 3 years ago

I took another look, and the provider already translates Geometry.IsWithinDistance to ST_DWithin since version 3.0.0 (see https://github.com/npgsql/efcore.pg/pull/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Can "ST_AsMVT" and "ST_AsMVTGeom" be supported?

roji commented 3 years ago

@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: https://github.com/dotnet/efcore/issues/22957

luo007 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

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

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

luo007 commented 11 months ago

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

roji commented 11 months ago

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