Fix INSERT VALUES alignment

allancascante · Aug 28, 2026 · Aug 27, 2026 · Aug 27, 2026 · Aug 27, 2026 · Aug 27, 2026
commit 93e0dafe0a73c909bf1e5e93c69fa3ccb59b4c4e
@@ -361,17 +361,29 @@ protected Boolean GenerateClauseBodyStart(Boolean newline, AlignmentPoint ap)
             return false;
         }
 
+        // Marks the river that lines up an INSERT statement's VALUES row constructors under its
+        // column list (e.g. "INSERT INTO t (a, b)" / "VALUES       (1, 2)"). Only skipped for the
+        // Indented + !AlignClauseBodies combination, where ValuesInsertSource instead puts the row
+        // list on its own indented line (see ShouldMoveInsertValuesToNewLine); every other option
+        // combination keeps the existing river behavior so current baselines are unaffected.
         protected void MarkInsertColumnsAlignmentPointWhenNecessary(AlignmentPoint ap)
         {
 #if !PIMODLANGUAGE
             Debug.Assert(ap != null, "Alignment point should not be null");
 #endif
-            if (ap != null)
+            if (ap != null && !ShouldMoveInsertValuesToNewLine())
             {
                 Mark(ap);
             }
         }
 
+        // True when an INSERT statement's VALUES row constructors should move to their own
+        // indented line instead of being padded (or single-spaced) onto the "VALUES" line.
+        protected Boolean ShouldMoveInsertValuesToNewLine()
+        {
+            return _options.ClauseBodyAlignment == ClauseBodyAlignment.Indented && !_options.AlignClauseBodies;
+        }
+
         protected void GenerateSeparatorForOrderBy()
         {
             GenerateNewLineOrSpace(_options.NewLineBeforeOrderByClause);

@@ -65,15 +65,30 @@ public override void ExplicitVisit(InsertSpecification node)
 
                 if (node.Columns.Count > 0)
                 {
-                    MarkInsertColumnsAlignmentPointWhenNecessary(insertColumns);
                     if (_options.MultilineInsertTargetsList)
                     {
                         ListGenerationOption option = ListGenerationOption.CreateOptionFromFormattingConfig(_options);
-                        GenerateFragmentList(node.Columns, option);
+                        if (option.NewLineBeforeOpenParenthesis || ShouldMoveInsertValuesToNewLine())
+                        {
+                            // No inline "(" to align under (it moves to its own line), or the VALUES
+                            // rows move to their own indented line: keep the pre-existing mark position.
+                            MarkInsertColumnsAlignmentPointWhenNecessary(insertColumns);
+                            GenerateFragmentList(node.Columns, option);
+                        }
+                        else
+                        {
+                            // Inline "(": mark insertColumns at the list's "(" (the same column the
+                            // non-multiline branch marks) so the VALUES row constructors line up under
+                            // the target list's opening parenthesis.
+                            GenerateFragmentList(node.Columns, option, insertColumns);
+                        }
                     }
                     else
                     {
+                        // Mark insertColumns right at "(" (after the separating space), matching where
+                        // ValuesInsertSource marks it for its own row list, so both parentheses line up.
                         GenerateSpace();
+                        MarkInsertColumnsAlignmentPointWhenNecessary(insertColumns);
                         GenerateParenthesisedCommaSeparatedList(node.Columns);
                     }
                 }

@@ -334,9 +334,12 @@ protected void GenerateParenthesisedCommaSeparatedList<T>(IList<T> list, Boolean
             }
         }
 
-        protected void GenerateFragmentList<T>(IList<T> list, ListGenerationOption option) where T : TSqlFragment
+        protected void GenerateFragmentList<T>(IList<T> list, ListGenerationOption option, AlignmentPoint openParenthesisAlignmentPoint = null) where T : TSqlFragment
         {
-            AlignmentPoint parentheses = new AlignmentPoint();
+            // When the caller supplies its own open-parenthesis alignment point, mark it at the "("
+            // so downstream content (e.g. an INSERT statement's VALUES row constructors) can line up
+            // under the list's opening parenthesis. Defaults to a private point for all other callers.
+            AlignmentPoint parentheses = openParenthesisAlignmentPoint ?? new AlignmentPoint();
             AlignmentPoint items = new AlignmentPoint();
 
             Boolean generateParentheses = (option.AlwaysGenerateParenthesis || (list.Count > 0 && option.Parenthesised));

@@ -25,15 +25,72 @@ public override void ExplicitVisit(ValuesInsertSource node)
             }
             else
             {
-                GenerateKeywordAndSpace(TSqlTokenType.Values);
-
-                MarkClauseBodyAlignmentWhenNecessary(true, clauseBody);
+                GenerateKeyword(TSqlTokenType.Values);
 
                 AlignmentPoint insertColumns = GetAlignmentPointForFragment(node, InsertColumns);
+                bool moveToNewLine = ShouldMoveInsertValuesToNewLine();
+
+                if (moveToNewLine)
+                {
+                    NewLineAndIndent();
+                }
+                else
+                {
+                    GenerateSpace();
+                    MarkClauseBodyAlignmentWhenNecessary(true, clauseBody);
+                    MarkInsertColumnsAlignmentPointWhenNecessary(insertColumns);
+                }
+
+                // insertColumns is null for a ValuesInsertSource scripted on its own (no enclosing
+                // INSERT/MERGE registered the column-list point); skip all re-anchoring in that case.
+                bool alignLeadingCommaRows = !moveToNewLine
+                    && _options.CommaPlacement == CommaPlacement.Leading
+                    && insertColumns != null;
+
+                if (alignLeadingCommaRows)
+                {
+                    // A plain leading comma after NewLine() would land 2 columns past the aligned "(".
+                    // Restore each continuation row to a neutral anchor, then re-mark insertColumns
+                    // after a right-aligned comma so every row's "(" lines up with the first row's.
+                    AlignmentPoint rowAnchor = new AlignmentPoint();
+                    PushAlignmentPoint(rowAnchor);
+
+                    bool firstRow = true;
+                    foreach (RowValue rowValue in node.RowValues)
+                    {
+                        if (!firstRow)
+                        {
+                            NewLine();
+                            GenerateRightAlignedCommaSeparator();
+                            // insertColumns is non-null here (guaranteed by alignLeadingCommaRows).
+                            Mark(insertColumns);
+                        }
+
+                        GenerateFragmentIfNotNull(rowValue);
+                        firstRow = false;
+                    }
+
+                    PopAlignmentPoint();
+                }
+                else
+                {
+                    // Align continuation rows under the first row when an enclosing INSERT/MERGE
+                    // registered insertColumns. For a bare source (null) we intentionally skip the
+                    // push: rows fall back to the statement's start anchor, which is null-safe and
+                    // avoids an unreachable branch (the non-moved bare-source path can't run in debug
+                    // builds because the null clauseBody/insertColumns marks above assert first).
+                    if (insertColumns != null)
+                    {
+                        PushAlignmentPoint(insertColumns);
+                    }
 
-                MarkInsertColumnsAlignmentPointWhenNecessary(insertColumns);
+                    GenerateCommaSeparatedList(node.RowValues, true, moveToNewLine);
 
-                GenerateCommaSeparatedList(node.RowValues, true);
+                    if (insertColumns != null)
+                    {
+                        PopAlignmentPoint();
+                    }
+                }
             }
 
             PopAlignmentPoint();

@@ -110,10 +110,10 @@
     <Setting name="ClauseBodyAlignment" type="ClauseBodyAlignment" default="ClauseBodyAlignment.Aligned">
       <Option value="ClauseBodyAlignment.Aligned">ClauseBodyAlignment_Aligned</Option>
       <Option value="ClauseBodyAlignment.Indented">ClauseBodyAlignment_Indented</Option>
-      <Summary>Gets or sets how a clause body (the part after FROM, WHERE, GROUP BY, etc.) is laid out. Aligned keeps the body on the keyword's line, lining all clause bodies up under a shared column past the widest keyword. Indented puts the body on its own new line, one indent level (IndentationSize) past the keyword, so nesting grows one step per level instead of drifting right. When Indented, AlignClauseBodies is ignored.</Summary>
+      <Summary>Gets or sets how a clause body (the part after FROM, WHERE, GROUP BY, etc.) is laid out. Aligned keeps the body on the keyword's line, lining all clause bodies up under a shared column past the widest keyword. Indented puts the body on its own new line, one indent level (IndentationSize) past the keyword, so nesting grows one step per level instead of drifting right. When Indented, AlignClauseBodies is ignored for clause bodies.</Summary>
     </Setting>
     <Setting name="AlignClauseBodies" type="bool" default="true">
-      <Summary>Gets or sets whether clause bodies are padded to line up under a shared column. Only applies when ClauseBodyAlignment is Aligned: true pads to the aligned column, false uses a single space after the keyword. Ignored when ClauseBodyAlignment is Indented.</Summary>
+      <Summary>Gets or sets whether clause bodies are padded to line up under a shared column. Only applies when ClauseBodyAlignment is Aligned: true pads to the aligned column, false uses a single space after the keyword. When ClauseBodyAlignment is Indented, this setting is generally ignored for clause bodies; however, it also affects an INSERT statement's VALUES row constructors: when ClauseBodyAlignment is Indented and this is false, the row constructors move to their own indented line instead of being padded to line up under the column list.</Summary>
     </Setting>
   </SettingGroup>
   <SettingGroup name="SELECT">

@@ -3,11 +3,11 @@ AS
 BEGIN
     INSERT INTO t1
     VALUES (1, 2),
-    (DEFAULT, 0),
-    (NULL, NULL);
+           (DEFAULT, 0),
+           (NULL, NULL);
     INSERT INTO t2
     VALUES ('aaa', 'bbb', 1),
-    ('ccc', 'ddd', 20);
+           ('ccc', 'ddd', 20);
 END
 
 
@@ -32,5 +32,5 @@ CREATE PROCEDURE p1
 AS
 BEGIN
     INSERT INTO t1 ($ACTION, $CUID, $ROWGUID)
-    VALUES        (1, 2, 3);
+    VALUES         (1, 2, 3);
 END
@@ -44,9 +44,9 @@ CREATE TABLE t2 (
 GO
 
 INSERT INTO t2 (Text)
-VALUES        (N'This is the first text.'),
-(N'Second sample text.'),
-(N'Third example text.');
+VALUES         (N'This is the first text.'),
+               (N'Second sample text.'),
+               (N'Third example text.');
 GO
 
 CREATE VIEW v_GeneratedChunksFromTable

@@ -48,7 +48,7 @@ public void TestSingleLineTargetsIsDefault()
             const string expected =
 @"
 INSERT  INTO t (a, b, c)
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
 
             AssertGenerated(input, options, expected);
         }
@@ -69,7 +69,7 @@ INSERT  INTO t (
     b,
     c
 )
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
 
             AssertGenerated(input, options, expected);
         }
@@ -85,7 +85,7 @@ public void TestSingleLineTargetsWhenDisabled()
             const string expected =
 @"
 INSERT  INTO t (a, b, c)
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
 
             AssertGenerated(input, options, expected);
         }
@@ -103,7 +103,7 @@ public void TestSingleTargetMultiline()
 INSERT  INTO t (
     a
 )
-VALUES        (1);";
+VALUES         (1);";
 
             AssertGenerated(input, options, expected);
         }
@@ -199,7 +199,7 @@ INSERT  INTO t (
   , b
   , c
 )
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
 
             AssertGenerated(input, options, expected);
         }
@@ -219,7 +219,36 @@ INSERT  INTO t (
     b,
     c
 )
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
+
+            AssertGenerated(input, options, expected);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestMultilineTargetsWithValuesMovedToNewLine()
+        {
+            // Multi-line target list combined with the Indented + AlignClauseBodies = false mode that
+            // moves the VALUES row constructors to their own indented line: the target list is still
+            // multi-line and the rows are not aligned under the target parenthesis.
+            const string input = "INSERT INTO t (a, b, c) VALUES (1, 2, 3), (4, 5, 6);";
+            var options = new SqlScriptGeneratorOptions
+            {
+                MultilineInsertTargetsList = true,
+                ClauseBodyAlignment = ClauseBodyAlignment.Indented,
+                AlignClauseBodies = false,
+            };
+            const string expected =
+@"
+INSERT INTO t (
+    a,
+    b,
+    c
+)
+VALUES
+    (1, 2, 3),
+    (4, 5, 6);";
 
             AssertGenerated(input, options, expected);
         }
@@ -273,7 +302,7 @@ INSERT  INTO t (
     a,
     b,
     c)
-VALUES        (1, 2, 3);";
+VALUES         (1, 2, 3);";
 
             AssertGenerated(input, options, expected);
         }
@@ -297,7 +326,7 @@ INSERT  INTO t (
     b
 )
 OUTPUT  inserted.a, inserted.b
-VALUES        (1, 2);";
+VALUES         (1, 2);";
 
             AssertGenerated(input, options, expected);
         }
@@ -317,7 +346,7 @@ INSERT  INTO t (
     b
 )
 OUTPUT  inserted.a INTO @log
-VALUES        (1, 2);";
+VALUES         (1, 2);";
 
             AssertGenerated(input, options, expected);
         }
@@ -391,7 +420,8 @@ INSERT INTO t (
         [SqlStudioTestCategory(Category.UnitTest)]
         public void TestMultilineTargetsWithMultiRowValues()
         {
-            // Only the target list is affected; a multi-row VALUES source is left as-is.
+            // Only the target list is affected; a multi-row VALUES source keeps aligning its
+            // continuation rows under the first row.
             const string input = "INSERT INTO t (a, b) VALUES (1, 2), (3, 4);";
             var options = new SqlScriptGeneratorOptions { MultilineInsertTargetsList = true };
             const string expected =
@@ -400,8 +430,8 @@ INSERT  INTO t (
     a,
     b
 )
-VALUES        (1, 2),
-(3, 4);";
+VALUES         (1, 2),
+               (3, 4);";
 
             AssertGenerated(input, options, expected);
         }