Skip to content

Get SQL databases working again #7653

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

Open
wants to merge 48 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.

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.
  • Investigate if this solves Critical Bug with List-Variables #2022

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

@TheLimeGlass TheLimeGlass requested a review from a team as a code owner April 15, 2025 18:50
@TheLimeGlass TheLimeGlass requested review from UnderscoreTud and sovdeeth and removed request for a team April 15, 2025 18:50
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.

Changes from @APickledWalrus in #5646 still not addressed

@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label May 15, 2025
@sovdeeth
Copy link
Member

sovdeeth commented Jun 3, 2025

Closing due to change requests outstanding for 6 months (#5646 (review))

@sovdeeth sovdeeth closed this Jun 3, 2025
@TheLimeGlass
Copy link
Contributor Author

huh?

@TheLimeGlass
Copy link
Contributor Author

Get Pickle to write the new requested changes. Open this pull request

@APickledWalrus APickledWalrus reopened this Jun 3, 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.

Looks good!

@TheLimeGlass TheLimeGlass requested a review from sovdeeth July 16, 2025 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) 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 reviews A PR that needs additional reviews 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
7 participants