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.

Sign up for GitHub

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

Npgsql: Use NpgsqlPoint .NET type for marshalling GEO_POINT types. Explore communicating and marshalling GeoJSON types. #782

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Npgsql: Use NpgsqlPoint .NET type for marshalling GEO_POINT types. Explore communicating and marshalling GeoJSON types. #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6770795
Npgsql: Use `NpgsqlPoint` .NET type for marshalling `GEO_POINT` types
amotl Dec 20, 2024
ba814ca
Npgsql: Explore communicating and marshalling GeoJSON types
amotl Dec 21, 2024
762b488
Npgsql: Mitigate warnings (chore)
amotl Dec 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions by-language/csharp-npgsql/BasicPoco.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ public class BasicPoco
public string? name { get; set; }
public int? age { get; set; }

public override bool Equals(object obj)
public override bool Equals(object? obj)
{
var other = (BasicPoco) obj;
return name == other.name && age == other.age;
var other = (BasicPoco?) obj;
return name == other?.name && age == other?.age;
}

public override int GetHashCode()
Expand Down
29 changes: 24 additions & 5 deletions by-language/csharp-npgsql/DemoProgram.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@ await Parser.Default.ParseArguments<Options>(args)
{
var connString = $"Host={options.Host};Port={options.Port};SSL Mode={options.SslMode};" +
$"Username={options.Username};Password={options.Password};Database={options.Database}";
Console.WriteLine($"Connecting to {connString}\n");

var dataSourceBuilder = new NpgsqlDataSourceBuilder(connString);
dataSourceBuilder.EnableDynamicJson();
await using var dataSource = dataSourceBuilder.Build();
await using var conn = dataSource.OpenConnection();
await using var conn = GetConnection(connString);

await DatabaseWorkloads.SystemQueryExample(conn);
await DatabaseWorkloads.BasicConversationExample(conn);
Expand All @@ -34,11 +30,34 @@ await Parser.Default.ParseArguments<Options>(args)
// await dwt.ArrayJsonDocumentExample();
await dwt.ObjectPocoExample();
await dwt.ArrayPocoExample();
await dwt.GeoJsonTypesExample();
conn.Close();
});

}

public static NpgsqlConnection GetConnection(string connString)
{
Console.WriteLine($"Connecting to database: {connString}\n");

// Enable JSON POCO mapping and PostGIS/GeoJSON Type Plugin.
// https://www.npgsql.org/doc/types/json.html
// https://www.npgsql.org/doc/types/geojson.html
var dataSourceBuilder = new NpgsqlDataSourceBuilder(connString);

// Enable JSON POCO mapping Plugin.
// https://www.npgsql.org/doc/types/json.html
dataSourceBuilder.EnableDynamicJson();

// Enable PostGIS/GeoJSON Type Plugin.
// https://www.npgsql.org/doc/types/geojson.html
// dataSourceBuilder.UseGeoJson();
Copy link
Member

@seut seut Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Isn't this sending GEOJson (so a map encoded in JSON) to CrateDB?
If so, this could work when explicitly casting the insert value to a map/object:

insert into geom (geo) values('{"coordinates": [8.308903076149363, 47.05038385401457], "type": "Point"}'::object);

or

insert into geom (geo) select '{"coordinates": [8.308903076149363, 47.05038385401457], "type": "Point"}'::object;

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your suggestions, here and below. I will investigate them and report back.


var dataSource = dataSourceBuilder.Build();
var conn = dataSource.OpenConnection();
return conn;
}

public class Options
{
[Option('h', "host", Required = false, HelpText = "Host name to connect to", Default = "localhost")]
Expand Down
116 changes: 114 additions & 2 deletions by-language/csharp-npgsql/DemoTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Data;
using System.Text.Json;
using System.Threading.Tasks;
using GeoJSON.Net.Geometry;
using Newtonsoft.Json;
using Npgsql;
using NpgsqlTypes;
Expand Down Expand Up @@ -166,12 +167,27 @@ INSERT INTO testdrive.example (
cmd.Parameters.AddWithValue("timestamp_tz", "1970-01-02T00:00:00+01:00");
cmd.Parameters.AddWithValue("timestamp_notz", "1970-01-02T00:00:00");
cmd.Parameters.AddWithValue("ip", "127.0.0.1");

// Container types
cmd.Parameters.AddWithValue("array", NpgsqlDbType.Json, new List<string>{"foo", "bar"});
cmd.Parameters.AddWithValue("object", NpgsqlDbType.Json, new Dictionary<string, string>{{"foo", "bar"}});
cmd.Parameters.AddWithValue("geopoint", new List<double>{85.43, 66.23});
// TODO: Check if `GEO_SHAPE` types can be represented by real .NET or Npgsql data types.

// Geospatial types

// GEO_POINT
// Alternatively to `NpgsqlPoint`, you can also use `List<double>{85.43, 66.23}`.
cmd.Parameters.AddWithValue("geopoint", new NpgsqlPoint(85.43, 66.23));

// GEO_SHAPE
// While `GEO_POINT` is transparently marshalled as `NpgsqlPoint`,
// `GEO_SHAPE` is communicated as scalar `string` type, using WKT or GeoJSON format.
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: CrateDB won't communicate this as string but as a JSON type, see also https://github.com/crate/crate/blob/master/server/src/main/java/io/crate/protocols/postgres/types/PGTypes.java#L66.

Maybe my previously commented workaround by casting the insert to an object will work?

// TODO: Possibly support transparently converging `GEO_SHAPE` to one of
// `NpgsqlLSeg`, `NpgsqlBox`, `NpgsqlPath`, `NpgsqlPolygon`, `NpgsqlCircle`.
cmd.Parameters.AddWithValue("geoshape", "POLYGON ((5 5, 10 5, 10 10, 5 10, 5 5))");

// Vector type
cmd.Parameters.AddWithValue("float_vector", new List<double> {1.1, 2.2, 3.3});

cmd.ExecuteNonQuery();
}

Expand Down Expand Up @@ -336,6 +352,102 @@ public async Task<List<BasicPoco>> ArrayPocoExample()
}
}

public async Task InsertGeoJsonTyped()
{
/***
* Verify Npgsql PostGIS/GeoJSON Type Plugin with CrateDB.
* https://www.npgsql.org/doc/types/geojson.html
*
* TODO: Does not work yet, because CrateDB communicates GEO_SHAPE as string?
* The error message is:
Comment on lines +361 to +362
Copy link
Member

Choose a reason for hiding this comment

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

See previous comment about a possible workaround.

*
* System.NotSupportedException : The NpgsqlDbType 'Geometry' isn't present in your
* database. You may need to install an extension or upgrade to a newer version.
*/
Console.WriteLine("Running InsertGeo");

// Insert single data point.
await using (var cmd = new NpgsqlCommand("""
INSERT INTO testdrive.example (
"geoshape"
) VALUES (
@geoshape
);
""", conn))
{
var point = new Point(new Position(85.43, 66.23));
cmd.Parameters.AddWithValue("geoshape", NpgsqlDbType.Geometry, point);
cmd.ExecuteNonQuery();
}

// Flush data.
await RefreshTable();
}
Comment on lines +355 to +385
Copy link
Member Author

Choose a reason for hiding this comment

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

It would be sweet to gain typed GeoJSON support, like the Npgsql PostGIS/GeoJSON Type Plugin might be providing it when talking to PostGIS. I don't know why it isn't working, the error message is:

System.NotSupportedException : The NpgsqlDbType 'Geometry' isn't present in your
database. You may need to install an extension or upgrade to a newer version.

Maybe it does not work, because CrateDB communicates GEO_SHAPE as exclusively as string when using the PostgrSQL wire protocol? Please advise if you see any options for improvements here.

/cc @seut, @surister, @kneth

Copy link

Choose a reason for hiding this comment

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

Is PostGIS' Geometry equivalent to CrateDB's geo_shape? Would a simple alias in the server do the trick?

Copy link
Member Author

Choose a reason for hiding this comment

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

When talking about PostGIS, and looking at compatibility concerns, it is not just about types, but also, and mostly, about operations on them.

PostGIS unlocks GDAL, while CrateDB unlocks JTS. Those are technically different animals, while they are still living in the same habitat. In this spirit, I figure that a simple alias will probably not be applicable, even if it would also be my dearest wish.

This doesn't mean we should not explore this area closer, how we could provide downstream compatibility, or at least a reasonable feature parity, possibly by other means.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would a simple alias in the server do the trick?

Maybe @seut has more insights into that. I will be so happy to also learn more about those details, and if they have been parts of any discussions in the past already.

Copy link
Member

Choose a reason for hiding this comment

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

From reading the PostGIS docs, the generic geometry type standalone looks like it could be mapped to our GEO_SHAPE type as it serves as a generic type for all concrete spatial types the same like our geo_shape does. But as far as I read, the PostGIS also allows to specify a concrete geometry subset on table definition, e.g. geometry(LINESTRING). This isn't possible at CrateDB, we cannot limit the allowed concrete shape of a geometry value.
So we maybe could alias the PostGIS geometry type without a concrete spatial type definition, but even so we'd need to do some (extensive) testing to ensure that this works as expected (in terms of SQL and PG compatibility), especially, the query/filter behaviour.
I suggest to open a feature request at CrateDB to implement the geometry data type with the alias of geo_shape as a possible solution.

Copy link
Member Author

@amotl amotl Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@seut: Thanks for your swift elaborations, and Happy New Year.
@kneth: Can I humbly ask you to follow @seut's advise and carry your proposal forward into a corresponding feature request ticket at crate/crate, already reusing @seut's statements on this topic to provide initial guidance?

Copy link

Choose a reason for hiding this comment

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

@amotl @seut I have created crate/crate#17187

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. 👍


public async Task InsertGeoJsonString()
{
/***
* Communicate GeoJSON types as strings, marshall from/to GeoJSON types manually.
*/
Console.WriteLine("Running InsertGeoRaw");

// Insert single data point.
await using (var cmd = new NpgsqlCommand("""
INSERT INTO testdrive.example (
"geoshape"
) VALUES (
@geoshape
);
""", conn))
{
var point = new Point(new Position(85.43, 66.23));
var poly = new Polygon([
new LineString([
new Position(longitude: 5.0, latitude: 5.0),
new Position(longitude: 5.0, latitude: 10.0),
new Position(longitude: 10.0, latitude: 10.0),
new Position(longitude: 10.0, latitude: 5.0),
new Position(longitude: 5.0, latitude: 5.0),
])
]);
// TODO: Can GEO_SHAPE types be directly marshalled to a .NET GeoJSON type?
// Currently, `InsertGeoJsonTyped` does not work yet.
cmd.Parameters.AddWithValue("geoshape", NpgsqlDbType.Json, JsonConvert.SerializeObject(point));
cmd.ExecuteNonQuery();

cmd.Parameters.Clear();

cmd.Parameters.AddWithValue("geoshape", NpgsqlDbType.Json, JsonConvert.SerializeObject(poly));
cmd.ExecuteNonQuery();
Comment on lines +403 to +421
Copy link
Member Author

Choose a reason for hiding this comment

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

What works is to communicate GeoJSON data using the NpgsqlDbType.Json type, but it needs manual marshalling like JsonConvert.SerializeObject(point).

Contrary to that, as mentioned above, the Npgsql PostGIS/GeoJSON Type Plugin enables to communicate .NET's GeoJSON types natively.

@simonprickett: Please let me know if you find any way do that already, which I might not have discovered yet. Thanks!

}

// Flush data.
await RefreshTable();

}

public async Task<Point?> GeoJsonTypesExample()
{
Console.WriteLine("Running GeoJsonTypesExample");

// Provision data.
await CreateTable();
// await InsertGeoJsonTyped();
await InsertGeoJsonString();

// Query back data.
await using (var cmd = new NpgsqlCommand("SELECT * FROM testdrive.example", conn))
await using (var reader = cmd.ExecuteReader())
{
reader.Read();
// TODO: Can GEO_SHAPE types be directly marshalled to a .NET GeoJSON type?
// Currently, `InsertGeoJsonTyped` does not work yet.
var obj = reader.GetFieldValue<JsonDocument>("geoshape");
var geoJsonObject = JsonConvert.DeserializeObject<Point>(obj.RootElement.ToString());
return (Point?) geoJsonObject;
Comment on lines +445 to +447
Copy link
Member

Choose a reason for hiding this comment

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

Reading data is not directly related to how data is ingested. Shouldn't the marshalling work when enabling GeoJSON by dataSourceBuilder.UseGeoJson();? Afaik the output JSON should be a valid GeoJSON format, at least for simple structures like Points.

}
Comment on lines +438 to +448
Copy link
Member Author

Choose a reason for hiding this comment

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

Dito: Manual procedures are currently needed when working with .NET's native GeoJSON types.

Here, the code uses reader.GetFieldValue<JsonDocument> for retrieval, and JsonConvert.DeserializeObject<Point>(...) for unmarshalling and type casting.

}

}

}
1 change: 1 addition & 0 deletions by-language/csharp-npgsql/demo.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="GeoJSON.Net" Version="1.4.1" />
<PackageReference Include="Npgsql" Version="9.0.2" />
<PackageReference Include="CommandLineParser" Version="2.9.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.12.0" />
Expand Down
35 changes: 24 additions & 11 deletions by-language/csharp-npgsql/tests/DemoProgramTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using System.Linq;
using System.Text.Json;
using System.Threading.Tasks;
using GeoJSON.Net.Geometry;
using Npgsql;
using NpgsqlTypes;
using Xunit;

namespace demo.tests
Expand All @@ -21,12 +23,7 @@ public DatabaseFixture()
{
CRATEDB_DSN = $"Host=localhost;Port=5432;Username=crate;Password=;Database=testdrive";
}
Console.WriteLine($"Connecting to {CRATEDB_DSN}\n");

var dataSourceBuilder = new NpgsqlDataSourceBuilder(CRATEDB_DSN);
dataSourceBuilder.EnableDynamicJson();
using var dataSource = dataSourceBuilder.Build();
Db = dataSource.OpenConnection();
Db = DemoProgram.GetConnection(CRATEDB_DSN);
}

public void Dispose()
Expand Down Expand Up @@ -129,11 +126,11 @@ public async Task TestAllTypesNativeExample()
// Assert.Equal(new Dictionary<string, string>{{"foo", "bar"}}, row["object"]);

// Geospatial types
// TODO: Unlock native data types?
// GEO_POINT and GEO_SHAPE types can be marshalled back and forth using STRING.
// GEO_POINT is using a tuple format, GEO_SHAPE is using the GeoJSON format.
// Assert.Equal(new List<double>{85.43, 66.23}, row["geopoint"]); // TODO
Assert.Equal("(85.42999997735023,66.22999997343868)", row["geopoint"].ToString()); // FIXME
// While `GEO_POINT` is transparently marshalled as `NpgsqlPoint`,
// `GEO_SHAPE` is communicated as scalar `string` type, using the GeoJSON format.
// TODO: Possibly support transparently converging `GEO_SHAPE` to one of
// `NpgsqlLSeg`, `NpgsqlBox`, `NpgsqlPath`, `NpgsqlPolygon`, `NpgsqlCircle`.
Assert.Equal(new NpgsqlPoint(85.42999997735023, 66.22999997343868), row["geopoint"]);
Assert.Equal("""{"coordinates":[[[5.0,5.0],[5.0,10.0],[10.0,10.0],[10.0,5.0],[5.0,5.0]]],"type":"Polygon"}""", row["geoshape"]);

// Vector type
Expand Down Expand Up @@ -215,5 +212,21 @@ public async Task TestArrayPocoExample()

}

[Fact]
public async Task TestGeoJsonTypesExample()
{
var conn = fixture.Db;

// Provision data.
var task = new DatabaseWorkloadsTypes(conn).GeoJsonTypesExample();
var point = await task.WaitAsync(TimeSpan.FromSeconds(0.5));

// Validate the outcome.
var coords = new Point(new Position(85.43, 66.23)).Coordinates;
Assert.Equal(coords.Latitude, point?.Coordinates.Latitude);
Assert.Equal(coords.Longitude, point?.Coordinates.Longitude);
Comment on lines +224 to +227
Copy link
Member Author

Choose a reason for hiding this comment

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

Can't directly compare GeoJSON object instances. Need to run the comparison on the individual coordinates inside. 🤷


}

}
}