Elk: use ports for columns in the sql table shape

[janydoe]

At the moment for the sql_table shape only TALA layout engine render connections to the exact columns.

But I think we could implement it for the ELK via "ports"

References:

• https://rtsys.informatik.uni-kiel.de/elklive/examples.html?e=general%2Fspacing%2Fports

[janydoe]

I think we could add ports for each columns with foreign key or primary key constraints

[alixander]

yeah 100%, should be doable in ELK. (related: #630 (comment))

[landmaj]

I got it somewhat working.

It is not perfect and works differently than TALA, since all connections leave from the right side and enter on the left side. There is a way to make ELK choose on which side a port should be placed, but it only does it to reduce number of crossings, it doesn't care about connection length or aesthetics. And I don't know if it would even work, since I can't figure out how to set correct Y coordinate for ports when portConstraints is set to FIXED_SIDE.

Would you be interested in a PR if I can make it work reliably?

[alixander]

@landmaj What's the behavior when you connect sql table row to non-sql table thing? Or sql table row to just sql table.

I think the leaving and entering on consistent sides isn't entirely a negative to everyone, so I wouldn't say that's a blocker.

A PR would be welcome! ❤️

[landmaj]

For some reason when you connect sql table row to sql table, the arrow goes to the upper left corner. I have no idea why but I should be able to fix it. Connecting sql table row to non-sql table thing works just fine, no issues there.

I found another issue - there is a piece of code in d2elklayout which tries to remove "ladders". It sometimes breaks merging edges and it looks ugly. I guess it was never an issue because I don't think it is possible to have merging edges if you don't use ports?

With ladder removal code:

Without ladder removal code:

And as you can see I figured out how to automatically choose left/right side. Thought I'm still not sure if implementing this feature, fixed sides or not, is actually worth it. The diagrams end up being much more spread out.

Here you can see an example with all the bugs mentioned above:

Same diagram rendered using D2 v0.6.1:

[alixander]

I guess it was never an issue because I don't think it is possible to have merging edges if you don't use ports?

I think that's right, the code doesn't account for merges.

That looks good! I think even if the routes are a little messier, it's entirely in the user's hands whether they want to activate it by specifying ports when declaring connections (assuming the "going to top left" bug gets fixed).

ELK options are a bit confusing and at times I've just trial-and-error'd combinations until one worked. The maintainer is pretty active on their chat though (glitter) and Github issues, which I've asked a few times.

[landmaj]

OK, thanks for feedback. I will get back to work but it may take some time, since I have a real work too. 😆 I will be back with a fully working implementation next weekend, I hope.

[alixander]

Yup understand, thank you!

[landmaj]

I think I got most of the issues fixed. You can see my commit above but I have to clean it up and add tests before PR. And I still need to test it on large diagrams.

Ladders

Before removing a ladder, I just check if the edge shares the destination coordinates with any other edge. This is a simple and stupid solution, but it works and should not affect the ladder removal logic for anything other than sql_table rows.

Object to sql_table and the other way around

All connections to and from a table (not a row) will now enter and leave from the top. Since I'm using FIXED_POS to set correct Y coordinate for row ports, I have to manually add and position ports for other connections to/from sql_table as well, otherwise they would be at (0,0). This works and looks good but has the side effect of hardcoding connection positions - they are positioned in the same order as declared in diagram source, from left to right. This may affect large diagrams with many connections to a single table.

The only other solution for top left corner issue I can think of would be to have a single port for all connections. But it would cause issues with self loops and tables with both incoming and outgoing connections.

Self loops

They work just fine. However I found out that sql_table row connections targeting the same table have SrcTableColumnIndex and DstTableColumnIndex set to nil. Nothing I can do about it - TALA has the same problem.
So as a result, table.row -> table.row is treated the same as table -> table.

Using a single row for both outgoing and incoming connections

The result is strange and you will end up with two-directional arrows. But this is really an edge case and it doesn't make sense in sql tables, so I don't think it's worth the time to fix.

And if you do stupid things, you get stupid results. I managed to create this in D2 Playground using TALA:

table: {
  shape: sql_table
  id: int
  fk: int
}
table -> table
obj -> table.id

[alixander]

Re ladders

Yeah perfect.

Re Object to sql table

Why all at the top vs defining ports e.g. every X pixels around the borders of the table?

Re self loop

I think this would be easier to do than the rest, since self loops are more manual, but can definitely be deferred to a followup issue.

Re connection blending

This is something that your original implementation of keeping incoming connections to one side and outgoing connections to another side would've helped with. I guess not in the case of loops since they're manual, but ideally incoming and outgoing don't share ports. Agreed it's okay to defer though.

---

Getting the bare minimum working and deferring edge cases to followup issues is 👍 . Thanks for the thoroughness in considering all those!

[landmaj]

I have a version I think I'm happy with. Let me know I you like it and I will add tests and create PR.

If connections leaving from rows have meaning, defining ports every X pixels around the border for table connections would be misleading. I settled on the following settings:

• incoming row connections: LEFT
• outgoing row connections: RIGHT
• incoming table connections: TOP
• outgoing table connections: BOTTOM

For direction: up table connections have switched sides, and for direction: left row connections are switched.

The end result looks good IMO.

primary: {
  shape: sql_table
  id: int {constraint: primary_key}
  primary_id: int {constraint: foreign_key}
  column: string
}

primary.primary_id -> primary.id
obj -> primary

secondary: {
  shape: sql_table
  id: int {constraint: primary_key}
  primary_id: int {constraint: foreign_key}
  column: string
}

secondary.primary_id -> primary.id
secondary -> obj

All table and column names are the same because I forgot to uncomment code which censored names from a real db (which I will show later).

Down

Up

Right

Left

[landmaj]

Real database

One connection is misaligned if the direction is left or right. I have no idea why, I just noticed it.

I also tested it on a diagram from #1668. It looks messy but the diagram is insane.

If you look at the bottom connection, it is not centered correctly. I think ELK or D2 moved my port for some reason.

[alixander]

Wow, nice work!

If connections leaving from rows have meaning, defining ports every X pixels around the border for table connections would be misleading.

Right, agreed.

---

Note to self to recommend direction: right/left for ERDs that utilize this in ELK.

---

i think the misalignments just come from hacks we do for padding on labels and containers. @gavin-ts can speak to this more.

---

I wonder if the code made the red connection first, gave it the first available port (which goes clockwise), and then defined the green one next, leading to the crossover. In which case, a post-processing step of swapping ports to reduce crossings would be helpful.

---

Let me know I you like it and I will add tests and create PR.

Yes please!

[alixander]

In that last diagram, the laddering code would've helped. I wonder why it didn't kick in here, they're not sharing a route.

(some of these are just notes for myself, no need to investigate)

[landmaj]

This is because I have to set fixed port positions and manually assign edges to ports, so they are in the order in which they were defined. I couldn't find a way to have both fixed ports on the sides and automatically placed on top/bottom.

I thought about creating an invisible node with the exact same dimentions inside the existing node. This way I could have FIXED_POS for one node and FIXED_SIDE for the other, so the top/bottom ports should be able to move during layout creation. It's just an idea, I don't know if it is even possible.

I will leave this issue for later, since connections to and from tables should not be that common if column connections work.

[landmaj]

In that last diagram, the laddering code would've helped. I wonder why it didn't kick in here, they're not sharing a route.

It should work better now, before it skipped all row connections, even if they never merged anywhere. It now checks if "corner" point is shared with any other route, so even long, merging edges should be able to benefit from ladder removal.

I also fixed misaligned connections. Instead of checking if end point is still attached to a node, for row connections it checks if new Y coordinate is within 1/3 row height from the row center.

This is diagam from #1668 rendered using code from just opened PR.

Some connections could look better but it's still better than without any straightening at all. Here is a version with all straightening logic disabled. The direction is changed because I apparently changed it yesterday and didn't notice before making a screenshot. The edges are identical, just mirrored.

And here is my DB. Topmost connection is now properly connected.

[landmaj]

The only issue left I can think of is when you specify diffrent arrowheads for multiple connections to a single row. Here is an example:

direction: right

a: {
    shape: sql_table
    id: int { constraint: primary_key }
}

b: {
    shape: sql_table
    id: int { constraint: primary_key }
    a_1: int { constraint: foreign_key }
    a_2: int { constraint: foreign_key }
}

b.a_1 -> a.id: {
    target-arrowhead.shape: cf-many
}
b.a_2 -> a.id

They are overlaid on each other. But I don't think there is any way to make it work.

[landmaj]

BTW this issue was a fun challenge but I think you should reconsider good first issue label. 🤣