Skip to content

Commit

Permalink
sql: replicating JSON empty array ordering found in Postgres
Browse files Browse the repository at this point in the history
Currently, cockroachdb#97928 and cockroachdb#99275 are responsible for laying out
a lexicographical ordering for JSON columns to be forward
indexable in nature. This ordering is based on the rules
posted by Postgres and is defined here: cockroachdb#99849

However, Postgres currently sorts the empty JSON array
before any other JSON values. An issue has been opened:
https://www.postgresql.org/message-id/17873-826fdc8bbcace4f1%40postgresql.org

Thus, this PR intends on replicating this behaviour until
the issue has been identified and resolved by Postgres.

Epic: CRDB-24501

Release note (sql change): This PR shall now place the empty JSON array
to have the lowest (highest) precedence when the JSON column
is being sorted in ascending (descending) order.
  • Loading branch information
Shivs11 committed Apr 6, 2023
1 parent dbdf486 commit d4a398a
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 24 deletions.
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/json
Original file line number Diff line number Diff line change
Expand Up @@ -1382,12 +1382,12 @@ INSERT INTO t VALUES
query T
SELECT x FROM t ORDER BY x;
----
[]
null
"crdb"
1
false
true
[]
[null]
[1]
[{"a": "b"}]
Expand Down Expand Up @@ -1430,12 +1430,12 @@ SELECT t1.x FROM t1 INNER LOOKUP JOIN t2 ON t1.x = t2.x
query T
SELECT x FROM t GROUP BY x ORDER BY x;
----
[]
null
"crdb"
1
false
true
[]
[null]
[1]
[{"a": "b"}]
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/rowenc/keyside/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func decodeJSONKey(buf []byte, dir encoding.Direction) (json.JSON, []byte, error
}
buf = buf[1:] // removing the terminator
jsonVal = json.FromDecimal(dec)
case encoding.JSONArray, encoding.JSONArrayDesc:
case encoding.JSONArray, encoding.JSONArrayDesc, encoding.JsonEmptyArray, encoding.JsonEmptyArrayDesc:
jsonVal, buf, err = decodeJSONArray(buf, dir)
if err != nil {
return nil, nil, errors.NewAssertionErrorWithWrappedErrf(err, "could not decode JSON Array")
Expand Down
61 changes: 41 additions & 20 deletions pkg/util/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,17 @@ const (

// Defining different key markers, for the ascending designation,
// for handling different JSON values.
jsonNullKeyMarker = voidMarker + 1
jsonStringKeyMarker = jsonNullKeyMarker + 1
jsonNumberKeyMarker = jsonStringKeyMarker + 1
jsonFalseKeyMarker = jsonNumberKeyMarker + 1
jsonTrueKeyMarker = jsonFalseKeyMarker + 1
jsonArrayKeyMarker = jsonTrueKeyMarker + 1
jsonObjectKeyMarker = jsonArrayKeyMarker + 1

// Postgres currently has a special case (maybe a bug) where
// the empty JSON Array sorts before all other JSON values.
jsonEmptyArrayKeyMarker = voidMarker + 1
jsonNullKeyMarker = jsonEmptyArrayKeyMarker + 1
jsonStringKeyMarker = jsonNullKeyMarker + 1
jsonNumberKeyMarker = jsonStringKeyMarker + 1
jsonFalseKeyMarker = jsonNumberKeyMarker + 1
jsonTrueKeyMarker = jsonFalseKeyMarker + 1
jsonArrayKeyMarker = jsonTrueKeyMarker + 1
jsonObjectKeyMarker = jsonArrayKeyMarker + 1

arrayKeyTerminator byte = 0x00
arrayKeyDescendingTerminator byte = 0xFF
Expand All @@ -126,13 +130,14 @@ const (

// Defining different key markers, for the descending designation,
// for handling different JSON values.
jsonNullKeyDescendingMarker = jsonObjectKeyMarker + 7
jsonStringKeyDescendingMarker = jsonNullKeyDescendingMarker - 1
jsonNumberKeyDescendingMarker = jsonStringKeyDescendingMarker - 1
jsonFalseKeyDescendingMarker = jsonNumberKeyDescendingMarker - 1
jsonTrueKeyDescendingMarker = jsonFalseKeyDescendingMarker - 1
jsonArrayKeyDescendingMarker = jsonTrueKeyDescendingMarker - 1
jsonObjectKeyDescendingMarker = jsonArrayKeyDescendingMarker - 1
jsonEmptyArrayKeyDescendingMarker = jsonObjectKeyMarker + 8
jsonNullKeyDescendingMarker = jsonEmptyArrayKeyDescendingMarker - 1
jsonStringKeyDescendingMarker = jsonNullKeyDescendingMarker - 1
jsonNumberKeyDescendingMarker = jsonStringKeyDescendingMarker - 1
jsonFalseKeyDescendingMarker = jsonNumberKeyDescendingMarker - 1
jsonTrueKeyDescendingMarker = jsonFalseKeyDescendingMarker - 1
jsonArrayKeyDescendingMarker = jsonTrueKeyDescendingMarker - 1
jsonObjectKeyDescendingMarker = jsonArrayKeyDescendingMarker - 1

// Terminators for JSON Key encoding.
jsonKeyTerminator byte = 0x00
Expand Down Expand Up @@ -1750,6 +1755,9 @@ const (
JSONArrayDesc Type = 39
JSONObject Type = 40
JSONObjectDesc Type = 41
// Special case
JsonEmptyArray Type = 42
JsonEmptyArrayDesc Type = 43
)

// typMap maps an encoded type byte to a decoded Type. It's got 256 slots, one
Expand Down Expand Up @@ -1810,6 +1818,10 @@ func slowPeekType(b []byte) Type {
return JSONArray
case m == jsonArrayKeyDescendingMarker:
return JSONArrayDesc
case m == jsonEmptyArrayKeyMarker:
return JsonEmptyArray
case m == jsonEmptyArrayKeyDescendingMarker:
return JsonEmptyArrayDesc
case m == jsonObjectKeyMarker:
return JSONObject
case m == jsonObjectKeyDescendingMarker:
Expand Down Expand Up @@ -1970,7 +1982,8 @@ func PeekLength(b []byte) (int, error) {
length, err := getArrayOrJSONLength(b[1:], dir, IsJSONKeyDone)
return 1 + length, err
case jsonArrayKeyMarker, jsonArrayKeyDescendingMarker,
jsonObjectKeyMarker, jsonObjectKeyDescendingMarker:
jsonObjectKeyMarker, jsonObjectKeyDescendingMarker,
jsonEmptyArrayKeyMarker, jsonEmptyArrayKeyDescendingMarker:
dir := Ascending
if (m == jsonArrayKeyDescendingMarker) ||
(m == jsonObjectKeyDescendingMarker) {
Expand Down Expand Up @@ -3461,12 +3474,20 @@ func EncodeJSONTrueKeyMarker(buf []byte, dir Direction) []byte {

// EncodeJSONArrayKeyMarker adds a JSON Array key encoding marker
// to buf and returns the new buffer.
func EncodeJSONArrayKeyMarker(buf []byte, dir Direction) []byte {
func EncodeJSONArrayKeyMarker(buf []byte, dir Direction, length int64) []byte {
switch dir {
case Ascending:
return append(buf, jsonArrayKeyMarker)
if length == 0 {
return append(buf, jsonEmptyArrayKeyMarker)
} else {
return append(buf, jsonArrayKeyMarker)
}
case Descending:
return append(buf, jsonArrayKeyDescendingMarker)
if length == 0 {
return append(buf, jsonEmptyArrayKeyDescendingMarker)
} else {
return append(buf, jsonArrayKeyDescendingMarker)
}
default:
panic("invalid direction")
}
Expand Down Expand Up @@ -3582,15 +3603,15 @@ func ValidateAndConsumeJSONKeyMarker(buf []byte, dir Direction) ([]byte, Type, e
case Descending:
switch typ {
case JSONNullDesc, JSONNumberDesc, JSONStringDesc, JSONFalseDesc,
JSONTrueDesc, JSONArrayDesc, JSONObjectDesc:
JSONTrueDesc, JSONArrayDesc, JSONObjectDesc, JsonEmptyArrayDesc:
return buf[1:], typ, nil
default:
return nil, Unknown, errors.Newf("invalid type found %s", typ)
}
case Ascending:
switch typ {
case JSONNull, JSONNumber, JSONString, JSONFalse, JSONTrue, JSONArray,
JSONObject:
JSONObject, JsonEmptyArray:
return buf[1:], typ, nil
default:
return nil, Unknown, errors.Newf("invalid type found %s", typ)
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -1978,7 +1978,7 @@ func (j jsonTrue) EncodeForwardIndex(buf []byte, dir encoding.Direction) ([]byte
}

func (j jsonArray) EncodeForwardIndex(buf []byte, dir encoding.Direction) ([]byte, error) {
buf = encoding.EncodeJSONArrayKeyMarker(buf, dir)
buf = encoding.EncodeJSONArrayKeyMarker(buf, dir, int64(len(j)))
buf = encoding.EncodeJSONValueLength(buf, dir, int64(len(j)))

var err error
Expand Down

0 comments on commit d4a398a

Please sign in to comment.