Support tinyint and time columns in synchronization - #20
Merged
Merged
Conversation
A table holding a SQL Server tinyint or time column could not be synchronized at all: SyncItemValue.DetectTypeOfObject threw NotSupportedException as soon as a provider read one, because System.Byte and System.TimeSpan have no SyncItemValueType. Rather than add enum members - which an older peer would reject on both the SyncProviderHttpClient and the SyncAgentController side - normalize the value to a type that already has one, so the wire format is unchanged: - byte, sbyte and ushort become Int32, uint becomes Int64; - TimeSpan becomes an invariant "c" formatted string, the same representation Microsoft.Data.Sqlite and EF Core use to store a TimeSpan (ticks must never be used: Microsoft.Data.Sqlite reads an integer into a TimeSpan as days); - char becomes a single character string, which the SQLite provider could already produce but nothing could carry. Apply side: - SqlServer/SqlServerCT ConvertToSqlType narrows to byte for SqlDbType.TinyInt (also covering the boolean a MySql TINYINT(1) yields) and parses a string back into a TimeSpan for SqlDbType.Time; - PostgreSQL and MySql convert a string back into a TimeSpan for their time and interval columns; - SqliteSyncProvider.GetValueFromRecord reads a nullable property through its underlying type and reads a TimeSpan explicitly, so a byte?/TimeSpan?/DateTime? column no longer falls through to GetValue() and is sent with the same SyncItemValueType the server sends down for the very same column.
- SqlServer and SqlServerCT: reject a time outside 00:00:00 to 23:59:59.9999999 with a clear error, instead of an opaque SqlClient failure or silent truncation. - PostgreSQL: read time and interval columns as TimeSpan, since Npgsql defaults time to TimeOnly, which cannot be synchronized. timetz and intervals carrying months remain unsupported and are documented as such. - SQLite: when a typed getter cannot parse a legacy value in a nullable column, fall back to the raw stored value and log a warning. - Tests: SQL Server, SqlServerCT, MySQL and PostgreSQL round trips against SQLite (direct, HTTP JSON, HTTP binary); old-client payloads accepted by every server provider; time range edges and rounding; MySQL tinyint flavours; SQLite nullable and legacy reads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Synchronizing a SQL Server
tinyintortimecolumn fails on the server withNotSupportedException: Type of value ('System.Byte') is not supported for synchronization(and the same for
System.TimeSpan). MySQLTIMEand PostgreSQLtime/intervalcolumns hit the same limitation.What changes
Nothing on the wire: no new
SyncItemValueTypemembers, so peers on older versions keep working.SyncItemValue):byte,sbyteandushorttravel asInt32,uintasInt64,charas a one-characterString, andTimeSpanas an invariant"c"formattedString, which is also how Microsoft.Data.Sqlite and EF Core store aTimeSpan.TimeSpanfortimeand narrows tobytefortinyint. A time outside 00:00:00 to 23:59:59.9999999 is rejected with a clear message instead of an opaque SqlClient error or silent truncation.timeandintervalcolumns are read asTimeSpan(Npgsql defaultstimetoTimeOnly, which cannot be synchronized) and applied back from the string form.timetzand intervals carrying months remain unsupported.TimeSpanforTIMEcolumns; everyTINYINTflavour maps to a carryable type.byte?orTimeSpan?uploads the same shape the server sends down. If a legacy stored value cannot be parsed, the reader falls back to the raw value and logs a warning.Tests
New
ColumnTypeMappingTests(30 tests):timecolumn, MySQL tinyint flavours, PostgreSQL unsupported types, SQLite nullable and legacy reads, and a guard thatSyncItemValueTypegains no members.Full suite run locally against SQL Server 2022, MySQL 8.4 and PostgreSQL 16 (Docker): all 155 existing tests still pass, and all 30 column-mapping tests pass.