-
Notifications
You must be signed in to change notification settings - Fork 15
Fix compatibility with PG16 #39
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
df7cb
commented
Jul 31, 2023
- Replace Abs with fabs
- Remove DatumGetPointer where PG16 got stricter typechecking
- Annotate expected/moc100 output files with PG major and architecture bits they are meant for
- Add expected output files for moc100 on PG16
Would it be premature to add PostgreSQL 16 to the Travis CI? Does Just curious, where did |
I'm not sure how the reference output file moc100_x.out is chosen for comparison. Is it performed automatically by test scripts? |
https://www.postgresql.org/docs/current/regress-variant.html
Of course PG16 is already supported. I'll add that shortly.
Abs was removed in PG16. |
@df7cb Thank you for the explanation. I'm ok with the patch but we have to fix tests for pg16. I'm not sure how to fix it yet. May be to change snapshot to the latest one? |
The problems that the testsuite has are independent from this pull request.
The issues should be fixed, but in a separate PR. This PR does fix the actual PG16 problems and I already applied it in the packages on apt.postgresql.org. |
I propose to enable and fix PG16 tests in another PR and then apply this patch. I can do it but I'm on vacation this week with limited access to the internet. |
@df7cb We fixed some warning messages that resulted in test failures. Could you please be so kind to re-sync your branch? Thank you in advance! |
* Replace Abs with fabs * Remove DatumGetPointer where PG16 got stricter typechecking * Annotate expected/moc100 output files with PG major and architecture bits they are meant for * Add expected output files for moc100 on PG16
Branch rebased. |
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.
Looks good to me! Let's merge. Thank you , @df7cb !