add support for PostgreSQL #45
Conversation
| @@ -102,7 +111,7 @@ protected function assertStoredMessageIsInDB(Envelope $envelope): void | |||
| /** @var MonitorIdStamp $monitorIdStamp */ | |||
| $monitorIdStamp = $envelope->last(MonitorIdStamp::class); | |||
| $this->assertNotFalse( | |||
| $connection->executeQuery('SELECT id FROM messenger_monitor WHERE id = :id', ['id' => $monitorIdStamp->getId()]) | |||
| $connection->executeQuery('SELECT id FROM messenger_monitor WHERE message_uid = :id', ['id' => $monitorIdStamp->getId()]) | |||
There was a problem hiding this comment.
pretty sure this was a bug in the tests that maria DB hid, but postgres exposed it. as $monitorIdStamp->getId() is a uuid, which threw an error trying to coerse to an int
bendavies
force-pushed
the
postgres-support
branch
from
885a1df
to
52041ac
Compare
.github/workflows/ci.yml
Outdated
| - "14" | ||
| dependencies: | ||
| - "lowest" | ||
| - "highest" |
There was a problem hiding this comment.
The matrix seems a bit excessive. We're already testing 8.0/8.1 and lowest/highest combination for mariadb. For postgres, I would choose just 8.1 and highest. For the postgres versions, are we doing anything fancy enough to reasonably think it would pass on one version but fail on another?
There was a problem hiding this comment.
fair comments, thanks.
I've revised it down to the latest postgres version now, which is the same as maria DB.
matrix size from 18 -> 4.
For postgres, I would choose just 8.1 and highest
Unsure about this - we still want to test that all combinations still work on postgres, regardless of if we've already test them on maria db, don't you think?
There was a problem hiding this comment.
In theory, we'd test every combination. But in practice, we should be pragmatic. Is it reasonable that Postgres specifically could cause a failure on 8.0 but not on 8.1? Perhaps (e.g. if you used an 8.1-specific syntax). For highest/lowest, that seems less likely. So I would be ok with removing highest/lowest but keeping 8.0 & 8.1
bendavies
force-pushed
the
postgres-support
branch
from
52041ac
to
bf37001
Compare
bendavies
force-pushed
the
postgres-support
branch
from
bf37001
to
e0168b6
Compare
|
Thanks Ben! |
Adds support and fixes issues for PostgresSQL.
Should be merged before #43