Skip to content

Commit 71ae04c

Browse files
stevehansenclaude
andcommitted
fix: Gate aggressive join stripping behind opt-in option
The join-condition exclusion logic from a198c97 strips INNER JOINs where the table is only referenced in its own ON clause. This removes filter conditions (e.g. AND b.Type = 'B') that affect row counts, causing incorrect results (29K vs 24K rows in production). Add AggressiveJoinStripping option (default false) to control this behavior. The default now uses the safe threshold-of-1 logic. Users can opt in via --aggressive-join-stripping on the CLI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 774a404 commit 71ae04c

File tree

4 files changed

+38
-5
lines changed

4 files changed

+38
-5
lines changed

src/SqlInliner.Tests/SimpleTests.cs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,14 @@ public void CountColumnReferences()
171171
public void StripJoinWithMultipleConditionsWhenUnused()
172172
{
173173
// A join with multiple ON conditions (e.g. AND b.Type = 'B') where the joined table
174-
// is only referenced in its own ON clause should still be stripped.
174+
// is only referenced in its own ON clause should still be stripped when aggressive mode is on.
175175
connection.AddViewDefinition(DatabaseConnection.ToObjectName("dbo", "VFilteredJoin"),
176176
"CREATE VIEW dbo.VFilteredJoin AS SELECT a.Id, a.Name FROM dbo.A a INNER JOIN dbo.B b ON a.BId = b.Id AND b.Type = 'B'");
177177

178178
const string viewSql = "CREATE VIEW dbo.VTest AS SELECT fj.Id, fj.Name FROM dbo.VFilteredJoin fj";
179179

180-
var inliner = new DatabaseViewInliner(connection, viewSql, options);
180+
var aggressiveOptions = new InlinerOptions { StripUnusedColumns = true, StripUnusedJoins = true, AggressiveJoinStripping = true };
181+
var inliner = new DatabaseViewInliner(connection, viewSql, aggressiveOptions);
181182
inliner.Errors.Count.ShouldBe(0);
182183

183184
var result = inliner.Result;
@@ -186,6 +187,24 @@ public void StripJoinWithMultipleConditionsWhenUnused()
186187
result.ConvertedSql.ShouldContain("dbo.A");
187188
}
188189

190+
[Test]
191+
public void KeepJoinWithMultipleConditionsWhenNotAggressive()
192+
{
193+
// Without AggressiveJoinStripping, a join with filter conditions in ON should be kept
194+
// because the ON clause acts as a filter for INNER JOINs.
195+
connection.AddViewDefinition(DatabaseConnection.ToObjectName("dbo", "VFilteredJoinSafe"),
196+
"CREATE VIEW dbo.VFilteredJoinSafe AS SELECT a.Id, a.Name FROM dbo.A a INNER JOIN dbo.B b ON a.BId = b.Id AND b.Type = 'B'");
197+
198+
const string viewSql = "CREATE VIEW dbo.VTest AS SELECT fj.Id, fj.Name FROM dbo.VFilteredJoinSafe fj";
199+
200+
var inliner = new DatabaseViewInliner(connection, viewSql, options);
201+
inliner.Errors.Count.ShouldBe(0);
202+
203+
var result = inliner.Result;
204+
result.ShouldNotBeNull();
205+
result.ConvertedSql.ShouldContain("dbo.B");
206+
}
207+
189208
[Test]
190209
public void KeepJoinWithMultipleConditionsWhenUsedInSelect()
191210
{

src/SqlInliner/DatabaseViewInliner.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,12 @@ private void DetectUnusedTablesToStrip(ReferencesVisitor references, List<TableR
325325
{
326326
var alias = referenced.Alias?.Value ?? referenced.SchemaObject.BaseIdentifier.Value;
327327

328-
// Exclude column references from the join condition that introduces this table,
329-
// so that filter conditions like "AND b.Type = 'B'" don't prevent stripping.
328+
// When AggressiveJoinStripping is enabled, exclude column references from the
329+
// join condition that introduces this table. This allows stripping joins where
330+
// the table is only referenced in its own ON clause (e.g. AND b.Type = 'B').
331+
// WARNING: This can change results for INNER JOINs where the ON clause filters rows.
330332
HashSet<ColumnReferenceExpression>? joinConditionRefs = null;
331-
if (references.JoinConditions.TryGetValue(referenced, out var searchCondition))
333+
if (options.AggressiveJoinStripping && references.JoinConditions.TryGetValue(referenced, out var searchCondition))
332334
joinConditionRefs = CollectColumnReferences(searchCondition);
333335

334336
var columns = references.ColumnReferences

src/SqlInliner/InlinerOptions.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ public sealed class InlinerOptions
1515
/// </summary>
1616
public bool StripUnusedJoins { get; set; }
1717

18+
/// <summary>
19+
/// Gets or sets whether join condition column references should be excluded from the usage count when stripping joins.
20+
/// When <c>true</c>, a table referenced only in its own ON clause (e.g. <c>INNER JOIN b ON a.Id = b.Id AND b.Type = 'X'</c>)
21+
/// will be stripped. This is more aggressive and can change results for INNER JOINs where the ON clause acts as a filter.
22+
/// Defaults to <c>false</c>.
23+
/// </summary>
24+
public bool AggressiveJoinStripping { get; set; }
25+
1826
/// <summary>
1927
/// Gets the recommended options that should be used for optimal results.
2028
/// </summary>

src/SqlInliner/Program.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ private static int Main(string[] args)
1616
var viewPathOption = new Option<FileInfo>("--view-path", "-vp") { Description = "The path of the view as a .sql file (including create statement)" };
1717
var stripUnusedColumnsOption = new Option<bool>("--strip-unused-columns", "-suc") { DefaultValueFactory = _ => true };
1818
var stripUnusedJoinsOption = new Option<bool>("--strip-unused-joins", "-suj");
19+
var aggressiveJoinStrippingOption = new Option<bool>("--aggressive-join-stripping") { Description = "Exclude join condition references from usage count when stripping joins (can change results for INNER JOINs)" };
1920
var generateCreateOrAlterOption = new Option<bool>("--generate-create-or-alter") { DefaultValueFactory = _ => true };
2021
var outputPathOption = new Option<FileInfo?>("--output-path", "-op") { Description = "Optional path of the file to write the resulting SQL to" };
2122
var logPathOption = new Option<FileInfo?>("--log-path", "-lp") { Description = "Optional path of the file to write debug information to" };
@@ -26,6 +27,7 @@ private static int Main(string[] args)
2627
viewPathOption,
2728
stripUnusedColumnsOption,
2829
stripUnusedJoinsOption,
30+
aggressiveJoinStrippingOption,
2931
generateCreateOrAlterOption,
3032
outputPathOption,
3133
logPathOption,
@@ -39,6 +41,7 @@ private static int Main(string[] args)
3941
var viewPath = parseResult.GetValue(viewPathOption);
4042
var stripUnusedColumns = parseResult.GetValue(stripUnusedColumnsOption);
4143
var stripUnusedJoins = parseResult.GetValue(stripUnusedJoinsOption);
44+
var aggressiveJoinStripping = parseResult.GetValue(aggressiveJoinStrippingOption);
4245
var generateCreateOrAlter = parseResult.GetValue(generateCreateOrAlterOption);
4346
var outputPath = parseResult.GetValue(outputPathOption);
4447
var logPath = parseResult.GetValue(logPathOption);
@@ -67,6 +70,7 @@ private static int Main(string[] args)
6770
{
6871
StripUnusedColumns = stripUnusedColumns,
6972
StripUnusedJoins = stripUnusedJoins,
73+
AggressiveJoinStripping = aggressiveJoinStripping,
7074
});
7175

7276
if (outputPath != null)

0 commit comments

Comments
 (0)