Skip to content

fix(mariadb): quote/escape unmapped JDBC values and keep now()/default bare in SQL conversion - #4

Closed
HandSonic wants to merge 10 commits into
mainfrom
fix/review-mariadb
Closed

HandSonic wants to merge 10 commits into
mainfrom
fix/review-mariadb

Conversation

@HandSonic

Copy link
Copy Markdown
Owner

Fixes verified review findings: (1) convertJDBCValueStrByType fell back to the raw unquoted super.convertJDBCValueByType for factory-unmapped types (VARCHAR/CHAR/INT/DATE/JSON/ENUM/SET), emitting unescaped values like O'Brien into exported INSERTs — now falls back to the Str variant with quoting/escaping; (2) convertSQLValueByType dropped the FUNCTION_SET short-circuit, quoting now()/default as string literals for TIMESTAMP/DATETIME — restored. Regression tests added. Part of OtterMind#2281.

…t bare in SQL value conversion (code review follow-up)

@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: 1445f4aa88

ℹ️ 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".


import static org.junit.jupiter.api.Assertions.assertEquals;

class MariaDBValueProcessorTest {

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 Make the new backend tests runnable in CI

This class, along with the other newly added *Test classes, is not exercised reliably by .github/workflows/ci.yml: the reactor has stale Surefire discovery and failure-ignore defaults, but the workflow invokes Maven without -Dsurefire.includes=**/*Test.java and -Dmaven.test.failure.ignore=false. CI can therefore report success without running these MariaDB regression assertions, or despite their failure; update the backend test command with the required overrides and verify a nonzero test summary.

AGENTS.md reference: AGENTS.md:L225-L247

Useful? React with 👍 / 👎.

Comment on lines +9 to 10
if (windows && community) {
return "Chat2DB Community";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the Community title on Linux

On Linux Community JCEF, OS.isWindows() is false while setupGenericPlatformSpecifics() still applies appName as the frame title, so this branch falls through to Chat2DB Pro. The packaged Community application is therefore visibly misidentified as a commercial edition on Linux; select the Community title whenever community is true and keep any platform-specific decision at the call site.

AGENTS.md reference: AGENTS.md:L44-L50

Useful? React with 👍 / 👎.

openai0229 and others added 5 commits July 28, 2026 11:44
… not leaked (code review follow-up) (OtterMind#2304)

Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
…2312)

Set the default nullable value of newly added columns in the table
editor to NullableType.Null, so new non-primary-key columns start as
nullable. Primary key columns are still forced to NOT NULL by the
existing handelPrimaryKey handler.

Closes OtterMind#2245

Co-authored-by: liulipengju <monkeycode-ai@chaitin.com>
Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
…m (code review follow-up) (OtterMind#2303)

Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
@HandSonic HandSonic closed this Aug 23, 2026
@HandSonic
HandSonic deleted the fix/review-mariadb branch August 23, 2026 08:13
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