From fb81e7a36062330a7d7b3e582cab0a1acd2f262d Mon Sep 17 00:00:00 2001 From: Samir Patel <48686912+54mir@users.noreply.github.com> Date: Fri, 27 May 2022 20:19:06 -0500 Subject: [PATCH] [FB-1462 FB-1393] Timestamp fix (#2082) * serialize base and epoch into req * fix and validate timestamp import * refactor overflow check and test --- client/orm.go | 4 ++++ field.go | 38 ++++++++++++++++++++++++++++++++++++-- field_test.go | 37 +++++++++++++++++++++++++++++++++++++ http_handler.go | 1 + 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/client/orm.go b/client/orm.go index c02573ff5..80f262600 100644 --- a/client/orm.go +++ b/client/orm.go @@ -739,6 +739,7 @@ type FieldOptions struct { foreignIndex string timeUnit string base int64 + epoch time.Time } // Type returns the type of the field. Currently "set", "int", or "time". @@ -837,6 +838,8 @@ func (fo FieldOptions) String() string { mopt["min"] = fo.min mopt["max"] = fo.max mopt["timeUnit"] = fo.timeUnit + mopt["base"] = fo.base + mopt["epoch"] = fo.epoch } if fo.fieldType != FieldTypeDefault { @@ -956,6 +959,7 @@ func OptFieldTypeTimestamp(epoch time.Time, timeUnit string) FieldOption { fo.min = pql.NewDecimal(MinTimestamp.UnixNano()/TimeUnitNanos(timeUnit), 0) fo.max = pql.NewDecimal(MaxTimestamp.UnixNano()/TimeUnitNanos(timeUnit), 0) fo.base = epochValue + fo.epoch = epoch } } diff --git a/field.go b/field.go index 34299400e..a14ad697c 100644 --- a/field.go +++ b/field.go @@ -195,6 +195,10 @@ func OptFieldTypeInt(min, max int64) FieldOption { // provide any respective configuration values. func OptFieldTypeTimestamp(epoch time.Time, timeUnit string) FieldOption { return func(fo *FieldOptions) error { + // Check if the epoch will overflow when converted to nano. + if err := CheckUnixNanoOverflow(epoch); err != nil { + return err + } epochValue := epoch.UnixNano() / TimeUnitNanos(timeUnit) if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) @@ -1406,6 +1410,7 @@ func (f *Field) SetValue(tx Tx, columnID uint64, value int64) (changed bool, err // Determine base value to store. baseValue := int64(value - bsig.Base) + requiredBitDepth := bitDepthInt64(baseValue) // Increase bit depth value if the unsigned value is greater. @@ -1723,7 +1728,7 @@ func (f *Field) importFloatValue(qcx *Qcx, columnIDs []uint64, values []float64, return f.importValue(qcx, columnIDs, ivalues, shard, options) } -// importFloatValue imports timestamp values. In current usage, this +// importTimestampValue imports timestamp values. In current usage, this // should only ever be called with data for a single shard; the API calls // around this are splitting it up per shard. func (f *Field) importTimestampValue(qcx *Qcx, columnIDs []uint64, values []time.Time, shard uint64, options *ImportOptions) error { @@ -1777,6 +1782,18 @@ func (f *Field) importValue(qcx *Qcx, columnIDs []uint64, values []int64, shard if value < min { min = value } + if f.Type() == FieldTypeTimestamp { + scale := (TimeUnitNanos(f.options.TimeUnit)) + offset := f.options.Base * scale + dur := value * scale + if offset > 0 { + if dur > math.MaxInt64-offset { + return errors.Wrap(ErrBSIGroupValueTooHigh, "value + epoch is too far from Unix epoch") + } + } else if dur < math.MinInt64-offset { + return errors.Wrap(ErrBSIGroupValueTooLow, "value + epoch is too far from Unix epoch") + } + } } // Determine the highest bit depth required by the min & max. @@ -1811,7 +1828,13 @@ func (f *Field) importValue(qcx *Qcx, columnIDs []uint64, values []int64, shard if bsig.Base != 0 { for i, v := range values { - values[i] = v - bsig.Base + // for Timestamps, values are already relative to their base (epoch) + // for other types (IntFields), values need to be subtracted from their base (either Min or Max) + if f.Type() == FieldTypeTimestamp { + values[i] = v + } else { + values[i] = v - bsig.Base + } } } @@ -2307,3 +2330,14 @@ func TimeUnitNanos(unit string) int64 { return int64(time.Nanosecond) } } + +func CheckUnixNanoOverflow(epoch time.Time) error { + if time.Unix(0, 0).After(epoch) { + if epoch.UnixNano() > 0 { + return errors.Errorf("custom epoch too far from Unix epoch: %s", epoch) + } + } else if epoch.UnixNano() < 0 { + return errors.Errorf("custom epoch too far from Unix epoch: %s", epoch) + } + return nil +} diff --git a/field_test.go b/field_test.go index e0623f999..8b35d6744 100644 --- a/field_test.go +++ b/field_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "math" "testing" + "time" "github.com/google/go-cmp/cmp" pilosa "github.com/molecula/featurebase/v3" @@ -290,3 +291,39 @@ func TestFieldInfoMarshal(t *testing.T) { t.Fatalf("expected %s, got %s", expected, a) } } + +func TestCheckUnixNanoOverflow(t *testing.T) { + tests := []struct { + name string + epoch time.Time + wantErr bool + }{ + { + name: "too small", + epoch: time.Unix(-1, math.MinInt64), + wantErr: true, + }, + { + name: "just right-1", + epoch: time.Unix(0, math.MinInt64), + wantErr: false, + }, + { + name: "just right-2", + epoch: time.Unix(0, math.MaxInt64), + wantErr: false, + }, + { + name: "too large", + epoch: time.Unix(1, math.MaxInt64), + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := pilosa.CheckUnixNanoOverflow(tt.epoch); (err != nil) != tt.wantErr { + t.Errorf("checkUnixNanoOverflow() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/http_handler.go b/http_handler.go index 7e17cf5a5..228e48d18 100644 --- a/http_handler.go +++ b/http_handler.go @@ -2012,6 +2012,7 @@ type fieldOptions struct { NoStandardView bool `json:"noStandardView,omitempty"` ForeignIndex *string `json:"foreignIndex,omitempty"` TTL *string `json:"ttl,omitempty"` + Base *int64 `json:"base,omitempty"` } func (o *fieldOptions) validate() error {