fix: improve OrderBy consistency and add XML documentation

Fix scaffolding round-trip mismatch by omitting default NULLS clauses to NULLS FIRST). Change OrderByExtensions default from bool to bool? with null default to prevent phantom migrations between API styles. Replace var with explicit types and triple-slash inline comments with double-slash per coding standards.

Author: sebastian-ederer

src/Eftdb.Design/Scaffolding/HypertableScaffoldingExtractor.cs

@@ -29,6 +29,7 @@ List<Dimension> AdditionalDimensions
 
             try
             {
+                // TODO: Evtl. CompessionSettings und CompressionConfiguration combinen?
                 Dictionary<(string, string), HypertableInfo> hypertables = [];
                 Dictionary<(string, string), bool> compressionSettings = GetCompressionSettings(connection);
 
@@ -210,10 +211,10 @@ FROM timescaledb_information.compression_settings
                     bool isNullsFirst = reader.GetBoolean(6);
 
                     string direction = isAscending ? "ASC" : "DESC";
-                    string nulls = isNullsFirst ? "NULLS FIRST" : "NULLS LAST";
+                    bool isDefaultNulls = (isAscending && !isNullsFirst) || (!isAscending && isNullsFirst);
+                    string nulls = isDefaultNulls ? "" : (isNullsFirst ? " NULLS FIRST" : " NULLS LAST");
 
-                    // Reconstruct the full string format: "colName DESC NULLS LAST"
-                    info.CompressionOrderBy.Add($"{columnName} {direction} {nulls}");
+                    info.CompressionOrderBy.Add($"{columnName} {direction}{nulls}");
                 }
             }
         }

src/Eftdb/Abstractions/OrderBy.cs

@@ -1,5 +1,7 @@
 using System.Linq.Expressions;
+using System.Text;
 
+// TODO: Evtl. in .Configuration.Hypertable statt .Abstractions verschieben?
 namespace CmdScale.EntityFrameworkCore.TimescaleDB.Abstractions
 {
     /// <summary>
@@ -18,13 +20,21 @@ namespace CmdScale.EntityFrameworkCore.TimescaleDB.Abstractions
     /// </param>
     public class OrderBy(string columnName, bool? isAscending = null, bool? nullsFirst = null)
     {
+        /// <summary>The name of the column to order by.</summary>
         public string ColumnName { get; } = columnName;
+
+        /// <summary>Ordering direction. True for ASC, false for DESC, null for database default.</summary>
         public bool? IsAscending { get; } = isAscending;
+
+        /// <summary>Null sorting behavior. True for NULLS FIRST, false for NULLS LAST, null for database default.</summary>
         public bool? NullsFirst { get; } = nullsFirst;
 
+        /// <summary>
+        /// Converts this ordering specification to a SQL clause fragment.
+        /// </summary>
         public string ToSql()
         {
-            var sb = new System.Text.StringBuilder(ColumnName);
+            StringBuilder sb = new(ColumnName);
 
             // Only append direction if explicitly set
             if (IsAscending.HasValue)
@@ -47,6 +57,11 @@ public string ToSql()
     /// </summary>
     public static class OrderByBuilder
     {
+        /// <summary>
+        /// Starts building an OrderBy specification for the specified property.
+        /// </summary>
+        /// <typeparam name="TEntity">The entity type containing the property.</typeparam>
+        /// <param name="expression">A lambda expression selecting the property to order by.</param>
         public static OrderByConfiguration<TEntity> For<TEntity>(Expression<Func<TEntity, object>> expression) => new(expression);
     }
 
@@ -57,8 +72,16 @@ public class OrderByConfiguration<TEntity>(Expression<Func<TEntity, object>> exp
     {
         private readonly string _propertyName = GetPropertyName(expression);
 
+        /// <summary>Creates an OrderBy using the database default direction.</summary>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy Default(bool? nullsFirst = null) => new(_propertyName, null, nullsFirst);
+
+        /// <summary>Creates an ascending OrderBy specification.</summary>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy Ascending(bool? nullsFirst = null) => new(_propertyName, true, nullsFirst);
+
+        /// <summary>Creates a descending OrderBy specification.</summary>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy Descending(bool? nullsFirst = null) => new(_propertyName, false, nullsFirst);
 
         // Helper to extract the string name from the expression
@@ -76,16 +99,24 @@ private static string GetPropertyName(Expression<Func<TEntity, object>> expressi
     /// <typeparam name="TEntity"></typeparam>
     public class OrderBySelector<TEntity>
     {
+        /// <summary>Creates an OrderBy using the database default direction for the selected property.</summary>
+        /// <param name="expression">A lambda expression selecting the property to order by.</param>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy By(Expression<Func<TEntity, object>> expression, bool? nullsFirst = null)
             => new(GetPropertyName(expression), null, nullsFirst);
 
+        /// <summary>Creates an ascending OrderBy specification for the selected property.</summary>
+        /// <param name="expression">A lambda expression selecting the property to order by.</param>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy ByAscending(Expression<Func<TEntity, object>> expression, bool? nullsFirst = null)
             => new(GetPropertyName(expression), true, nullsFirst);
 
+        /// <summary>Creates a descending OrderBy specification for the selected property.</summary>
+        /// <param name="expression">A lambda expression selecting the property to order by.</param>
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
         public OrderBy ByDescending(Expression<Func<TEntity, object>> expression, bool? nullsFirst = null)
             => new(GetPropertyName(expression), false, nullsFirst);
 
-        // Internal helper to get property names from expressions
         private static string GetPropertyName(Expression<Func<TEntity, object>> expression)
         {
             if (expression.Body is MemberExpression m) return m.Member.Name;
@@ -103,8 +134,8 @@ public static class OrderByExtensions
         /// Creates an ascending OrderBy instance.
         /// </summary>
         /// <param name="columnName">The name of the column to order by.</param>
-        /// <param name="nullsFirst">Whether nulls should appear first.</param>
-        public static OrderBy Ascending(this string columnName, bool nullsFirst = false)
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
+        public static OrderBy Ascending(this string columnName, bool? nullsFirst = null)
         {
             return new OrderBy(columnName, true, nullsFirst);
         }
@@ -113,8 +144,8 @@ public static OrderBy Ascending(this string columnName, bool nullsFirst = false)
         /// Creates a descending OrderBy instance.
         /// </summary>
         /// <param name="columnName">The name of the column to order by.</param>
-        /// <param name="nullsFirst">Whether nulls should appear first.</param>
-        public static OrderBy Descending(this string columnName, bool nullsFirst = false)
+        /// <param name="nullsFirst">Optional null sorting behavior. Null uses database default.</param>
+        public static OrderBy Descending(this string columnName, bool? nullsFirst = null)
         {
             return new OrderBy(columnName, false, nullsFirst);
         }

src/Eftdb/Configuration/Hypertable/HypertableConvention.cs

@@ -44,21 +44,21 @@ public void ProcessEntityTypeAdded(IConventionEntityTypeBuilder entityTypeBuilde
 
                 if (attribute.ChunkSkipColumns != null && attribute.ChunkSkipColumns.Length > 0)
                 {
-                    /// Chunk skipping requires compression to be enabled
+                    // Chunk skipping requires compression to be enabled
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.EnableCompression, true);
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.ChunkSkipColumns, string.Join(",", attribute.ChunkSkipColumns));
                 }
 
                 if (attribute.CompressionSegmentBy != null && attribute.CompressionSegmentBy.Length > 0)
                 {
-                    /// SegmentBy requires compression to be enabled
+                    // SegmentBy requires compression to be enabled
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.EnableCompression, true);
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.CompressionSegmentBy, string.Join(", ", attribute.CompressionSegmentBy));
                 }
 
                 if (attribute.CompressionOrderBy != null && attribute.CompressionOrderBy.Length > 0)
                 {
-                    /// OrderBy requires compression to be enabled
+                    // OrderBy requires compression to be enabled
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.EnableCompression, true);
                     entityTypeBuilder.HasAnnotation(HypertableAnnotations.CompressionOrderBy, string.Join(", ", attribute.CompressionOrderBy));
                 }

src/Eftdb/Configuration/Hypertable/HypertableTypeBuilder.cs

@@ -178,10 +178,10 @@ public static EntityTypeBuilder<TEntity> WithCompressionOrderBy<TEntity>(
             this EntityTypeBuilder<TEntity> entityTypeBuilder,
             Func<OrderBySelector<TEntity>, IEnumerable<OrderBy>> orderSelector) where TEntity : class
         {
-            var selector = new OrderBySelector<TEntity>();
-            var rules = orderSelector(selector);
+            OrderBySelector<TEntity> selector = new();
+            IEnumerable<OrderBy> rules = orderSelector(selector);
 
-            return entityTypeBuilder.WithCompressionOrderBy(rules.ToArray());
+            return entityTypeBuilder.WithCompressionOrderBy([.. rules]);
         }
 
         /// <summary>

src/Eftdb/Generators/HypertableOperationGenerator.cs

@@ -322,7 +322,7 @@ private static string QuoteOrderByList(IEnumerable<string> orderByClauses)
         {
             return string.Join(", ", orderByClauses.Select(clause =>
             {
-                var parts = clause.Split(' ', 2);
+                string[] parts = clause.Split(' ', 2);
                 string col = parts[0];
                 string suffix = parts.Length > 1 ? " " + parts[1] : "";
 

src/Eftdb/Internals/Features/Hypertables/HypertableModelExtractor.cs

@@ -67,12 +67,12 @@ public static IEnumerable<CreateHypertableOperation> GetHypertables(IRelationalM
                 if (!string.IsNullOrWhiteSpace(orderByString))
                 {
                     compressionOrderBy = [];
-                    var clauses = orderByString.Split(',', StringSplitOptions.TrimEntries);
+                    string[] clauses = orderByString.Split(',', StringSplitOptions.TrimEntries);
 
-                    foreach (var clause in clauses)
+                    foreach (string clause in clauses)
                     {
                         // Split by the first space to separate PropertyName from Directions (ASC/DESC/NULLS)
-                        var parts = clause.Split(' ', 2, StringSplitOptions.RemoveEmptyEntries);
+                        string[] parts = clause.Split(' ', 2, StringSplitOptions.RemoveEmptyEntries);
                         if (parts.Length > 0)
                         {
                             string propName = parts[0];

tests/Eftdb.Tests/Integration/HypertableIntegrationTests.cs

@@ -452,7 +452,7 @@ public async Task Should_Create_Hypertable_With_CompressionSegmentBy()
 
         List<CompressionSettingInfo> settings = await GetCompressionSettingsAsync(context, "segment_by_metrics");
 
-        var tenantSetting = settings.FirstOrDefault(s => s.ColumnName == "TenantId");
+        CompressionSettingInfo? tenantSetting = settings.FirstOrDefault(s => s.ColumnName == "TenantId");
         Assert.NotNull(tenantSetting);
 
         Assert.Equal(1, tenantSetting.SegmentByIndex);
@@ -503,12 +503,12 @@ public async Task Should_Create_Hypertable_With_CompressionOrderBy()
         List<CompressionSettingInfo> settings = await GetCompressionSettingsAsync(context, "order_by_metrics");
 
         // Verify Timestamp (DESC)
-        var tsSetting = settings.First(s => s.ColumnName == "Timestamp");
+        CompressionSettingInfo tsSetting = settings.First(s => s.ColumnName == "Timestamp");
         Assert.NotNull(tsSetting.OrderByIndex); // Should be ordered
         Assert.False(tsSetting.IsAscending);    // DESC
 
         // Verify Value (ASC, NULLS FIRST)
-        var valSetting = settings.First(s => s.ColumnName == "Value");
+        CompressionSettingInfo valSetting = settings.First(s => s.ColumnName == "Value");
         Assert.NotNull(valSetting.OrderByIndex);
         Assert.True(valSetting.IsAscending);    // ASC (Default)
         Assert.True(valSetting.IsNullsFirst);   // NULLS FIRST
@@ -554,11 +554,11 @@ public async Task Should_Create_Hypertable_With_FullCompressionSettings()
         List<CompressionSettingInfo> settings = await GetCompressionSettingsAsync(context, "full_comp_metrics");
 
         // DeviceId should be Segment #1
-        var deviceSetting = settings.First(s => s.ColumnName == "DeviceId");
+        CompressionSettingInfo deviceSetting = settings.First(s => s.ColumnName == "DeviceId");
         Assert.Equal(1, deviceSetting.SegmentByIndex);
 
         // Timestamp should be Order #1 (DESC)
-        var tsSetting = settings.First(s => s.ColumnName == "Timestamp");
+        CompressionSettingInfo tsSetting = settings.First(s => s.ColumnName == "Timestamp");
         Assert.Equal(1, tsSetting.OrderByIndex);
         Assert.False(tsSetting.IsAscending);
     }

tests/Eftdb.Tests/Integration/HypertableScaffoldingExtractorTests.cs

@@ -214,7 +214,7 @@ public async Task Should_Extract_CompressionSegmentBy()
         Dictionary<(string Schema, string TableName), object> result = extractor.Extract(connection);
 
         Assert.Single(result);
-        var info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
+        HypertableScaffoldingExtractor.HypertableInfo info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
 
         // Compression should be enabled
         Assert.True(info.CompressionEnabled);
@@ -268,14 +268,14 @@ public async Task Should_Extract_CompressionOrderBy()
         Dictionary<(string Schema, string TableName), object> result = extractor.Extract(connection);
 
         Assert.Single(result);
-        var info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
+        HypertableScaffoldingExtractor.HypertableInfo info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
 
         Assert.True(info.CompressionEnabled);
 
         Assert.Equal(2, info.CompressionOrderBy.Count);
 
         // Extractor reconstructs the string: "ColumnName [ASC|DESC] [NULLS FIRST|LAST]"
-        Assert.Equal("Timestamp DESC NULLS FIRST", info.CompressionOrderBy[0]);
+        Assert.Equal("Timestamp DESC", info.CompressionOrderBy[0]);
         // Note: Default for ASC is usually NULLS LAST in Postgres, but if we set NULLS FIRST explicitly:
         Assert.Equal("Value ASC NULLS FIRST", info.CompressionOrderBy[1]);
     }
@@ -322,7 +322,7 @@ public async Task Should_Extract_Full_Compression_Configuration()
         Dictionary<(string Schema, string TableName), object> result = extractor.Extract(connection);
 
         Assert.Single(result);
-        var info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
+        HypertableScaffoldingExtractor.HypertableInfo info = (HypertableScaffoldingExtractor.HypertableInfo)result[("public", "Metrics")];
 
         // SegmentBy
         Assert.Single(info.CompressionSegmentBy);