Added custom array example with documentation.

[marvin-hansen]

Added example code with a comprehensive Readme that covers the bulk of the custom array concepts and how they relate to each other.

In response to discussion: #4157

However, the Readme needs review and the tests need a fix for a serialization error.

[marvin-hansen]

Quick update:

• Tests are fixed
• Readme got edited and checked
• Added the clippy lint check to the GH action so it runs together with all the others on CI

Only thing I could not figure out was how to add the tests to CI.

Should be straight forward to review this PR.

[weiznich]

Thanks for working on this. This is already a really great starting point.

I left a bunch of comments on various things. Some of them are just more information that might be helpful for you and others that read that example. Some are minor stylistic comments and a few are things that we should likely improve. If something is unclear just let me know and I will try to explain it with more details.

That written: If that example is merged I will try to convert it into a guide on the web page. It's already a really good starting point, it mostly needs styling/linking for that. This hopefully makes it much more discoverable how to perform that kind of extensions.

[weiznich]

Only thing I could not figure out was how to add the tests to CI.

The tests are automatically picked up: https://github.com/diesel-rs/diesel/actions/runs/10411801607/job/28836874448?pr=4169#step:27:356. I'm not sure why they fail but I can have a look at them next week if you want.

[marvin-hansen]

@weiznich Thank you so much for the substantial commentary.

The bulk of your comments are relatively easy to fix. I still trying to figure out the details about how Diesel handles custom schema, so your comments here are invaluable and I most likely will update the Readme once I updated the code so that everything resides in the custom schema. Still learning...

I have fixed the failing tests. Apparently, the CI picks up the test automatically and I simply wasn't running a migration prior to test execution. Once I added an embedded migration, the test seems to pass for now.
On that topic, I also added a section about Integration testing to the Readme to complete the document in terms of covering all the important topics.

Moving the content to the Website would be great!

I look into all the details over the weekend and update the PR

[marvin-hansen]

Update:

• Changed dotenv to dotenvy; (see comment)
• Moved Endpoint impl into type definition file. (See comment)
• Switched example code and tests from PGConnectionPool to single PGConnection. (See comment)
• Moved all custom types into custom schema smdb. See comment
• Added a DatabaseError to PgProtocolType in from_sql implementation to convey that the DB contains something that isn't expected. See comment.

Updating the Readme next.

[marvin-hansen]

Thanks a lot for the consistency checks as I didn't had the time to do it. Overall, I already committed the bulk of corrections.

I want to take a closer look at your suggestion to encapsulate tests into a transaction. I get the idea of basically piggy backing on the two phase commit mechanism inherent to transactions. I just have to write and run some sample tests to see how this works out in practice.

I think if that works out, it's sensible to update the test suite and the Readme.

Background is, I'm building and testing my Rust mono repo with Bazel and my CI runs already hundreds of tests in parallel. However, for DB integration tests at scale, the aborted transaction trick may work out and speeds things up further.

[marvin-hansen]

Thanks a lot for the consistency checks as I didn't had the time to do it. Overall, I already committed the bulk of corrections.

I want to take a closer look at your suggestion to encapsulate tests into a transaction. I get the idea of basically piggy backing on the two phase commit mechanism inherent to transactions. I just have to write and run some sample tests to see how this works out in practice.

I think if that works out, it's sensible to update the test suite and the Readme.

Background is, I'm building and testing my Rust mono repo with Bazel and my CI runs already hundreds of tests in parallel. However, for DB integration tests at scale, the aborted transaction trick may work out and speeds things up further.

[marvin-hansen]

@weiznich

Update summary:

• Readme was updated with all those minor corrections you have flagged
• Updated run_db_migration in lib by removing unnecessary check
• Added revert_db_migration to lib; it's used to cleanup the DB after sequential testing.
• Added parallel test suite using test transaction, as you laid out in a previous comment.
• Updated Readme section on testing.

Just one question: Does the test transaction aborts automatically when it goes out of scope?
I did not find any abort or rollback method, but somehow all tests run flawlessly and I wonder why?

Either way, please review these changes and if there
is nothing in the way, feel free to merge whenever ready.

[weiznich]

Thanks for the update. I will try to do a final review (and likely some additional minor adjustments based on that) this week. Otherwise I think this is fine now.

Just one question: Does the test transaction aborts automatically when it goes out of scope?
I did not find any abort or rollback method, but somehow all tests run flawlessly and I wonder why?

It relays on the fact that you drop the connection itself at the end of the test. This will also drop any uncommited changes for that connection.

[marvin-hansen]

@weiznich

Thank you for the detailed corrections and the links to the docs in the Readme. Also, thank you for getting the parallel tests in shape. While things worked locally on my box, apparently on CI each test really needs an unique connection.

It was my first try with the dangling transaction test style so there is stil a bit of learning ahead. Anyways, just want to thank you for your time and effort to get this PR done.

[weiznich]

While things worked locally on my box, apparently on CI each test really needs an unique connection.

The problem there was that you likely had already applied the migrations manually. The tests run against an empty database and postgres do not allow to modify the schema from more than one connection at once. That's worked around by having the schema modification itself in the test transaction as well. So it doesn't get commited and there are no problem with concurrent access.

Thanks again for writing this up. I think that's now a good starting point for at least two guides: One for custom types and one for testing. Now I only need to find the capacity to do the last bit of polishing to put it on the web page.

[marvin-hansen]

Now this makes sense as I've just seen the same issue on my CI (BuildBuddy).

As for the guide, I don't have the capacity to do that as my project progresses, but it might be an option to open an issue with a call for contribution with some guidance and some links to an example. This would be a great first PR for new contributors.

[weiznich]

As for the guide, I don't have the capacity to do that as my project progresses, but it might be an option to open an issue with a call for contribution with some guidance and some links to an example. This would be a great first PR for new contributors.

I'm already really grateful that you wrote this PR. I will find the time at some point for the necessary work to turn this into a full guide, I just wanted to manage expectations that this likely won't be the case tomorrow.