#40709 Fix DROP confirmation dialog layout for long SQL by EastLord · Pull Request #40714 · dbeaver/dbeaver

EastLord · 2026-04-07T14:13:34Z

Test Steps

Open an SQL Editor
Execute a script where the first statement is a DROP statement and the full script is long, for example:

DROP TABLE IF EXISTS bug_40709_demo;

CREATE TABLE bug_40709_demo (
    id VARCHAR(32) NOT NULL,
    code_001 VARCHAR(32),
    code_002 VARCHAR(32),
    code_003 VARCHAR(32),
    code_004 VARCHAR(32),
    code_005 VARCHAR(32),
    code_006 VARCHAR(32),
    code_007 VARCHAR(32),
    code_008 VARCHAR(32),
    code_009 VARCHAR(32),
    code_010 VARCHAR(32),
    code_011 VARCHAR(32),
    code_012 VARCHAR(32),
    code_013 VARCHAR(32),
    code_014 VARCHAR(32),
    code_015 VARCHAR(32),
    code_016 VARCHAR(32),
    code_017 VARCHAR(32),
    code_018 VARCHAR(32),
    code_019 VARCHAR(32),
    code_020 VARCHAR(32),
    name_001 VARCHAR(128),
    name_002 VARCHAR(128),
    name_003 VARCHAR(128),
    name_004 VARCHAR(128),
    name_005 VARCHAR(128),
    name_006 VARCHAR(128),
    name_007 VARCHAR(128),
    name_008 VARCHAR(128),
    name_009 VARCHAR(128),
    name_010 VARCHAR(128),
    flag_001 BOOLEAN DEFAULT FALSE,
    flag_002 BOOLEAN DEFAULT FALSE,
    flag_003 BOOLEAN DEFAULT FALSE,
    flag_004 BOOLEAN DEFAULT FALSE,
    flag_005 BOOLEAN DEFAULT FALSE,
    amount_001 DECIMAL(18,2),
    amount_002 DECIMAL(18,2),
    amount_003 DECIMAL(18,2),
    amount_004 DECIMAL(18,2),
    amount_005 DECIMAL(18,2),
    qty_001 INT,
    qty_002 INT,
    qty_003 INT,
    qty_004 INT,
    qty_005 INT,
    remark_001 VARCHAR(256),
    remark_002 VARCHAR(256),
    remark_003 VARCHAR(256),
    remark_004 VARCHAR(256),
    remark_005 VARCHAR(256),
    created_by VARCHAR(64),
    created_time TIMESTAMP,
    updated_by VARCHAR(64),
    updated_time TIMESTAMP,
    ext_001 VARCHAR(128),
    ext_002 VARCHAR(128),
    ext_003 VARCHAR(128),
    ext_004 VARCHAR(128),
    ext_005 VARCHAR(128),
    ext_006 VARCHAR(128),
    ext_007 VARCHAR(128),
    ext_008 VARCHAR(128),
    ext_009 VARCHAR(128),
    ext_010 VARCHAR(128),
    ext_011 VARCHAR(128),
    ext_012 VARCHAR(128),
    ext_013 VARCHAR(128),
    ext_014 VARCHAR(128),
    ext_015 VARCHAR(128),
    ext_016 VARCHAR(128),
    ext_017 VARCHAR(128),
    ext_018 VARCHAR(128),
    ext_019 VARCHAR(128),
    ext_020 VARCHAR(128),
    PRIMARY KEY (id)
);

COMMENT ON TABLE bug_40709_demo IS 'Bug 40709 reproduction table';
COMMENT ON COLUMN bug_40709_demo.code_001 IS 'long comment 001';
COMMENT ON COLUMN bug_40709_demo.code_002 IS 'long comment 002';
COMMENT ON COLUMN bug_40709_demo.code_003 IS 'long comment 003';
COMMENT ON COLUMN bug_40709_demo.code_004 IS 'long comment 004';
COMMENT ON COLUMN bug_40709_demo.code_005 IS 'long comment 005';
COMMENT ON COLUMN bug_40709_demo.name_001 IS 'long comment 006';
COMMENT ON COLUMN bug_40709_demo.name_002 IS 'long comment 007';
COMMENT ON COLUMN bug_40709_demo.name_003 IS 'long comment 008';
COMMENT ON COLUMN bug_40709_demo.name_004 IS 'long comment 009';
COMMENT ON COLUMN bug_40709_demo.name_005 IS 'long comment 010';
COMMENT ON COLUMN bug_40709_demo.remark_001 IS 'long comment 011';
COMMENT ON COLUMN bug_40709_demo.remark_002 IS 'long comment 012';
COMMENT ON COLUMN bug_40709_demo.remark_003 IS 'long comment 013';
COMMENT ON COLUMN bug_40709_demo.remark_004 IS 'long comment 014';
COMMENT ON COLUMN bug_40709_demo.remark_005 IS 'long comment 015';
COMMENT ON COLUMN bug_40709_demo.ext_001 IS 'long comment 016';
COMMENT ON COLUMN bug_40709_demo.ext_002 IS 'long comment 017';
COMMENT ON COLUMN bug_40709_demo.ext_003 IS 'long comment 018';
COMMENT ON COLUMN bug_40709_demo.ext_004 IS 'long comment 019';
COMMENT ON COLUMN bug_40709_demo.ext_005 IS 'long comment 020';
COMMENT ON COLUMN bug_40709_demo.ext_006 IS 'long comment 021';
COMMENT ON COLUMN bug_40709_demo.ext_007 IS 'long comment 022';
COMMENT ON COLUMN bug_40709_demo.ext_008 IS 'long comment 023';
COMMENT ON COLUMN bug_40709_demo.ext_009 IS 'long comment 024';
COMMENT ON COLUMN bug_40709_demo.ext_010 IS 'long comment 025';

3.Verify the confirmation dialog appears before execution

After Fix

ShadelessFox

Thanks for the contribution. The concept is good, but the implementation could've been better. :^)

EastLord · 2026-04-08T04:58:55Z

@ShadelessFox Thanks for the review. I’ve refactored the implementation to reuse ConfirmationDialog instead of keeping this logic in SQLEditor. Could you please take another look and let me know if there are any other issues?

ShadelessFox · 2026-04-08T13:38:41Z

+        @NotNull String id,
+        int type,
+        @Nullable String codeBlockText,
+        @Nullable String dialogBoundsSettingsId,


Don't, we can use id

ShadelessFox · 2026-04-08T13:39:13Z

+        Shell parent,
+        String title,
+        String message,
+        String toggleMessage,
+        boolean toggleState,
+        String key,


Please infer nullability annotations

EastLord · 2026-04-08T15:01:25Z

@ShadelessFox Thanks for the review and the suggestions. I’ve updated the implementation accordingly. Please let me know if there are any other issues.

ShadelessFox · 2026-04-08T15:34:54Z

+        if (queryTextForDisplay.length() > MAX_QUERY_PREVIEW_LENGTH) {
+            // Truncate string. Too big strings may freeze UI.
+            queryTextForDisplay = CommonUtils.truncateString(queryTextForDisplay, MAX_QUERY_PREVIEW_LENGTH) +
+                "... (truncated " + (queryTextForDisplay.length() - MAX_QUERY_PREVIEW_LENGTH) + " characters)";
+        }


Use org.jkiss.utils.StringUtils#truncateText. (I don't think the number of characters truncated should be included)

This code comes from

I see, can you introduce a method for it, then?

Thanks, I’ve updated it according to your suggestion and switched to org.jkiss.utils.StringUtils#truncateText, without including the number of truncated characters.

About the old SQLQueryJob logic you pointed out: if we want to keep that style and extract it into a shared method, I’m not fully sure where the best place for that helper would be.

Eek, let's keep the SQLQueryJob logic intact. We can't extract it to the org.jkiss.utils module as it would require proper localization, but since it must not depend on any Eclipse plugins, we won't have access to org.eclipse.osgi.util.NLS there.

Thank you for the detailed explanation and for your patience. For the current version of the code, is there anything else that still needs to be changed?

Please remove if (queryTextForDisplay.length() > MAX_QUERY_PREVIEW_LENGTH) check. It's already handled by StringUtils#truncateText. Don't forget to inline the variable

Thanks, updated accordingly. I removed the explicit length check and inlined the variable, so it now directly uses StringUtils.truncateText(...).

…t` for query preview

EastLord · 2026-04-08T15:55:32Z

@ShadelessFox Thanks for the review and the suggestions. I’ve updated the implementation accordingly.

EastLord · 2026-04-10T09:59:14Z

@ShadelessFox Could you please review the code ? Thank you !

ShadelessFox · 2026-04-10T10:21:30Z

Sorry, I forgot to submit my review. Fixed :^)

…xt with `StringUtils.truncateText`.

ShadelessFox

LTGM :^)

E1izabeth · 2026-04-13T10:39:03Z

We passed your PR to our QA team for testing.

EastLord marked this pull request as ready for review April 7, 2026 14:13

ShadelessFox requested changes Apr 7, 2026

View reviewed changes

Comment thread plugins/org.jkiss.dbeaver.ui.editors.sql/src/org/jkiss/dbeaver/ui/editors/sql/SQLEditor.java Outdated

Comment thread plugins/org.jkiss.dbeaver.ui.editors.sql/src/org/jkiss/dbeaver/ui/editors/sql/SQLEditor.java Outdated

dbeaver#40709 Fix DROP confirmation dialog layout for long SQL

11bf8a9

EastLord force-pushed the 40709-fix-drop-confirm-dialog-scroll branch from 96245ef to 11bf8a9 Compare April 8, 2026 04:58

ShadelessFox reviewed Apr 8, 2026

View reviewed changes

Refactor DROP confirmation preview into ConfirmationDialog

be88b8d

EastLord requested a review from ShadelessFox April 8, 2026 15:02

ShadelessFox reviewed Apr 8, 2026

View reviewed changes

Refactor SQL DROP confirmation dialog to use `StringUtils.truncateTex…

d38c18e

…t` for query preview

EastLord requested a review from ShadelessFox April 8, 2026 15:55

Simplify SQL DROP confirmation dialog by directly truncating query te…

e4ec29b

…xt with `StringUtils.truncateText`.

E1izabeth added the external label Apr 13, 2026

ShadelessFox approved these changes Apr 13, 2026

View reviewed changes

ShadelessFox requested a review from E1izabeth April 13, 2026 10:02

E1izabeth approved these changes Apr 13, 2026

View reviewed changes

Conversation

EastLord commented Apr 7, 2026

Test Steps

After Fix

Uh oh!

ShadelessFox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EastLord commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EastLord commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EastLord commented Apr 8, 2026

Uh oh!

EastLord commented Apr 10, 2026

Uh oh!

ShadelessFox commented Apr 10, 2026

Uh oh!

ShadelessFox left a comment

Choose a reason for hiding this comment

Uh oh!

E1izabeth commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants