ronantakizawa/codereview-bench huggingface dataset:
Repository: codenotary/immudb
File: embedded/sql/engine.go (Go)

Code before:
			return err
		}

		if tableID != table.id {
			return ErrCorruptedData
		}

		err = e.loadIndexes(table, catalogSnap)
		if err != nil {
			return err
		}

		if table.autoIncrementPK {
			encMaxPK, err := e.loadMaxPK(dataSnap, table)
			if err == store.ErrNoMoreEntries {
				continue
			}
			if err != nil {
				return err
			}

			if len(encMaxPK) != 8 {
				return ErrCorruptedData
			}

			table.maxPK = int64(binary.BigEndian.Uint64(encMaxPK) + math.MaxInt64 + 1)
		}
	}

	return nil
}

func indexKeyFrom(cols []*Column) string {
	var buf bytes.Buffer

	for _, col := range cols {
		buf.WriteString(strconv.FormatUint(uint64(col.id), 16))
	}

	return buf.String()
}

func (e *Engine) loadMaxPK(dataSnap *store.Snapshot, table *Table) ([]byte, error) {
	pkReaderSpec := &store.KeyReaderSpec{
		Prefix:    e.mapKey(PIndexPrefix, EncodeID(table.db.id), EncodeID(table.id), EncodeID(PKIndexID)),
		DescOrder: true,
	}

	pkReader, err := dataSnap.NewKeyReader(pkReaderSpec)
	if err != nil {
		return nil, err

Review comment (suggestion):
I would go for just plain cast between signed and unsigned value here:

```suggestion
			table.maxPK = int64(binary.BigEndian.Uint64(encMaxPK))
```

It would be backwards-compatible and IIRC such casts are well-defined in golang, and that's how it's done in go's stdlib: https://cs.opensource.google/go/go/+/refs/tags/go1.17.1:src/encoding/binary/binary.go;l=329

Code after:
			return err
		}

		if tableID != table.id {
			return ErrCorruptedData
		}

		err = e.loadIndexes(table, catalogSnap)
		if err != nil {
			return err
		}

		if table.autoIncrementPK {
			encMaxPK, err := e.loadMaxPK(dataSnap, table)
			if err == store.ErrNoMoreEntries {
				continue
			}
			if err != nil {
				return err
			}

			if len(encMaxPK) != 8 {
				return ErrCorruptedData
			}

			// map to signed integer space
			encMaxPK[0] ^= 0x80

			table.maxPK = int64(binary.BigEndian.Uint64(encMaxPK))
		}
	}

	return nil
}

func indexKeyFrom(cols []*Column) string {
	var buf bytes.Buffer

	for _, col := range cols {
		buf.WriteString(strconv.FormatUint(uint64(col.id), 16))
	}

	return buf.String()
}

func (e *Engine) loadMaxPK(dataSnap *store.Snapshot, table *Table) ([]byte, error) {
	pkReaderSpec := &store.KeyReaderSpec{
		Prefix:    e.mapKey(PIndexPrefix, EncodeID(table.db.id), EncodeID(table.id), EncodeID(PKIndexID)),
		DescOrder: true,
	}
Repository: codenotary/immudb
File: embedded/sql/engine.go (Go)

Code before:
				return nil, ErrInvalidValue
			}

			if maxLen > 0 && len(strVal) > maxLen {
				return nil, ErrMaxLengthExceeded
			}

			// len(v) + v
			encv := make([]byte, EncLenLen+len(strVal))
			binary.BigEndian.PutUint32(encv[:], uint32(len(strVal)))
			copy(encv[EncLenLen:], []byte(strVal))

			return encv, nil
		}
	case IntegerType:
		{
			intVal, ok := val.(int64)
			if !ok {
				return nil, ErrInvalidValue
			}

			// map to unsigned integer space
			// len(v) + v
			var encv [EncLenLen + 8]byte
			binary.BigEndian.PutUint32(encv[:], uint32(8))
			binary.BigEndian.PutUint64(encv[EncLenLen:], uint64(intVal+math.MaxInt64+1))

			return encv[:], nil
		}
	case BooleanType:
		{
			boolVal, ok := val.(bool)
			if !ok {
				return nil, ErrInvalidValue
			}

			// len(v) + v
			var encv [EncLenLen + 1]byte
			binary.BigEndian.PutUint32(encv[:], uint32(1))
			if boolVal {
				encv[EncLenLen] = 1
			}

			return encv[:], nil
		}
	case BLOBType:
		{
			var blobVal []byte

			if val != nil {
				v, ok := val.([]byte)

Review comment (suggestion):
Same here, mapping from signed to unsigned space can be done by just a simple type conversion, no need to shift values:

```suggestion
			binary.BigEndian.PutUint64(encv[EncLenLen:], uint64(intVal))
```

Code after:
		{
			strVal, ok := val.(string)
			if !ok {
				return nil, ErrInvalidValue
			}

			if maxLen > 0 && len(strVal) > maxLen {
				return nil, ErrMaxLengthExceeded
			}

			// len(v) + v
			encv := make([]byte, EncLenLen+len(strVal))
			binary.BigEndian.PutUint32(encv[:], uint32(len(strVal)))
			copy(encv[EncLenLen:], []byte(strVal))

			return encv, nil
		}
	case IntegerType:
		{
			intVal, ok := val.(int64)
			if !ok {
				return nil, ErrInvalidValue
			}

			// map to unsigned integer space
			// len(v) + v
			var encv [EncLenLen + 8]byte
			binary.BigEndian.PutUint32(encv[:], uint32(8))
			binary.BigEndian.PutUint64(encv[EncLenLen:], uint64(intVal))

			return encv[:], nil
		}
	case BooleanType:
		{
			boolVal, ok := val.(bool)
			if !ok {
				return nil, ErrInvalidValue
			}

			// len(v) + v
			var encv [EncLenLen + 1]byte
			binary.BigEndian.PutUint32(encv[:], uint32(1))
			if boolVal {
				encv[EncLenLen] = 1
			}

			return encv[:], nil
		}
	case BLOBType:
		{
			var blobVal []byte
Repository: codenotary/immudb
File: embedded/sql/engine.go (Go)

Code before:
			if len(strVal) > maxLen {
				return nil, ErrMaxLengthExceeded
			}

			// value + padding + len(value)
			encv := make([]byte, maxLen+EncLenLen)
			copy(encv, []byte(strVal))
			binary.BigEndian.PutUint32(encv[len(encv)-EncLenLen:], uint32(len(strVal)))

			return encv, nil
		}
	case IntegerType:
		{
			if maxLen != 8 {
				return nil, ErrCorruptedData
			}

			intVal, ok := val.(int64)
			if !ok {
				return nil, ErrInvalidValue
			}

			// v
			// map to unsigned integer space
			var encv [8]byte
			binary.BigEndian.PutUint64(encv[:], uint64(intVal+math.MaxInt64+1))

			return encv[:], nil
		}
	case BooleanType:
		{
			if maxLen != 1 {
				return nil, ErrCorruptedData
			}

			boolVal, ok := val.(bool)
			if !ok {
				return nil, ErrInvalidValue
			}

			// v
			var encv [1]byte
			if boolVal {
				encv[0] = 1
			}

			return encv[:], nil
		}
	case BLOBType:
		{
			blobVal, ok := val.([]byte)

Review comment (suggestion):
```suggestion
			binary.BigEndian.PutUint64(encv[:], uint64(intVal))
```

Code after:
				return nil, ErrInvalidValue
			}

			if len(strVal) > maxLen {
				return nil, ErrMaxLengthExceeded
			}

			// value + padding + len(value)
			encv := make([]byte, maxLen+EncLenLen)
			copy(encv, []byte(strVal))
			binary.BigEndian.PutUint32(encv[len(encv)-EncLenLen:], uint32(len(strVal)))

			return encv, nil
		}
	case IntegerType:
		{
			if maxLen != 8 {
				return nil, ErrCorruptedData
			}

			intVal, ok := val.(int64)
			if !ok {
				return nil, ErrInvalidValue
			}

			// v
			// map to unsigned integer space
			var encv [8]byte
			binary.BigEndian.PutUint64(encv[:], uint64(intVal))
			encv[0] ^= 0x80

			return encv[:], nil
		}
	case BooleanType:
		{
			if maxLen != 1 {
				return nil, ErrCorruptedData
			}

			boolVal, ok := val.(bool)
			if !ok {
				return nil, ErrInvalidValue
			}

			// v
			var encv [1]byte
			if boolVal {
				encv[0] = 1
			}

			return encv[:], nil
Repository: codenotary/immudb
File: embedded/sql/engine.go (Go)

Code before:
	}

	vlen := int(binary.BigEndian.Uint32(b[:]))
	voff := EncLenLen

	if vlen < 0 || len(b) < voff+vlen {
		return nil, 0, ErrCorruptedData
	}

	switch colType {
	case VarcharType:
		{
			v := string(b[voff : voff+vlen])
			voff += vlen

			return &Varchar{val: v}, voff, nil
		}
	case IntegerType:
		{
			if vlen != 8 {
				return nil, 0, ErrCorruptedData
			}

			buff := [8]byte{}
			copy(buff[8-vlen:], b[voff:voff+vlen])
			v := binary.BigEndian.Uint64(buff[:])
			voff += vlen

			// map to signed integer space
			v += math.MaxInt64 + 1

			return &Number{val: int64(v)}, voff, nil
		}
	case BooleanType:
		{
			if vlen != 1 {
				return nil, 0, ErrCorruptedData
			}

			v := b[voff] == 1
			voff += 1

			return &Bool{val: v}, voff, nil
		}
	case BLOBType:
		{
			v := b[voff : voff+vlen]
			voff += vlen

			return &Blob{val: v}, voff, nil
		}

Review comment (suggestion):
Since 8 bytes are now required, the extra buffer is no longer needed:

```suggestion
			v := binary.BigEndian.Uint64(b[voff:])
```

Code after:

func DecodeValue(b []byte, colType SQLValueType) (TypedValue, int, error) {
	if len(b) < EncLenLen {
		return nil, 0, ErrCorruptedData
	}

	vlen := int(binary.BigEndian.Uint32(b[:]))
	voff := EncLenLen

	if vlen < 0 || len(b) < voff+vlen {
		return nil, 0, ErrCorruptedData
	}

	switch colType {
	case VarcharType:
		{
			v := string(b[voff : voff+vlen])
			voff += vlen

			return &Varchar{val: v}, voff, nil
		}
	case IntegerType:
		{
			if vlen != 8 {
				return nil, 0, ErrCorruptedData
			}

			v := binary.BigEndian.Uint64(b[voff:])
			voff += vlen

			return &Number{val: int64(v)}, voff, nil
		}
	case BooleanType:
		{
			if vlen != 1 {
				return nil, 0, ErrCorruptedData
			}

			v := b[voff] == 1
			voff += 1

			return &Bool{val: v}, voff, nil
		}
	case BLOBType:
		{
			v := b[voff : voff+vlen]
			voff += vlen

			return &Blob{val: v}, voff, nil
		}
	}
Repository: codenotary/immudb
File: embedded/sql/engine.go (Go)

Code before:

	if vlen < 0 || len(b) < voff+vlen {
		return nil, 0, ErrCorruptedData
	}

	switch colType {
	case VarcharType:
		{
			v := string(b[voff : voff+vlen])
			voff += vlen

			return &Varchar{val: v}, voff, nil
		}
	case IntegerType:
		{
			if vlen != 8 {
				return nil, 0, ErrCorruptedData
			}

			buff := [8]byte{}
			copy(buff[8-vlen:], b[voff:voff+vlen])
			v := binary.BigEndian.Uint64(buff[:])
			voff += vlen

			// map to signed integer space
			v += math.MaxInt64 + 1

			return &Number{val: int64(v)}, voff, nil
		}
	case BooleanType:
		{
			if vlen != 1 {
				return nil, 0, ErrCorruptedData
			}

			v := b[voff] == 1
			voff += 1

			return &Bool{val: v}, voff, nil
		}
	case BLOBType:
		{
			v := b[voff : voff+vlen]
			voff += vlen

			return &Blob{val: v}, voff, nil
		}
	}

	return nil, 0, ErrCorruptedData
}

Review comment (suggestion):
I would just use a simple cast:

```suggestion
			return &Number{val: int64(v)}, voff, nil
```

Code after:
	}

	vlen := int(binary.BigEndian.Uint32(b[:]))
	voff := EncLenLen

	if vlen < 0 || len(b) < voff+vlen {
		return nil, 0, ErrCorruptedData
	}

	switch colType {
	case VarcharType:
		{
			v := string(b[voff : voff+vlen])
			voff += vlen

			return &Varchar{val: v}, voff, nil
		}
	case IntegerType:
		{
			if vlen != 8 {
				return nil, 0, ErrCorruptedData
			}

			v := binary.BigEndian.Uint64(b[voff:])
			voff += vlen

			return &Number{val: int64(v)}, voff, nil
		}
	case BooleanType:
		{
			if vlen != 1 {
				return nil, 0, ErrCorruptedData
			}

			v := b[voff] == 1
			voff += 1

			return &Bool{val: v}, voff, nil
		}
	case BLOBType:
		{
			v := b[voff : voff+vlen]
			voff += vlen

			return &Blob{val: v}, voff, nil
		}
	}

	return nil, 0, ErrCorruptedData
}
Repository: codenotary/immudb
File: embedded/sql/parser_test.go (Go)

Code before:
	"fmt"
	"testing"

	"github.com/stretchr/testify/require"
)

func TestEmptyInput(t *testing.T) {
	_, err := ParseString("")
	require.Error(t, err)
}

func TestCreateDatabaseStmt(t *testing.T) {
	testCases := []struct {
		input          string
		expectedOutput []SQLStmt
		expectedError  error
	}{
		{
			input:          "CREATE DATABASE db1",
			expectedOutput: []SQLStmt{&CreateDatabaseStmt{DB: "db1"}},
			expectedError:  nil,
		},
		{
			input:          "CREATE db1",
			expectedOutput: nil,
			expectedError:  errors.New("syntax error: unexpected IDENTIFIER, expecting INDEX"),
		},
	}

	for i, tc := range testCases {
		res, err := ParseString(tc.input)
		require.Equal(t, tc.expectedError, err, fmt.Sprintf("failed on iteration %d", i))

		if tc.expectedError == nil {
			require.Equal(t, tc.expectedOutput, res, fmt.Sprintf("failed on iteration %d", i))
		}
	}
}

func TestUseDatabaseStmt(t *testing.T) {
	testCases := []struct {
		input          string
		expectedOutput []SQLStmt
		expectedError  error
	}{
		{
			input:          "USE DATABASE db1",
			expectedOutput: []SQLStmt{&UseDatabaseStmt{DB: "db1"}},
			expectedError:  nil,
		},
		{

Review comment (suggestion):
I don't understand why those errors have changed. Technically, after `CREATE` there could still be `DATABASE`, `TABLE` or `[UNIQUE] INDEX`

Code after:
	"fmt"
	"testing"

	"github.com/stretchr/testify/require"
)

func TestEmptyInput(t *testing.T) {
	_, err := ParseString("")
	require.Error(t, err)
}

func TestCreateDatabaseStmt(t *testing.T) {
	testCases := []struct {
		input          string
		expectedOutput []SQLStmt
		expectedError  error
	}{
		{
			input:          "CREATE DATABASE db1",
			expectedOutput: []SQLStmt{&CreateDatabaseStmt{DB: "db1"}},
			expectedError:  nil,
		},
		{
			input:          "CREATE db1",
			expectedOutput: nil,
			expectedError:  errors.New("syntax error: unexpected IDENTIFIER, expecting DATABASE or TABLE or UNIQUE or INDEX"),
		},
	}

	for i, tc := range testCases {
		res, err := ParseString(tc.input)
		require.Equal(t, tc.expectedError, err, fmt.Sprintf("failed on iteration %d", i))

		if tc.expectedError == nil {
			require.Equal(t, tc.expectedOutput, res, fmt.Sprintf("failed on iteration %d", i))
		}
	}
}

func TestUseDatabaseStmt(t *testing.T) {
	testCases := []struct {
		input          string
		expectedOutput []SQLStmt
		expectedError  error
	}{
		{
			input:          "USE DATABASE db1",
			expectedOutput: []SQLStmt{&UseDatabaseStmt{DB: "db1"}},
			expectedError:  nil,
		},
		{
Repository: codenotary/immudb
File: pkg/database/all_ops.go (Go)

Code before:
				if len(x.ZAdd.Set) == 0 || len(x.ZAdd.Key) == 0 {
					return nil, store.ErrIllegalArguments
				}

				if x.ZAdd.AtTx > 0 && !x.ZAdd.BoundRef {
					return nil, store.ErrIllegalArguments
				}

				if req.NoWait && (x.ZAdd.AtTx != 0 || !x.ZAdd.BoundRef) {
					return nil, ErrNoWaitOperationMustBeSelfContained
				}

				_, exists := kmap[sha256.Sum256(x.ZAdd.Key)]

				if req.NoWait && !exists {
					return nil, ErrNoWaitOperationMustBeSelfContained
				}

				if !req.NoWait {
					if !exists || x.ZAdd.AtTx > 0 {
						// check referenced key exists and it's not a reference
						refEntry, err := d.getAt(EncodeKey(x.ZAdd.Key), x.ZAdd.AtTx, 0, index, tx)
						if err != nil {
							return nil, err
						}
						if refEntry.ReferencedBy != nil {
							return nil, ErrReferencedKeyCannotBeAReference
						}
					}
				}

				// zAdd arguments are converted in regular key value items and then atomically inserted
				key := EncodeKey(x.ZAdd.Key)

				if x.ZAdd.BoundRef && x.ZAdd.AtTx == 0 {
					e = EncodeZAdd(x.ZAdd.Set, x.ZAdd.Score, key, txID)
				} else {
					e = EncodeZAdd(x.ZAdd.Set, x.ZAdd.Score, key, x.ZAdd.AtTx)
				}
			}

			entries[i] = e
		}

		return entries, nil
	}

	hdr, err := d.st.CommitWith(callback, !req.NoWait)
	if err != nil {
		return nil, err
	}

Review comment (suggestion):
Same as with the `Op_Ref` - we could add some extra checks here to make recursive and cyclic references much harder to add.

Code after:
				} else {
					e = EncodeReference(x.Ref.Key, nil, x.Ref.ReferencedKey, x.Ref.AtTx)
				}

			case *schema.Op_ZAdd:
				if len(x.ZAdd.Set) == 0 || len(x.ZAdd.Key) == 0 {
					return nil, store.ErrIllegalArguments
				}

				if x.ZAdd.AtTx > 0 && !x.ZAdd.BoundRef {
					return nil, store.ErrIllegalArguments
				}

				if req.NoWait && (x.ZAdd.AtTx != 0 || !x.ZAdd.BoundRef) {
					return nil, fmt.Errorf(
						"%w: can only set references to keys added within same transaction, please use bound references with AtTx set to 0",
						ErrNoWaitOperationMustBeSelfContained)
				}

				_, exists := kmap[sha256.Sum256(x.ZAdd.Key)]

				if req.NoWait && !exists {
					return nil, fmt.Errorf(
						"%w: can not create a reference into a set for a key that was not set in the same transaction",
						ErrNoWaitOperationMustBeSelfContained)
				}

				if !req.NoWait {
					if !exists || x.ZAdd.AtTx > 0 {
						// check referenced key exists and it's not a reference
						refEntry, err := d.getAt(EncodeKey(x.ZAdd.Key), x.ZAdd.AtTx, 0, index, tx)
						if err != nil {
							return nil, err
						}
						if refEntry.ReferencedBy != nil {
							return nil, ErrReferencedKeyCannotBeAReference
						}
					}
				}

				// zAdd arguments are converted in regular key value items and then atomically inserted
				key := EncodeKey(x.ZAdd.Key)

				if x.ZAdd.BoundRef && x.ZAdd.AtTx == 0 {
					e = EncodeZAdd(x.ZAdd.Set, x.ZAdd.Score, key, txID)
				} else {
					e = EncodeZAdd(x.ZAdd.Set, x.ZAdd.Score, key, x.ZAdd.AtTx)
				}
			}

			entries[i] = e
Repository: codenotary/immudb
File: embedded/store/indexer.go (Go)

Code before:

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

	http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package store

import (
	"crypto/sha256"
	"encoding/binary"
	"sync"
	"time"

	"github.com/codenotary/immudb/embedded/tbtree"
	"github.com/codenotary/immudb/embedded/watchers"
	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/promauto"
)

type indexer struct {
	path string

	store *ImmuStore
	tx    *Tx

	index *tbtree.TBtree

	cancellation chan struct{}
	wHub         *watchers.WatchersHub

	state     int
	stateCond *sync.Cond

	closed bool

	compactionMutex sync.Mutex
	mutex           sync.Mutex
}

type runningState = int

const (
	running runningState = iota

Review comment (suggestion):
I think we should avoid adding prometheus as a direct dependency at the embedded store. I think we should think in a different approach to get the required data from the store

Code after:

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

	http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package store

import (
	"crypto/sha256"
	"encoding/binary"
	"path/filepath"
	"sync"
	"time"

	"github.com/codenotary/immudb/embedded/tbtree"
	"github.com/codenotary/immudb/embedded/watchers"
	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/promauto"
)

type indexer struct {
	path string

	store *ImmuStore
	tx    *Tx

	index *tbtree.TBtree

	cancellation chan struct{}
	wHub         *watchers.WatchersHub

	state     int
	stateCond *sync.Cond

	closed bool

	compactionMutex sync.Mutex
	mutex           sync.Mutex

	metricsLastCommittedTrx prometheus.Gauge
	metricsLastIndexedTrx   prometheus.Gauge
}
Repository: codenotary/immudb
File: embedded/store/preconditions.go (Go)

Code before:

type PreconditionKeyNotModifiedAfterTx struct {
	Key  []byte
	TxID uint64
}

func (cs *PreconditionKeyNotModifiedAfterTx) String() string { return "KeyNotModifiedAfterTxID" }

func (cs *PreconditionKeyNotModifiedAfterTx) Validate(st *ImmuStore) error {
	if len(cs.Key) == 0 {
		return ErrInvalidPreconditionNullKey
	}

	if len(cs.Key) > st.maxKeyLen {
		return ErrInvalidPreconditionMaxKeyLenExceeded
	}

	if cs.TxID == 0 {
		return ErrInvalidPreconditionInvalidTxID
	}

	return nil
}

func (cs *PreconditionKeyNotModifiedAfterTx) Check(idx KeyIndex) (bool, error) {
	valRef, err := idx.Get(cs.Key)
	if err != nil && !errors.Is(err, tbtree.ErrKeyNotFound) {
		return false, err
	}

	if err != nil {
		// Key does not exist thus not modified at all
		return true, nil
	}

	return valRef.Tx() <= cs.TxID, nil
}


Review comment (bug):
Deleted entries are considered not modified after given TX - I'm not sure if this is a good approach.

Deletion of an entry could result in valid constraint check that leads to removal of some intermediate changes made to the key.

e.g.:

* Key has value X set at tx 1, two writers read that value
* Writer 1 adds update to the key in tx 2 (using precondition not modified since tx 1)
* Someone deletes the key in tx 3
* Writer 2 updates the key with precondition not modified since tx 1 - this succeeds

Shouldn't writer 2 receive an error that the constraint has failed?

Code after:

type PreconditionKeyNotModifiedAfterTx struct {
	Key  []byte
	TxID uint64
}

func (cs *PreconditionKeyNotModifiedAfterTx) String() string { return "KeyNotModifiedAfterTxID" }

func (cs *PreconditionKeyNotModifiedAfterTx) Validate(st *ImmuStore) error {
	if len(cs.Key) == 0 {
		return ErrInvalidPreconditionNullKey
	}

	if len(cs.Key) > st.maxKeyLen {
		return ErrInvalidPreconditionMaxKeyLenExceeded
	}

	if cs.TxID == 0 {
		return ErrInvalidPreconditionInvalidTxID
	}

	return nil
}

func (cs *PreconditionKeyNotModifiedAfterTx) Check(idx KeyIndex) (bool, error) {
	// get the latest entry (it could be deleted or even expired)
	valRef, err := idx.GetWith(cs.Key)
	if err != nil && errors.Is(err, ErrKeyNotFound) {
		// key does not exist thus not modified at all
		return true, nil
	}
	if err != nil {
		return false, err
	}

	return valRef.Tx() <= cs.TxID, nil
}
Repository: codenotary/immudb
File: embedded/store/immustore_test.go (Go)

Code before:
		err = otx.Set([]byte("key4"), nil, []byte("value4"))
		require.NoError(t, err)

		err = otx.AddPrecondition(&PreconditionKeyNotModifiedAfterTx{Key: []byte("nonExistentKey"), TxID: 1})
		require.NoError(t, err)

		_, err = otx.Commit()
		require.NoError(t, err)
	})

	// insert an expirable entry
	otx, err = immuStore.NewTx()
	require.NoError(t, err)

	md := NewKVMetadata()
	err = md.ExpiresAt(time.Now().Add(1 * time.Second))
	require.NoError(t, err)

	err = otx.Set([]byte("expirableKey"), md, []byte("expirableValue"))
	require.NoError(t, err)

	hdr, err := otx.Commit()
	require.NoError(t, err)

	// wait for entry to be expired
	for {
		time.Sleep(100 * time.Millisecond)

		_, err = immuStore.Get([]byte("expirableKey"))
		if err != nil && errors.Is(err, ErrKeyNotFound) {
			break
		}

		require.NoError(t, err)
	}

	t.Run("must not be modified after constraint should not pass when if expired and expiration was set after specified tx", func(t *testing.T) {
		otx, err = immuStore.NewTx()
		require.NoError(t, err)

		err = otx.Set([]byte("key5"), nil, []byte("value5"))
		require.NoError(t, err)

		err = otx.AddPrecondition(&PreconditionKeyNotModifiedAfterTx{Key: []byte("expirableKey"), TxID: hdr.ID - 1})
		require.NoError(t, err)

		_, err = otx.Commit()
		require.ErrorIs(t, err, ErrPreconditionFailed)
	})

	t.Run("must not exist constraint should pass when if expired", func(t *testing.T) {

Review comment (bug):
```suggestion
	for i:=0;; i++ {
        require.Less(i, 20, "Entry expiration failed")
```

Code after:
		err = otx.Set([]byte("key4"), nil, []byte("value4"))
		require.NoError(t, err)

		err = otx.AddPrecondition(&PreconditionKeyNotModifiedAfterTx{Key: []byte("nonExistentKey"), TxID: 1})
		require.NoError(t, err)

		_, err = otx.Commit()
		require.NoError(t, err)
	})

	// insert an expirable entry
	otx, err = immuStore.NewTx()
	require.NoError(t, err)

	md := NewKVMetadata()
	err = md.ExpiresAt(time.Now().Add(1 * time.Second))
	require.NoError(t, err)

	err = otx.Set([]byte("expirableKey"), md, []byte("expirableValue"))
	require.NoError(t, err)

	hdr, err := otx.Commit()
	require.NoError(t, err)

	// wait for entry to be expired
	for i := 0; ; i++ {
		require.Less(t, i, 20, "entry expiration failed")

		time.Sleep(100 * time.Millisecond)

		_, err = immuStore.Get([]byte("expirableKey"))
		if err != nil && errors.Is(err, ErrKeyNotFound) {
			break
		}

		require.NoError(t, err)
	}

	t.Run("must not be modified after constraint should not pass when if expired and expiration was set after specified tx", func(t *testing.T) {
		otx, err = immuStore.NewTx()
		require.NoError(t, err)

		err = otx.Set([]byte("key5"), nil, []byte("value5"))
		require.NoError(t, err)

		err = otx.AddPrecondition(&PreconditionKeyNotModifiedAfterTx{Key: []byte("expirableKey"), TxID: hdr.ID - 1})
		require.NoError(t, err)

		_, err = otx.Commit()
		require.ErrorIs(t, err, ErrPreconditionFailed)
	})