Repository navigation
Conversation
…GenericMetaDataConverter (code review follow-up)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 929683dbaa
ℹ️ 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".
| if (desired.isEmpty()) { | ||
| return new ArrayList<>(); |
There was a problem hiding this comment.
Delete Redis keys when their final member is removed
When a user removes every member from an existing Redis set, desired is empty and this branch returns no commands, so the update reports success while leaving the original key and all members unchanged. The same early-return behavior was added to ListTypeScript, HashTypeScript, and ZSetTypeScript; emit the appropriate removals or delete the key when the desired collection is empty, with focused coverage for deleting the final member.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
| } else { | ||
| allNull = false; | ||
| valueList.add("'" + v + "'"); | ||
| valueList.add(EasyStringUtils.escapeAndQuoteString(v.toString())); |
There was a problem hiding this comment.
Preserve backslashes in imported Excel cell values
On the Excel-to-H2 import path, escapeAndQuoteString doubles backslashes in addition to SQL single quotes. H2 regular string literals treat backslashes as data, so importing values such as Windows paths or escape-looking text stores extra backslashes compared with the spreadsheet. Use H2-appropriate literal escaping—preferably bound parameters—so quote safety does not alter cell contents.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
| columnType.setSupportLength(type.getPrecision() != null && type.getPrecision() > 0); | ||
| columnType.setSupportScale(type.getMaximumScale() != null && type.getMaximumScale() > 0); |
There was a problem hiding this comment.
Derive generic size capabilities from CREATE_PARAMS
For generic drivers, JDBC PRECISION is the maximum precision of the type, not evidence that a length clause is accepted; for example, INTEGER commonly reports precision 10 while CREATE_PARAMS is empty. This marks such types as supportLength=true and enables users to generate invalid declarations such as INTEGER(10). Determine length and scale support from Type.createParams (for example, length or precision,scale) and add cases where numeric precision exists without creation parameters.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
| columnType.setSupportScale(type.getMaximumScale() != null && type.getMaximumScale() > 0); | ||
| columnType.setSupportNullable(type.getNullable() != null | ||
| && type.getNullable() != DatabaseMetaData.typeNoNulls); | ||
| columnType.setSupportAutoIncrement(Boolean.TRUE.equals(type.getAutoIncrement())); |
There was a problem hiding this comment.
Populate default-value support in the generic fallback
For databases whose generic configuration has no columnTypes, this hook still leaves supportDefaultValue at its default false. The table editor gates its default-value control on that flag, so the stated fix continues to hide DEFAULT editing for TDENGINE, Elasticsearch, and other fallback users. Set an appropriate generic default capability or derive it from explicit driver configuration, and cover the flag in the converter regression test.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
| issues: | ||
| types: [opened, reopened, closed] |
There was a problem hiding this comment.
Sync contribution-label transitions to Project status
When maintainers add or remove contribution/good-first-issue or contribution/help-wanted during triage, this workflow never runs because labeled and unlabeled are excluded. The scheduled reconciliation also skips every open Issue, even though fallbackIssueStatus explicitly maps those labels to Ready and their absence to Backlog, so the Project item can remain permanently in its previous lifecycle state. Handle label events and reconcile open Issue statuses from current labels.
Useful? React with 👍 / 👎.
| const issueMap = new Map(); | ||
| for (const issue of await client.getClosingIssues(pullRequest.number)) issueMap.set(issue.number, issue); | ||
| for (const number of closingIssueNumbers(pullRequest.body)) { |
There was a problem hiding this comment.
Reset issues removed from a PR's closing references
When an edited PR removes Closes #N, this map contains only the references that remain in the new body or GitHub's current closingIssuesReferences. Issue #N, which an earlier run moved to In Progress or In Review, is therefore never visited or returned to Ready/Backlog; the scheduled reconciliation skips open Issues as well. On edited, compare against changes.body.from or perform a full linked-PR reconciliation so removed references are reset.
Useful? React with 👍 / 👎.
…seTableSchema default, header overwrite, tilde expansion, download failure logging, cutName literal prefix (code review follow-up)
…sult-tab-icon feat(sql-editor): use table icon for result tabs
…supportDefaultValue (codex review)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 630b59bbac
ℹ️ 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".
| columnType.setSupportDefaultValue(true); | ||
| columnType.setSupportAutoIncrement(Boolean.TRUE.equals(type.getAutoIncrement())); |
There was a problem hiding this comment.
Align generic capability flags with emitted DDL
When a database without configured columnTypes uses GenericMetaData—including TDENGINE, Elasticsearch, BigQuery, Informix, and GBase8s—these flags expose Default Value and Auto Increment controls, but the inherited DefaultSqlBuilder.buildCreateTable() renders only type/size, nullability, and comments; it never reads defaultValue or autoIncrement. Consequently, values entered through the newly enabled controls silently disappear from the generated SQL. Fresh evidence beyond the earlier default-value comment is the current unconditional flags combined with this concrete generic-builder path, which also drops auto-increment; implement these clauses in the owning dialect builder or leave unsupported capabilities disabled.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
# Conflicts: # chat2db-community-client/scripts/i18n-source-hashes.json
…(adversarial review)
… 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>
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>
… 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>
…enum to COCKROACHDB (code review follow-up) (OtterMind#2300) Co-authored-by: zgq <openai0229@gmail.com>
fix(core-tools): harden pagination, JDBC utilities, and shared helpers
Fixes verified review finding: IGenericMetaDataConverter.type2columnType mapped only typeName, leaving all supportXxx flags false for drivers whose generic.json declares no columnTypes (ELASTICSEARCH, TDENGINE), so the table editor omitted NULL/DEFAULT/length clauses. Flags are now derived from JDBC DatabaseMetaData.getTypeInfo (@AfterMapping), preserving explicit config flags. Regression tests added. Part of OtterMind#2281.