Skip to content

Commit

Permalink
Fix timestamp size to include alignment, also fix timestamp in union (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Ulimo authored Jan 10, 2025
1 parent d7f1444 commit 1abf5d7
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Apache.Arrow.Memory;
using Apache.Arrow.Types;
using FlowtideDotNet.Core.ColumnStore.DataValues;
using FlowtideDotNet.Core.ColumnStore.Serialization;
using FlowtideDotNet.Core.ColumnStore.TreeStorage;
using FlowtideDotNet.Core.ColumnStore.Utils;
using FlowtideDotNet.Storage.DataStructures;
Expand Down Expand Up @@ -335,8 +336,9 @@ public void RemoveAt(in int index)
if (_valueColumns[i] != null)
{
var (arrowArray, arrowType) = _valueColumns[i].ToArrowArray(nullBuffer, nullCount);
var customMetadata = EventArrowSerializer.GetCustomMetadata(arrowType);
typeIds.Add((int)arrowType.TypeId);
fields.Add(new Field("", arrowType, true));
fields.Add(new Field("", arrowType, true, metadata: customMetadata));
childArrays.Add(arrowArray);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public struct TimestampTzValue : IDataValue, IComparable<TimestampTzValue>
const long UnixEpochTicks = DaysTo1970 * TicksPerDay;

public long ticks;
public short offset;
public long offset;

public TimestampTzValue(long ticks, short offset)
public TimestampTzValue(long ticks, long offset)
{
this.ticks = ticks;
this.offset = offset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal class TimestampTzType : FixedSizeBinaryType, ICustomArrowType
public const string ExtensionName = "flowtide.timestamptz";
public static readonly TimestampTzType Default = new TimestampTzType();

public TimestampTzType() : base(10)
public TimestampTzType() : base(16)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,11 @@ public void Visit(FixedSizeBinaryArray array)
_typeId = ArrowTypeId.Timestamp;
break;
default:
throw new NotImplementedException();
throw new NotImplementedException(typeName);
}
return;
}
throw new NotImplementedException();
throw new NotImplementedException("No metadata field");
}
}
}
26 changes: 26 additions & 0 deletions tests/FlowtideDotNet.Core.Tests/ColumnStore/ArrowTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -712,5 +712,31 @@ public void TestDecimalInMapSerializeDeserialize()
Assert.Equal(2.0m, map);

}

[Fact]
public void TimestampInUnionSerializeDeserialize()
{
Column column = new Column(GlobalMemoryManager.Instance);
column.Add(new TimestampTzValue(1, 0));
column.Add(NullValue.Instance);
column.Add(new Int64Value(2));

var recordBatch = EventArrowSerializer.BatchToArrow(new EventBatchData(
[
column
]), column.Count);

MemoryStream memoryStream = new MemoryStream();

var writer = new ArrowStreamWriter(memoryStream, recordBatch.Schema, true);
writer.WriteRecordBatch(recordBatch);
writer.Dispose();
memoryStream.Position = 0;
var reader = new ArrowStreamReader(memoryStream, new Apache.Arrow.Memory.NativeMemoryAllocator(), true);
var deserializedRecordBatch = reader.ReadNextRecordBatch();
var deserializedBatch = EventArrowSerializer.ArrowToBatch(deserializedRecordBatch, GlobalMemoryManager.Instance);

Assert.Equal(1, column.GetValueAt(0, default).AsTimestamp.ticks);
}
}
}

1 comment on commit 1abf5d7

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 1abf5d7 Previous: 4ae8b85 Ratio
FlowtideDotNet.Benchmarks.Stream.StreamBenchmark.InnerJoin 560479750 ns (± 19654114.38606572) 592945200 ns (± 35581532.03160469) 0.95
FlowtideDotNet.Benchmarks.Stream.StreamBenchmark.LeftJoin 658325680 ns (± 31266504.27328297) 658813590 ns (± 45349397.746520415) 1.00
FlowtideDotNet.Benchmarks.Stream.StreamBenchmark.ProjectionAndNormalization 213586610 ns (± 11971102.044061497) 201853112.5 ns (± 5564610.621581839) 1.06
FlowtideDotNet.Benchmarks.Stream.StreamBenchmark.SumAggregation 224977590 ns (± 12446105.517630806) 228276677.7777778 ns (± 6383403.230796598) 0.99
FlowtideDotNet.Benchmarks.Stream.StreamBenchmark.ListAggWithMapAggregation 2445497905.5555553 ns (± 86964002.43924367) 2311069716.6666665 ns (± 65590138.42863499) 1.06

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.