Skip to content

Conversation

@ffried
Copy link
Contributor

@ffried ffried commented Nov 14, 2025

The integer type in non-SQLite databases is too small to hold Date.now() values.
This increases the type to bigint. To deal with existing databases, alter table commands were added. They should simply succeed in case the type already matches (which is the case for newly created databases).
This fixes #136.

Also, in MySQL you can't use text with indices (unless specifying a prefix length). This is why I've changed the data type to varchar(4096) for these columns. Note that no alter table is needed for these, because the creation of the table already fails. The size (4096) is currently arbitrarily chosen and can be set to anything large enough (up to 65,535).

Copilot AI review requested due to automatic review settings November 14, 2025 16:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses database compatibility issues by increasing integer column types to bigint (to properly store Date.now() values) and changing indexed text columns to varchar(4096) for MySQL compatibility. It also adds migration logic for existing databases and refactors the periodic cleanup into a separate function.

Key Changes:

  • Changed integer columns storing timestamps to bigint across all tables
  • Introduced MySQL-compatible varchar(4096) for indexed text columns
  • Added migration logic with changeIntToBigInt() function to update existing databases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tiagozip
Copy link
Owner

thanks, ill start reviewing and texting later. just to be sure, can you confirm that varchar(4096) does indeed work on both SQLite, postgres and MySQL?

@ffried
Copy link
Contributor Author

ffried commented Nov 15, 2025

It's only used for MySQL. Postgres an sqlite continue to use text.

@ancashoria
Copy link
Contributor

I have tested this PR and can confirm it fixes this bug

@ancashoria
Copy link
Contributor

After more trial and error, it seems there are still issues with Postgres compatibility

@ffried
Copy link
Contributor Author

ffried commented Nov 27, 2025

After more trial and error, it seems there are still issues with Postgres compatibility

This might totally be the case, since I've only fixed the most obvious once (namely the ones storing timestamps).
Can you elaborate on which issues are still present?

@ancashoria
Copy link
Contributor

ancashoria commented Nov 27, 2025

One issue I encountered was on the redeem request where I got column reference "count" is ambiguous which sounds like an sql error to me.

I also noticed that the UI graph for a site I triggered a few solve requests was not displaying anything, although the count was counting them.

Lastly, I think this SQL query is not working because I got "Invalid site key or secret" when atempting a siteverify. I double checked my secret and all looks good.

@ffried
Copy link
Contributor Author

ffried commented Nov 27, 2025

@ancashoria

  • The count issue is obvious but also simple to fix (fully qualifying works the same no all DBs). I've changed that.
  • The brackets in the select you referenced shouldn't be needed anyways - maybe that fixes it? I don't see any other issues with that select otherwise. Also, this is not a siteverify - this is the challenge request. The error message is also incorrect there, since there's no secret involved. I've adjusted that.
  • As for the graphs, I'm not sure where the issue stems from. Have you tried this with e.g. SQLite?

Can you retest with the changes and let me know if they work for you?

@ancashoria
Copy link
Contributor

Sure, I'll give it a shot.

@tiagozip tiagozip marked this pull request as draft November 30, 2025 19:02
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.

[BUG] [PostgreSQL] Unable to login

3 participants