nix: add postgrest-with-oriole-18#4845
Conversation
717f0b4 to
50a6046
Compare
This comment was marked as resolved.
This comment was marked as resolved.
6b817d2 to
1b44dfa
Compare
This comment was marked as resolved.
This comment was marked as resolved.
1b44dfa to
146115e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
146115e to
5a9fc47
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
5a9fc47 to
f1d0e8e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
f1d0e8e to
0150b05
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
After doing that, only a few tests fail where we hardcoded a certain expectation of cost. The cost changed because a different plan was chosen - instead of "Seq Scan" it uses "Custom Scan": The query should essentially be a simple PG 17: Oriole: |
|
There's one more thing that fails, which is the build on MacOS. I tracked the error message down to the thread that resulted in postgres/postgres@0dceba2, which does not seem to be present in |
9c079fc to
7b8048e
Compare
Looking at this some more makes me wonder whether the regular Seq Scan has just associated increased cost estimations now - and that's why the index scan is chosen in this case? Any idea, @akorotkov? |
|
@wolfgangwalther, OrioleDB implements index-organized tables. Even if there is no primary key, OrioleDB organizes the table as an index over an invisible CTID column. It has an equivalent to seq scan, but it's internally different and has a different estimate. |
7b8048e to
cb272f3
Compare
Thanks - that's what I suspected. So the different cost estimates / plans in this case are OK. I already tried building on MacOS - works now. This means we have all test failures figured out, no questions remain. (can't merge this before we sort out packaging issues in Nixpkgs, though) |
cb272f3 to
2755a26
Compare
|
Maybe we should add a small guide on https://docs.postgrest.org/en/v14/integrations/ after this is merged, showing how to use the NixOS postgres module with Orioledb, also mentioning that we test against it. (at some point I remember we had a Greenplum integration, but wasn't really tested). |
242ed78 to
9d69c96
Compare
9d69c96 to
d36fcb5
Compare
This is resolved now, Still need to find a way to special-case the tests for the different cost estimates and write that small integration/howto. Edit: Currently waiting on NixOS/nixpkgs#540609, though, because |
|
There now seems to be another failing test that we either missed earlier or that came along in the meantime - I think I updated orioledb to a newer version before merging it in Nixpkgs, so that could be the case. (test results are not online, I only have them locally with a fixed gdal/postgis) It's the Comparing PG 17 and orioledb 17: Orioledb does not show the query identifier on |
@wolfgangwalther Can't reproduce this branch locally, I get a postgis compile error (after considerable time compiling). How do you manage to run the tests to get the above error? |
I updated my nixpkgs branch locally to one that contains the fix for that compilation problem. It's from some random PR, so I did not push it. This PR has merged and I'm watching the channels at https://nixpk.gs/pr-tracker.html?pr=540826 - once this hits nixpkgs-unstable, I will update this branch and push it. You can then see it in CI. Edit: Note that this will also include NixOS/nixpkgs#541173, i.e. will update orioledb's postgres from 17 to 18. So we might see different results anyway. |
d36fcb5 to
5a4de49
Compare
|
(did not test, just pushed the new nixpkgs pin - let's see what CI says, we might need to fix more things, always a chance on a bump) |
5a4de49 to
e5bbe6c
Compare
In my local tests this did not make a difference. I pushed builds to cachix, so hopefully the CI job will not timeout this time. Let's see... |
|
I have no idea why the style-check fails in CI, it does not for me locally. |
That has happened before for me too. I think the CI picks a different (newer) version of the python linter. |
Hiding this will also hide errors when they happen... In my case, my nix-shell's TMPDIR was too long, so black was failing silently because it couldn't create a unix socket for its forkserver.
Nixpkgs has removed support for x86_64-darwin, so removing that from CI's flake check. A x86_64-darwin binary is still built via Stack.
e5bbe6c to
07a3614
Compare
That should certainly not happen because Nix pins all those dependencies for us. It was a different problem: Fixed that and then it went on to reformat the same files as CI. Now the only remaining failure will be the spec test failure about the missing Query Identifier. We should probably report that upstream. |
|
Reported upstream orioledb/postgres#72 |
Tests PostgREST against orioledb, which is packaged on a temporary branch on my Nixpkgs fork. Thus, not ready for merging, yet.
We will be able to see some failing spec tests, though - did not check the other test suites, yet.
Open issues:
SELECTwithANYclause uses a more costly custom/index scan in orioledb. It's questionable whether that's a bug or whether we'd just need to special case the cost estimation here.TODO: