Skip to content

Use prepared statements for database operations - #284

Merged
BenCodez merged 8 commits into
masterfrom
agent/use-prepared-statements
Aug 5, 2026
Merged

Use prepared statements for database operations#284
BenCodez merged 8 commits into
masterfrom
agent/use-prepared-statements

Conversation

@BenCodez

@BenCodez BenCodez commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • bind values in SQLite user lookups and updates
  • bind values in MySQL, MariaDB, and PostgreSQL user inserts, lookups, updates, and deletes
  • bind values in global-data database operations
  • add regression coverage for quotes and punctuation in stored values

Why

Building values directly into query strings makes ordinary names and stored values containing quotes or punctuation fail to round-trip correctly. Prepared statements keep SQL structure separate from values and make behavior consistent across supported database engines.

Validation

  • parsed all changed Java sources successfully
  • added focused Mockito and JUnit coverage for SQLite binding
  • full Maven validation will run in GitHub Actions because the local environment does not include Maven or a Java 21 compiler

@BenCodez
BenCodez marked this pull request as ready for review August 5, 2026 22:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4d495baea3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +919 to +923
if (value.isBoolean()) {
return value.getBoolean();
}
if (value.isInt()) {
return value.getInt();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bind text-backed global values as strings

When PostgreSQL global data stores a boolean or integer in a dynamically created column, addColumn has declared that column as TEXT, but this helper now binds a typed Boolean or Integer. PostgreSQL does not implicitly assign those parameter types to a text column, so calls such as GlobalDataHandler.setBoolean fail instead of storing the value; the previous quoted literals were coercible to the target type. Bind these values as their string representations for text-backed columns, or create columns with matching database types.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b15cb64e77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +896 to +897
if (value.isBoolean()) {
return value.getBoolean();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bind boolean user values as text

On PostgreSQL, the built-in boolean user keys such as CheckWorld and isBedrock are declared as VARCHAR(5) by UserDataKeyBoolean, but this helper binds their values as typed booleans. PostgreSQL cannot assign a boolean parameter to a varchar column, so both inserts and updates of these keys fail instead of persisting the user data; convert boolean values to their string representations when targeting these text-backed columns.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dcbb26e761

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +922 to +923
if (value.isInt()) {
return String.valueOf(value.getInt());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve integer bindings for PostgreSQL integer columns

When a global key is backed by an actual PostgreSQL INTEGER column—such as one recognized or converted by alterColumnType(key, "INT") and tracked in intColumns—this converts its DataValueInt into a varchar parameter. PostgreSQL will reject the resulting insert or update because a character-varying parameter cannot be assigned to an integer column, so GlobalDataHandler.setInt stops persisting that key. Fresh evidence beyond the earlier text-column report is the class's explicit support for INTEGER-backed global columns; select the bound representation from the column's actual type rather than converting every integer to text.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05f7f99338

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +933 to +934
if (value.isInt()) {
return isIntColumn(column) ? value.getInt() : String.valueOf(value.getInt());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Verify the database type before binding global integers

When PostgreSQL alterColumnType(key, "INT") attempts to convert a text-backed global column, its ALTER ... TYPE INTEGER lacks the required USING expression and can fail, but the method still adds the key to intColumns; this branch consequently binds future setInt values as typed integers to the unchanged text column, causing inserts and updates to fail. Fresh evidence beyond the earlier integer-binding report is that alterColumnType marks the column as integer regardless of whether its asynchronous ALTER succeeds, so isIntColumn does not establish the actual schema type.

Useful? React with 👍 / 👎.

@BenCodez
BenCodez merged commit 9e7b9bc into master Aug 5, 2026
4 checks passed
@BenCodez
BenCodez deleted the agent/use-prepared-statements branch August 5, 2026 23:05
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.

1 participant