Repository navigation
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a1ce41ff5
ℹ️ 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".
Comment on lines
+301
to
+304
| script.append(RedisConstants.COMMAND_LIST_REMOVE_PREFIX).append(getRedisValue(keyName)) | ||
| .append(RedisConstants.COMMAND_LIST_REMOVE_ONE_FRAGMENT) | ||
| .append(getRedisValue(StringUtils.defaultString(oldRow.get(RedisConstants.FIELD_VALUE)))) | ||
| .append(SQLConstants.LINE_SEPARATOR); |
There was a problem hiding this comment.
When an editable Redis list contains duplicate values, deleting any occurrence other than the first generates LREM key 1 value, which removes the first matching element from the head rather than the selected grid row. This silently deletes the wrong data; use the recorded row number with a position-preserving deletion strategy, and account for index shifts when applying multiple list edits.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
Comment on lines
26
to
+28
| userId = 0L; | ||
| } | ||
| String id = System.currentTimeMillis() + "" + Math.floorMod(userId, 1000); | ||
| return Long.parseLong(id); | ||
| long timestamp = LAST_TIMESTAMP.updateAndGet( |
There was a problem hiding this comment.
The in-memory timestamp sequence can advance beyond wall-clock time during bursts, but it resets on restart. For example, two calls in millisecond t generate timestamp portions t and t+1; restarting in millisecond t+1 generates the second ID again for the same user bucket. Because these IDs back persistent storage entries and detail filenames, that collision can overwrite an existing record; use a restart-safe persisted sequence or another collision-resistant identifier.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
Comment on lines
+4
to
+5
| issues: | ||
| types: [opened, reopened, closed] |
There was a problem hiding this comment.
When a maintainer adds or removes contribution/help-wanted or contribution/good-first-issue after an issue is opened, this workflow does not run because it subscribes only to opened, reopened, and closed events. The scheduled reconciliation also ignores open issues, even though fallbackIssueStatus derives Ready from these labels, so labeled tasks remain stuck in Inbox or Backlog and never appear in the contributor-ready project views. Add labeled/unlabeled triggers and apply the fallback status, or make scheduled reconciliation fetch labels and update open issues.
Useful? React with 👍 / 👎.
| if (community) { | ||
| static String resolve(boolean windows, boolean community, boolean localEdition) { | ||
| if (windows && community) { | ||
| return "Chat2DB Community"; |
There was a problem hiding this comment.
For a packaged Community desktop launch on Linux, windows is false while community is true, so this condition falls through to Chat2DB Pro; MainJFrame.setupGenericPlatformSpecifics then uses that value as the visible window title. This gives the Community Linux package a commercial product identity, whereas the previous resolver returned Community on every platform. Resolve Community independently of Windows, or gate only the platform-specific title application rather than the product identity.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
| int endRow = offset + pageSize; | ||
| StringBuilder sqlBuilder = new StringBuilder(sql.length() + 120); | ||
| sqlBuilder.append(SQL_ROW_NUMBER_PREFIX); | ||
| sqlBuilder.append(sql); |
There was a problem hiding this comment.
On SQL Server versions before 2012, a normal query ending in ; is inserted verbatim into the derived table, producing FROM (SELECT ...;) TMP_PAGE, which SQL Server rejects because the terminator occurs inside the subquery. The new fallback therefore makes paginated execution fail for semicolon-terminated queries on that runtime; remove the trailing terminator before wrapping or construct the pagination query through a statement-aware rewrite.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
…sult-tab-icon feat(sql-editor): use table icon for result tabs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e0bfa588a
ℹ️ 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".
Comment on lines
+307
to
+309
| script.append("LSET ").append(getRedisValue(keyName)) | ||
| .append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(index) | ||
| .append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR).append(getRedisValue(tombstone)) |
There was a problem hiding this comment.
When multiple list rows are deleted, every command uses the row’s original index even though each preceding LREM shortens the list. For example, deleting original rows 1 and 2 in that order removes rows 1 and 3, leaving row 2 untouched; deleting the first and last rows can instead make the later LSET out of range. Fresh evidence in this revision is that the new LSET/LREM implementation still processes the frontend's deleteRows in selection order without adjusting indices. Sort positional deletes by descending original index or otherwise account for prior removals.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
| // Positional delete so a later duplicate occurrence removes exactly the | ||
| // edited row: LSET idx <tombstone> then LREM 1 <tombstone>. Value-based | ||
| // LREM would always hit the FIRST duplicate instead. | ||
| String tombstone = "__chat2db_deleted__" + index; |
There was a problem hiding this comment.
If the list already contains the generated value __chat2db_deleted__<index> before the selected row, LSET creates a duplicate marker and LREM ... 1 removes the pre-existing value from the head. The selected row is then left containing the marker, silently corrupting two elements. Use a position-preserving strategy that cannot collide with user data, or verify a generated marker is absent before applying it.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
Comment on lines
+240
to
+242
| if (StringUtils.isNotBlank(newField)) { | ||
| appendCommand(script, RedisConstants.COMMAND_HASH_SET_PREFIX, keyName, newField, | ||
| newRow.get(RedisConstants.FIELD_VALUE)); |
There was a problem hiding this comment.
When a hash contains an empty-string field name, changing its value generates no HSET because newField fails isNotBlank; deleting that row is similarly ignored. Empty fields are supported by the plugin's normal hash creation/update path, which excludes only null, so grid saves silently diverge for this valid stored data. Test for null rather than blankness when deciding whether to emit the hash mutation.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
Comment on lines
+232
to
+233
| } while (!RedisConstants.SCAN_INITIAL_CURSOR.equals(cursor) | ||
| && scanCalls < RedisScanConfig.DEFAULT.maxScanCallsPerRequest()); |
There was a problem hiding this comment.
Whenever Redis returns a nonzero cursor from the tenth SCAN, this new loop exits because maxScanCallsPerRequest() is 10 and returns the accumulated list without any continuation token or incomplete-result indication. Consequently, tables() still omits keys in larger keyspaces, and the same cap in match() regresses sparse searches that previously recursed until cursor zero or the result target was reached. Legacy APIs without a continuation cursor must finish the scan, or expose an explicit continuation instead of silently returning incomplete metadata.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
Comment on lines
175
to
181
| StringBuilder stringBuilder = new StringBuilder(); | ||
| stringBuilder.append(RedisConstants.REDIS_MULTI_COMMAND); | ||
| stringBuilder.append(renameOrUpdateTtl(queryResult)); | ||
| for (ResultOperation operation : operations) { | ||
| stringBuilder.append(buildOperationScript(redisKeyType, queryResult.getTableName(), headerList, operation)); | ||
| } | ||
| stringBuilder.append(RedisConstants.REDIS_EXEC_COMMAND); |
There was a problem hiding this comment.
The Redis grid-save path sends this generated script through DbDlTemplateServiceImpl.redisExecuteUpdate, which explicitly skips lines equal to MULTI or EXEC and executes every intervening command separately. Thus the new multi-command edits are not atomic despite these wrappers; for example, a connection failure after SREM but before SADD, or between the list LSET and LREM, leaves partially modified user data. Execute the transaction commands on the same connection rather than stripping them, or use a single atomic server-side operation.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
Comment on lines
+354
to
+360
| private void appendCommand(StringBuilder script, String commandPrefix, String keyName, String... args) { | ||
| script.append(commandPrefix).append(getRedisValue(keyName)); | ||
| for (String arg : args) { | ||
| script.append(RedisConstants.COMMAND_ARGUMENT_SEPARATOR) | ||
| .append(getRedisValue(StringUtils.defaultString(arg))); | ||
| } | ||
| script.append(SQLConstants.LINE_SEPARATOR); |
There was a problem hiding this comment.
When an edited key, field, or value contains \n or \r, getRedisValue leaves that character literal in the generated command. The save path then uses DbDlTemplateServiceImpl.redisExecuteUpdate, which splits the script on every carriage return or newline, so a multiline value such as hello\nworld becomes two malformed Redis commands instead of one mutation. The frontend explicitly preserves multiline cell values, so execute structured commands or split with a quote-aware parser rather than using physical lines as command boundaries.
AGENTS.md reference: AGENTS.md:L56-L58
Useful? React with 👍 / 👎.
… 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>
…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>
…OtterMind#1914) (OtterMind#2172) * fix(mysql): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(mysql): validate hex literals and double backticks in identifier processor (OtterMind#1914) - MysqlDmlValueTemplate.wrapHex: require hex digits for the 0x template, matching wrapBit's requireBitLiteral - MysqlBinaryProcessor/MysqlVarBinaryProcessor: only pass values through raw when they are well-formed 0x hex literals (isHexLiteral), closing the 0x-prefixed injection passthrough in generated DML - MysqlIdentifierProcessor.quoteIdentifier: double embedded backticks via MysqlSqlEscapes.quoteIdentifierRaw instead of wrapping verbatim - add regression tests for all three paths * fix(mysql): escape truncate/copy table identifiers and add regression tests (OtterMind#1914) * refactor(mysql): move escaping into MysqlIdentifierProcessor per maintainer review (OtterMind#1914) - MysqlIdentifierProcessor (SPI ISQLIdentifierProcessor): INSTANCE, always-quote backtick quoteIdentifier with one-pair strip + doubling, escapeString with backslash-first-then-quote doubling, static escapeIdentifier for quoted templates - MysqlMetaData call sites use getSQLIdentifierProcessor(); builders/managers/enums/ value processors use MysqlIdentifierProcessor.INSTANCE - non-escapable validation moved to MysqlSqlGuards (engine/charset/collation names, numeric defaults, bit/hex literals, definers, asc/desc, enum constants, enum/set lists) - MysqlSqlEscapes removed; tests migrated to MysqlIdentifierProcessorTest (630 green) * fix(mysql): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(mysql): finalize SQL escaping boundaries --------- Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#1914) (OtterMind#2177) * fix(h2): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(h2): escape identifiers and literals across sql builder surface (OtterMind#1914) Override quoteIdentifier/quoteQualifiedIdentifier/buildTableName/buildColumns and reimplement buildCreateTable/buildAlterTable/buildUpdate/buildTemplate/ buildCreateDatabase in H2SqlBuilder so every identifier is double-quote escaped and every comment literal is single-quote escaped. Validate metadata TYPE_NAME against an allow-list and neutralize hostile COLUMN_DEF defaults in generated DDL. Apply the export SCRIPT NODATA sentinel before substituting the escaped schema name so schema names containing NODATA are not corrupted. Document the raw-name contract on H2SqlEscapes and add attack-string tests for the builder, manager and metadata paths. * fix(h2): validate UI-supplied column types in create/alter table DDL (OtterMind#1914) Route column.getColumnType() in buildCreateTable and generateAlterColumnSql (ADD/MODIFY) through H2SqlEscapes.requireSafeTypeName so a hostile type string cannot break out of generated DDL. Adds attack-string rejection tests. * refactor(h2): move escaping into H2IdentifierProcessor per maintainer review (OtterMind#1914) - new H2IdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - H2Meta overrides getSQLIdentifierProcessor(); metadata call sites use it - builders/managers use H2IdentifierProcessor.INSTANCE - non-escapable validation moved to H2SqlGuards (type names, column defaults) - H2SqlEscapes removed; tests migrated (27 green) * fix(h2): restore dual-track quoteIdentifier contract (null passthrough, conditional for SPI, always for DDL) (OtterMind#1914) The pilot branch predated the dual-track contract; its always-quote processor had the same null->"" garbage and completion regression the other modules were fixed for. * fix(h2): finalize identifier and DDL escaping --------- Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#1914) (OtterMind#2173) * fix(db2): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(db2): close fallback-column and db2look option-string injection paths (OtterMind#1914) - buildCreateColumnSql fallback: validate columnType against a strict type-shape whitelist and escape the column name as a delimited identifier instead of delegating to the raw SPI buildDefaultColumn - tableDDL: reject schema/table names containing double quotes, which would otherwise break out of the db2look -td/-t option string - add regression tests for both paths plus SET SCHEMA, dropTable and copyTable escaping composition * fix(db2): quote inherited drop/truncate paths and tighten type/default validation (OtterMind#1914) - DB2SqlBuilder: override buildDropTable/buildTruncateTable so qualified table names are quoted via Db2SqlEscapes instead of the SPI identity - DB2DBManager: override truncateTable with the same quoting - DB2ColumnTypeEnum.getByType: strip size suffix so VARCHAR(10) resolves to the enum and reaches the validated fallback instead of being skipped - DB2ColumnTypeEnum: drop parens/comma from DEFAULT_VALUE_PATTERN to block column-definition breakout via crafted default values - add regression tests for all four paths * fix(db2): harden default-value and type validators against breakout payloads (OtterMind#1914) - DEFAULT_VALUE_PATTERN: drop single-quote from the allowed class so crafted defaults cannot toggle string-literal context inside the enclosing CREATE/ALTER statement - anchor DEFAULT_VALUE_PATTERN, UNIT_PATTERN and FALLBACK_COLUMN_TYPE_PATTERN with \A/\z instead of ^/$ so a trailing newline can no longer smuggle input past validation - validateAscOrDesc needs no anchor change: it uses exact string equality, which is already absolute - add rejection tests for quote/comma/paren breakout defaults, trailing-newline bypasses, and GET_DDL_TOKEN option-string quoting * refactor(db2): move escaping into Db2IdentifierProcessor per maintainer review (OtterMind#1914) - new Db2IdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - DB2MetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builders/managers/enums use Db2IdentifierProcessor.INSTANCE - non-escapable validation moved to Db2SqlGuards (default values, length units, fallback column types, index sort directions) - Db2SqlEscapes removed; tests migrated (23 green) * fix(db2): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(db2): preserve identifier and default semantics --------- Co-authored-by: zgq <openai0229@gmail.com>
…terMind#2340) * feat(client): add line and block comment shortcuts for SQL editor * test(i18n): update source hashes for setting and monaco locales * fix(client): unbind native Monaco comment keybindings to make shortcut setting authoritative * fix(client): support physical key recording and add shifted punctuation shortcut tests * fix(client): preserve shortcut capture constraints --------- Co-authored-by: zgq <openai0229@gmail.com>
Adds configurable, context-aware keyboard shortcuts for open table, edit table, create table, and refresh in the Chat2DB Community database tree.
…paths (OtterMind#1914) (OtterMind#2174) * fix(clickhouse): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(clickhouse): allow enum '=' type args and keep DEFAULT/Nullable in type fallback (OtterMind#1914) * fix(clickhouse): skip Nullable wrap for AggregateFunction fallback, lock enum dash rejection (OtterMind#1914) * refactor(clickhouse): move escaping into ClickHouseIdentifierProcessor per maintainer review (OtterMind#1914) - new ClickHouseIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with backtick doubling, escapeString with backslash + single-quote doubling - ClickHouseMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builders/managers/enums use ClickHouseIdentifierProcessor.INSTANCE - non-escapable validation moved to ClickHouseSqlGuards (column type expressions, engine whitelist, column default expressions) - ClickHouseSqlEscapes removed; tests migrated (24 green) * fix(clickhouse): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank pass through, plain non-keyword identifiers stay unquoted, everything else is wrapped in backticks with embedded backticks doubled; versioned overload delegates - new quoteIdentifierAlways(String) carries the old unconditional semantics for DDL-generation call sites migrated from ClickHouseSqlEscapes - quoteIdentifierIgnoreCase stays the always-quote, case-preserving variant - override removeIdentifierQuote/isQuoteIdentifier so backtick-quoted identifiers round-trip through completion/matching consumers - add ClickHouse reserved keyword set used by the conditional check - tests cover conditional vs always behavior incl. null passthrough (30 green) * fix(clickhouse): preserve legal SQL escaping semantics (OtterMind#1914) --------- Co-authored-by: zgq <openai0229@gmail.com>
OtterMind#1914) (OtterMind#2195) * fix(oracle): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(oracle): move escaping into OracleIdentifierProcessor per maintainer review (OtterMind#1914) - strengthen OracleIdentifierProcessor: INSTANCE, escapeString with single-quote doubling, static escapeIdentifier/quoteIdentifierAlways - OracleMetaData processor singleton delegates to INSTANCE; private escapeSqlLiteral (OtterMind#2052) unified to processor escapeString - builders/DBManager/enums/value processors use the processor (or OracleSqlGuards for non-escapable validation) - OracleSqlEscapes removed; tests migrated (33 green) * fix(oracle): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(oracle): preserve legal SQL escaping semantics (OtterMind#1914) --------- Co-authored-by: zgq <openai0229@gmail.com>
…aths (OtterMind#1914) (OtterMind#2196) * fix(sqlserver): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(sqlserver): move escaping into SqlServerIdentifierProcessor per maintainer review (OtterMind#1914) - strengthen SqlServerIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier always bracket-quotes with ] doubling, escapeString doubles single quotes only (no backslash doubling in T-SQL) - SqlServerMetaData.getSQLIdentifierProcessor() returns the shared INSTANCE; metadata call sites use it (statics use INSTANCE directly) - builders/DBManager/enums/XML+Geography value processors rewired to SqlServerIdentifierProcessor.INSTANCE / static escapeIdentifier - collation validation moved to public SqlServerSqlGuards - SqlServerIdentifierUtils removed; tests migrated to SqlServerIdentifierProcessorTest (49 green) * fix(sqlserver): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(sqlserver): preserve safe identifier and expression semantics (OtterMind#1914) --------- Co-authored-by: zgq <openai0229@gmail.com>
OtterMind#1914) (OtterMind#2175) * fix(xugudb): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(xugudb): quote identifiers in select/insert builders and harden column fallback (OtterMind#1914) - Override quoteIdentifier/quoteQualifiedIdentifier/buildSelectTable/buildTableName/ buildColumns in XUGUDBSqlBuilder so inherited DefaultSqlBuilder DQL/DML paths (exportTableData, insert, update) no longer concatenate raw identifiers - Replace raw IColumnBuilder.buildDefaultColumn fallback with escaped name + anchored type-shape validation (reject otherwise) - Return trimmed values from default-value/unit validators - Document raw-identifier contract on XugudbSqlEscapes.escapeIdentifier - Add attack-string tests: malicious schema/table/column via buildSelectTable and buildInsert, comment literal end-to-end, fallback name/type, validator trimming * fix(xugudb): tighten function-default pattern validation + round-trip tests (OtterMind#1914) - DEFAULT_VALUE_PATTERN function-call branch now requires balanced single-quoted literals as arguments ('([^']|'')*' allowed, bare quotes rejected), closing the unbalanced-quote gap in default-value validation - Add tests: unbalancedQuoteInFunctionDefaultIsRejected, balancedQuotedArgsInFunctionDefaultAreAccepted - Fix extra closing paren typo in the c3 case * refactor(xugudb): move escaping into XugudbIdentifierProcessor per maintainer review (OtterMind#1914) - new XugudbIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - XUGUDBMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builder/manager/enums use XugudbIdentifierProcessor.INSTANCE - non-escapable validation moved to XugudbSqlGuards (column defaults, length units) - XugudbSqlEscapes removed; tests migrated (21 green) * fix(xugudb): unroll quoted-literal regex to avoid CodeQL polynomial-regex alert (OtterMind#1914) * fix(xugudb): replace DEFAULT validator regex with linear scanners (CodeQL ReDoS) (OtterMind#1914) * fix(xugudb): restore dual-track quoteIdentifier contract (null passthrough, conditional for SPI, always for DDL) (OtterMind#1914) Found in re-review: processor was always-quote with null->"" garbage, breaking SPI completion paths; no quoteIdentifierAlways existed. * fix(xugudb): complete identifier escaping coverage --------- Co-authored-by: zgq <openai0229@gmail.com>
…paths (OtterMind#1914) (OtterMind#2194) * fix(postgresql): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(postgresql): move escaping into PostgreSQLIdentifierProcessor per maintainer review (OtterMind#1914) - PostgreSQLIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier always double-quotes with embedded-quote doubling, escapeString with single-quote doubling; INSTANCE shared by call sites - PostgreSQLMetaData.getSQLIdentifierProcessor() returns the processor; metadata call sites use it (POSTGRE_SQL_IDENTIFIER_PROCESSOR removed) - builders/managers/enums/templates use PostgreSQLIdentifierProcessor.INSTANCE - non-escapable validation moved to PostgreSqlGuards (pg names, default expressions, bit/hex literals, enum options) - PostgreSqlEscapes removed; tests migrated (38 green) * fix(postgresql): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank passthrough, valid non-keyword identifiers returned unquoted, everything else wrapped with embedded-quote doubling; versioned overload delegates to it - quoteIdentifierIgnoreCase is the always-quote SPI variant (preserves case) - new quoteIdentifierAlways(String) for DDL-generation call sites migrated from PostgreSqlEscapes.quoteIdentifier (DBManager, SqlBuilder DDL paths, column/index type enums, MetaData format/getMetaDataName) - buildTableName/buildColumns keep conditional processor quoting - tests cover conditional, always-quote, ignoreCase and null passthrough (40 green) * fix(postgresql): keep mixed-case identifiers quoted in conditional quoteIdentifier (OtterMind#1914) PostgreSQL folds unquoted identifiers to lowercase, so the conditional quoteIdentifier must quote names containing uppercase (as Oracle/DM do). Found in adversarial confirmation. * fix(postgresql): complete identifier-safe DDL generation --------- Co-authored-by: zgq <openai0229@gmail.com>
…ths (OtterMind#1914) (OtterMind#2197) * fix(kingbase): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(kingbase): move escaping into KingBaseSQLIdentifierProcessor per maintainer review (OtterMind#1914) - strengthen KingBaseSQLIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier always double-quotes with embedded-quote doubling, escapeString doubles single quotes; INSTANCE for call sites without MetaData access - KingBaseMetaData returns the processor from getSQLIdentifierProcessor(); metadata call sites use it (KINGBASE_SQL_IDENTIFIER_PROCESSOR removed) - DBManager/builder/enums use KingBaseSQLIdentifierProcessor.INSTANCE - non-escapable validation moved to KingBaseSqlGuards (safe expressions, index methods) - KingBaseSqlEscapes removed; tests migrated (15 green) * fix(kingbase): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(kingbase): complete identifier-safe DDL generation --------- Co-authored-by: zgq <openai0229@gmail.com>
OtterMind#1914) (OtterMind#2204) * fix(sqlite): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(sqlite): move escaping into SqliteIdentifierProcessor per maintainer review (OtterMind#1914) - SqliteIdentifierProcessor gains INSTANCE, escapeString (single-quote doubling) and static escapeIdentifier; quoteIdentifier overloads delegate - SqliteMetaData/DBManager use getSQLIdentifierProcessor()/INSTANCE - builders/enums use SqliteIdentifierProcessor.escapeIdentifier - non-escapable validation moved to SqliteSqlGuards (safe names, type names, column defaults, line comments) - SqliteSqlEscapes removed; tests migrated (16 green) * fix(sqlite): complete identifier-safe SQL generation --------- Co-authored-by: zgq <openai0229@gmail.com>
…ths (OtterMind#1914) (OtterMind#2203) * fix(redshift): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(redshift): move escaping into RedshiftIdentifierProcessor per maintainer review (OtterMind#1914) - new RedshiftIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - RedshiftMetaData overrides getSQLIdentifierProcessor(); SHOW CREATE TABLE call site uses RedshiftIdentifierProcessor.INSTANCE - RedshiftSqlEscapes removed; tests migrated (9 green) * fix(redshift): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank passthrough, valid plain identifiers returned unquoted, otherwise wrapped with double quotes after stripping one pair and doubling embedded quotes - new quoteIdentifierAlways(String) for DDL-generation sites; buildShowCreateTableSql uses it (was RedshiftSqlEscapes.quoteIdentifier) - quoteIdentifierIgnoreCase keeps the always-quote SPI variant meaning; versioned overload delegates to quoteIdentifier(String) - tests cover both conditional and always behaviors incl. null passthrough * fix(redshift): blank passthrough in quoteIdentifierAlways (no empty quoted identifier) (OtterMind#1914) * fix(redshift): align identifier contract with PostgreSQL Reuse the verified PostgreSQL processor, preserve raw identifier round trips, and handle optional SHOW CREATE TABLE schema qualification. Co-authored-by: HandSonic <8078023+handsonic@users.noreply.github.com> --------- Co-authored-by: zgq <openai0229@gmail.com>
…OtterMind#1914) (OtterMind#2176) * fix(sundb): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * fix(sundb): tighten DEFAULT pattern and escape second-order DDL metadata (OtterMind#1914) - DEFAULT_VALUE_PATTERN: drop quotes, commas and parentheses so a default value cannot smuggle literals, inject extra column definitions or close the column definition early (e.g. "0) --"). Paren-less SUNDB default functions (SYSDATE, CURRENT_TIMESTAMP, USER, SEQ.NEXTVAL) still pass. - SUNDBMetaData.tableDDL: quote identifiers originating from DB metadata (column names, constraint names/columns, tablespace names, index schema/table/column names) in generated DDL. - getIndexName: always quote schema and index name; drop the unquoted PRIMARY_KEY_INDEX special case. - Tests: default-pattern rejection payloads, alter-table rename escaping, copyTable executed SQL, export-comment literal escaping, getIndexName quoting (incl. PRIMARY_KEY_INDEX names). * fix(sundb): allow quoted string-literal DEFAULTs, keep DDL-reshape rejection (OtterMind#1914) dd55337's bare-token-only DEFAULT_VALUE_PATTERN rejected legitimate string-literal defaults (DEFAULT 'Y', '0', 'O''Brien', '1970-01-01', ''). New pattern is an anchored alternation: \A(?:(?!.*--)[A-Za-z0-9_ .+-]+|'(?:[^']|'')*')\z - bare token: unchanged charset, plus a lookahead rejecting '--' - quoted literal: quotes may only appear as doubled '' pairs, which exactly mirrors SQL string-literal tokenization, so anything accepted is parsed by the DB as one inert string token Still rejects: 0) --, 0 --, 1, x INT, now(), unbalanced quotes, 'a' 'b', 'a'||'b', 'a'--, 'a'; DROP TABLE x--. * fix(sundb): accept typed date/time literals in DEFAULT pattern (OtterMind#1914) Extend the quoted-literal branch with an optional single leading keyword: (?:[A-Za-z]+ )?'(?:[^']|'')*' — restores DATE '2024-01-01' and TIMESTAMP '...' defaults flagged in the gp-25 regression review. The keyword is letters-only so it cannot break out; the literal still requires doubled interior quotes. Parenthesized calls (TO_DATE(...), SYS_GUID()) stay rejected and are documented as such in the PR. Tests: accept set += DATE '2024-01-01', TIMESTAMP '2024-01-01 00:00:00'; reject set += TO_DATE('2024-01-01','YYYY-MM-DD'), SYS_GUID(), DATE '2024-01-01'--. Suite: 17/17 green. * refactor(sundb): move escaping into SUNDBIdentifierProcessor per maintainer review (OtterMind#1914) - new SUNDBIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - SUNDBMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builders/managers/enums use SUNDBIdentifierProcessor.INSTANCE - non-escapable validation moved to SUNDBSqlGuards (ASC/DESC, DEFAULT expression, VARCHAR unit) - SUNDBSqlEscapes removed; tests migrated (17 green) * fix(sundb): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank passthrough, valid non-keyword identifiers returned unquoted, else wrapped with embedded-quote doubling (restores SPI contract for completion/matching) - new quoteIdentifierAlways(String): unconditional wrap (null -> null), used only at the DDL-generation call sites that commit 0ff5f8b migrated from the always-quote SUNDBSqlEscapes.quoteIdentifier - quoteIdentifierIgnoreCase stays the always-quote variant; the versioned overload delegates to quoteIdentifier(String) - tests cover both conditional and always behaviors incl. null passthrough (21 green) * fix(sundb): complete identifier and DDL hardening * fix(sundb): close final DDL guard gaps --------- Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#1914) (OtterMind#2198) * fix(dm): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(dm): move escaping into DMIdentifierProcessor per maintainer review (OtterMind#1914) - strengthen DMIdentifierProcessor (SPI ISQLIdentifierProcessor): always-quote quoteIdentifier with embedded-quote doubling, escapeString with single-quote doubling, static escapeIdentifier for pre-quoted templates, INSTANCE - DMMetaData.getSQLIdentifierProcessor() returns the shared INSTANCE; metadata call sites use it (quoteIdentifier / escapeString / method refs) - builders/DBManager/enums use DMIdentifierProcessor.INSTANCE / static escapeIdentifier - non-escapable DEFAULT-expression validation moved to public DMSqlGuards - DMSqlEscapes removed; tests migrated to DMIdentifierProcessorTest (11 green) * fix(dm): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(dm): unroll quoted-literal regex to avoid CodeQL polynomial-regex alert (OtterMind#1914) * fix(dm): replace DEFAULT validator regex with linear scanners (CodeQL ReDoS) (OtterMind#1914) Balanced-paren + quoted-literal aware scanning keeps NVL(SUM(x),0) and quoted defaults accepted; semicolons and unbalanced input rejected. * fix(dm): complete identifier-safe SQL generation --------- Co-authored-by: zgq <openai0229@gmail.com>
…aths (OtterMind#1914) (OtterMind#2200) * fix(snowflake): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(snowflake): move escaping into SnowflakeIdentifierProcessor per maintainer review (OtterMind#1914) - new SnowflakeIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - SnowflakeMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builder/enums/DBManager use SnowflakeIdentifierProcessor.INSTANCE - non-escapable validation moved to SnowflakeSqlGuards (name tokens, default expressions, index sort direction); USE SCHEMA keeps unquoted+whitelist schema - SnowflakeSqlEscapes removed; tests migrated (19 green) * fix(snowflake): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(snowflake): complete identifier safety paths Preserve identifier round trips and Snowflake case folding, quote inherited DML builder paths, and validate raw defaults, custom types, index metadata, and clustering clauses. Co-authored-by: HandSonic <8078023+handsonic@users.noreply.github.com> --------- Co-authored-by: zgq <openai0229@gmail.com>
…tterMind#1914) (OtterMind#2201) * fix(hive): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(hive): move escaping into HiveIdentifierProcessor per maintainer review (OtterMind#1914) - new HiveIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with backtick doubling, escapeString doubling backslashes then single quotes - HiveMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it - builder/DBManager/enums use HiveIdentifierProcessor.INSTANCE - non-escapable validation moved to HiveSqlGuards (engine/charset/collation names, numeric defaults, index sort direction) - HiveSqlEscapes removed; tests migrated (18 green) * fix(hive): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(hive): complete identifier hardening review --------- Co-authored-by: zgq <openai0229@gmail.com>
…hs (OtterMind#1914) (OtterMind#2199) * fix(generic): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(generic): move escaping into GenericIdentifierProcessor per maintainer review (OtterMind#1914) - new GenericIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote default + dialect-parameterized char variant, escapeString with single-quote doubling - GenericMetaData overrides getSQLIdentifierProcessor() - template-aware validation moved to GenericSqlGuards (sanitizeTemplateValue, requireSafeIdentifier AS-IS), escaping delegated to the processor - GenericSqlEscapes removed; tests migrated (12 green) * fix(generic): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) * fix(generic): align template escaping with identifier SPI * fix(generic): preserve raw boundary quotes in always-quote --------- Co-authored-by: zgq <openai0229@gmail.com>
…OtterMind#1914) (OtterMind#2202) * fix(oscar): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(oscar): move escaping into OscarIdentifierProcessor per maintainer review (OtterMind#1914) - strengthen OscarIdentifierProcessor: shared INSTANCE, escapeString with single-quote doubling, static escapeIdentifier for quoted-template content - OscarMetaData call sites use getSQLIdentifierProcessor().escapeString(...) - non-escapable validation moved to OscarSqlGuards (default expressions, length units, sort orders) - OscarSqlEscapes removed; tests migrated (15 green) * fix(oscar): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank pass through, valid non-keyword identifiers return unquoted, everything else wraps with embedded-quote doubling; versioned overload delegates to it - new quoteIdentifierAlways(String) carries the old always-quote SqlEscapes semantics (null -> null, else wrap with doubling) - quoteIdentifierIgnoreCase keeps its SPI always-quote, case-preserving meaning by delegating to quoteIdentifierAlways, so DDL-generation sites (OscarUtils/builers/enums) keep emitting quoted identifiers - tests cover conditional and always behaviors incl. null passthrough (15 green) * fix(oscar): unroll quoted-literal regex to avoid CodeQL polynomial-regex alert (OtterMind#1914) '([^']|'')*' -> '[^']*(?:''[^']*)*' (same language, no backtracking) * fix(oscar): replace DEFAULT validators with linear scanners (CodeQL ReDoS) (OtterMind#1914) Regexes for quoted literals and function-call defaults are replaced by O(n) character scanners; numeric/keyword patterns (simple classes) stay. * fix(oscar): close identifier and type escaping gaps (OtterMind#1914) --------- Co-authored-by: zgq <openai0229@gmail.com>
…a/DDL paths (OtterMind#1914) (OtterMind#2205) * fix(oceanbase-oracle): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(oceanbase-oracle): move escaping into OceanbaseOracleIdentifierProcessor per maintainer review (OtterMind#1914) - new OceanbaseOracleIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - OceanbaseOracleMetaData overrides getSQLIdentifierProcessor(); DDL builder helpers use OceanbaseOracleIdentifierProcessor.INSTANCE - OceanbaseOracleSqlEscapes removed; tests migrated (5 green) * fix(oceanbase-oracle): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (OtterMind#1914) - quoteIdentifier(String) is conditional again: null/blank passthrough, plain non-reserved identifiers returned unquoted (fixes GenericSqlCompletionEngine raw-vs-quoted mismatches); other inputs are double-quote wrapped with embedded-quote doubling - new quoteIdentifierAlways(String) for DDL-generation call sites that require unconditional quoting; quoteIdentifierIgnoreCase is the always-quote SPI variant and delegates to it - processor now extends OracleIdentifierProcessor to inherit the Oracle reserved-keyword set; versioned overload delegates to quoteIdentifier(String) - tests cover conditional and always behaviors incl. null passthrough (10 green) * fix(oceanbase-oracle): blank passthrough in quoteIdentifierAlways (no empty quoted identifier) (OtterMind#1914) * fix(oceanbase-oracle): align identifier processor with Oracle --------- Co-authored-by: zgq <openai0229@gmail.com>
OtterMind#2206) * fix(mongodb): escape SQL identifiers and literals in metadata/DDL paths (OtterMind#1914) * refactor(mongodb): rename MongodbSqlEscapes to MongodbSqlGuards per maintainer review (OtterMind#1914) - non-SQL module: no SQL identifier/string escaping, default ISQLIdentifierProcessor left untouched - requireMongoName allowlist + escapeJsonString are validation/JSON guards, not SQL escaping; class renamed accordingly - callers rewired (DBManager/MetaData/ScriptExecutor/SqlBuilder) - tests migrated to MongodbSqlGuardsTest (8 green) * fix(mongodb): escape generated command contexts * fix(mongodb): align editable and count collection paths --------- Co-authored-by: zgq <openai0229@gmail.com>
… fix XLXS typo, align PinTable hashCode (OtterMind#2293) * fix(core-domain-api): volatile async state, escape quoted SQL values, fix XLXS typo, align PinTable hashCode (code review follow-up) * fix(domain-api): use shared SQL literal escaping --------- Co-authored-by: zgq <openai0229@gmail.com>
openai0229
force-pushed
the
fix/review-redis
branch
from
July 30, 2026 00:04
1e0bfa5 to
0a1dade
Compare
…opagate SQL errors; fix pinned duplication and completion fallback (OtterMind#2309) * fix(core-domain-core): escape excel-import DDL identifiers/comments, propagate import SQL errors, dedupe pinned tables, fix schema-less completion fallback (code review follow-up) * fix(domain-core): do not double backslashes in H2-targeted Excel import values (codex review) --------- Co-authored-by: zgq <openai0229@gmail.com>
…elete emptied keys, loop+dedupe SCAN paths (code review follow-up)
…delete for duplicate values (codex review)
openai0229
force-pushed
the
fix/review-redis
branch
from
July 30, 2026 00:11
0a1dade to
60a723b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes verified review findings: (1) dropTable built DEL with a raw unquoted key name (wrong-key deletion on names with spaces); (2) buildByQueryResult iterated edits in an empty loop, silently dropping all grid edits — now emits per-type mutation commands inside MULTI/EXEC; (3) RENAME/EXPIRE built with raw names and EXPIRE emitted for sentinel TTLs; (4) deleting every field/member left an empty key instead of DEL; (5) SCAN listing truncated after the first batch — now loops the cursor with dedupe. Regression tests added. Part of OtterMind#2281.