Skip to content

fix(bi): guard empty data array in EditorChartSql before destructuring - #2164

Merged
openai0229 merged 4 commits into
OtterMind:mainfrom
Aias00:fix/2162-editorchartsql-crash
Aug 3, 2026
Merged

openai0229 merged 4 commits into
OtterMind:mainfrom
Aias00:fix/2162-editorchartsql-crash

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2162

Summary

EditorChartSql.onExecuteSQLCallback destructured data[0] without guarding the empty case. Running a non-result SQL statement (DDL/INSERT/UPDATE) in the BI chart SQL editor yields data = [], so data[0] is undefined and the destructure throws. Added if (!data.length) return; before the destructure.

Verification

  • tsc + eslint clean for EditorChartSql/index.tsx.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

Copilot AI review requested due to automatic review settings July 26, 2026 07:13
@Aias00
Aias00 requested a review from openai0229 as a code owner July 26, 2026 07:13

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openai0229

Copy link
Copy Markdown
Contributor

This guard prevents destructuring an empty result, but the PR is not ready to merge because it leaves stale chart state behind. If a successful SELECT populates metadata and the next statement returns no rows (for example DDL/DML), the early return preserves the previous metaData / databaseInfo, so the editor can display information from the wrong execution.

Please explicitly clear or replace the previous metadata while preserving the current execution's database context. Add an automated sequence test for non-empty result followed by empty result, plus an initially empty result, and rebase onto the latest main.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty-result guard still leaves stale metadata from the previous execution, and the required execution-sequence tests are missing. The current head does not address the detailed review comment already posted, so this remains blocked pending an updated commit and re-review.

onExecuteSQLCallback returned early when data was empty, preserving
the previous execution's metaData/databaseInfo. Now explicitly clears
metaData (empty dataList/headerList) while preserving the current
execution's database context. Prevents displaying information from
the wrong execution.

Fixes OtterMind#2162

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00
Aias00 force-pushed the fix/2162-editorchartsql-crash branch from 271b029 to caf9b38 Compare July 28, 2026 13:56
@Aias00

Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main. Fixed the stale-metadata issue: when data is empty, the callback now explicitly clears metaData (empty dataList/headerList) while preserving the current execution's databaseInfo. This prevents displaying chart data from a previous execution.

if (!data.length) {
  setDatabaseInfoAndMetaData({
    databaseInfo: { ... }, // current execution context
    metaData: { dataList: [], headerList: [] }, // cleared
  });
  return;
}

tsc + eslint clean.

@openai0229

Copy link
Copy Markdown
Contributor

Blocking on the current head: the real path in SQLExecute filters results with item.dataList !== null and can invoke EditorChartSql.onExecuteSQLCallback with an empty array. The new if (!data.length) branch clears state and returns, but this PR adds no automated coverage for that early-return path.

Please add an actual handler/component sequence test for non-empty data followed by empty data, asserting that stale metadata is cleared and the current database context is preserved, plus initially empty and normal non-empty cases.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the empty-data guard, targeted tests, lint, YAML validation, diff checks, and the full Community build. Required checks are passing on the current head.

@openai0229
openai0229 merged commit 19e87a7 into OtterMind:main Aug 3, 2026
17 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

EditorChartSql onExecuteSQLCallback crashes on empty data array (non-result SQL)

3 participants