Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
vitcpp merged 3 commits intopostgrespro:masterfromcybertec-postgresql:pg16
Aug 16, 2023

Conversation

df7cb
Copy link
Contributor

  • 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

@esabol
Copy link
Contributor

esabol commentedJul 31, 2023
edited
Loading

Would it be premature to add PostgreSQL 16 to the Travis CI? Doesapt.postgresql.org.sh support PostgreSQL 16 yet? If 16 does work, then I suggest adding it to.travis.yml in this PR.

Just curious, where didAbs() come from in the first place? Is that an older PostgreSQL thing?

@vitcpp
Copy link
Contributor

I'm not sure how the reference output file moc100_x.out is chosen for comparison. Is it performed automatically by test scripts?

@df7cb
Copy link
ContributorAuthor

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

Would it be premature to add PostgreSQL 16 to the Travis CI? Doesapt.postgresql.org.sh support PostgreSQL 16 yet? If 16 does work, then I suggest adding it to.travis.yml in this PR.

Of course PG16 is already supported. I'll add that shortly.

Just curious, where didAbs() come from in the first place? Is that an older PostgreSQL thing?

Abs was removed in PG16.

@vitcpp
Copy link
Contributor

@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?

@df7cb
Copy link
ContributorAuthor

The problems that the testsuite has are independent from this pull request.

  • On Ubuntu bionic 18.04 (now), there is no PG16 since bionic's zstd is too old so I don't provide that on apt.postgresql.org
  • On Ubuntu jammy 22.04, all jobs fail because of compiler warnings (-Werror) in other areas of the code
  • On Ubuntu focal 20.04 (which I now put into this PR), only PG10 and PG11 fail, again for something that this PR doesn't touch

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.

esabol reacted with thumbs up emoji

@vitcpp
Copy link
Contributor

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.

esabol reacted with thumbs up emoji

@esabol
Copy link
Contributor

@vitcpp wrote:

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.

This sounds to me like a very good plan.

Thank you,@df7cb, for detailing the issues with the test suite and such.

@dura0ok
Copy link

@df7cb I cherry-picked all your commits to my branch, made a small change and the tests passed.
You can see in#41
@vitcpp@esabol what do you think?

esabol reacted with thumbs up emoji

@vitcpp
Copy link
Contributor

@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
@df7cb
Copy link
ContributorAuthor

Branch rebased.

esabol and vitcpp reacted with thumbs up emoji

Copy link
Contributor

@esabolesabol left a 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 !

vitcpp reacted with thumbs up emoji
@vitcppvitcpp merged commit1709fd5 intopostgrespro:masterAug 16, 2023
@df7cbdf7cb deleted the pg16 branchOctober 5, 2023 13:37
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@esabolesabolesabol approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@df7cb@esabol@vitcpp@dura0ok

[8]ページ先頭

©2009-2025 Movatter.jp