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

Get SQL databases working again #7653

Open
wants to merge 40 commits into
base: dev/feature
Choose a base branch
from

Conversation

TheLimeGlass
Copy link
Contributor

@TheLimeGlass TheLimeGlass commented Feb 27, 2025

Description

  • Adds H2 as a configurable database. Will probably make default eventually or maybe in this pull request?
  • Removes SQLibrary and adds https://github.com/brettwooldridge/HikariCP as the main SQL wrapper.
  • Revamped API for better registration and handling of JDBC Java drivers.
  • Added a commit changes option which is essentially how Skript used to operate. Changed the JDBC standard to auto commit after every edit (Make the 5-min variable saving period configurable #2007). This is standard and better for performance. Skript didn't for some reason. I speculate it was to allow MySQL to sync with other servers, so either way it's an option now.

Perks

  • Better configuration options with HikariCP.
  • Way better performance with HikariCP.
  • Easier implementation and not relying on a small project like SQLibary.
  • Thread pooling potential with HikariCP.
  • This new API design allows for quite literally any JDBC implementation database.
  • Uses the SpigotLibraryLoader to load the resources at runtime rather than shadowing into the jar.

TODO

  • Test MySQL.
  • Test SQLite.
  • Test H2.
  • Run beta testing in the SkriptLang Discord.
  • Adds tests.
  • In H2 use H2's MERGE INTO instead of INSERT.
  • Forgot to add a check that HikariCP classes were loaded by SpigotLibraryLoader.
  • Rename SQLStorage to something with JDBC reference, maybe JdbcStorage as it's not only SQL anymore.
  • Test that monitor changes still do something.
  • Attach a SkriptAddon instance to the registration, so Skript knows where a storage is coming from.
  • Investigate if this solves Critical Bug with List-Variables #2022

Optional

  • Write up an API tutorial for registering custom storage types.
  • Document all the possible options for databases or have annotations for a documentation system.
  • See if Skript could handle async and multithreading now that the database has the possibility.
  • Maybe add MongoDB support More DataBase support ? MongoDB for example #1787
  • Change the existing behavior of dumping every variable into ram on server start.

Notes:

  • Should we just remove SQLite support? It's not like anyone is using it right now, it doesn't work currently.
  • H2 is a flat file SQL implementation that supports asynchronous operations and multi threaded unlike SQLite in both those regards.
  • This does not impact anything to do with the default flat file CSV.
  • This mainly only affects JDBC driver databases. The rework is still a viable experiment started by bensku https://github.com/SkriptLang/Skript/tree/feature/variables2

Testing and using this jar

To test this experimental feature out, go to the "checks" tab and then click a Java version on the side and then click the nightly artifacts to get a built jar of the latest commit.


Target Minecraft Versions: any
Requirements: none
Related Issues: #1168, #2007, #1478 and #3948

@Pikachu920
Copy link
Member

could you push the SkriptLang branch to your fork, then do the work there? once things are ready you can open a PR from that branch to SkriptLang

@Pikachu920 Pikachu920 closed this Feb 27, 2025
@TheLimeGlass

This comment was marked as resolved.

@Pikachu920
Copy link
Member

Pikachu920 commented Feb 27, 2025

I guess I'm confused why we'd merge into enhancement/sql instead of having your work on your fork's branch and then you can open a PR from the fork branch to dev/feature

@TheLimeGlass

This comment was marked as resolved.

@TheLimeGlass

This comment was marked as resolved.

@sovdeeth sovdeeth reopened this Mar 10, 2025
@sovdeeth sovdeeth changed the base branch from enhancement/sql to dev/feature March 10, 2025 21:28
@sovdeeth sovdeeth changed the title Enhancement/sql update to 2.10.1 Get SQL databases working again Mar 10, 2025
@sovdeeth sovdeeth mentioned this pull request Mar 10, 2025
16 tasks
@sovdeeth sovdeeth added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. enhancement Feature request, an issue about something that could be improved, or a PR improving something. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. variables Related to variables and/or storing them. labels Mar 10, 2025
Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

See existing change requests from pickle at previous pr, too: #5646 (review)

Comment on lines +167 to +169
### VS Code ###
.vscode/

Copy link
Member

Choose a reason for hiding this comment

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

shouldn't be in this pr

@TheLimeGlass TheLimeGlass requested a review from sovdeeth March 12, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. enhancement Feature request, an issue about something that could be improved, or a PR improving something. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. variables Related to variables and/or storing them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants