Skip to content

Commit

Permalink
reverting #537 (#555)
Browse files Browse the repository at this point in the history
  • Loading branch information
aloneguid committed Sep 30, 2024
1 parent 28baeee commit 202f421
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 265 deletions.
1 change: 1 addition & 0 deletions docs/release-history.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
### Bugs fixed

- Nullable `Enum`s were not correctly unwrapped to primitive types, by @cliedeman in #551.
- Reverting #537 due to it breaking binary compatibility in 4.25.0. Thanks to @NeilMacMullen for reporting this.

## 4.25.0

Expand Down
28 changes: 14 additions & 14 deletions src/Parquet.Test/DataAnalysis/DataFrameReaderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@ namespace Parquet.Test.DataAnalysis {
public class DataFrameReaderTest : TestBase {

[Theory]
[InlineData(typeof(short), false, (short)1, (short)2)]
[InlineData(typeof(short?), false, null, (short)2)]
[InlineData(typeof(int), false, 1, 2)]
[InlineData(typeof(int?), false, null, 2)]
[InlineData(typeof(bool), false, true, false)]
[InlineData(typeof(bool?), false, true, null)]
[InlineData(typeof(long), false, 1L, 2L)]
[InlineData(typeof(long?), false, 1L, 2L)]
[InlineData(typeof(ulong), false, 1UL, 2UL)]
[InlineData(typeof(ulong?), false, 1UL, 2UL)]
[InlineData(typeof(string), false, "1", "2")]
[InlineData(typeof(string), true, null, "2")]
public async Task Roundtrip_all_types(Type t, bool makeNullable, object? el1, object? el2) {
[InlineData(typeof(short), (short)1, (short)2)]
[InlineData(typeof(short?), null, (short)2)]
[InlineData(typeof(int), 1, 2)]
[InlineData(typeof(int?), null, 2)]
[InlineData(typeof(bool), true, false)]
[InlineData(typeof(bool?), true, null)]
[InlineData(typeof(long), 1L, 2L)]
[InlineData(typeof(long?), 1L, 2L)]
[InlineData(typeof(ulong), 1UL, 2UL)]
[InlineData(typeof(ulong?), 1UL, 2UL)]
[InlineData(typeof(string), "1", "2")]
[InlineData(typeof(string), null, "2")]
public async Task Roundtrip_all_types(Type t, object? el1, object? el2) {

// arrange
using var ms = new MemoryStream();
Expand All @@ -34,7 +34,7 @@ public async Task Roundtrip_all_types(Type t, bool makeNullable, object? el1, ob


// make schema
var schema = new ParquetSchema(new DataField(t.Name, t, isNullable: makeNullable?true:null, isCompiledWithNullable: true));
var schema = new ParquetSchema(new DataField(t.Name, t));

// make data
using(ParquetWriter writer = await ParquetWriter.CreateAsync(schema, ms)) {
Expand Down
2 changes: 1 addition & 1 deletion src/Parquet.Test/DictionaryEncodingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public async Task DictionaryEncodingTest2() {
"zzz",
};

var dataField = new DataField<string>("string", true);
var dataField = new DataField<string>("string");
var parquetSchema = new ParquetSchema(dataField);

using var stream = new MemoryStream();
Expand Down
26 changes: 26 additions & 0 deletions src/Parquet.Test/Serialisation/ParquetSerializerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,32 @@ public async Task InterfaceProperty_Serialize() {
Assert.Equivalent(data, data2);
}

/*class OneRequiredStringProperty {
[ParquetRequired]
public string S { get; set; } = "";
}
class OneOptionalStringProperty {
public string? S { get; set; }
}
[Fact]
public async Task String_RequiredIntoOptional_Serde() {
OneRequiredStringProperty[] dataRequired = [
new OneRequiredStringProperty { S = "a" },
new OneRequiredStringProperty { S = "b" },
];
using var ms = new MemoryStream();
await ParquetSerializer.SerializeAsync(dataRequired, ms);
ms.Position = 0;
IList<OneOptionalStringProperty> dataOptional = await ParquetSerializer.DeserializeAsync<OneOptionalStringProperty>(ms);
Assert.Equivalent(dataRequired, dataOptional);
}*/

#if NET6_0_OR_GREATER

record RecordContainingDateAndtimeOnly {
Expand Down
21 changes: 0 additions & 21 deletions src/Parquet.Test/Serialisation/SchemaReflectorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -550,27 +550,6 @@ public void Strings_OptionalAndRequired() {
Assert.True(s.DataFields[2].IsNullable);
}


class PocoClassNotNullable {
[JsonPropertyName("id")]
public long Id { get; set; }

[JsonPropertyName("value")]
public string Value { get; set; } = null!;

[JsonPropertyName("frequency")]
public double Frequency { get; set; }
}

[Fact]
public void Strings_NotNullable() {
ParquetSchema s = typeof(PocoClassNotNullable).GetParquetSchema(true);
Assert.False(s.DataFields[0].IsNullable);
Assert.False(s.DataFields[1].IsNullable);
Assert.False(s.DataFields[2].IsNullable);
}


public interface IInterface {
int Id { get; set; }
}
Expand Down
13 changes: 0 additions & 13 deletions src/Parquet/Extensions/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,6 @@ public static bool IsNullable(this Type t) {
(ti.IsGenericType && ti.GetGenericTypeDefinition() == typeof(Nullable<>));
}

public static bool IsNullableStrict(this Type t) {
TypeInfo ti = t.GetTypeInfo();

return
(ti.IsGenericType && ti.GetGenericTypeDefinition() == typeof(Nullable<>));
}

public static bool IsSystemNullable(this Type t) {
TypeInfo ti = t.GetTypeInfo();

Expand All @@ -135,12 +128,6 @@ public static Type GetNonNullable(this Type t) {
return ti.GenericTypeArguments[0];
}

public static bool CanNullifyType(this Type t) {
TypeInfo ti = t.GetTypeInfo();

return !ti.IsClass;
}

public static Type GetNullable(this Type t) {
TypeInfo ti = t.GetTypeInfo();

Expand Down
2 changes: 1 addition & 1 deletion src/Parquet/Parquet.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="IronCompress" Version="1.6.0" />
<PackageReference Include="IronCompress" Version="1.6.2" />
<PackageReference Include="Microsoft.Data.Analysis" Version="0.21.1" />
<PackageReference Include="Microsoft.IO.RecyclableMemoryStream" Version="3.0.1" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Parquet/ParquetExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public static async Task<DataFrame> ReadParquetAsDataFrameAsync(
public static async Task WriteAsync(this DataFrame df, Stream outputStream, CancellationToken cancellationToken = default) {
// create schema
var schema = new ParquetSchema(
df.Columns.Select(col => new DataField(col.Name, col.DataType.GetNullable(), isNullable: col.DataType.CanNullifyType() ? null : col.NullCount > 0)));
df.Columns.Select(col => new DataField(col.Name, col.DataType.GetNullable())));

using ParquetWriter writer = await ParquetWriter.CreateAsync(schema, outputStream, cancellationToken: cancellationToken);
using ParquetRowGroupWriter rgw = writer.CreateRowGroup();
Expand Down
14 changes: 6 additions & 8 deletions src/Parquet/Schema/DataField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,18 @@ public bool IsArray {
/// </summary>
/// <param name="name">Field name</param>
/// <param name="clrType">CLR type of this field. The type is internally discovered and expanded into appropriate Parquet flags.</param>
/// <param name="isCompiledWithNullable">Indicates if the source type was compiled with nullable enabled or not.</param>
/// <param name="isNullable">When set, will override <see cref="IsNullable"/> attribute regardless whether passed type was nullable or not.</param>
/// <param name="isArray">When set, will override <see cref="IsArray"/> attribute regardless whether passed type was an array or not.</param>
/// <param name="propertyName">When set, uses this property to get the field's data. When not set, uses the property that matches the name parameter.</param>
public DataField(string name, Type clrType, bool? isNullable = null, bool? isArray = null, string? propertyName = null, bool? isCompiledWithNullable = null)
public DataField(string name, Type clrType, bool? isNullable = null, bool? isArray = null, string? propertyName = null)
: base(name, SchemaType.Data) {

Discover(clrType, isCompiledWithNullable ?? true, out Type baseType, out bool discIsArray, out bool discIsNullable);
Discover(clrType, out Type baseType, out bool discIsArray, out bool discIsNullable);
ClrType = baseType;
if(!SchemaEncoder.IsSupported(ClrType)) {
if(baseType == typeof(DateTimeOffset)) {
throw new NotSupportedException($"{nameof(DateTimeOffset)} support was dropped due to numerous ambiguity issues, please use {nameof(DateTime)} from now on.");
}
else {
} else {
throw new NotSupportedException($"type {clrType} is not supported");
}
}
Expand Down Expand Up @@ -131,7 +129,7 @@ internal Array UnpackDefinitions(Array definedData, Span<int> definitionLevels)
internal bool IsDeltaEncodable => DeltaBinaryPackedEncoder.IsSupported(ClrType);

/// <inheritdoc/>
public override string ToString() =>
public override string ToString() =>
$"{Path} ({ClrType}{(_isNullable ? "?" : "")}{(_isArray ? "[]" : "")})";

private Type BaseClrType {
Expand Down Expand Up @@ -168,7 +166,7 @@ public override bool Equals(object? obj) {

#region [ Type Resolution ]

private static void Discover(Type t, bool isCompiledWithNullable, out Type baseType, out bool isArray, out bool isNullable) {
private static void Discover(Type t, out Type baseType, out bool isArray, out bool isNullable) {
baseType = t;
isArray = false;
isNullable = false;
Expand All @@ -183,7 +181,7 @@ private static void Discover(Type t, bool isCompiledWithNullable, out Type baseT
isArray = true;
}

if (baseType.IsNullable()) {
if(baseType.IsNullable()) {
baseType = baseType.GetNonNullable();
isNullable = true;
}
Expand Down
80 changes: 0 additions & 80 deletions src/Parquet/Serialization/HttpEncoder.cs

This file was deleted.

Loading

0 comments on commit 202f421

Please sign in to comment.