Skip to content

Commit

Permalink
sql: stop producing inv idx keys for NULL
Browse files Browse the repository at this point in the history
Previously, an inverted index on a JSON column would contain an index
entry for every row that had a SQL NULL value in the JSON column,
despite the the fact that the query planner does not use inverted
indexes to satisfy an IS NULL predicate. This commit changes the
behavior of inverted indexing to cease producing these index keys.

As a sidebar, Postgres also does not include NULL values in its inverted
indexes, nor does Elastic, so I don't think this behavior goes against
user expectation. In addition, the optimizer already did not use the
index for IS NULL predicates, so there will be no behavior changes for
users of inverted indexes.

This change has a side effect of orphaning any entries for NULL datums
that might already exist in an inverted index. These will not be cleaned
up, even if a particular row is updated to not contain NULL, until the
index is dropped or truncated. This problem is rare, and could only
conceivably be problematic for tables with many NULL values in a column
that's indexed with an inverted index, a very unusual case.

Release note: None
  • Loading branch information
jordanlewis committed Mar 1, 2020
1 parent 1d10843 commit 1e71a13
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -3612,7 +3612,7 @@ may increase either contention or retry errors, or both.`,
Fn: func(_ *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
arg := args[0]
if arg == tree.DNull {
return tree.NewDInt(tree.DInt(1)), nil
return tree.DZero, nil
}
n, err := json.NumInvertedIndexEntries(tree.MustBeDJSON(arg).JSON)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/span/span_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ func (s *Builder) encodeConstraintKey(
if err != nil {
return nil, false, err
}
if len(keys) == 0 {
err := errors.AssertionFailedf("trying to use null key in index lookup")
return nil, false, err
}
if len(keys) > 1 {
err := errors.AssertionFailedf("trying to use multiple keys in index lookup")
return nil, false, err
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/sqlbase/index_encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,9 +857,11 @@ func EncodeInvertedIndexKeys(
// concatenates it with `inKey`and returns a list of buffers per
// path. The encoded values is guaranteed to be lexicographically
// sortable, but not guaranteed to be round-trippable during decoding.
// A (SQL) NULL input Datum produces no keys, because inverted indexes
// cannot and do not need to satisfy the predicate col IS NULL.
func EncodeInvertedIndexTableKeys(val tree.Datum, inKey []byte) (key [][]byte, err error) {
if val == tree.DNull {
return [][]byte{encoding.EncodeNullAscending(inKey)}, nil
return nil, nil
}
switch t := tree.UnwrapDatum(nil, val).(type) {
case *tree.DJSON:
Expand Down

0 comments on commit 1e71a13

Please sign in to comment.