Поддержка YDB в Jimmer ORM

[Rinoyt]

Добавлены диалект и вспомогательные классы для поддержки YDB в Jimmer ORM.

[robot-vibe-db]

Severity: Major
Confidence: High

If block.apply(connection) throws a RuntimeException (or any unchecked exception), the catch (SQLException) block is skipped, so connection.rollback() is never called. The connection is then closed by try-with-resources, but whether the JDBC driver auto-rolls-back on close() is driver-specific and not guaranteed by the JDBC spec.

This can lead to partially committed data if the YDB driver doesn't auto-rollback on close.

Suggested fix: add a generic catch (Exception ex) block (or a finally that rolls back if not committed), for example:

try {
    connection.setAutoCommit(false);
    R result = block.apply(connection);
    connection.commit();
    return result;
} catch (Exception ex) {
    connection.rollback();
    if (ex instanceof SQLException sqlEx && (i < maxRetries - 1) && isRetryable(sqlEx)) {
        // continue retry loop
    } else {
        throw (ex instanceof RuntimeException re) ? re : new RuntimeException(ex);
    }
}

[robot-vibe-db]

Severity: Major
Confidence: Medium

The propagation parameter is accepted but completely ignored. Jimmer's TxConnectionManager.executeTransaction contract expects the implementation to respect propagation semantics (REQUIRED, REQUIRES_NEW, NESTED, etc.).

Currently, every call creates a brand-new independent transaction, which is equivalent to REQUIRES_NEW. If a caller passes REQUIRED expecting to join an existing transaction, this implementation silently creates a separate one, which can break transactional consistency.

Suggested fix: either implement propagation support (e.g., using a ThreadLocal to track the current connection/transaction) or document that this class only supports REQUIRES_NEW and throw UnsupportedOperationException for other modes.

[robot-vibe-db]

Severity: Major
Confidence: Medium

The batchSize parameter is validated in the constructor (batchSize < 1 throws) and passed to the superclass, but the save() method here batches all keys in a single executeBatch() call regardless of batchSize. Since both delete() and deleteAll() are overridden, the superclass's chunking logic (which would split work into batchSize-sized batches) is bypassed.

For a large collection of cache keys, this could cause memory pressure or exceed YDB's batch statement limits.

Suggested fix: chunk the keys collection into batchSize-sized groups and call executeBatch() for each chunk:

int count = 0;
for (Object key : keys) {
    // ... setString calls ...
    stmt.addBatch();
    if (++count % batchSize == 0) {
        stmt.executeBatch();
    }
}
if (count % batchSize != 0) {
    stmt.executeBatch();
}

(where batchSize needs to be stored as an instance field)

[robot-vibe-db]

Severity: Major
Confidence: High

--restart=always and --rm are mutually exclusive Docker flags. On Docker 18.06+ this produces an error:

Conflicting options: --restart and --rm

The script will fail to start the container.

Suggested fix: remove either --rm (if you want the container to persist and auto-restart) or --restart=always (if you want a disposable container that is removed on stop).

[robot-vibe-db]

Severity: Minor
Confidence: High

getIsolationClient() builds a JSqlClient without calling .setDialect(new YdbDialect()). This means the client returned to tests (used in AbstractTransactionTest, KeysetTest) uses Jimmer's default dialect rather than the YDB dialect.

Currently the tests only execute simple queries where the default dialect happens to produce compatible SQL, but this will silently produce incorrect SQL for any YDB-specific operations (UPSERT, type mappings, etc.).

Suggested fix:

return new YqlClient(
    (JSqlClientImplementor) JSqlClient.newBuilder()
        .setDialect(new YdbDialect())
        .setConnectionManager(new YdbTxConnectionManager(dataSource))
        .setExecutor(executor)
        .build());

[robot-vibe-db]

Severity: Minor
Confidence: Medium

classToYdbType and classToJdbcType are declared as public static final but the concrete type is a mutable HashMap. Any code that has access to these fields can call put(), remove(), or clear() on them, corrupting the shared mapping for all callers.

Suggested fix: wrap them with Collections.unmodifiableMap() after the static initializer block populates a temporary mutable map, or use Map.ofEntries(...) for an immutable map.

[robot-vibe-db]

Severity: Minor
Confidence: Medium

liquibase-core is declared at compile scope but is not imported or used anywhere in the main source code (src/main/java). This adds a significant transitive dependency tree (~3 MB+) to the artifact for no functional benefit.

If Liquibase is intended for future use, consider changing the scope to provided or test. If it's not needed, remove it.

[robot-vibe-db]

AI Review Summary

Verdict: ✅ No critical issues found

Critical issues

No critical issues found.

Other findings

• Major | High: RetryConnectionManager.executeTransaction — RuntimeException from the user block does not trigger explicit rollback; relies on driver-specific close behavior — RetryConnectionManager.java:38
• Major | Medium: RetryConnectionManager.executeTransaction — Propagation parameter is accepted but completely ignored — RetryConnectionManager.java:30
• Major | Medium: UuidTransactionCacheOperator.save() — batchSize constructor parameter is validated but never enforced; all keys are batched in a single executeBatch() call — UuidTransactionCacheOperator.java:107
• Major | High: install.sh — --restart=always and --rm flags are mutually exclusive in Docker and will cause the script to fail — install.sh:3
• Minor | High: AbstractTest.getIsolationClient() — YdbDialect is not set on the builder; test client uses the default Jimmer dialect instead of YDB — AbstractTest.java:59
• Minor | Medium: YdbClassMapping — classToYdbType and classToJdbcType are mutable HashMap exposed as public static final; any caller can mutate them — YdbClassMapping.java:24
• Minor | Medium: pom.xml — liquibase-core is at compile scope but not used anywhere in main sources — pom.xml:74

---

This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.