sql: inverted index fixes#45564
Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom Mar 3, 2020
Merged
Conversation
Member
|
This change is |
64bf331 to
35498f0
Compare
rohany
reviewed
Mar 2, 2020
| query T kvtrace | ||
| INSERT INTO d VALUES(0, '{"a": "b"}') | ||
| ---- | ||
| CPut /Table/53/1/0/0 -> /TUPLE/ |
Contributor
There was a problem hiding this comment.
This seems strange -- where b in the value?
Member
Author
There was a problem hiding this comment.
you mean the /TUPLE/ thing? I think we don't have something defined for arrays or json - it's like this with or without my patch.
Member
Author
There was a problem hiding this comment.
We didn't implement PrettyPrintValueEncoded for these types. I'll send a separate PR for that.
rohany
reviewed
Mar 2, 2020
There's no reason to return duplicate keys when generating inverted index keys for JSON. This can happen because JSON arrays can have duplicate values. The KV system will eventually deduplicate these keys too, but in a less efficient way. Release note: None
This commit paves the way to having inverted indexes not return any keys, for datums like NULL and empty arrays. Release note: None
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
35498f0 to
7b09b62
Compare
Contributor
|
LGTM |
Member
Author
|
bors r+ |
Contributor
Build succeeded |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is based on #45563, and split out from #45157.
Closes #45154.
Closes #32468.
Miscellaneous fixes for inverted indexes.