zeroshade commented on code in PR #571:
URL: https://github.com/apache/iceberg-go/pull/571#discussion_r2370148702
##########
table/sorting.go:
##########
@@ -125,24 +128,105 @@ const (
)
// A default Sort Order indicating no sort order at all
-var UnsortedSortOrder = SortOrder{OrderID: UnsortedSortOrderID, Fields:
[]SortField{}}
+var UnsortedSortOrder = SortOrder{orderID: UnsortedSortOrderID, fields:
[]SortField{}}
// SortOrder describes how the data is sorted within the table.
//
// Data can be sorted within partitions by columns to gain performance. The
// order of the sort fields within the list defines the order in which the
// sort is applied to the data.
type SortOrder struct {
- OrderID int `json:"order-id"`
- Fields []SortField `json:"fields"`
+ orderID int
+ fields []SortField
+}
+
+func (s SortOrder) OrderID() int {
+ return s.orderID
+}
+
+func (s SortOrder) Fields() iter.Seq[SortField] {
+ return slices.Values(s.fields)
+}
+
+func (s SortOrder) Len() int {
+ return len(s.fields)
+}
+
+func (s SortOrder) MarshalJSON() ([]byte, error) {
+ type Alias struct {
+ OrderID int `json:"order-id"`
+ Fields []SortField `json:"fields"`
+ }
+
+ return json.Marshal(Alias{
+ s.orderID,
+ s.fields,
+ })
+}
+
+func (s *SortOrder) UnmarshalJSON(b []byte) error {
+ type Alias struct {
+ OrderID int `json:"order-id"`
+ Fields []SortField `json:"fields"`
+ }
+ aux := Alias{-1, nil}
+
+ if err := json.Unmarshal(b, &aux); err != nil {
+ return err
+ }
+
+ if len(aux.Fields) == 0 && aux.OrderID == -1 {
+ aux.Fields = []SortField{}
+ aux.OrderID = 0
+ }
+
+ if aux.OrderID == -1 {
+ aux.OrderID = InitialSortOrderID
+ }
+
+ newOrder, err := NewSortOrder(aux.OrderID, aux.Fields)
+ if err != nil {
+ return err
+ }
+
+ *s = newOrder
+
+ return nil
+}
+
+func NewSortOrder(orderID int, fields []SortField) (SortOrder, error) {
Review Comment:
needs a docstring comment
##########
table/metadata_internal_test.go:
##########
@@ -911,6 +904,8 @@ func TestMetadataV2Validation(t *testing.T) {
"schemas": [{"type":"struct","schema-id":0,"fields":[]}],
"partition-specs": [{"spec-id": 0, "fields": []}],
"properties": {},
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id": 0, "fields": []}],
Review Comment:
are these actually required? I thought they were optional and would use the
default?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
