From 1df2b598885e7266ecd5ef565db07c93a5ad3ea2 Mon Sep 17 00:00:00 2001 From: Huzaifa Iftikhar Date: Sun, 6 Sep 2026 01:39:46 +0500 Subject: [PATCH 1/2] fix the endless loop in join and merge when both suffixes are the same SetSuffixForDuplicatedColumnNames renames a column that is in both frames. the existing one gets leftSuffix and the new one gets rightSuffix, and it repeats until the two names differ. when both suffixes are the same string the names stay equal after every round, so nothing ends the loop and both names grow forever. it now throws an ArgumentException naming the two parameters and the column. the check sits inside the helper rather than in Join and Merge, because equal suffixes are fine when the two frames share no column name, and nothing is renamed in that case. --- src/Microsoft.Data.Analysis/DataFrame.Join.cs | 9 +++++ .../Strings.Designer.cs | 9 +++++ src/Microsoft.Data.Analysis/Strings.resx | 3 ++ .../DataFrameTests.Join.cs | 34 +++++++++++++++++++ .../DataFrameTests.Merge.cs | 13 +++++++ 5 files changed, 68 insertions(+) diff --git a/src/Microsoft.Data.Analysis/DataFrame.Join.cs b/src/Microsoft.Data.Analysis/DataFrame.Join.cs index fe4b44e0d5..0f26ec3144 100644 --- a/src/Microsoft.Data.Analysis/DataFrame.Join.cs +++ b/src/Microsoft.Data.Analysis/DataFrame.Join.cs @@ -26,6 +26,15 @@ public partial class DataFrame private void SetSuffixForDuplicatedColumnNames(DataFrame dataFrame, DataFrameColumn column, string leftSuffix, string rightSuffix) { int index = dataFrame._columnCollection.IndexOf(column.Name); + + // The loop below appends leftSuffix to the existing column and rightSuffix to the new one until the two + // names differ. If both suffixes are the same the names stay equal however many times they are appended, + // so there is no name that ends the loop. + if (index != -1 && leftSuffix == rightSuffix) + { + throw new ArgumentException(string.Format(Strings.SuffixesMustBeDifferent, nameof(leftSuffix), nameof(rightSuffix), column.Name), nameof(rightSuffix)); + } + while (index != -1) { // Pre-existing column. Change name diff --git a/src/Microsoft.Data.Analysis/Strings.Designer.cs b/src/Microsoft.Data.Analysis/Strings.Designer.cs index 69f0daf9bc..9aeb46ff38 100644 --- a/src/Microsoft.Data.Analysis/Strings.Designer.cs +++ b/src/Microsoft.Data.Analysis/Strings.Designer.cs @@ -492,6 +492,15 @@ internal static string StreamDoesntSupportReading { } } + /// + /// Looks up a localized string similar to {0} and {1} must be different when both DataFrames contain a column called '{2}'. + /// + internal static string SuffixesMustBeDifferent { + get { + return ResourceManager.GetString("SuffixesMustBeDifferent", resourceCulture); + } + } + /// /// Looks up a localized string similar to Value name '{0}' matches an existing column name. /// diff --git a/src/Microsoft.Data.Analysis/Strings.resx b/src/Microsoft.Data.Analysis/Strings.resx index bc58112290..faaa77037a 100644 --- a/src/Microsoft.Data.Analysis/Strings.resx +++ b/src/Microsoft.Data.Analysis/Strings.resx @@ -261,6 +261,9 @@ Stream doesn't support reading + + {0} and {1} must be different when both DataFrames contain a column called '{2}' + Value name '{0}' matches an existing column name diff --git a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs index e782296dd9..f8ac428203 100644 --- a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs +++ b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs @@ -82,6 +82,40 @@ public void TestJoin() VerifyJoin(join, left, right, JoinAlgorithm.Inner); } + [Theory] + [InlineData(JoinAlgorithm.Left)] + [InlineData(JoinAlgorithm.Right)] + [InlineData(JoinAlgorithm.FullOuter)] + [InlineData(JoinAlgorithm.Inner)] + public void TestJoin_SameSuffixOnBothSides_Issue6128(JoinAlgorithm joinAlgorithm) + { + DataFrame left = MakeDataFrameWithNumericColumns(3, false); + DataFrame right = MakeDataFrameWithNumericColumns(4, false); + + // Both frames have a column called "Int", and one suffix cannot make the two names different, + // so the join used to keep renaming for ever instead of coming back + Assert.Throws(() => left.Join(right, "_same", "_same", joinAlgorithm)); + Assert.Throws(() => left.Join(right, "", "", joinAlgorithm)); + + // different suffixes still work + DataFrame join = left.Join(right, "_left", "_right", joinAlgorithm); + Assert.Equal(left.Columns.Count + right.Columns.Count, join.Columns.Count); + } + + [Fact] + public void TestJoin_SameSuffixWithNoSharedColumnNames() + { + DataFrame left = new DataFrame(new Int32DataFrameColumn("Left", new int?[] { 0, 1, 2 })); + DataFrame right = new DataFrame(new Int32DataFrameColumn("Right", new int?[] { 0, 1, 2 })); + + // no name is shared, so nothing is renamed and the suffixes are never used + DataFrame join = left.Join(right, "_same", "_same"); + + Assert.Equal(2, join.Columns.Count); + Assert.Equal("Left", join.Columns[0].Name); + Assert.Equal("Right", join.Columns[1].Name); + } + private void VerifyJoin(DataFrame join, DataFrame left, DataFrame right, JoinAlgorithm joinAlgorithm) { Int64DataFrameColumn mapIndices = new Int64DataFrameColumn("map", join.Rows.Count); diff --git a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs index dbd689ff3d..0fad610bab 100644 --- a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs +++ b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs @@ -1013,6 +1013,19 @@ public void TestMerge_CorrectColumnTypes() Assert.NotNull(merge.Columns.GetDateTimeColumn("DateTime_right")); } + [Fact] + //Issue 6128 + public void TestMerge_SameSuffixOnBothSides() + { + DataFrame left = MakeDataFrameWithNumericColumns(3, false); + DataFrame right = MakeDataFrameWithNumericColumns(4, false); + + Assert.Throws(() => left.Merge(right, "Int", "Int", "_same", "_same")); + + DataFrame merge = left.Merge(right, "Int", "Int"); + Assert.Equal(left.Columns.Count + right.Columns.Count, merge.Columns.Count); + } + private void VerifyMerge(DataFrame merge, DataFrame left, DataFrame right, JoinAlgorithm joinAlgorithm) { if (joinAlgorithm == JoinAlgorithm.Left || joinAlgorithm == JoinAlgorithm.Inner) From 45d2f6f1f6b383147ebfe3c7b672db68664bcfc8 Mon Sep 17 00:00:00 2001 From: Huzaifa Iftikhar Date: Fri, 11 Sep 2026 23:30:37 +0500 Subject: [PATCH 2/2] Rename the same suffix tests and drop the issue reference As asked in review. --- test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs | 2 +- test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs index f8ac428203..62624bcc9c 100644 --- a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs +++ b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Join.cs @@ -87,7 +87,7 @@ public void TestJoin() [InlineData(JoinAlgorithm.Right)] [InlineData(JoinAlgorithm.FullOuter)] [InlineData(JoinAlgorithm.Inner)] - public void TestJoin_SameSuffixOnBothSides_Issue6128(JoinAlgorithm joinAlgorithm) + public void TestJoin_ColumnNamesCollide_ThrowsOnSameSuffixOnBothSides(JoinAlgorithm joinAlgorithm) { DataFrame left = MakeDataFrameWithNumericColumns(3, false); DataFrame right = MakeDataFrameWithNumericColumns(4, false); diff --git a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs index 0fad610bab..9a563ee525 100644 --- a/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs +++ b/test/Microsoft.Data.Analysis.Tests/DataFrameTests.Merge.cs @@ -1014,8 +1014,7 @@ public void TestMerge_CorrectColumnTypes() } [Fact] - //Issue 6128 - public void TestMerge_SameSuffixOnBothSides() + public void TestMerge_ColumnNamesCollide_ThrowsOnSameSuffixOnBothSides() { DataFrame left = MakeDataFrameWithNumericColumns(3, false); DataFrame right = MakeDataFrameWithNumericColumns(4, false);