Skip to content

Commit

Permalink
Fix generic type validation to support datetimes and non-ASCII string…
Browse files Browse the repository at this point in the history
…s. (#291)

* Throw `ArgumentException` on type mismatches.

* Fix and centralize generic type validation.

And also document which type is compatible with which `DataType`.

* Improve type checking for `FragmentInfo` methods when the type is string.

If the generic type is string, we check ourselves that the `DataType` is a string one. This ensures the exception thrown is `ArgumentException` instead of `TileDBException`.
The exception message also is more descriptive.

* Obsolete the converters between TileDB data types and .NET types.

* Use the correct obsoletion code in two cases.

The obsoletions in question have not yet shipped.

* Validate the data type in a couple more cases.

* Suppress obsoletion warning `TILEDB0013` in the `TileDB.CSharp` project.
  • Loading branch information
teo-tsirpanis authored Oct 17, 2023
1 parent 7ca45f8 commit e645f63
Show file tree
Hide file tree
Showing 18 changed files with 320 additions and 89 deletions.
16 changes: 15 additions & 1 deletion docs/obsoletions.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Following [the deprecation policy of TileDB Embedded][core-deprecation], obsolet
|Diagnostic codes|Deprecated in version|Removed in version|
|----------------|---------------------|------------------|
|[`TILEDB0001`](#TILEDB0001)[`TILEDB0011`](#TILEDB0011)|5.3.0|5.5.0|
|[`TILEDB0012`](#TILEDB0012)[`TILEDB0012`](#TILEDB0012)|5.7.0|5.9.0|
|[`TILEDB0012`](#TILEDB0012)[`TILEDB0013`](#TILEDB0013)|5.7.0|5.9.0|

## `TILEDB0001` - Enum value names that start with `TILEDB_` were replaced with C#-friendly names.

Expand Down Expand Up @@ -320,3 +320,17 @@ The obsoleted APIs fall into the following categories:
- Types with the name `tiledb_***_t` were made public again only to support the APIs of the safe handles above. They have little other use on their own. You should use APIs in the `TileDB.CSharp` namespace instead.

[core-deprecation]: https://github.com/TileDB-Inc/TileDB/blob/dev/doc/policy/api_changes.md

## `TILEDB0013` - The `EnumUtils.TypeToDataType` and `EnumUtils.DataTypeToType` methods are obsolete and will be removed in a future version.

<a name="TILEDB0013"></a>

The `EnumUtils.TypeToDataType` and `EnumUtils.DataTypeToType` methods convert between TileDB data types and .NET types. Given that there is no one-to-one correspondence between these two and for legacy reasons, these methods sometimes return wrong results and were obsoleted.

### Version introduced

5.7.0

### Recommended action

If you are performing queries on arrays of unknown schema, you can use the `Query.UnsafeSetDataBuffer` and `Query.UnsafeSetWriteDataBuffer` methods to set a data buffer to a query without type validation.
12 changes: 2 additions & 10 deletions sources/TileDB.CSharp/Array.cs
Original file line number Diff line number Diff line change
Expand Up @@ -501,15 +501,11 @@ static void GetDomain<T>(Array array, string dimName, uint i, NonEmptyDomain non
/// <exception cref="ArgumentException"><typeparamref name="T"/> is not the dimension's type.</exception>
public (T Start, T End, bool IsEmpty) NonEmptyDomain<T>(uint index) where T : struct
{
var datatype = EnumUtil.TypeToDataType(typeof(T));
using (var schema = Schema())
using (var domain = schema.Domain())
using (var dimension = domain.Dimension(index))
{
if (datatype != dimension.Type())
{
throw new ArgumentException("Array.NonEmptyDomain, not valid datatype!");
}
ErrorHandling.CheckDataType<T>(dimension.Type());
}

SequentialPair<T> data;
Expand All @@ -531,15 +527,11 @@ static void GetDomain<T>(Array array, string dimName, uint i, NonEmptyDomain non
/// <exception cref="ArgumentException"><typeparamref name="T"/> is not the dimension's type.</exception>
public (T Start, T End, bool IsEmpty) NonEmptyDomain<T>(string name) where T : struct
{
var datatype = EnumUtil.TypeToDataType(typeof(T));
using (var schema = Schema())
using (var domain = schema.Domain())
using (var dimension = domain.Dimension(name))
{
if (datatype != dimension.Type())
{
throw new ArgumentException("Array.NonEmptyDomain, not valid datatype!");
}
ErrorHandling.CheckDataType<T>(dimension.Type());
}

using var ms_name = new MarshaledString(name);
Expand Down
2 changes: 1 addition & 1 deletion sources/TileDB.CSharp/ArrayMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ IEnumerator IEnumerable.GetEnumerator()

private void put_metadata<T>(string key, T[] value, tiledb_datatype_t tiledb_datatype) where T : struct
{
ErrorHandling.ThrowIfManagedType<T>();
ErrorHandling.CheckDataType<T>((DataType)tiledb_datatype);
if (string.IsNullOrEmpty(key) || value.Length == 0)
{
throw new ArgumentException("ArrayMetadata.put_metadata, null or empty key-value!");
Expand Down
2 changes: 1 addition & 1 deletion sources/TileDB.CSharp/Attribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public ulong CellSize()
/// <param name="data">An array of values that will be used as the fill value.</param>
private void SetFillValue<T>(T[] data) where T : struct
{
ErrorHandling.ThrowIfManagedType<T>();
ErrorHandling.CheckDataType<T>(Type());
if (data.Length == 0)
{
throw new ArgumentException("Attribute.SetFillValue, data is empty!");
Expand Down
4 changes: 2 additions & 2 deletions sources/TileDB.CSharp/Dimension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public DataType Type()
/// <returns></returns>
public (T Start, T End) GetDomain<T>() where T : struct
{
ErrorHandling.ThrowIfManagedType<T>();
ErrorHandling.CheckDataType<T>(Type());
void* value_p;

using var ctxHandle = _ctx.Handle.Acquire();
Expand All @@ -163,7 +163,7 @@ public DataType Type()
/// <returns></returns>
public T TileExtent<T>() where T : struct
{
ErrorHandling.ThrowIfManagedType<T>();
ErrorHandling.CheckDataType<T>(Type());
using var ctxHandle = _ctx.Handle.Acquire();
using var handle = _handle.Acquire();
void* value_p;
Expand Down
Loading

0 comments on commit e645f63

Please sign in to comment.