Fix ER diagram rendering due to "." - enclose in quotes

[ErikEJ]

fixes #894

[ErikEJ]

@nmummau Could you have a look at this?

[Copilot]

Pull request overview

This PR updates Mermaid ER diagram generation to handle schema-qualified table names containing . by enclosing table identifiers in quotes, preventing Mermaid parsing/rendering issues.

Changes:

• Quote table names (and FK relationship endpoints) in generated Mermaid erDiagram output.
• Fix a method name typo (AddColum → AddColumn) in the diagram generator.
• Update the ER diagram sample markdown and the diagram builder unit test to expect quoted table names.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
test/TestProject/TestProject_erdiagram.md	Updates the sample Mermaid ER diagram to use a quoted schema-qualified table name.
test/DacpacTool.Tests/DiagramBuilderTests.cs	Updates assertions to expect quoted table names in generated diagram output.
src/DacpacTool/Diagram/DatabaseModelToMermaid.cs	Quotes schema-qualified table names in entity declarations and FK relationship lines; fixes AddColumn naming.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[nmummau]

I'm packing the SDK locally and doing a test with my project. Standby.

[nmummau]

@ErikEJ I have a computed persisted column that is now breaking the parsing

Rendering the diagram is erroring with this now

Parse error on line 9:
...ted((CONVERT(TINYINT,6)))(NULL) FK    I
-----------------------^
Expecting 'BLOCK_STOP', 'ATTRIBUTE_WORD', 'ATTRIBUTE_KEY', 'COMMENT', got ','

You can repro by putting this into the existing test file test\TestProject\Tables\MyTable.sql

CREATE TABLE [dbo].[MyTable]
(
    Column1 nvarchar(100),
    Column2 int
);
GO

CREATE TABLE dbo.InventoryNodeTypeLookup (
    InventoryNodeTypeCode  TINYINT        NOT NULL,
    CONSTRAINT PK_dbo_InventoryNodeTypeLookup PRIMARY KEY CLUSTERED (InventoryNodeTypeCode)
);
GO

CREATE TABLE dbo.InventoryNode (
    InventoryNodeId        INT     NOT NULL IDENTITY (1, 1),
    InventoryNodeTypeCode  TINYINT NOT NULL,
    CONSTRAINT PK_dbo_InventoryNode PRIMARY KEY CLUSTERED (InventoryNodeId),
    CONSTRAINT FK_dbo_InventoryNode_InventoryNodeTypeLookup FOREIGN KEY (InventoryNodeTypeCode) REFERENCES dbo.InventoryNodeTypeLookup (InventoryNodeTypeCode)
);
GO

-- This unique index allows us to enforce the type code on dbo.Bin table via foreign key.
CREATE UNIQUE NONCLUSTERED INDEX UQ_dbo_InventoryNode_InventoryNodeId_InventoryNodeTypeCode
    ON dbo.InventoryNode (InventoryNodeId, InventoryNodeTypeCode);
GO

CREATE TABLE dbo.Bin (
    BinId           INT NOT NULL IDENTITY (1, 1),
    InventoryNodeId INT NOT NULL,
    InventoryNodeTypeCode AS (CONVERT(TINYINT, 8)) PERSISTED NOT NULL,
    CONSTRAINT PK_dbo_Bin PRIMARY KEY CLUSTERED (BinId),
    CONSTRAINT FK_dbo_Bin_dbo_InventoryNodeTypeCode FOREIGN KEY (InventoryNodeTypeCode) REFERENCES dbo.InventoryNodeTypeLookup (InventoryNodeTypeCode),
    CONSTRAINT FK_dbo_Bin_InventoryNodeId_InventoryNodeTypeCode_dbo_InventoryNode FOREIGN KEY (InventoryNodeId, InventoryNodeTypeCode) REFERENCES dbo.InventoryNode (InventoryNodeId, InventoryNodeTypeCode),
);
GO

It renders a diagram like this that cannot be parsed

erDiagram
  "dbo.MyTable" {
    Column1 nvarchar(100)(NULL) 
    Column2 int(NULL) 
  }
  "dbo.InventoryNodeTypeLookup" {
    InventoryNodeTypeCode tinyint PK
  }
  "dbo.InventoryNode" {
    InventoryNodeId int PK
    InventoryNodeTypeCode tinyint FK
  }
  "dbo.InventoryNode" }o--|| "dbo.InventoryNodeTypeLookup" : FK_dbo_InventoryNode_InventoryNodeTypeLookup
  "dbo.Bin" {
    BinId int PK
    InventoryNodeId int FK
    InventoryNodeTypeCode computed((CONVERT(TINYINT,8)))(NULL) FK
  }
  "dbo.Bin" }o--o| "dbo.InventoryNodeTypeLookup" : FK_dbo_Bin_dbo_InventoryNodeTypeCode
  "dbo.Bin" }o--o| "dbo.InventoryNode" : FK_dbo_Bin_InventoryNodeId_InventoryNodeTypeCode_dbo_InventoryNode

Loading

[nmummau]

Wrapping it in "" fixes it. Is that reasonable to squeeze into this fix?
"InventoryNodeTypeCode computed((CONVERT(TINYINT,8)))(NULL) FK"

[nmummau]

It's a wider issue with computed columns. Cause another one is having issues, not in my example but

A column like this

IsReservedExpected AS CONVERT(BIT, CASE 1 WHEN SomethingIsTrue THEN 1 ELSE 0 END)

will renders in the diagram markdown as:
IsReservedExpected computed(CONVERT(BIT,CASE1WHENSomethingIsTrueTHEN1ELSE0END))(NULL)

Fails to parse

Parse error on line 15:
...computed(CONVERT(BIT,CASE1WHENSomethingI
-----------------------^
Expecting 'BLOCK_STOP', 'ATTRIBUTE_WORD', 'ATTRIBUTE_KEY', 'COMMENT', got ','

[ErikEJ]

@nmummau I vote to get this out, I think we need a more general solution in next iteration for the various edge cases - and I think I do not have enough knowledge just yet to uncover them all.

[ErikEJ]

@nmummau we are not sanitising column names at all just now, so that is probably a bad start.

[ErikEJ]

I will add that

[ErikEJ]

@nmummau Added a fix for the store type

[ErikEJ]

Wrapping it in "" fixes it. Is that reasonable to squeeze into this fix?
"InventoryNodeTypeCode computed((CONVERT(TINYINT,8)))(NULL) FK"

Actually this will render an invalid diagram

[nmummau]

LGTM
I pulled your forked changes locally, packed the SDK, and built my project. The rendered diagram is healthy 🎊

[jmezach]

@ErikEJ I'll override and merge this

[jmezach]

@ErikEJ Can you cherry pick the merge commit to the 4.1 release branch and then update the version number in version.json on that branch? That should trigger the release flow

[ErikEJ]

@jmezach I will try tomorrow

[ErikEJ]

@nmummau @jmezach 4.1.1 is out now