-
Notifications
You must be signed in to change notification settings - Fork 22
Support boolean restrictions #78
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
Conversation
mfenniak
left a comment
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.
Thanks for the proposed contribution!
I'm wondering if there's one case that could be considered here that isn't:
SELECT * FROM MyTable WHERE NOT flag; -- inverted var
It would be great if you could add to one of the test suites all of the supported cases -- multicorn_regression_test.sql would be ideal as there's a small Test quals section there that could be expanded with the new boolean operations to ensure that they work in all environments and are maintained in the future.
A very small nit but it looks like there's a mix of tab & space indentation in the newly added code, if you don't mind standardizing it to match the source file that would be amazing.
I could be wrong, but I don't think this is either a |
I think I have fixed it. |
I would love to add tests, but I have not yet succeeded in running the existing test suite. |
I run a little unusual dev environment (running NixOS) so I can't advise a whole lot on this -- in NixOS, or if Nix is installed, it's documented in the README (https://github.com/pgsql-io/multicorn2?tab=readme-ov-file#integration-tests). But outside of that environment... theoretically it should just require If you have troubles, we could merge this in and I can add a test... but I took a quick shot at it and ran into a concern you might want to review first. I added these test statements near the end of ...
SELECT * from testmulticorn where test3 = 12::money;
SELECT * from testmulticorn where test1 = '12 €';
-- Test boolean operation pushdown
ALTER FOREIGN TABLE testmulticorn alter test1 type bool;
select * from testmulticorn where test1;
select * from testmulticorn where test1 = TRUE;
select * from testmulticorn where test1 = FALSE;
select * from testmulticorn where test1 IS TRUE;
select * from testmulticorn where test1 IS FALSE;
select * from testmulticorn where test1 IS UNKNOWN;
select * from testmulticorn where test1 IS NOT UNKNOWN;
DROP USER MAPPING FOR current_user SERVER multicorn_srv;
DROP EXTENSION multicorn cascade;
...Resulted in a warning and no quals being pushed down, in the |
Done. |
Using |
I wasn't ready to cut a PR for this, but I have playing with adding a dev.container. |
|
@bovlb Sorry for the inconvenience on testing -- I'll try to write up some useful dev testing instructions in the future. In the meantime I popped in my tests after your |
Currently, queries like:
Are not passed down in quals. This change fixes that by adding support for BooleanTest and Var.
Tested in PostgreSQL 17