Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash while modifying index with drag & drop #385

Closed
fifonik opened this issue Nov 5, 2018 · 12 comments

Comments

Projects
None yet
3 participants
@fifonik
Copy link

commented Nov 5, 2018

Steps to reproduce this issue

  1. Create test table:
CREATE TABLE `test` (
	`a` INT(11) NULL DEFAULT NULL,
	`b` INT(11) NULL DEFAULT NULL,
	`c` INT(11) NULL DEFAULT NULL,
	INDEX `idx` (`a`, `b`, `c`)
);
  1. Go to table's 'Indexes' tab & open the index 'idx'
  2. Drag item 'a' below item 'c' until line below item 'c' is displayed
  3. Drop it

Current behavior

Crash when item is dropped.
If I drop item 'a' just on the item 'c' when there is no line under the 'c' item -- no crash.
If I drop 'a' item below 'b' when the line is displayed -- no crash, however the item 'a' would be at the very bottom, but not between item 'b' and item 'c' where it was dropped.
If I drop the item 'a' above item 'b' when the line is displayed -- no crash, however it would appeared below item 'b'.
Looks like drop index is not calculated correctly in this case.

Expected behavior

I hope not to crash would be better behaviour :)

Environment

  • HeidiSQL version: 9.5.0.5325, 9.5.0.5295 (have not checked with earlier versions)
  • Operating system: Windows 10 x64
@fifonik

This comment has been minimized.

Copy link
Author

commented Jan 14, 2019

Anyone to confirm?
I have the same behaviour on two different PCs.

@rentalhost

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

Confirmed with 5453.

The behavior of index tab is a bit confuses. Sometimes it replaces with target position, another times it allows move to above or below target. Very random. The second case will crash after move below target.

Report attached: issue-385.txt

@ansgarbecker ansgarbecker added this to the v10.1 milestone Jan 14, 2019

@fifonik

This comment has been minimized.

Copy link
Author

commented Mar 17, 2019

Looks like this line is causing the issue (table_editor.pas):
if Mode = dmBelow then Inc(ColPos);

The easiest fix would be to add index range check:

if Mode = dmBelow then Inc(ColPos);
+if ColPos > Node.Parent.ChildCount then ColPos := Node.Parent.ChildCount;

However, I think the code should also check if the source idx is higher than target idx or not as currently the logic is inconsistent. For example:

  • if we drag 'a' (first part of index) and drop it onto 'b' (second part of index), the 'a' is inserted after 'b'
  • if we drag 'b' (second part of index) and drop it onto 'a' (first part of index) the 'b' is inserted before 'a'
    I'm talking about dropping onto the item, not 'just before the item' and 'just after the item' when we can see some lines that is displaying exact dropping position.
@fifonik

This comment has been minimized.

Copy link
Author

commented May 19, 2019

Thanks for that. It is not crashing any longer, but when you drag the 'c' in example above and drop it between 'a' and 'b' it actually dropping before 'a'.

@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

commented May 20, 2019

Yes, can confirm that.

@ansgarbecker ansgarbecker reopened this May 20, 2019

@rentalhost

This comment has been minimized.

Copy link
Collaborator

commented May 20, 2019

Additionally look that: https://youtu.be/-st2vkMZM6U

  • First 10 seconds: It too need force rendering of this list, because it causes a visual glitch when you move the indexes.
  • After: another thing that happens is that you could move indexes in two different ways, one will always move to top of target, and the another will display an auxiliar marker that will make possible move to top or bottom.
@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

commented May 20, 2019

The main issue was I did not take into account whether the dropped column comes from above or from below. This should now be fixed, along with that missing repaint issue.

@rentalhost rentalhost added fixed and removed fixed labels May 20, 2019

@fifonik

This comment has been minimized.

Copy link
Author

commented May 20, 2019

It works fine when I'm trying to re-arrange columns in existing index.

However, when creating a new index and dropping columns from column table (that is below) it still does not work correctly.
For the table created with script above:

  • Click 'Add' for creating a new index (would be named 'Index 2')
  • Drag column 'c' from columns table and drop it onto the new index. All good for now
  • Drag column 'b' from columns table and drop it after 'c' in the new index. The 'b' will be added before 'c' (not after 'c' where it was supposed to be)
  • Drag column 'a' from columns table and drop it after 'c' in the new index. The 'a' will be added between 'b' and 'c' (not after 'c' where it was supposed to be)

Sorry for that. I'm just a messenger.

@fifonik

This comment has been minimized.

Copy link
Author

commented Jun 4, 2019

Should I create a new ticket or this one will be re-opened (as it was only partially fixed)?
Thanks.

@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2019

Ah, sorry, I already read it, but forgot about it somehow. No new ticket please, I am reopening this one.

@ansgarbecker

This comment has been minimized.

Copy link
Collaborator

commented Jun 8, 2019

Next build should also fix that wrong position when dropping a column from the columns list. Also, dragging a column above or below an index is blocked, as that makes no sense for a column. Only dragging it onto an index is allowed. Dragging between existing columns of an index is of course allowed also.

@fifonik

This comment has been minimized.

Copy link
Author

commented Jun 10, 2019

All OK now. Many thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.