Skip to content

Conversation

@frin1
Copy link
Contributor

@frin1 frin1 commented Sep 27, 2025

Add postgres support

  • Introduced a dialect field in service structs (DepositService, OrderService, RewardService, TradeService, WithdrawService) to handle database-specific SQL generation.
  • Added ensureDialect method to initialize the dialect if not set.
  • Updated SQL queries to use dialect-aware methods for table and column names, ensuring compatibility with MySQL and PostgreSQL.
  • Refactored insert and upsert logic to utilize dialect-specific UUID handling and SQL generation.
  • Improved test coverage for PostgreSQL dialect handling in RewardService and TradingVolume queries.
  • Added a new configuration file for PostgreSQL connection settings.

@frin1 frin1 requested a review from c9s as a code owner September 27, 2025 09:24
@CLAassistant
Copy link

CLAassistant commented Sep 27, 2025

CLA assistant check
All committers have signed the CLA.

@c9s
Copy link
Owner

c9s commented Sep 27, 2025

You need to use the migration tool c9s/rockhopper and make sure this PR works well with rockhopper

@c9s
Copy link
Owner

c9s commented Sep 27, 2025

Also, could you please split the changes like fixes should be in another PR

For supporting postgresql, it could be incremental backward compatible changes in multiple PRs, it doesn't have to be in one large PR

@frin1
Copy link
Contributor Author

frin1 commented Sep 27, 2025

You need to use the migration tool c9s/rockhopper and make sure this PR works well with rockhopper

For creating go code from the migrations, I used rockhopper compile --config rockhopper_postgres.yaml --output pkg/migrations/postgres. Is there anything else I need to do with rockhopper?

@frin1
Copy link
Contributor Author

frin1 commented Sep 27, 2025

Also, could you please split the changes like fixes should be in another PR

For supporting postgresql, it could be incremental backward compatible changes in multiple PRs, it doesn't have to be in one large PR

I have reverted the fixes.

@c9s
Copy link
Owner

c9s commented Sep 29, 2025

Thanks for your pull request,

but CI build failed, could you please check? thanks

Upgrading Go is acceptable, but you need to split the PRs and each one should focus on its topic

for example, this commit 7bdeebb is not related to postgres.

since this project is used by many companies on production, we would prefer low impact PR comes first.

c9s and others added 5 commits September 29, 2025 07:05
even if uncover position is dust or zero, we might still have covered
position to be processed, things like canceling order and returning
the pending position for the next round
- Introduced a `dialect` field in service structs (DepositService, OrderService, RewardService, TradeService, WithdrawService) to handle database-specific SQL generation.
- Added `ensureDialect` method to initialize the dialect if not set.
- Updated SQL queries to use dialect-aware methods for table and column names, ensuring compatibility with MySQL and PostgreSQL.
- Refactored insert and upsert logic to utilize dialect-specific UUID handling and SQL generation.
- Improved test coverage for PostgreSQL dialect handling in RewardService and TradingVolume queries.
- Added a new configuration file for PostgreSQL connection settings.
@frin1 frin1 closed this Sep 29, 2025
@frin1 frin1 deleted the feature/postgres branch September 29, 2025 05:18
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