-
-
Notifications
You must be signed in to change notification settings - Fork 56
feat(#305): add support for PostGIS's types of GEOGRAPHY, GEOMETRY and their array variations
#421
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
Changes from 5 commits
a9aabf3
3801e55
9fa3471
1b9d623
d0fb182
90f6509
2b6a808
b1eeaf4
8787b19
94b9b46
1679b8b
76d0b6d
8f6d4a8
2f8b760
3717176
caab1d8
9ee1f11
63e1350
e69e412
10fcc61
c1d7a3d
f8e91a9
ef344b1
3c87171
a442120
d1b5e36
d873c4b
382c20a
687cb6a
002cbd3
20e8abe
94dc439
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,14 +60,26 @@ in | |||||||||||||||||||||||||||||||||||||||||||||||
| services.postgres = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| enable = true; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Use PostgreSQL 17 to match Docker Compose and CI | ||||||||||||||||||||||||||||||||||||||||||||||||
| package = pkgs.postgresql_17; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| listen_addresses = "127.0.0.1"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| port = config.env.POSTGRES_PORT; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| initialDatabases = [ { name = config.env.POSTGRES_DB; } ]; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Enable PostGIS extension | ||||||||||||||||||||||||||||||||||||||||||||||||
| extensions = extensions: [ | ||||||||||||||||||||||||||||||||||||||||||||||||
| extensions.postgis | ||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| initialScript = '' | ||||||||||||||||||||||||||||||||||||||||||||||||
| CREATE ROLE "${config.env.POSTGRES_USER}" | ||||||||||||||||||||||||||||||||||||||||||||||||
| WITH SUPERUSER LOGIN PASSWORD '${config.env.POSTGRES_PASSWORD}'; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| -- Enable PostGIS extension in the database | ||||||||||||||||||||||||||||||||||||||||||||||||
| \c ${config.env.POSTGRES_DB} | ||||||||||||||||||||||||||||||||||||||||||||||||
| CREATE EXTENSION IF NOT EXISTS postgis; | ||||||||||||||||||||||||||||||||||||||||||||||||
| ''; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idempotency issue: creating the default 'postgres' role will fail
- initialScript = ''
- CREATE ROLE "${config.env.POSTGRES_USER}"
- WITH SUPERUSER LOGIN PASSWORD '${config.env.POSTGRES_PASSWORD}';
-
- -- Enable PostGIS extension in the database
- \c ${config.env.POSTGRES_DB}
- CREATE EXTENSION IF NOT EXISTS postgis;
- '';
+ initialScript = ''
+ DO $$
+ BEGIN
+ IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '${config.env.POSTGRES_USER}') THEN
+ CREATE ROLE "${config.env.POSTGRES_USER}" WITH SUPERUSER LOGIN PASSWORD '${config.env.POSTGRES_PASSWORD}';
+ ELSE
+ ALTER ROLE "${config.env.POSTGRES_USER}" WITH LOGIN PASSWORD '${config.env.POSTGRES_PASSWORD}';
+ END IF;
+ END
+ $$;
+
+ -- Enable PostGIS extension in the target database
+ \c ${config.env.POSTGRES_DB}
+ CREATE EXTENSION IF NOT EXISTS postgis;
+ '';Optionally, if you want the custom user to own the database:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\DBAL\Types\Exceptions; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
|
|
||
| /** | ||
| * @since 3.5 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| final class InvalidGeographyForDatabaseException extends ConversionException | ||
| { | ||
| private static function create(string $message, mixed $value): self | ||
| { | ||
| return new self(\sprintf($message, \var_export($value, true))); | ||
| } | ||
|
|
||
| public static function forInvalidType(mixed $value): self | ||
| { | ||
| return self::create('Database value must be a string, %s given', $value); | ||
| } | ||
|
|
||
| public static function forInvalidFormat(mixed $value): self | ||
| { | ||
| return self::create('Invalid geography format in database: %s', $value); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\DBAL\Types\Exceptions; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
|
|
||
| /** | ||
| * @since 3.5 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| final class InvalidGeographyForPHPException extends ConversionException | ||
| { | ||
| private static function create(string $message, mixed $value): self | ||
| { | ||
| return new self(\sprintf($message, \var_export($value, true))); | ||
| } | ||
|
|
||
| public static function forInvalidType(mixed $value): self | ||
| { | ||
| return self::create('Value must be a Geography value object, %s given', $value); | ||
| } | ||
|
|
||
| public static function forInvalidFormat(mixed $value): self | ||
| { | ||
| return self::create('Invalid Geography value object format: %s', $value); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\DBAL\Types\Exceptions; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
|
|
||
| /** | ||
| * @since 3.5 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| final class InvalidGeometryForDatabaseException extends ConversionException | ||
| { | ||
| private static function create(string $message, mixed $value): self | ||
| { | ||
| return new self(\sprintf($message, \var_export($value, true))); | ||
| } | ||
|
|
||
| public static function forInvalidType(mixed $value): self | ||
| { | ||
| return self::create('Database value must be a string, %s given', $value); | ||
| } | ||
|
|
||
| public static function forInvalidFormat(mixed $value): self | ||
| { | ||
| return self::create('Invalid geometry format in database: %s', $value); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\DBAL\Types\Exceptions; | ||
|
|
||
| use Doctrine\DBAL\Types\ConversionException; | ||
|
|
||
| /** | ||
| * @since 3.5 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| final class InvalidGeometryForPHPException extends ConversionException | ||
| { | ||
| private static function create(string $message, mixed $value): self | ||
| { | ||
| return new self(\sprintf($message, \var_export($value, true))); | ||
| } | ||
|
|
||
| public static function forInvalidType(mixed $value): self | ||
| { | ||
| return self::create('Value must be a Geometry value object, %s given', $value); | ||
| } | ||
|
|
||
| public static function forInvalidFormat(mixed $value): self | ||
| { | ||
| return self::create('Invalid Geometry value object format: %s', $value); | ||
| } | ||
| } | ||
|
Comment on lines
+14
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainSanity-check usage paths for this exception in Geometry DBAL type To avoid dead code, verify that convertToDatabaseValue/convertToPHPValue in Geometry type actually throw these factories on invalid PHP input or malformed value-object state. Run this to confirm call sites exist: 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Searching for InvalidGeometryForPHPException usage…"
rg -nP '\bInvalidGeometryForPHPException::forInvalid(?:Type|Format)\(' -C2
echo
echo "Looking for Geometry DBAL type and potential throw sites…"
rg -nP '(?s)class\s+Geometry\b.*?(convertTo(Database|PHP)Value)\s*\(' -C3 --type=php || trueLength of output: 846 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Inspecting the start of Geometry.php for class declaration..."
sed -n '1,200p' src/MartinGeorgiev/Doctrine/DBAL/Types/Geometry.php || true
echo
echo "Searching for convertToDatabaseValue definitions and usage…"
rg -nP 'function\s+convertToDatabaseValue' -C5 src/MartinGeorgiev/Doctrine/DBAL/Types/Geometry.php || true
echo
echo "Searching for convertToPHPValue definitions and usage…"
rg -nP 'function\s+convertToPHPValue' -C5 src/MartinGeorgiev/Doctrine/DBAL/Types/Geometry.php || true
echo
echo "Searching for forInvalidFormat calls in Geometry.php…"
rg -nP 'InvalidGeometryForPHPException::forInvalidFormat' -C3 src/MartinGeorgiev/Doctrine/DBAL/Types/Geometry.php || trueLength of output: 3035 Remove unused The
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace MartinGeorgiev\Doctrine\DBAL\Types; | ||
|
|
||
| use Doctrine\DBAL\Platforms\AbstractPlatform; | ||
| use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidGeographyForDatabaseException; | ||
| use MartinGeorgiev\Doctrine\DBAL\Types\Exceptions\InvalidGeographyForPHPException; | ||
| use MartinGeorgiev\Doctrine\DBAL\Types\ValueObject\Exceptions\InvalidWktSpatialDataException; | ||
| use MartinGeorgiev\Doctrine\DBAL\Types\ValueObject\WktSpatialData; | ||
|
|
||
| /** | ||
| * Implementation of PostGIS GEOGRAPHY data type. | ||
| * | ||
| * @see https://postgis.net/docs/using_postgis_dbmanagement.html | ||
| * @since 3.5 | ||
| * | ||
| * @author Martin Georgiev <martin.georgiev@gmail.com> | ||
| */ | ||
| final class Geography extends BaseType | ||
| { | ||
| protected const TYPE_NAME = 'geography'; | ||
|
|
||
| public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string | ||
| { | ||
| if ($value === null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!$value instanceof WktSpatialData) { | ||
| throw InvalidGeographyForPHPException::forInvalidType($value); | ||
| } | ||
|
|
||
| return (string) $value; | ||
| } | ||
|
|
||
| public function convertToPHPValue($value, AbstractPlatform $platform): ?WktSpatialData | ||
| { | ||
| if ($value === null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!\is_string($value)) { | ||
| throw InvalidGeographyForDatabaseException::forInvalidType($value); | ||
| } | ||
|
|
||
| try { | ||
| return WktSpatialData::fromWkt($value); | ||
| } catch (InvalidWktSpatialDataException) { | ||
| throw InvalidGeographyForDatabaseException::forInvalidFormat($value); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Create the extension before querying PostGIS_Version()
On some images/databases, PostGIS is installed but not enabled in the target DB. Explicitly creating the extension avoids flakiness.
Apply this tweak to the verification step:
Minor: the earlier echo lines use "\n", which many shells won’t interpret without -e. Consider printf for consistent newlines.
🏁 Script executed:
Length of output: 631
Enable PostGIS extension before querying its version
The workflow currently lists available extensions but never creates/enables PostGIS in the test database, which can lead to flakiness on images where PostGIS isn’t auto-enabled.
• File: .github/workflows/integration-tests.yml
• Around lines 108–112
Apply this diff:
- name: Verify available PostGIS extensions run: | echo "Available extensions:" PGPASSWORD=postgres psql -h localhost -U postgres -d postgres_doctrine_test -c "SELECT * FROM pg_available_extensions;" + echo "Enabling PostGIS extension (idempotent):" + PGPASSWORD=postgres psql -h localhost -U postgres -d postgres_doctrine_test -c "CREATE EXTENSION IF NOT EXISTS postgis;" - name: Verify PostGIS installation run: | - echo "\nVerifying PostGIS installation:" - PGPASSWORD=postgres psql -h localhost -U postgres -d postgres_doctrine_test -c "SELECT PostGIS_Version();" + # Use printf or echo -e for consistent newlines + printf "\nVerifying PostGIS installation:\n" + PGPASSWORD=postgres psql -h localhost -U postgres -d postgres_doctrine_test -c "SELECT PostGIS_Version();" + # Optional sanity check for a PostGIS function + PGPASSWORD=postgres psql -h localhost -U postgres -d postgres_doctrine_test -c "SELECT COUNT(*) FROM pg_proc WHERE proname='st_astext';"📝 Committable suggestion
🤖 Prompt for AI Agents