Skip to content

Commit ae7600a

Browse files
craig[bot]mgartnerangles-n-daemonsspilchen
committed
146114: opt: remove unnecessary RLS allocation and refactor r=mgartner a=mgartner #### opt: remove unnecessary RLS allocation ``` name old time/op new time/op delta Phases/json-insert/Prepared/AssignPlaceholdersNoNorm 3.89µs ± 1% 3.72µs ± 1% -4.45% name old alloc/op new alloc/op delta Phases/json-insert/Prepared/AssignPlaceholdersNoNorm 3.05kB ± 0% 3.00kB ± 0% -1.57% name old allocs/op new allocs/op delta Phases/json-insert/Prepared/AssignPlaceholdersNoNorm 22.0 ± 0% 21.0 ± 0% -4.55% ``` Release note: None #### opt: add RowLevelSecurityMeta.Copy Release note: None 146280: structlogging: hot ranges log modify interval loop check to be 1m r=angles-n-daemons a=angles-n-daemons structlogging: hot ranges log modify interval loop check to be 1m It seems like the interval loop checker for hot ranges was set to 1s for testing, and accidentally committed. This change reverts the original change. Fixes: #138767 Epic: CRDB-43150 Release note: None 146287: sql: maintain index dependencies during truncate r=spilchen a=spilchen When a table is truncated, new IDs are assigned to all its indexes. However, we previously failed to update any existing back-references to these index IDs in the table descriptor. This prevented the truncate from succeeding. This change ensures that all references to old index IDs are properly remapped to the new ones, which allows the truncate to succeed. Fixes #146065 Epic: none Release note (bug fix): Fixed a bug that prevented TRUNCATE from succeeding if any indexes on the table had back-reference dependencies, such as from a view or function referencing the index. Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Brian Dillmann <[email protected]> Co-authored-by: Matt Spilchen <[email protected]>
4 parents d0cd2cf + 8a20e81 + 00bab4f + 18f0fa4 commit ae7600a

File tree

5 files changed

+95
-18
lines changed

5 files changed

+95
-18
lines changed

pkg/server/structlogging/hot_ranges_log.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ var ReportTopHottestRanges int32 = 5
2929

3030
// CheckInterval is the interval at which the system checks
3131
// whether or not to log the hot ranges.
32-
var CheckInterval = time.Second
32+
var CheckInterval = time.Minute
3333

3434
// TestLoopChannel triggers the hot ranges logging loop to start again.
3535
// It's useful in the context of a test, where we don't want to wait

pkg/sql/logictest/testdata/logic_test/truncate

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,64 @@ TRUNCATE bar
8888
statement ok
8989
DROP TABLE bar;
9090

91+
subtest index_dependencies
92+
93+
statement ok
94+
CREATE TABLE t0(c0 INT UNIQUE CHECK(true));
95+
96+
statement ok
97+
INSERT INTO t0 VALUES (0),(1),(2);
98+
99+
# Create two dependencies on the index. One from a view and one from a
100+
# function.
101+
statement ok
102+
CREATE VIEW v0 AS SELECT c0 FROM t0 @{FORCE_INDEX = t0_c0_key};
103+
104+
statement ok
105+
create function get_min_t0() RETURNS INT LANGUAGE SQL AS $$ SELECT c0 FROM t0@t0_c0_key ORDER BY c0 LIMIT 1; $$;
106+
107+
query I
108+
SELECT * FROM v0 ORDER BY c0;
109+
----
110+
0
111+
1
112+
2
113+
114+
query I
115+
SELECT get_min_t0();
116+
----
117+
0
118+
119+
statement ok
120+
TRUNCATE TABLE t0;
121+
122+
query I
123+
SELECT * FROM v0 ORDER BY c0;
124+
----
125+
126+
query I
127+
SELECT get_min_t0();
128+
----
129+
NULL
130+
131+
# Ensure the index dependency was copied over.
132+
statement error pq: index "t0_c0_key" is in use as unique constraint.*
133+
DROP INDEX t0@t0_c0_key;
134+
135+
statement ok
136+
DROP INDEX t0@t0_c0_key CASCADE;
137+
138+
# Ensure the cascade got rid of the view
139+
statement error pq: relation "v0" does not exist
140+
SELECT * FROM v0;
141+
142+
# Ensure the cascade got rid of the function
143+
statement error pq: unknown function: get_min_t0()
144+
SELECT get_min_t0();
145+
146+
statement ok
147+
DROP TABLE t0;
148+
91149
subtest truncate_30547
92150

93151
statement ok

pkg/sql/opt/metadata.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,7 @@ func (md *Metadata) CopyFrom(from *Metadata, copyScalarFn func(Expr) Expr) {
334334
// We cannot copy the bound expressions; they must be rebuilt in the new memo.
335335
md.withBindings = nil
336336

337-
md.rlsMeta = from.rlsMeta
338-
md.rlsMeta.PoliciesApplied = make(map[TableID]PoliciesApplied)
339-
for id, policies := range from.rlsMeta.PoliciesApplied {
340-
md.rlsMeta.PoliciesApplied[id] = policies.Copy()
341-
}
337+
md.rlsMeta = from.rlsMeta.Copy()
342338
}
343339

344340
// MDDepName stores either the unresolved DataSourceName or the StableID from

pkg/sql/opt/row_level_security.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,23 @@ func (r *RowLevelSecurityMeta) RefreshNoPoliciesAppliedForTable(tableID TableID)
104104
}
105105
}
106106

107+
// Copy returns a deep copy of the RowLevelSecurityMeta.
108+
func (r *RowLevelSecurityMeta) Copy() RowLevelSecurityMeta {
109+
cpy := RowLevelSecurityMeta{
110+
IsInitialized: r.IsInitialized,
111+
User: r.User,
112+
HasAdminRole: r.HasAdminRole,
113+
NoPoliciesApplied: r.NoPoliciesApplied,
114+
}
115+
for id, policies := range r.PoliciesApplied {
116+
if cpy.PoliciesApplied == nil {
117+
cpy.PoliciesApplied = make(map[TableID]PoliciesApplied)
118+
}
119+
cpy.PoliciesApplied[id] = policies.Copy()
120+
}
121+
return cpy
122+
}
123+
107124
// PoliciesApplied stores the set of policies that were applied to a table.
108125
type PoliciesApplied struct {
109126
// NoForceExempt is true if the policies were exempt because they were the

pkg/sql/truncate.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -209,18 +209,12 @@ func (p *planner) truncateTable(ctx context.Context, id descpb.ID, jobDesc strin
209209
}
210210
}
211211

212-
// Create new ID's for all of the indexes in the table.
213-
{
214-
version := p.ExecCfg().Settings.Version.ActiveVersion(ctx)
215-
// Temporarily empty the mutation jobs slice otherwise the descriptor
216-
// validation performed by AllocateIDs will fail: the Mutations slice
217-
// has been emptied but MutationJobs only gets emptied later on.
218-
mutationJobs := tableDesc.MutationJobs
219-
tableDesc.MutationJobs = nil
220-
if err := tableDesc.AllocateIDs(ctx, version); err != nil {
221-
return err
222-
}
223-
tableDesc.MutationJobs = mutationJobs
212+
// Allocate new IDs for all indexes in the table descriptor. We use the
213+
// AllocateIDsWithoutValidation variant because some DependedOnBy references
214+
// may still point to the old index IDs, which we haven't remapped yet.
215+
// Full validation will occur later when writeSchemaChange is called.
216+
if err := tableDesc.AllocateIDsWithoutValidation(ctx, true /*createMissingPrimaryKey*/); err != nil {
217+
return err
224218
}
225219

226220
// Construct a mapping from old index ID's to new index ID's.
@@ -229,6 +223,18 @@ func (p *planner) truncateTable(ctx context.Context, id descpb.ID, jobDesc strin
229223
indexIDMapping[oldIndexes[idx.Ordinal()].ID] = idx.GetID()
230224
}
231225

226+
// Remap index IDs in the DependedOnBy references using the new index ID mapping.
227+
for i := range tableDesc.DependedOnBy {
228+
ref := &tableDesc.DependedOnBy[i]
229+
if ref.IndexID != 0 {
230+
var ok bool
231+
ref.IndexID, ok = indexIDMapping[ref.IndexID]
232+
if !ok {
233+
return errors.AssertionFailedf("could not find index ID %d in mapping", ref.IndexID)
234+
}
235+
}
236+
}
237+
232238
// Create schema change GC jobs for all of the indexes.
233239
dropTime := timeutil.Now().UnixNano()
234240
droppedIndexes := make([]jobspb.SchemaChangeGCDetails_DroppedIndex, 0, len(oldIndexes))

0 commit comments

Comments
 (0)