sql, ui: add index used by statement#92351
Conversation
|
This change is |
rytaft
left a comment
There was a problem hiding this comment.
Very nice!
Reviewed 15 of 17 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @maryliag)
pkg/roachpb/app_stats.proto line 121 at r2 (raw file):
// indexes is the list of indexes used by the particular plan when executing the statement. repeated string indexes = 30;
nit: isn't 29 next?
pkg/sql/opt/exec/execbuilder/relational.go line 733 at r2 (raw file):
errors.AssertionFailedf("expected inverted index scan to have an inverted constraint") } b.IndexesUsed = util.CombineUniqueString(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", idx.Table().ID(), idx.ID())})
It's unfortunate that CombineUniqueString reallocates a new slice each time it is called. I think it would be more efficient to store the IndexesUsed in a map[string]struct{}or a map[int]util.FastIntSet while in the execbuilder. Then you can convert to a []string for the instrumentationHelper.
also a nit: idx.Table() -> tab
pkg/sql/opt/exec/execbuilder/relational.go line 1991 at r2 (raw file):
// starts with the PK columns in order (#40749). pri := tab.Index(cat.PrimaryIndex) b.IndexesUsed = util.CombineUniqueString(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", pri.Table().ID(), pri.ID())})
nit: you already have pri.Table() with tab
pkg/sql/opt/exec/execbuilder/relational.go line 2286 at r2 (raw file):
tab := md.Table(join.Table) idx := tab.Index(join.Index) b.IndexesUsed = util.CombineUniqueString(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", idx.Table().ID(), idx.ID())})
nit: idx.Table() -> tab
pkg/sql/opt/exec/execbuilder/relational.go line 2448 at r2 (raw file):
tab := md.Table(join.Table) idx := tab.Index(join.Index) b.IndexesUsed = util.CombineUniqueString(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", idx.Table().ID(), idx.ID())})
nit: idx.Table() -> tab
pkg/sql/opt/exec/execbuilder/relational.go line 2553 at r2 (raw file):
[]string{fmt.Sprintf("%d@%d", leftIndex.Table().ID(), leftIndex.ID())}) b.IndexesUsed = util.CombineUniqueString(b.IndexesUsed, []string{fmt.Sprintf("%d@%d", rightIndex.Table().ID(), rightIndex.ID())})
nit: leftIndex.Table() -> leftTable, rightIndex.Table() -> rightTable
pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding.go line 131 at r2 (raw file):
// "rowsRead", // "nodes", // "indexes,
nit: formatting is a bit messed up
Part Of cockroachdb#82615 This commits collects the list of indexes used by the statement execution and and saves on `system.statement_statistics` and `crdb_internal.statement_statistics` under the statistics columns, inside the statistics key. To get the information ``` SELECT statistics -> 'statistics' ->> 'indexes' from crdb_internal.statement_statistics ``` The value is saved as an array of string with format `tableId@indexId`. Release note (sql change): Add list of indexes used by the query on the statistics column on system.statement_statistics and crdb_internal.statement_statistics tables. The format is `tableID@indexID`.
maryliag
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/roachpb/app_stats.proto line 121 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: isn't 29 next?
There is a PR introducing the 29 here, so I want to avoid the conflict and already jump to 30
pkg/sql/opt/exec/execbuilder/relational.go line 733 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
It's unfortunate that
CombineUniqueStringreallocates a new slice each time it is called. I think it would be more efficient to store theIndexesUsedin amap[string]struct{}or amap[int]util.FastIntSetwhile in theexecbuilder. Then you can convert to a[]stringfor theinstrumentationHelper.also a nit:
idx.Table()->tab
Done for the nit
For the combine unique, I decided to improve the function instead: #92396
Let me know what you think
pkg/sql/opt/exec/execbuilder/relational.go line 1991 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: you already have
pri.Table()withtab
Done
pkg/sql/opt/exec/execbuilder/relational.go line 2286 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit:
idx.Table()->tab
Done
pkg/sql/opt/exec/execbuilder/relational.go line 2448 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit:
idx.Table()->tab
Done
pkg/sql/opt/exec/execbuilder/relational.go line 2553 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit:
leftIndex.Table()->leftTable,rightIndex.Table()->rightTable
Done
pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil/json_encoding.go line 131 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: formatting is a bit messed up
Done
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status:
pkg/sql/opt/exec/execbuilder/relational.go line 733 at r2 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
Done for the nit
For the combine unique, I decided to improve the function instead: #92396
Let me know what you think
Sounds good!
|
TFTR! |
|
Build succeeded: |
Part Of #82615
This commits collects the list of indexes used by the statement execution and and saves on
system.statement_statisticsandcrdb_internal.statement_statisticsunder the statistics columns, inside the statistics key.To get the information
The value is saved as an array of string with format
tableId@indexId.Release note (sql change): Add list of indexes used by the query on the statistics column on system.statement_statistics and crdb_internal.statement_statistics tables. The format is
tableID@indexID.