Skip to content

fix: MySQL compatibility for JDBC plugin temporal writes - #132

Open
xudamao2015 wants to merge 2 commits into
openmemind:mainfrom
xudamao2015:fix/mysql-compatibility
Open

fix: MySQL compatibility for JDBC plugin temporal writes#132
xudamao2015 wants to merge 2 commits into
openmemind:mainfrom
xudamao2015:fix/mysql-compatibility

Conversation

@xudamao2015

Copy link
Copy Markdown

fix JDBC 插件时间字段写入与 admin memory 查询的 MySQL 兼容问题

- InstantTypeHandler: write java.time.Instant as JDBC Timestamp instead of
  Instant.toString() (ISO-8601 with 'Z' + nanosecond precision). MySQL DATETIME
  rejects the ISO string with 'Incorrect datetime value'; SQLite TEXT and
  PostgreSQL TIMESTAMPTZ tolerated it. setTimestamp is portable across drivers.
- MemoryAdminService.listMemories: the COUNT query wraps a derived table
  without an alias; MySQL requires every derived table to have an alias
  ('Every derived table must have its own alias'). Add 'memory_count' alias.
The JDBC plugin (memind-plugin-jdbc-core) writes java.time.Instant as
ISO-8601 strings (Instant.toString()) via Jdbi and graph operations.
MySQL DATETIME(3) rejects those strings with 'Incorrect datetime value';
only SQLite TEXT and PostgreSQL TIMESTAMPTZ tolerated them.

- JdbiTemporalSupport: bind Instant as JDBC Timestamp; parseInstant now
  also accepts the 'yyyy-MM-dd HH:mm:ss[.SSS]' space-separated format.
- JdbcGraphOperations: dialect-aware setTemporal() (setString for SQLite,
  setTimestamp for MySQL/PostgreSQL) used by entity/mention/link/alias/
  cooccurrence writes; the varargs binder passes Instant directly instead
  of a pre-formatted string.

SQLite behavior is unchanged (still stores ISO-8601 text).
@xudamao2015 xudamao2015 changed the title fix: MySQL compatibility for JDBC plugin temporal writesFix/mysql compatibility fix: MySQL compatibility for JDBC plugin temporal writes Aug 4, 2026
@starboyate

Copy link
Copy Markdown
Collaborator

@xudamao2015 Thanks for the fix — the MySQL direction and the derived-table alias look right. However, I don’t think this is safe to merge yet:

  • On SQLite, setTimestamp() stores epoch-millis text, while existing rows use ISO/date strings. Mixing these formats breaks time ordering and range queries after an upgrade.
  • On MySQL with a non-UTC connection timezone, setTimestamp() followed by getString() and UTC parsing shifts the timestamp (for example, 07:00Z round-trips as 15:00Z in Asia/Shanghai).
  • The new Jdbi parser turns a valid offset value such as 2026-04-27T15:00:00+08:00 into the invalid ...+08:00Z.

spotless:check also currently fails in JdbcGraphOperations. Could we keep temporal handling dialect-aware and add round-trip tests for SQLite and non-UTC MySQL before merging?

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.

2 participants