Copilot commented on code in PR #7183:
URL: https://github.com/apache/ignite-3/pull/7183#discussion_r2599087015
##########
modules/platforms/dotnet/Apache.Ignite.Benchmarks/Apache.Ignite.Benchmarks.csproj:
##########
@@ -29,6 +29,7 @@
<ItemGroup>
<PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
+ <PackageReference Include="BenchmarkDotNet.Diagnostics.dotMemory"
Version="0.15.8" />
Review Comment:
[nitpick] The addition of `BenchmarkDotNet.Diagnostics.dotMemory` package
seems unrelated to the main purpose of this PR (adding mapper interfaces).
Consider whether this dependency addition is necessary and whether it should be
in a separate commit.
```suggestion
```
##########
modules/platforms/dotnet/Apache.Ignite.Benchmarks/Program.cs:
##########
@@ -18,10 +18,10 @@
namespace Apache.Ignite.Benchmarks;
using BenchmarkDotNet.Running;
-using Compute;
+using Table.Serialization;
internal static class Program
{
// IMPORTANT: Disable Netty leak detector when using a real Ignite server
for benchmarks.
- private static void Main() => BenchmarkRunner.Run<PlatformJobBenchmarks>();
+ private static void Main() =>
BenchmarkRunner.Run<SerializerHandlerReadBenchmarks>();
Review Comment:
The Program.cs file has been modified to run
`SerializerHandlerReadBenchmarks` instead of the original
`PlatformJobBenchmarks`. This should not be committed as part of the PR - it
appears to be a local development change that should be reverted before merging.
```suggestion
private static void Main() =>
BenchmarkRunner.Run<PlatformJobBenchmarks>();
```
##########
modules/platforms/dotnet/Apache.Ignite/Internal/Proto/BinaryTuple/BinaryTupleBuilder.cs:
##########
@@ -1016,8 +1016,8 @@ public void AppendObjectWithType(
/// </summary>
/// <param name="value">Value.</param>
/// <returns>Resulting column type.</returns>
- public ColumnType AppendObjectAndGetType(
- object? value)
+ /// <typeparam name="T">Value type.</typeparam>
+ public ColumnType AppendObjectAndGetType<T>(T? value)
Review Comment:
[nitpick] The change from `public ColumnType AppendObjectAndGetType(object?
value)` to `public ColumnType AppendObjectAndGetType<T>(T? value)` with generic
type parameter is good for performance (reduces boxing), but this appears to be
an unrelated change to the main purpose of this PR. Consider whether this
should be in a separate commit or PR for cleaner git history.
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Table/RecordViewPrimitiveTests.cs:
##########
@@ -88,6 +92,113 @@ private async Task TestKey<T>(T val, string tableName)
Assert.IsNotNull(table, "Table must exist: " + tableName);
- await TestKey(val, table!.GetRecordView<T>());
+ var recordView = UseMapper
+ ? table!.GetRecordView(new PrimitiveMapper<T>())
+ : table!.GetRecordView<T>();
+
+ await TestKey(val, recordView);
+ }
+
+ private class PrimitiveMapper<T> : IMapper<T>
+ where T : notnull
+ {
+ public void Write(T obj, ref RowWriter rowWriter, IMapperSchema schema)
+ {
+ var col = schema.Columns[0];
+
+ switch (col.Type)
+ {
+ case ColumnType.Boolean:
+ rowWriter.WriteBool((bool)(object)obj);
+ break;
+ case ColumnType.Int8:
+ rowWriter.WriteByte((sbyte)(object)obj);
+ break;
+ case ColumnType.Int16:
+ rowWriter.WriteShort((short)(object)obj);
+ break;
+ case ColumnType.Int32:
+ rowWriter.WriteInt((int)(object)obj);
+ break;
+ case ColumnType.Int64:
+ rowWriter.WriteLong((long)(object)obj);
+ break;
+ case ColumnType.Float:
+ rowWriter.WriteFloat((float)(object)obj);
+ break;
+ case ColumnType.Double:
+ rowWriter.WriteDouble((double)(object)obj);
+ break;
+ case ColumnType.Decimal:
+ if (obj is BigDecimal bd)
+ {
+ rowWriter.WriteBigDecimal(bd);
+ }
+ else
+ {
+ rowWriter.WriteDecimal((decimal)(object)obj);
+ }
+
+ break;
+ case ColumnType.String:
+ rowWriter.WriteString((string)(object)obj);
+ break;
+ case ColumnType.Date:
+ rowWriter.WriteDate((LocalDate)(object)obj);
+ break;
+ case ColumnType.Time:
+ rowWriter.WriteTime((LocalTime)(object)obj);
+ break;
+ case ColumnType.Datetime:
+ rowWriter.WriteDateTime((LocalDateTime)(object)obj);
+ break;
+ case ColumnType.Timestamp:
+ rowWriter.WriteTimestamp((Instant)(object)obj);
+ break;
+ case ColumnType.Uuid:
+ rowWriter.WriteGuid((Guid)(object)obj);
+ break;
+ case ColumnType.ByteArray:
+ rowWriter.WriteBytes((byte[])(object)obj);
+ break;
+ default:
+ Assert.Fail("Unsupported column type: " + col.Type);
+ break;
+ }
+
+ for (int i = 1; i < schema.Columns.Count; i++)
+ {
+ rowWriter.Skip();
+ }
+ }
+
+ public T Read(ref RowReader rowReader, IMapperSchema schema)
+ {
+ var col = schema.Columns[0];
+
+ return col.Type switch
+ {
+ ColumnType.Boolean => (T)(object)rowReader.ReadBool()!,
+ ColumnType.Int8 => (T)(object)rowReader.ReadByte()!,
+ ColumnType.Int16 => (T)(object)rowReader.ReadShort()!,
+ ColumnType.Int32 => (T)(object)rowReader.ReadInt()!,
+ ColumnType.Int64 => (T)(object)rowReader.ReadLong()!,
+ ColumnType.Float => (T)(object)rowReader.ReadFloat()!,
+ ColumnType.Double => (T)(object)rowReader.ReadDouble()!,
+ ColumnType.Decimal => typeof(T) == typeof(BigDecimal)
+ ? (T)(object)rowReader.ReadBigDecimal()!
+ : (T)(object)rowReader.ReadDecimal()!,
+ ColumnType.String => (T)(object)rowReader.ReadString()!,
+ ColumnType.Date => (T)(object)rowReader.ReadDate()!,
+ ColumnType.Time => (T)(object)rowReader.ReadTime()!,
+ ColumnType.Datetime => (T)(object)rowReader.ReadDateTime()!,
+ ColumnType.Timestamp => (T)(object)rowReader.ReadTimestamp()!,
+ ColumnType.Uuid => (T)(object)rowReader.ReadGuid()!,
+ ColumnType.ByteArray => (T)(object)rowReader.ReadBytes()!,
+
+ // ReSharper disable PatternIsRedundant
+ ColumnType.Null or ColumnType.Period or ColumnType.Duration or
_ => default!
Review Comment:
Pattern always matches.
```suggestion
_ => default!
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
