Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
138048: roachtest: lower the foreground throughput in disk bandwidth test r=itsbilal a=aadityasondhi

The test fails due to excessive amount of foreground traffic. This patch lowers the bytes written by the foreground traffic.

Fixes #137881

Release note: None

138295: sql: Fix virtual index scan for crdb_internal.create_type_statements r=spilchen a=spilchen

When querying the crdb_internal.create_type_statements virtual table, two access paths are possible: a full table scan or a virtual index scan. We observed inconsistent behavior between these paths. The full table scan returned correct data, but the virtual index scan could return types for the wrong database (if one was specified) or panic with a null pointer dereference when querying across all databases. This change corrects the virtual index access pattern to ensure consistency with the full table scan.

Epic: None
Closes #137579
Release note (bug fix): Fixed issues with the virtual index scan on crdb_internal.create_type_statements, ensuring consistent results when querying user-defined types (UDTs) across databases.

Co-authored-by: Aaditya Sondhi <[email protected]>
Co-authored-by: Matt Spilchen <[email protected]>
  • Loading branch information
3 people committed Jan 6, 2025
3 parents 3d3d3fc + 09bb542 + 615725a commit ae2c24b
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func registerDiskBandwidthOverload(r registry.Registry) {

c.Run(ctx, option.WithNodes(c.WorkloadNode()),
"./cockroach workload init kv --drop --insert-count=400 "+
"--max-block-bytes=1024 --min-block-bytes=1024"+foregroundDB+url)
"--max-block-bytes=512 --min-block-bytes=512"+foregroundDB+url)

c.Run(ctx, option.WithNodes(c.WorkloadNode()),
"./cockroach workload init kv --drop --insert-count=400 "+
Expand Down
18 changes: 18 additions & 0 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -3560,6 +3560,24 @@ CREATE TABLE crdb_internal.create_type_statements (
if err != nil || typDesc == nil {
return false, err
}

// Handle special cases for the database descriptor provided. There are
// two scenarios:
// 1) A database descriptor is provided, but the type belongs to a
// different database. In this case, the row will be filtered out as
// it doesn't pertain to the requested database.
// 2) The database descriptor is nil. This occurs when requesting types
// across all databases (e.g., SELECT .. FROM "".crdb_internal.create_type_statements).
// In this case, the row will be returned, but a database lookup is
// needed to populate columns that depend on the database descriptor.
if db != nil && typDesc.GetParentID() != db.GetID() {
return false, nil
} else if db == nil {
db, err = p.byIDGetterBuilder().WithoutDropped().Get().Database(ctx, typDesc.GetParentID())
if err != nil {
return false, err
}
}
return writeCreateTypeDescRow(ctx, db, scName, p, typDesc, addRow)
},
},
Expand Down
25 changes: 22 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
Original file line number Diff line number Diff line change
Expand Up @@ -1677,13 +1677,32 @@ DELETE FROM system.role_members WHERE member = 'non_cached_user';
statement ok
DROP USER real_user;

subtest end
subtest cross_db

statement ok
CREATE TYPE other_db.public.enum1 AS ENUM ('yo');

# Ensure that the cross-DB lookup of UDTs works.
# Ensure that the cross-DB lookup of UDTs works. This uses the full table scan.
query ITTITTT
SELECT * FROM "".crdb_internal.create_type_statements WHERE descriptor_id = (('other_db.public.enum1'::regtype::int) - 100000)::oid
SELECT * FROM "".crdb_internal.create_type_statements WHERE descriptor_name = 'enum1' and database_name = 'other_db'
----
121 other_db public 151 enum1 CREATE TYPE other_db.public.enum1 AS ENUM ('yo') {yo}

# This uses the virtual index. descriptor_id is an int in the vtable.
query ITTITTT
SELECT * FROM "".crdb_internal.create_type_statements WHERE descriptor_id = (('other_db.public.enum1'::regtype::int) - 100000)
----
121 other_db public 151 enum1 CREATE TYPE other_db.public.enum1 AS ENUM ('yo') {yo}

# Repeat above two queries but apply only for the current database. We do this by omitting the "".
# This one uses the full table scan.
query ITTITTT
SELECT * FROM crdb_internal.create_type_statements WHERE descriptor_name = 'enum1'
----

# This one uses the virtual index
query ITTITTT
SELECT * FROM crdb_internal.create_type_statements WHERE descriptor_id = (('other_db.public.enum1'::regtype::int) - 100000)
----

subtest end

0 comments on commit ae2c24b

Please sign in to comment.