sql: add DisableNotVisibleIndex to ScanFlags #85239

wenyihu6 · 2022-07-28T15:14:09Z

This commit adds a flag to ScanFlags to indicate when the invisible index
feature should be enabled in buildScan. The invisible index feature should be
enabled by default but temporarily disabled during any unique or foreign key
constraint check. More details on how the decision was made and which flag to
pass in docs/RFCS/20220628_invisible_index.md. Note that no logic has been
added to the optimizer yet. This commit just passes the correct flag to
buildScan and shows the effect of the flag in the output of test data.

Assists: #72576

See also: #85354

Release note: none

cockroach-teamcity · 2022-07-28T15:14:19Z

This change is

wenyihu6 · 2022-07-30T01:04:06Z

The first commit comes from #84912.

This commit adds the logic of the invisible index feature to the optimizer. After this commit has been merged, the invisible index feature should be fully functional with `CREATE INDEX` and `CREATE TABLE`. Assists: cockroachdb#72576 See also: cockroachdb#85239 Release note (sql change): creating a not visible index using `CREATE TABLE … (INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported.

wenyihu6 · 2022-08-04T22:33:34Z

pkg/sql/opt/optbuilder/testdata/fk-on-update-cascade line 1575 at r6 (raw file):

Previously, wenyihu6 (Wenyi Hu) wrote…

Yeah. I think its because of how we are setting the flag for ExprFmtFlags. Because ShowAll has 0000 0000, it is actually hiding the flag for ExprFmtAlwaysShowNotVisibleIndexInfo. Note how all the other flags have Hide... and ExprFmtAlwaysShowNotVisibleIndexInfo is the only flag that is showing things. So I would need to change the value for ShowAll and HideAll to make things right. I want to make another PR so that the flags can take into account of flags that are showing things.

Michael and I talked about this for a bit and decided that staying with our existing flags is probably better. Initially, I was thinking about introducing show flags and writing the const ExprFmtShowAll and ExprFmtHideAll separately by doing OR of every show flags and OR of every hide flags. But if we introduce show flags, we might need to change the logic at some places. For example, here, we can no longer just use this map for every flags. We have to introduce a new map for show flags as well.

This commit adds a flag to ScanFlags to indicate when the invisible index feature should be enabled in buildScan. The invisible index feature should be enabled by default but temporarily disabled during any unique or foreign key constraint check. More details on how the decision was made and which flag to pass in `docs/RFCS/20220628_invisible_index.md`. Note that no logic has been added to the optimizer yet. This commit just passes the correct flag to buildScan and shows the effect of the flag in the output of test data. Assists: cockroachdb#72576 See also: cockroachdb#85354 Release note: none

This commit adds the logic of the invisible index feature to the optimizer. After this commit has been merged, the invisible index feature should be fully functional with `CREATE INDEX` and `CREATE TABLE`. Assists: cockroachdb#72576 See also: cockroachdb#85239 Release note (sql change): creating a not visible index using `CREATE TABLE … (INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported.

michae2

Excellent. Good job digging into the exprfmt flags!

Reviewed 17 of 18 files at r11, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @rhu713, and @wenyihu6)

pkg/sql/opt/memo/expr.go line 379 at r11 (raw file):

	// table to generate equivalent memo groups. If DisableNotVisibleIndex is
	// true, optimizer will also generate equivalent memo group using the
	// invisible index. Otherwise, optimizer will ignore the invisible indexes.

This is a great comment! 💯

michae2

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @rhu713, and @wenyihu6)

pkg/sql/opt/memo/expr_format.go line 75 at r11 (raw file):

// a flag that show things, ShowAll = 0000..000 would actually turn this flag
// off. Thus, in order for ExprFmtShowAll and ExprFmtHideAll to be correct, we
// can only add flags to hide things but not flags to show things.

Thank you for taking the time to document this, as well. Really thoughtful.

wenyihu6 · 2022-08-05T22:17:38Z

TFTRs!

bors r+

craig · 2022-08-06T00:27:20Z

Build succeeded:

Bazel Essential CI (Cockroach)

This commit adds the logic of the invisible index feature to the optimizer. After this commit has been merged, the invisible index feature should be fully functional with `CREATE INDEX` and `CREATE TABLE`. Assists: cockroachdb#72576 See also: cockroachdb#85239 Release note (sql change): creating a not visible index using `CREATE TABLE … (INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported.

This commit adds the logic of the invisible index feature to the optimizer. After this commit has been merged, the invisible index feature should be fully functional with `CREATE INDEX` and `CREATE TABLE`. Assists: cockroachdb#72576 See also: cockroachdb#85239 Release note (sql change): creating a not visible index using `CREATE TABLE …(INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported.

77070: sql: add SHOW STATISTICS WITH FORECAST r=michae2 a=michae2 **sql/stats: replace eval.Context with tree.CompareContext** Most uses of eval.Context in the sql/stats package can actually be tree.CompareContext instead, so make the replacement. Release note: None **sql/stats: bump histogram version to 2** In 22.2 as of 963deb8 we support multiple histograms for trigram- indexed strings. Let's bump the histogram version for this change, as we may want to know whether multiple histograms are possible for a given row in system.table_statistics. (I suspect that during upgrades to 22.2 the 22.1 statistics builder will choke on these statistics, so maybe we should also backport a version check to 22.1.) Also update avgRefreshTime to work correctly in multiple-histogram cases. Release note: None **sql/stats: teach histogram.adjustCounts to remove empty buckets** Sometimes when adjusting counts down we end up with empty buckets in the histogram. They don't hurt anything, but they take up some memory (and some brainpower when examining test results). So, teach adjustCounts to remove them. Release note: None **sql/stats: always use non-nil buckets for empty-table histograms** After 82b5926 I've been using the convention that nil histogram buckets = no histogram, and non-nil-zero-length histogram buckets = histogram on empty table. This is mostly useful for testing but is also important for forecasting histograms. Fix a spot that wasn't following this convention. Also, add some empty-table testcases and some other testcases for histogram.adjustCounts. Release note: None **sql/stats: make ordering of SHOW STATISTICS more deterministic** Make two changes to produce more deterministic SHOW STATISTICS output: 1. Sort column IDs when creating statistics. We already use `FastIntSet` in both create_stats.go and statistics_builder.go to ignore ordering when gathering statistics by column set, but the column ordering from CREATE STATISTICS leaks into `system.table_statistics` and can affect SQL on that table, such as SHOW STATISTICS and various internal DELETE statements. 2. Order by column IDs and statistic IDs when reading from `system.table_statistics` in both SHOW STATISTICS and the stats cache. This will ensure SHOW STATISTICS always produces the same output, and shows us rows in the same order as the stats cache sees them (well, reverse order of the stats cache). Release note (sql change): Make SHOW STATISTICS output more deterministic. **sql/stats: forecast table statistics** Add function to forecast table statistics based on observed statistics. These forecasts are based on linear regression models over time. For each set of columns with statistics, we construct a linear regression model over time for each statistic (row count, null count, distinct count, average row size, and histogram). If all models are good fits then we produce a statistics forecast for the set of columns. Assists: #79872 Release note: None **sql: add SHOW STATISTICS WITH FORECAST** Add a new WITH FORECAST option to SHOW STATISTICS which calculates and displays forecasted statistics along with the existing table statistics. Also, forbid injecting forecasted stats. Assists: #79872 Release note (sql change): Add a new WITH FORECAST option to SHOW STATISTICS which calculates and displays forecasted statistics along with the existing table statistics. 85673: storage: Incrementally calculate range key stats in CheckSSTConflicts r=erikgrinaker a=itsbilal This change updates CheckSSTConflicts to incrementally calculate stats in the presence of range keys in the SST being ingested. This avoids expensive stats recomputation after AddSSTable, as previously we were marking stats as estimates if an SST with range keys was added. Fixes #83405. Release note: None. 85794: sql: add not visible index to optimizer r=wenyihu6 a=wenyihu6 This commit adds the logic of the invisible index feature to the optimizer. After this commit has been merged, the invisible index feature should be fully functional with `CREATE INDEX` and `CREATE TABLE`. Assists: #72576 See also: #85239 Release note (sql change): creating a not visible index using `CREATE TABLE …(INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported. 85974: cluster-ui: update active execution and sessions details r=xinhaoz a=xinhaoz Fixes #85968 Closes #85912 Closes #85973 This commit adds new details to the active execution details pages: full scan (both stmt and txn), priority (txn only), and last retry reason (txn only). New information is also added to the sessions table and details pages: transaction count, active duration, recent txn fingerprint ids (cache size comes from a cluster setting). This commit also fixes a bug in the sessions overview UI where the duration for closed sessions was incorrectly calcualted based on the current time instead of the session end time. Release note (ui change): the following fields have been added to the active stmt/txn details pages: - Full Scan: indicates if the execution contains a full scan - Last Retry Reason (txn page only): the last recorded reason the txn was retried - Priority (txn page only): the txn priority The following fields have been added to the sessions table and page: - Transaction count: the number of txns executed by the session - Session active duration: the time a session spent executing txns - Session most recent fingerprint ids ------------------------------------- Retry reason populated example: <img width="855" alt="image" src="https://user-images.githubusercontent.com/20136951/184201435-6d585d9b-13a9-4e87-86dd-718f03f9e92a.png"> https://www.loom.com/share/e396d5aa7dda4d5995227154c6b5076b Co-authored-by: Michael Erickson <[email protected]> Co-authored-by: Bilal Akhtar <[email protected]> Co-authored-by: wenyihu3 <[email protected]> Co-authored-by: Xin Hao Zhang <[email protected]>

wenyihu6 force-pushed the 3-add-flag branch 5 times, most recently from 15c7aba to dd1b5ae Compare July 29, 2022 17:19

wenyihu6 changed the title ~~[WIP] sql: add DisableNotVisibleIndex to ScanFlags~~ sql: add DisableNotVisibleIndex to ScanFlags Jul 29, 2022

wenyihu6 force-pushed the 3-add-flag branch from dd1b5ae to a2bdce1 Compare July 30, 2022 00:16

wenyihu6 mentioned this pull request Jul 30, 2022

sql: notices for NotVisible Indexes #85354

Merged

wenyihu6 force-pushed the 3-add-flag branch from a2bdce1 to 6f541ee Compare July 30, 2022 00:21

wenyihu6 force-pushed the 3-add-flag branch from 6f541ee to 2784fa1 Compare July 30, 2022 01:02

wenyihu6 marked this pull request as ready for review July 30, 2022 01:02

wenyihu6 requested a review from a team as a code owner July 30, 2022 01:02

wenyihu6 requested review from a team and rhu713 July 30, 2022 01:02

wenyihu6 requested review from michae2 and mgartner August 1, 2022 14:06

wenyihu6 force-pushed the 3-add-flag branch 2 times, most recently from 266afe2 to ba68a0b Compare August 2, 2022 13:28

wenyihu6 force-pushed the 3-add-flag branch from ba68a0b to e051880 Compare August 2, 2022 13:34

wenyihu6 force-pushed the 3-add-flag branch from 06bcc12 to 8608101 Compare August 4, 2022 23:20

wenyihu6 force-pushed the 3-add-flag branch from 8608101 to f4965eb Compare August 5, 2022 01:15

wenyihu6 requested a review from michae2 August 5, 2022 15:24

wenyihu6 mentioned this pull request Aug 5, 2022

sql: add ALTER INDEX … NOT VISIBLE to parser #85473

Merged

wenyihu6 requested a review from mgartner August 5, 2022 16:27

michae2 approved these changes Aug 5, 2022

View reviewed changes

michae2 reviewed Aug 5, 2022

View reviewed changes

craig bot merged commit cc7a3e7 into cockroachdb:master Aug 6, 2022

wenyihu6 mentioned this pull request Aug 9, 2022

sql: add not visible index to optimizer #85794

Merged

wenyihu6 deleted the 3-add-flag branch September 1, 2022 20:48

mgartner mentioned this pull request Nov 12, 2024

opt: invisible index improvements #135018

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: add DisableNotVisibleIndex to ScanFlags #85239

sql: add DisableNotVisibleIndex to ScanFlags #85239

wenyihu6 commented Jul 28, 2022 •

edited

Loading

cockroach-teamcity commented Jul 28, 2022

wenyihu6 commented Jul 30, 2022

wenyihu6 commented Aug 4, 2022

michae2 left a comment

michae2 left a comment

wenyihu6 commented Aug 5, 2022

craig bot commented Aug 6, 2022

sql: add DisableNotVisibleIndex to ScanFlags #85239

sql: add DisableNotVisibleIndex to ScanFlags #85239

Conversation

wenyihu6 commented Jul 28, 2022 • edited Loading

cockroach-teamcity commented Jul 28, 2022

wenyihu6 commented Jul 30, 2022

wenyihu6 commented Aug 4, 2022

michae2 left a comment

Choose a reason for hiding this comment

michae2 left a comment

Choose a reason for hiding this comment

wenyihu6 commented Aug 5, 2022

craig bot commented Aug 6, 2022

wenyihu6 commented Jul 28, 2022 •

edited

Loading