wjones127 commented on code in PR #36122:
URL: https://github.com/apache/arrow/pull/36122#discussion_r1232610389
##########
csharp/src/Apache.Arrow/C/CArrowSchemaImporter.cs:
##########
@@ -281,21 +283,53 @@ public Field GetAsField()
bool nullable =
_cSchema->GetFlag(CArrowSchema.ArrowFlagNullable);
- return new Field(fieldName, GetAsType(), nullable);
+ return new Field(fieldName, GetAsType(), nullable,
GetMetadata(_cSchema->metadata));
}
public Schema GetAsSchema()
{
ArrowType fullType = GetAsType();
if (fullType is StructType structType)
{
- return new Schema(structType.Fields, default);
+ return new Schema(structType.Fields,
GetMetadata(_cSchema->metadata));
}
else
{
throw new ArgumentException("Imported type is not a struct
type, so it cannot be converted to a schema.");
}
}
+
+ private unsafe static IReadOnlyDictionary<string, string>
GetMetadata(byte* metadata)
+ {
+ if (metadata == null)
+ {
+ return null;
+ }
+
+ IntPtr ptr = (IntPtr)metadata;
+ int count = Marshal.ReadInt32(ptr);
+ if (count == 0)
+ {
+ return null;
+ }
+ ptr += 4;
+
+ Dictionary<string, string> result = new Dictionary<string,
string>(count);
+ for (int i = 0; i < count; i++)
+ {
+ result[ReadString(ref ptr)] = ReadString(ref ptr);
+ }
+ return result;
+ }
+
+ private unsafe static string ReadString(ref IntPtr ptr)
Review Comment:
Might want to name more specifically here:
```suggestion
private unsafe static string ReadMetadataString(ref IntPtr ptr)
```
##########
csharp/src/Apache.Arrow/C/CArrowSchemaImporter.cs:
##########
@@ -281,21 +283,53 @@ public Field GetAsField()
bool nullable =
_cSchema->GetFlag(CArrowSchema.ArrowFlagNullable);
- return new Field(fieldName, GetAsType(), nullable);
+ return new Field(fieldName, GetAsType(), nullable,
GetMetadata(_cSchema->metadata));
}
public Schema GetAsSchema()
{
ArrowType fullType = GetAsType();
if (fullType is StructType structType)
{
- return new Schema(structType.Fields, default);
+ return new Schema(structType.Fields,
GetMetadata(_cSchema->metadata));
}
else
{
throw new ArgumentException("Imported type is not a struct
type, so it cannot be converted to a schema.");
}
}
+
+ private unsafe static IReadOnlyDictionary<string, string>
GetMetadata(byte* metadata)
+ {
+ if (metadata == null)
+ {
+ return null;
+ }
+
+ IntPtr ptr = (IntPtr)metadata;
+ int count = Marshal.ReadInt32(ptr);
+ if (count == 0)
Review Comment:
Perhaps, to be somewhat defensive:
```suggestion
if (count <= 0)
```
##########
csharp/src/Apache.Arrow/C/CArrowSchemaImporter.cs:
##########
@@ -281,21 +283,53 @@ public Field GetAsField()
bool nullable =
_cSchema->GetFlag(CArrowSchema.ArrowFlagNullable);
- return new Field(fieldName, GetAsType(), nullable);
+ return new Field(fieldName, GetAsType(), nullable,
GetMetadata(_cSchema->metadata));
}
public Schema GetAsSchema()
{
ArrowType fullType = GetAsType();
if (fullType is StructType structType)
{
- return new Schema(structType.Fields, default);
+ return new Schema(structType.Fields,
GetMetadata(_cSchema->metadata));
}
else
{
throw new ArgumentException("Imported type is not a struct
type, so it cannot be converted to a schema.");
}
}
+
+ private unsafe static IReadOnlyDictionary<string, string>
GetMetadata(byte* metadata)
+ {
+ if (metadata == null)
+ {
+ return null;
+ }
+
+ IntPtr ptr = (IntPtr)metadata;
+ int count = Marshal.ReadInt32(ptr);
+ if (count == 0)
+ {
+ return null;
+ }
+ ptr += 4;
+
+ Dictionary<string, string> result = new Dictionary<string,
string>(count);
+ for (int i = 0; i < count; i++)
+ {
+ result[ReadString(ref ptr)] = ReadString(ref ptr);
+ }
+ return result;
+ }
+
+ private unsafe static string ReadString(ref IntPtr ptr)
+ {
+ int length = Marshal.ReadInt32(ptr);
Review Comment:
Perhaps we should also throw an exception if length is negative? Or will
`GetString` already do that?
##########
csharp/test/Apache.Arrow.Tests/CDataInterfacePythonTests.cs:
##########
Review Comment:
Could you add a test that involves some multi-byte unicode characters?
##########
csharp/src/Apache.Arrow/C/CArrowSchemaExporter.cs:
##########
@@ -239,6 +240,51 @@ private static long GetFlags(IArrowType datatype, bool
nullable = true)
}
}
+ private unsafe static byte*
ConstructMetadata(IReadOnlyDictionary<string, string> metadata)
+ {
+ if (metadata == null || metadata.Count == 0)
+ {
+ return null;
+ }
+
+ int size = 4;
+ int[] lengths = new int[metadata.Count * 2];
+ int i = 0;
+ foreach (KeyValuePair<string, string> pair in metadata)
+ {
+ size += 8;
+ lengths[i] = Encoding.UTF8.GetByteCount(pair.Key);
+ size += lengths[i++];
+ lengths[i] = Encoding.UTF8.GetByteCount(pair.Value);
+ size += lengths[i++];
+ }
+
+ IntPtr result = Marshal.AllocHGlobal(size);
+ Marshal.WriteInt32(result, metadata.Count);
+ byte* ptr = (byte*)result + 4;
+ i = 0;
+ foreach (KeyValuePair<string, string> pair in metadata)
+ {
+ WriteString(ref ptr, lengths[i++], pair.Key);
+ WriteString(ref ptr, lengths[i++], pair.Value);
+ }
+
+ Debug.Assert((long)(IntPtr)ptr - (long)result == size);
+
+ return (byte*)result;
+ }
+
+ private unsafe static void WriteString(ref byte* ptr, int length,
string str)
Review Comment:
```suggestion
private unsafe static void WriteMetadataString(ref byte* ptr, int
length, string str)
```
--
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]
