Skip to content

chore: bump pgx to v5 #1032

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

Merged
merged 1 commit into from
Mar 24, 2025
Merged

chore: bump pgx to v5 #1032

merged 1 commit into from
Mar 24, 2025

Conversation

iwpnd
Copy link
Member

@iwpnd iwpnd commented Mar 5, 2025

description

  • bump pgx to v5
  • refactor registering types on connect
  • hstore now is a buildin type with changed behaviour, it was required to treat it separately in decipherFields

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far! Thank you for kicking off this upgrade. I left some minor comments inline on my first pass.

@iwpnd
Copy link
Member Author

iwpnd commented Mar 11, 2025

I wanted to test the hstore tags and noticed that both in master and in my branch the tags do not show properly in the ui. Default tags are ignored as well. @ARolek

update:
was able to get tags in master disabling the file cache. the fields property doesn't work either. the only way to get tags into features is by extending the sql statement to include the fields.

update2:
ok, all of those options are postgis provider only. 💡

@iwpnd
Copy link
Member Author

iwpnd commented Mar 11, 2025

tests will pass once #1033 is accepted :)

@iwpnd iwpnd force-pushed the chore/bump-pgx-v5 branch from b8ca315 to f99af44 Compare March 13, 2025 08:14
@iwpnd iwpnd marked this pull request as ready for review March 13, 2025 08:20
@iwpnd iwpnd requested a review from gdey as a code owner March 13, 2025 08:20
@iwpnd iwpnd requested a review from ARolek March 13, 2025 08:20
Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few questions. Overall it looks good.

@iwpnd
Copy link
Member Author

iwpnd commented Mar 18, 2025

@ARolek can you please give this another look? 👀

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks excellent! My comments are rather minor, and not blockers. I'm going to approve this PR as is. If you choose to land the slog PR and then adjust LogAdapter in this PR, please re-tag me for review. I would like to see how that all comes together.

@ARolek
Copy link
Member

ARolek commented Mar 21, 2025

@iwpnd review in! Sorry for the delayed response. Heading out on vacation this weekend and it has been an intense week trying to wrap everything up. This PR looks great. Thank you for tackling this upgrade!

@iwpnd iwpnd force-pushed the chore/bump-pgx-v5 branch from 4757566 to 19a3d36 Compare March 22, 2025 12:35
chore: restore previous behaviour of storing hstore oid and reuse
chore: remove interface{}, use any
chore: scripts -> testdata
test: testdata migration script and tegola.dump in repo
fix: pgx/v4 logger behaviour
revert: keep behaviour of not forwarding primary key to tile tags
@iwpnd iwpnd force-pushed the chore/bump-pgx-v5 branch from 19a3d36 to b4a11d3 Compare March 22, 2025 12:38
@iwpnd
Copy link
Member Author

iwpnd commented Mar 22, 2025

Are you also okay with it @gdey? Feel free to merge if you are. :)

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gdey gdey merged commit cd42cb0 into go-spatial:master Mar 24, 2025
11 checks passed
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.

3 participants