Skip to content
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

Limit Postgres MIN/MAX to an arity of one #4526

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eygraber
Copy link
Contributor

No description provided.

@eygraber
Copy link
Contributor Author

eygraber commented Aug 21, 2023

Changes like this are going to make the compiler tests a little more difficult as-is because there might need to be different variants of tests for different dialects (e.g. max takes the proper type in ExpressionTest would need to account for sqlite allowing multiple arguments to MAX but other dialects limiting it to one argument).

Any thoughts on how to address that? I was originally thinking of having packages of tests for common behavior, and then individual ones for each dialect as needed. It can get clunky in cases where most dialects have one behavior (like with MAX), i.e. should each dialect have the same test, or put one test in the common package for the majority of the dialects and have other tests for specially behaving dialects as needed?

Edit: maybe that wasn't the best example since the tests aren't really needed for non sqlite dialects since they only operate on one argument, and those tests are related to picking the correct type when multiple arguments are passed.

@eygraber
Copy link
Contributor Author

eygraber commented Feb 2, 2024

@hfhbd any thoughts on this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant