sql: builtint for jsonb array to string array conversion #112609
Conversation
|
This change is |
maryliag
force-pushed
the
builtin-convert-array
branch
from
dd31bf8
to
0ebae1e
Compare
There was a problem hiding this comment.
Looks good to me, I only have nits.
(Although I'd probably include this commit as part of #112350 since we'll want to backport both anyways, and we'd see how the new builtin is being utilized.)
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @maryliag)
-- commits line 2 at r1:
nit: "builtint".
-- commits line 10 at r1:
Do you think external users will use it? If not, we could mark the new builtin FunctionProperties.Undocumented = true, and then it won't be included into the docs and we won't have to mention it in the release note. I guess we do have string_to_array, so perhaps adding another documented similar builtin is reasonable. Up to you.
nit: mention the builtin name in the release note itself too.
pkg/sql/logictest/testdata/logic_test/array line 1208 at r1 (raw file):
query T SELECT jsonb_array_to_string_array(j -> 'a') from str_arr order by j ASC
nit: capitalize FROM and ORDER BY here and below.
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Do you think external users will use it? If not, we could mark the new builtin
FunctionProperties.Undocumented = true, and then it won't be included into the docs and we won't have to mention it in the release note. I guess we do havestring_to_array, so perhaps adding another documented similar builtin is reasonable. Up to you.nit: mention the builtin name in the release note itself too.
I had the same struggle, but since I notice on our code we have to do this conversion in several places I thought there is a good chance other users might need it to, so decided to go with the public approach. I will make the updates on release note to add the name
maryliag
force-pushed
the
builtin-convert-array
branch
from
0ebae1e
to
1881024
Compare
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: "builtint".
Fixed
pkg/sql/logictest/testdata/logic_test/array line 1208 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: capitalize
FROMandORDER BYhere and below.
Fixed
maryliag
force-pushed
the
builtin-convert-array
branch
from
1881024
to
775d9cc
Compare
maryliag
force-pushed
the
builtin-convert-array
branch
from
775d9cc
to
452536c
Compare
This commit introduces the function `jsonb_array_to_string_array` that converts a JSONB array to a string array format. Epic: none Release note (sql change): New builtin `jsonb_array_to_string_array` that converts JSONB array to string array.
maryliag
force-pushed
the
builtin-convert-array
branch
from
452536c
to
e252306
Compare
112350: sql: fix sql activity cache for large cardinality r=maryliag a=maryliag Note to reviewers: first commit being reviewed on #112609 --- Recreated from #112071 tldr: fix activity update job failing from conflict, caching to much info when their is high cardinatlity in the stats, and fix cache to store the correct top stats. 1. Fix activity update failure from upserting same row multiple times. This was caused by 2 nodes having the same statement, but one only executed it 2 or 3 time and the other node executed it enough to generate an index recommendation. The different index recommendation caused duplicate rows based on primary key to be inserted. It now uses index recommendation from the merged stats which uses the latest existing recommendation (we don't update if there is no recommendation). 2. Customers can have transactions that have over 1k unique statement fingerprint ids. This makes it unreasonable to cache all of the statement information for all the top transactions. This commit removes the logic to cache all the statement information for the top transactions which could cause the table to grow to over 100k for a single hour. This requires the call to go the statistics table to get all the necessary statement information for the txn details. 3. The activity cache logic was not correctly selecting the top CPU json column. It was using the computed index column name instead of the json field name. The values shown on the UI were correct because the endpoint still aggregated several columns using the aggregated stats rather than the separate column. 4. The activity cache logic was using the execution count instead of the statistics count to select the top queries. In most cases this still generates the correct top queries, and the UI uses the aggregated stats which showed the correct value. Fixes: #111780 Fixes: #111224 Release note (sql change): Fix the SQL activity update job to avoid conflicts on update, reduces the amount of data cached to just what the overview page requires, and fix the correctess of the top queries. Co-authored-by: maryliag <marylia@cockroachlabs.com>
This commit introduces the function
jsonb_array_to_string_arraythat converts a JSONB array to a string array format.Epic: none
Release note (sql change): New builtin that converts JSONB array to string array.