Skip to content

fix(cockroachdb): use PostgreSQL grammar for SQL parsing and align dbType on COCKROACHDB - #2

Closed
HandSonic wants to merge 125 commits into
mainfrom
fix/review-cockroachdb
Closed

HandSonic wants to merge 125 commits into
mainfrom
fix/review-cockroachdb

Conversation

@HandSonic

Copy link
Copy Markdown
Owner

Fixes verified review findings: (1) CockroachSqlParser extended the MySQL parser although CockroachDB speaks the PostgreSQL dialect — now extends PgsqlSqlParser (UPSERT/ILIKE/::-casts/$$ bodies/double-quoted identifiers parse correctly); (2) dbType identity mismatch: syntax plugin registered as COCKROACH while cockroachdb.json declares COCKROACHDB — enum renamed to COCKROACHDB so string and enum lookups converge. Regression tests added. Part of OtterMind#2281.

@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: 1a6fe5dc62

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

return List.of();
}
ValueRange range = index < values.size() ? values.get(index) : new ValueRange(cursor, cursor);
boolean active = values.isEmpty() ? index == 0 : index == values.size() - 1;

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 Advance the active INSERT hint after a trailing comma

For PostgreSQL input such as INSERT INTO demo (id, name) VALUES (1, , valueRanges omits the empty trailing segment, so values.size() remains 1 and this marks column 0 (id) active instead of column 1 (name). The editor consequently highlights and displays the type hint for the previous value while the user enters the next one. Preserve the trailing empty value or derive the active index from top-level separators, and cover this incomplete-row case with a focused plugin test.

AGENTS.md reference: AGENTS.md:L144-L145

Useful? React with 👍 / 👎.

{record.context.schemaName || record.context.databaseName || 'SQL'}&gt;
</span>
<SQLPreview className={styles.sql} sql={record.sql} source="execution-console" />
<div className={styles.scrollArea} ref={scrollRef}>

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 Stop following when the user scrolls away

In the frontend execution console, the scroll container no longer has an onScroll handler, so manually scrolling away from the latest edge leaves followLatest true. During an ongoing execution, every new record or ResizeObserver notification then calls alignToLatest and snaps the user back, making earlier output impractical to inspect unless they discover and disable the context-menu option first. Restore an order-aware scroll handler that clears followLatest when the user leaves the relevant edge.

Useful? React with 👍 / 👎.

static String resolve(boolean community, boolean localEdition) {
if (community) {
static String resolve(boolean windows, boolean community, boolean localEdition) {
if (windows && 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.

P1 Badge Preserve the Community title on Linux

In a packaged Community Linux desktop run, OS.isWindows() is false while ConfigUtils.isCommunity() is true, so this branch falls through to Chat2DB Pro; MainJFrame.setupGenericPlatformSpecifics then applies that value with setTitle. This visibly rebrands the Community package as Pro on Linux. Resolve Community identity independently of OS, and handle any macOS title suppression at the call site.

AGENTS.md reference: AGENTS.md:L44-L47

Useful? React with 👍 / 👎.

Comment on lines +570 to +571
{inspectorOpen && inspectorPortalTarget &&
createPortal(

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 Keep the result inspector visible outside the workspace

When SearchResult is rendered on a non-workspace surface such as the chat code-block result, activating the row, value, or aggregate inspector now portals it only into workspace-result-inspector-portal. That host is owned by WorkspaceRight; while another main tab is active its containing workspace component is hidden, or it may not be mounted yet, so inspectorPortalTarget is absent or invisible and the inspector that was formerly inline appears to do nothing. Keep an inline fallback for these callers or move the host to a shared visible layout.

Useful? React with 👍 / 👎.

int endRow = offset + pageSize;
StringBuilder sqlBuilder = new StringBuilder(sql.length() + 120);
sqlBuilder.append(SQL_ROW_NUMBER_PREFIX);
sqlBuilder.append(sql);

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 ordered queries in legacy SQL Server pagination

For SQL Server 2008 or older, or whenever the server version is unavailable, an input such as SELECT * FROM users ORDER BY id is inserted verbatim into the new FROM (...) TMP_PAGE wrapper. SQL Server rejects ORDER BY in a derived table unless it is paired with constructs such as TOP or OFFSET, so this legacy pagination branch fails for common sorted result queries. Extract the top-level ordering into ROW_NUMBER() OVER (...) or use compatible TOP pagination, and add focused plugin coverage.

AGENTS.md reference: AGENTS.md:L144-L145

Useful? React with 👍 / 👎.

openai0229 and others added 23 commits July 28, 2026 11:17
# Conflicts:
#	chat2db-community-client/scripts/i18n-source-hashes.json
… 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>
…t bare in SQL value conversion (code review follow-up) (OtterMind#2302)

Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
…er-growing JDBC URL on reconnect (code review follow-up) (OtterMind#2289)

Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
…-dialect plugin (code review follow-up) (OtterMind#2287)

Co-authored-by: openai0229 <136558319+openai0229@users.noreply.github.com>
…ery connections (OtterMind#2179)

Co-authored-by: zgq <openai0229@gmail.com>
…d#2254)

* fix(导出): 🐛 修复客户端导出连接无响应

- 使用 JCEF 应用内保存对话框处理文件选择、取消和保存失败
- 补充导出准备中、成功、取消和失败反馈及多语言文案
- 新增前端导出流程与 JCEF 保存处理器回归测试

* ci: 👷 .gitignore

---------

Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#2298)

* fix(result-grid): preserve change tracking after no-op undo

* fix(result-grid): restore change tracking after revert failures

* fix(result-grid): keep operation records consistent on failures

* test(result-grid): verify nested tracking suspension

---------

Co-authored-by: zgq <openai0229@gmail.com>
…Mind#2192) (OtterMind#2314)

* fix(clickhouse): preserve Decimal precision when scale is null (OtterMind#2192)

Change the guard condition from (columnSize == null || decimalDigits == null) to
(columnSize == null) only. This prevents an early return that drops precision when
scale is null but precision is set. Also fix the size-only branch to output
Decimal(P,0) instead of Decimal(P) since ClickHouse requires both P and S.
Co-authored-by: monkeycode-ai <monkeycode-ai@chaitin.com>

* test(clickhouse): cover Decimal precision rendering

---------

Co-authored-by: liulipengju <monkeycode-ai@chaitin.com>
Co-authored-by: zgq <openai0229@gmail.com>
…erMind#2313)

Replace key={index} with key={item.treeNodeType} in SelectedList
rendering to prevent React from incorrectly reusing DOM nodes when
the selection list reorders (e.g. when switching database -> schema
-> table). Each item has a guaranteed unique treeNodeType, making
it a stable key.

Closes OtterMind#2153

Co-authored-by: liulipengju <monkeycode-ai@chaitin.com>
…tab-name

fix(ui): refine workspace tabs and navigation
…rMind#2234)

* feat(spi): add quoteIdentifierAlways to ISQLIdentifierProcessor

Add a new SPI method that unconditionally quotes an identifier,
preserving the exact name regardless of case or keyword status.
Embedded delimiter characters are escaped per the dialect convention
(double-quote "" for ", backtick `` for `, bracket ]] for ]).

Fix removeIdentifierQuote to unescape doubled delimiters, satisfying
the round-trip contract: removeIdentifierQuote(quoteIdentifierAlways(raw)).equals(raw).

Implementations:
- DefaultSQLIdentifierProcessor: double-quote + escape " -> ""
- MysqlIdentifierProcessor: backtick + escape ` -> ``
- SqlServerIdentifierProcessor: bracket + escape ] -> ]]

Also add SNOWFLAKE_SQL_IDENTIFIER_PROCESSOR static field to
SnowflakeMetaData, mirroring PostgreSQL/KingBase, to give Snowflake
column-type enums access to the processor for downstream quoting PRs.

Tests: 36 tests covering conditional vs always-quote, embedded
delimiters, round-trip, reserved keywords, and case preservation
across Default, MySQL, and SqlServer processors.

Addresses the maintainer review on OtterMind#2187 requesting the shared SPI
contract be extended before rebasing the individual dialect quoting PRs.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(spi): enforce identifier quoting contract

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
Aias00 and others added 28 commits August 3, 2026 22:10
…d#2386)

* fix(spi): make ConnectionPool cleanup thread a named daemon (OtterMind#2383)

The static initializer started a permanent cleanup loop as a user
(non-daemon) thread, so the JVM could not exit cleanly once the pool class
was loaded — the process hung on shutdown in desktop/embedded/test runs.
Mark the thread as a daemon and name it 'chat2db-conn-pool-cleanup' so it
does not block JVM exit and is attributable in thread dumps.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(spi): verify cleanup thread is daemon

* test: initialize connection pool before thread assertion

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#2380)

* fix(view): /api/rdb/view/delete drops the view instead of the table (OtterMind#2377)

The /delete endpoint was documented as deleting a database view but called
tableService.dropTable, which generates DROP TABLE against a same-named
object — destroying a table instead of removing the view. Build a
DbViewDeleteRequest (tableName -> viewName) and delegate to viewService.drop,
mirroring the /drop endpoint. The TableDeleteRequest input contract is
preserved so existing callers are unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix: bind view deletion request body

* test: verify view delete delegation

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…d#2398)

* fix(converter): delete temp files after DbConverter uploads (OtterMind#2397)

The four upload endpoints (/ncx/upload, /upload, /dbp/upload,
/chat2db/upload) created a temp file via transferToTempFile and never
deleted it, leaking a .tmp file per upload. Wrap each use in try/finally
and delete the temp file best-effort (never throws so it does not mask
the primary result).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(converter): avoid false temp cleanup warnings

* fix: keep temp cleanup best effort

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…d#2404)

* fix(rdb): null-guard view/function/procedure list endpoints (OtterMind#2403)

DbFunctionController/DbProcedureController/DbViewController list endpoints
called .size() directly on the service/converter result, NPEing when it
returned null (some JDBC drivers return null for empty result sets).
DbTriggerController already guarded with CollectionUtils.isNotEmpty. Align
the three with a null-safe size computation.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(rdb): normalize null metadata lists

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(chat): write correct shape in setCurrentChat and handle fetch failure (OtterMind#2401)

setCurrentChat optimistically set currentChat[page] to the whole chat object
(a Record with workspace/dashboard/chat slots) instead of the ChatVO at
chat[page], and chained getChatDetailById without a .catch — so a fetch
rejection left the wrong-shape Record in the slot, making
currentChat[page].id/.title/.chatDetails undefined (blank title, empty
list) until a later successful fetch. Set [page]: chat[page] and restore the
slot to chat[page] on fetch failure.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(chat): avoid stale state writes after fetch failure

* fix(chat): merge async updates into current state

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#2400)

* fix(workspace): cap persisted workspaceTabList to prevent localStorage quota overflow (OtterMind#2399)

workspaceTabList (with per-tab full ddl) is persisted to localStorage with no
count cap, and createConsole's 100-tab guard is commented out. Over time this
overflows localStorage quota, throwing QuotaExceededError on subsequent
setItem calls (including unrelated keys sharing the origin). Cap the
persisted tab count at the persistence layer (keep most recent 100); the
in-memory list is unaffected so there is no UX change, and
getPersistableActiveConsoleId already handles an activeConsoleId missing
from the capped list.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* perf: cap persisted tabs before serialization

* docs: clarify persisted tab cap scope

* test(workspace): cover empty persisted tab lists

* test(workspace): run persistence cap contract in CI

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…lls (OtterMind#2382)

* fix(result-grid): stop infinite recursion when editing large-value cells (OtterMind#2379)

Editing a large-value cell re-entered the change_cell_value listener
indefinitely: the listener called tableInstance.changeCellValue to restore
the original display, VTable re-fired change_cell_value synchronously
(oldValue !== changedValue), and the only guard (currentValue ===
changedValue) could never match because every programmatic write flipped
the two values, exhausting the call stack and crashing the result tab.

Add a reentry flag set around the programmatic restore so the synthetic
re-fire is suppressed. The flag is only set during our own restore, so
genuine user edits are unaffected.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(result-grid): cover large-value restore reentry

* test(result-grid): cover restore failure cleanup

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
… reset (OtterMind#2406)

* fix(result-grid): re-apply create/delete highlights after arrangement reset (OtterMind#2405)

The cellChangeRecordList effect cleared customCellStyleArrangement (all
arrangements, including create/delete) to fix VTable A->B->A update-color
cycling, then re-applied only custom-update-cell. The create/delete
effects only re-run when their own deps change, so a plain cell edit left
green (new-row) / red (deleted-row) arrangements cleared until the next
sort/filter. Re-apply create/delete arrangements in the same effect after
the reset, and add them to the dependency list.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* refactor: use forEach for highlight updates

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…Hooks (OtterMind#2421)

* fix(org-settings): move hooks above early return to satisfy Rules-of-Hooks (OtterMind#2420)

useMemo/useEffect were called after 'if (!curOrg) return null', so the
first render (curOrg null) ran N hooks and the next render (curOrg
populated) ran N+1, crashing the org settings page with 'Rendered more
hooks than during the previous render'. Move the hooks above the early
return and guard inside them, preserving the intent to query users once
the org is loaded.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(org-settings): make owner check boolean

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(tree): guard updateTreeData against null list (OtterMind#2422)

updateTreeData did list.map(...) with no null guard. getTreeData/clearTreeStore
null out treeData, and handleLoadData passes the (possibly null) origin into
setTreeData, so a refresh racing a lazy-load threw null.map and crashed the
tree view. Guard with Array.isArray and return [].

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(tree): preserve loading state during lazy updates

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(tree): make findNode/getParentNode null-tolerant (OtterMind#2424)

Tree store actions pass get().treeData! into findNode/getParentNode, but
treeData is legitimately null (initial, getTreeData, clearTreeStore). The
functions dereferenced tree.length with no guard -> null.length NPE on
rename/delete/close-connection right after login or during a refresh. Guard
the loop with 'tree &&' so a null tree yields undefined instead of throwing.
Signatures unchanged to avoid call-site type ripple.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(tree): make node lookup contracts nullable

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(sql): escape single quotes in COMMENT text (OtterMind#2430)

COMMENT text was interpolated raw between single quotes, so a comment
containing an apostrophe (e.g. employee's id) produced malformed DDL the
DB rejected, and a crafted comment could break out of the literal. Double
embedded single quotes (SQL-standard) at the four comment sites, with null
guards. Backslash is intentionally not doubled (matching current behavior)
to avoid corrupting comments on standard-conforming dialects.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(sql): cover comment literal edge cases

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…OtterMind#2433)

* fix(sql): clamp pageNo/pageSize to prevent paging offset int overflow (OtterMind#2432)

executeSQL/executeSQLStreaming computed offset = (pageNo - 1) * pageSize with
int arithmetic and no bounds, so a large client-controlled pageNo wrapped to
a small or negative int: setMaxRows(negative) threw SQLException (500), or
setMaxRows(smallPositive) silently capped and returned first-page data for a
huge page number. Clamp pageNo>=1, pageSize to [1, MAX_PAGE_SIZE], and pageNo
to <= Integer.MAX_VALUE/pageSize so the product fits int.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* refactor(sql): centralize safe page bounds

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…erMind#2427)

* fix(mongodb): avoid Collectors.toMap NPE on null document values (OtterMind#2426)

columns/indexes built objectMap via Collectors.toMap(Entry::getKey,
Entry::getValue), whose Map.merge throws NPE on a null value. MongoDB
element documents commonly contain null-valued fields (e.g. unset types),
so the downstream Objects.nonNull guards never ran. Use a HashMap copy
(permits null values) instead.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(mongodb): cover null document metadata

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…kslash doubling (OtterMind#2429)

* fix(oceanbase-oracle): COMMENT DDL use quoteStringLiteral to stop backslash doubling (OtterMind#2428)

tableDDL built COMMENT ON ... IS via EasyStringUtils.escapeAndQuoteString,
which doubles both ' and \. The base OracleMetaData correctly uses
OracleIdentifierProcessor.quoteStringLiteral (only doubles '), and OceanBase
Oracle mode treats \ as a literal, so comments containing \ were regenerated
as \ and corrupted when the DDL was re-applied. Use the dialect's
quoteStringLiteral, mirroring the base class.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(oceanbase-oracle): cover comment literal DDL

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…igMap (OtterMind#2434) (OtterMind#2435)

testModelConfig -> resolveTestApiKey read the cross-user shared userConfigMap
(plain HashMap) and its inner ArrayList without the lock, while every other
accessor mutates them under synchronized. Concurrent save/delete (same user:
inner-list add/removeIf CME; different user: HashMap rehash) corrupted the
store. Make resolveTestApiKey synchronized (same this lock as siblings),
covering the only unsynchronized read without holding the lock across the
network test call in testModelConfig.

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…t scale set (OtterMind#2439)

* fix(postgresql): buildDataType NPE for NUMERIC when precision null but scale set (OtterMind#2438)

When columnSize was null but decimalDigits was set, the DECIMAL/NUMERIC
branch fell to the else and unboxed null columnSize -> NPE. Restructure so
a null precision returns the bare type (precision is required to specify
scale). Affects the PG family (GaussDB/OpenGauss/CockroachDB/Redshift).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(postgresql): cover numeric metadata edge cases

* refactor(postgresql): normalize numeric type formatting

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…erMind#2441)

* fix(sql-parser): tolerate duplicate table names in context parse (OtterMind#2440)

Collectors.toMap(Table::getName, t->t) had no merge function, so a
same-named table/view pair threw IllegalStateException, and the catch-all
returned an empty fallback, silently killing completion/hover context for
that console. Add a merge function to keep the first entry.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(sql-parser): cover duplicate metadata names

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…ind#2445)

* fix(sql): clamp buildPageLimit offset/pageSize to valid range (OtterMind#2444)

buildPageLimit appended LIMIT <pageSize> or LIMIT <offset>,<pageSize> from
the request with no validation: pageSize 0 -> LIMIT 0 (silently no rows),
negative -> LIMIT -5 (SQL syntax error 500). Clamp offset>=0 and pageSize>=1.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test(sql): cover page limit clamping

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(sql): bound format/valid_select SQL size (OtterMind#2446)

format/valid_select accepted an unbounded sql field, unlike contextParser/
quickParser which cap at 50000 chars, so a multi-megabyte SQL could hang the
formatter/parser. Add @notblank and @SiZe(max=50000) on both DTOs (the
controller already uses @Valid).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* test: cover SQL request validation bounds

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
OtterMind#2451)

The packaging scripts baked -noverify (= -Xverify:none) into the installed
launcher's JVM options, disabling the application classloader bytecode
verifier so a tampered class file on the classpath bypasses verification.
Combined with the CDN auto-update path, this lowers the bar for running
untrusted bytecode. Remove it; verification is on by default.

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…nd#2453)

* fix(storage): do not persist transient Namespace.dataSources (OtterMind#2452)

DataSourceStorage read paths (getNamespaceDatasource/buildTree) mutate
Namespace.dataSources on shared instances; the model persisted both
dataSources and datasourceIds, so a read followed by any namespace write
serialized a stale dataSources snapshot to disk. datasourceIds is the
source of truth; mark dataSources @JsonIgnore so the transient hydration
is never persisted.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(storage): ignore hydrated namespace data in fastjson

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…econnect (OtterMind#2455)

* fix(bigquery): strip managed extendInfo keys before re-injecting on reconnect (OtterMind#2454)

getConnection reused the same extendInfo list and unconditionally appended
ProjectId/OAuthServiceAcctEmail/OAuthType/OAuthPvtKeyPath, so reconnects
accumulated duplicate managed keys. Strip them first so reconnects replace.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(bigquery): copy extend info before deduplication

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…OtterMind#2457)

* fix(export): gate SET FOREIGN_KEY_CHECKS on the MySQL-protocol family (OtterMind#2456)

exportTables unconditionally wrote SET FOREIGN_KEY_CHECKS=0/1, MySQL syntax
that Presto/Kylin/BigQuery/GBase8s/DuckDB/CockroachDB/Redshift reject. Emit
it only for the MySQL-protocol family (MYSQL/MARIADB/TIDB/STARROCKS/DORIS/
OCEANBASE) based on the connection dbType.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* refactor(export): model MySQL protocol family

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(tree-storage): make deleteNode atomic (OtterMind#2458)

deleteNode was not synchronized; it called getNodes (live list), removeNode
(synchronized, releases lock), createTree (synchronized, re-acquires). A
concurrent updatePosition could interleave between removeNode and createTree,
so createTree serialized the interleaved state and one mutation was lost.
Make deleteNode synchronized (same this lock, reentrant with the siblings)
to hold the lock across the composite.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(tree-storage): persist empty tree deletions

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
* fix(storage): make SmallDataStorage.saveDataList atomic (OtterMind#2466)

saveDataList truncated the storage file to empty then appended each record
with a separate open/write/close; a crash/disk-full mid-loop left the file
partially written, permanently losing all records after the truncation point
on reload. Build the full content in memory, write to a temp file in the
same directory, then Files.move (atomic rename, with best-effort fallback)
over the target so the storage file is either the previous or the new
content, never a partial write.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(storage): harden atomic file replacement

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
…rMind#2469)

* fix(tree): show error and unblock delete-table modal on failure (OtterMind#2468)

deleteTable.onOk had no .catch on mysqlService.deleteTable; on failure the
promise rejected, openModal(false) never ran, and the modal stayed stuck
open with no feedback. Add a .catch that surfaces the error via
staticMessage.error (modal stays open so the user can cancel/retry).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: liuhy <liuhongyu@apache.org>

* fix(tree): surface delete table API errors

---------

Signed-off-by: liuhy <liuhongyu@apache.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: zgq <openai0229@gmail.com>
@HandSonic HandSonic closed this Aug 23, 2026
@HandSonic
HandSonic deleted the fix/review-cockroachdb 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.

9 participants