From 2d4df658e47e276364be3d33c42ebc6856e86f70 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Mon, 13 Dec 2021 09:32:22 +0530 Subject: [PATCH] Treat all NULL values differently in UNIQUE indexes SQL standard can be interpreted differently: either NULL values are all unique (SQLite, PostgreSQL, ... and now Genji) or they are considered equal (SQL Server, ...). --- document/array.go | 1 - document/encoding/encodingtest/testing.go | 2 +- internal/expr/expr_test.go | 21 ------------- internal/sql/scanner/scanner.go | 2 +- internal/stream/index.go | 33 +++++++++++--------- sqltests/INSERT/unique.sql | 28 +++++++++++++++++ sqltests/INSERT/unique_composite.sql | 38 +++++++++++++++++++++++ 7 files changed, 87 insertions(+), 38 deletions(-) create mode 100644 sqltests/INSERT/unique.sql create mode 100644 sqltests/INSERT/unique_composite.sql diff --git a/document/array.go b/document/array.go index 08c3f675c..1b9007e6d 100644 --- a/document/array.go +++ b/document/array.go @@ -55,7 +55,6 @@ func ArrayContains(a types.Array, v types.Value) (bool, error) { // ValueBuffer is an array that holds values in memory. type ValueBuffer struct { Values []types.Value - err error } // NewValueBuffer creates a buffer of values. diff --git a/document/encoding/encodingtest/testing.go b/document/encoding/encodingtest/testing.go index a29d266d1..d4fa6468b 100644 --- a/document/encoding/encodingtest/testing.go +++ b/document/encoding/encodingtest/testing.go @@ -134,7 +134,7 @@ func testDocumentGetByField(t *testing.T, codecBuilder func() encoding.Codec) { assert.NoError(t, err) require.Equal(t, types.NewTextValue("john"), v) - v, err = d.GetByField("d") + _, err = d.GetByField("d") assert.ErrorIs(t, err, document.ErrFieldNotFound) } diff --git a/internal/expr/expr_test.go b/internal/expr/expr_test.go index 02adff750..691a95085 100644 --- a/internal/expr/expr_test.go +++ b/internal/expr/expr_test.go @@ -23,29 +23,8 @@ var doc types.Document = func() types.Document { var envWithDoc = environment.New(doc) -var envWithDocAndKey *environment.Environment = func() *environment.Environment { - env := environment.New(doc) - env.Set(environment.TableKey, types.NewTextValue("string")) - env.Set(environment.DocPKKey, types.NewBlobValue([]byte("foo"))) - return env -}() - var nullLiteral = types.NewNullValue() -func testExpr(t testing.TB, exprStr string, env *environment.Environment, want types.Value, fails bool) { - t.Helper() - - e, err := parser.NewParser(strings.NewReader(exprStr)).ParseExpr() - assert.NoError(t, err) - res, err := e.Eval(env) - if fails { - assert.Error(t, err) - } else { - assert.NoError(t, err) - require.Equal(t, want, res) - } -} - func TestString(t *testing.T) { var operands = []string{ `10.4`, diff --git a/internal/sql/scanner/scanner.go b/internal/sql/scanner/scanner.go index 881536ca9..52cdc4c52 100644 --- a/internal/sql/scanner/scanner.go +++ b/internal/sql/scanner/scanner.go @@ -636,7 +636,7 @@ func scanBareIdent(r io.RuneScanner) string { if err != nil { break } else if !isIdentChar(ch) { - r.UnreadRune() + _ = r.UnreadRune() break } else { _, _ = buf.WriteRune(ch) diff --git a/internal/stream/index.go b/internal/stream/index.go index 08b27a318..2a2ab210d 100644 --- a/internal/stream/index.go +++ b/internal/stream/index.go @@ -150,11 +150,7 @@ func (op *IndexValidateOperator) Iterate(in *environment.Environment, fn func(ou return err } - var newEnv environment.Environment - return op.Prev.Iterate(in, func(out *environment.Environment) error { - newEnv.SetOuter(out) - doc, ok := out.GetDocument() if !ok { return errors.New("missing document") @@ -162,28 +158,37 @@ func (op *IndexValidateOperator) Iterate(in *environment.Environment, fn func(ou vs := make([]types.Value, 0, len(info.Paths)) + // if the indexes values contain NULL somewhere, + // we don't check for unicity. + // cf: https://sqlite.org/lang_createindex.html#unique_indexes + var hasNull bool for _, path := range info.Paths { v, err := path.GetValueFromDocument(doc) if err != nil { + hasNull = true v = types.NewNullValue() + } else if v.Type() == types.NullValue { + hasNull = true } vs = append(vs, v) } - duplicate, key, err := idx.Exists(vs) - if err != nil { - return err - } - if duplicate { - return &errs.ConstraintViolationError{ - Constraint: "UNIQUE", - Paths: info.Paths, - Key: key, + if !hasNull { + duplicate, key, err := idx.Exists(vs) + if err != nil { + return err + } + if duplicate { + return &errs.ConstraintViolationError{ + Constraint: "UNIQUE", + Paths: info.Paths, + Key: key, + } } } - return fn(&newEnv) + return fn(out) }) } diff --git a/sqltests/INSERT/unique.sql b/sqltests/INSERT/unique.sql new file mode 100644 index 000000000..e96483732 --- /dev/null +++ b/sqltests/INSERT/unique.sql @@ -0,0 +1,28 @@ +-- setup: +CREATE TABLE test (a int unique, b int); + +-- test: same value +INSERT INTO test (a, b) VALUES (1, 1); +INSERT INTO test (a, b) VALUES (1, 1); +-- error: + +-- test: same value, same statement +INSERT INTO test (a, b) VALUES (1, 1), (1, 1); +-- error: + +-- test: different values +INSERT INTO test (a, b) VALUES (1, 1), (2, 2); +/* result: +{a: 1, b: 1} +{a: 2, b: 2} +*/ + +-- test: NULL +INSERT INTO test (b) VALUES (1), (2); +INSERT INTO test (a, b) VALUES (NULL, 3); +SELECT a, b FROM test; +/* result: +{a: NULL, b: 1} +{a: NULL, b: 2} +{a: NULL, b: 3} +*/ diff --git a/sqltests/INSERT/unique_composite.sql b/sqltests/INSERT/unique_composite.sql new file mode 100644 index 000000000..c19e3a1d5 --- /dev/null +++ b/sqltests/INSERT/unique_composite.sql @@ -0,0 +1,38 @@ +-- setup: +CREATE TABLE test (a int, b int, c int, d int, UNIQUE (a, b, c)); + +-- test: same value +INSERT INTO test (a, b, c, d) VALUES (1, 1, 1, 1); +INSERT INTO test (a, b, c, d) VALUES (1, 1, 1, 1); +-- error: + +-- test: same value, same statement +INSERT INTO test (a, b, c, d) VALUES (1, 1, 1, 1), (1, 1, 1, 1); +-- error: + +-- test: different values +INSERT INTO test (a, b, c, d) VALUES (1, 1, 1, 1), (1, 2, 1, 1); +/* result: +{a: 1, b: 1, c: 1, d: 1} +{a: 1, b: 2, c: 1, d: 1} +*/ + +-- test: NULL +INSERT INTO test (d) VALUES (1), (2); +INSERT INTO test (c, d) VALUES (3, 3); +INSERT INTO test (c, d) VALUES (3, 3); +INSERT INTO test (b, c, d) VALUES (4, 4, 4); +INSERT INTO test (b, c, d) VALUES (4, 4, 4); +INSERT INTO test (a, b, c, d) VALUES (5, null, 5, 5); +INSERT INTO test (a, c, d) VALUES (5, 5, 5); +SELECT a, b, c, d FROM test; +/* result: +{a: NULL, b: NULL, c: NULL, d: 1} +{a: NULL, b: NULL, c: NULL, d: 2} +{a: NULL, b: NULL, c: 3, d: 3} +{a: NULL, b: NULL, c: 3, d: 3} +{a: NULL, b: 4, c: 4, d: 4} +{a: NULL, b: 4, c: 4, d: 4} +{a: 5, b: NULL, c: 5, d: 5} +{a: 5, b: NULL, c: 5, d: 5} +*/