Fix DataFrame column clone behavior

src/Microsoft.Data.Analysis/DataFrameColumn.cs

@@ -212,7 +212,9 @@ public object this[long rowIndex]
         /// <summary>
         /// Clones the column.
         /// </summary>
-        /// <param name="numberOfNullsToAppend">The number of null values to append to the copied values.</param>
+        /// <param name="numberOfNullsToAppend">
+        /// The number of null values to append to the copied values. Columns that do not support null values append default values instead.
+        /// </param>
         /// <returns>A new <see cref="DataFrameColumn"/></returns>
         public DataFrameColumn Clone(long numberOfNullsToAppend = 0) => CloneImplementation(numberOfNullsToAppend);
 
@@ -229,7 +231,9 @@ public object this[long rowIndex]
         /// <see langword="true"/> to process integer values in <paramref name="mapIndices"/> in reverse order;
         /// otherwise, <see langword="false"/>. This parameter does not affect a Boolean map.
         /// </param>
-        /// <param name="numberOfNullsToAppend">The number of null values to append after the selected values.</param>
+        /// <param name="numberOfNullsToAppend">
+        /// The number of null values to append after the selected values. Columns that do not support null values append default values instead.
+        /// </param>
         /// <returns>A new <see cref="DataFrameColumn"/></returns>
         public DataFrameColumn Clone(DataFrameColumn mapIndices, bool invertMapIndices = false, long numberOfNullsToAppend = 0) => CloneImplementation(mapIndices, invertMapIndices, numberOfNullsToAppend);
 

src/Microsoft.Data.Analysis/DataFrameColumns/VBufferDataFrameColumn.cs

@@ -267,13 +267,35 @@ private VBufferDataFrameColumn<T> CloneImplementation(PrimitiveDataFrameColumn<i
             return ret;
         }
 
-        /// <inheritdoc cref="DataFrameColumn.Clone(DataFrameColumn, bool, long)"/>
+        /// <summary>
+        /// Clones the column, selecting and ordering values according to <paramref name="mapIndices"/>.
+        /// </summary>
+        /// <param name="mapIndices">
+        /// A Boolean, <see cref="int"/>, or <see cref="long"/> column that determines which values to copy.
+        /// For a Boolean column, the value at each position selects the value at the same position when it is
+        /// <see langword="true"/>. For an integer column, each value is a zero-based index into this column;
+        /// the map order determines the result order, and repeated indices produce repeated values.
+        /// </param>
+        /// <param name="invertMapIndices">
+        /// <see langword="true"/> to process integer values in <paramref name="mapIndices"/> in reverse order;
+        /// otherwise, <see langword="false"/>. This parameter does not affect a Boolean map.
+        /// </param>
+        /// <param name="numberOfNullsToAppend">
+        /// The number of default <see cref="VBuffer{T}"/> values to append after the selected values.
+        /// </param>
+        /// <returns>A new <see cref="VBufferDataFrameColumn{T}"/>.</returns>
         public new VBufferDataFrameColumn<T> Clone(DataFrameColumn mapIndices, bool invertMapIndices, long numberOfNullsToAppend)
         {
             return (VBufferDataFrameColumn<T>)CloneImplementation(mapIndices, invertMapIndices, numberOfNullsToAppend);
         }
 
-        /// <inheritdoc cref="DataFrameColumn.Clone(long)"/>
+        /// <summary>
+        /// Clones the column.
+        /// </summary>
+        /// <param name="numberOfNullsToAppend">
+        /// The number of default <see cref="VBuffer{T}"/> values to append to the copied values.
+        /// </param>
+        /// <returns>A new <see cref="VBufferDataFrameColumn{T}"/>.</returns>
         public new VBufferDataFrameColumn<T> Clone(long numberOfNullsToAppend = 0)
         {
             return (VBufferDataFrameColumn<T>)CloneImplementation(numberOfNullsToAppend);
@@ -299,6 +321,9 @@ protected override DataFrameColumn CloneImplementation(DataFrameColumn mapIndice
                 clone = Clone();
             }
 
+            for (long i = 0; i < numberOfNullsToAppend; i++)
+                clone.Append(default);
+
             return clone;
         }
 
@@ -309,6 +334,9 @@ protected override DataFrameColumn CloneImplementation(long numberOfNullsToAppen
             for (long i = 0; i < Length; i++)
                 ret[i] = this[i];
 
+            for (long i = 0; i < numberOfNullsToAppend; i++)
+                ret.Append(default);
+
             return ret;
         }
 

src/Microsoft.Data.Analysis/PrimitiveDataFrameColumn.cs

@@ -547,14 +547,10 @@ public PrimitiveDataFrameColumn<T> Clone(PrimitiveDataFrameColumn<int> mapIndice
         /// <returns>A new <see cref="PrimitiveDataFrameColumn{T}"/>.</returns>
         public PrimitiveDataFrameColumn<T> Clone(IEnumerable<long> mapIndices)
         {
-            IEnumerator<long> rows = mapIndices.GetEnumerator();
             PrimitiveDataFrameColumn<T> ret = CreateNewColumn(Name);
-            long numberOfRows = 0;
-            while (rows.MoveNext() && numberOfRows < Length)
+            foreach (long rowIndex in mapIndices)
             {
-                numberOfRows++;
-                var curRow = rows.Current;
-                var value = _columnContainer[curRow];
+                var value = _columnContainer[rowIndex];
                 ret.Append(value);
             }
             return ret;

test/Microsoft.Data.Analysis.Tests/PrimitiveDataFrameColumnTests.cs

@@ -439,6 +439,20 @@ public void TestNotNullableColumnCloneWithIndicesMapAsEnumerableInt()
                 Assert.Equal(column[indicesMap[i]], clonedColumn[i]);
         }
 
+        [Fact]
+        public void TestColumnCloneWithEnumerableIndicesLongerThanColumn()
+        {
+            var column = new Int32DataFrameColumn("Int column", values: new[] { 10, 20 });
+            var longIndicesMap = new long[] { 0, 1, 0 };
+            var intIndicesMap = new int[] { 1, 0, 1 };
+
+            PrimitiveDataFrameColumn<int> longIndicesClone = column.Clone(longIndicesMap);
+            PrimitiveDataFrameColumn<int> intIndicesClone = column.Clone(intIndicesMap);
+
+            Assert.Equal(new int?[] { 10, 20, 10 }, longIndicesClone);
+            Assert.Equal(new int?[] { 20, 10, 20 }, intIndicesClone);
+        }
+
         [Fact]
         public void TestNullableColumnCloneWithIndicesMapAndSmallerSize()
         {

test/Microsoft.Data.Analysis.Tests/VBufferColumnTests.cs

@@ -46,6 +46,29 @@ public void TestVBufferColumn_Indexer()
             Assert.Equal(0, vBufferColumn[0].GetValues()[4]);
         }
 
+        [Fact]
+        public void TestVBufferColumn_CloneAppendsDefaultValues()
+        {
+            var first = new VBuffer<int>(2, new[] { 1, 2 });
+            var second = new VBuffer<int>(2, new[] { 3, 4 });
+            var column = new VBufferDataFrameColumn<int>("VBuffer", new[] { first, second });
+            var mapIndices = new Int32DataFrameColumn("Indices", new[] { 1 });
+
+            VBufferDataFrameColumn<int> clone = column.Clone(numberOfNullsToAppend: 2);
+            VBufferDataFrameColumn<int> mappedClone = column.Clone(mapIndices, invertMapIndices: false, numberOfNullsToAppend: 2);
+
+            Assert.Equal(4, clone.Length);
+            Assert.Equal(first.GetValues().ToArray(), clone[0].GetValues().ToArray());
+            Assert.Equal(second.GetValues().ToArray(), clone[1].GetValues().ToArray());
+            Assert.Equal(0, clone[2].GetValues().Length);
+            Assert.Equal(0, clone[3].GetValues().Length);
+
+            Assert.Equal(3, mappedClone.Length);
+            Assert.Equal(second.GetValues().ToArray(), mappedClone[0].GetValues().ToArray());
+            Assert.Equal(0, mappedClone[1].GetValues().Length);
+            Assert.Equal(0, mappedClone[2].GetValues().Length);
+        }
+
         [X64Fact("32-bit doesn't allow to allocate more than 2 Gb")]
         public void TestVBufferColumn_Indexer_MoreThanMaxInt()
         {