sql: Benchmark has_schema_privilege, has_sequence_privilege, has_table_privilege #75855
Conversation
|
This change is |
|
You'll need to update the expectations file. Also, consider adding cases where you set up more than one of each of the things. |
| // prevents database exists error | ||
| Setup: `DROP DATABASE IF EXISTS d; CREATE DATABASE d`, | ||
| Stmt: `SELECT has_database_privilege('d', 'CREATE')`, | ||
| }, |
There was a problem hiding this comment.
can you add more tests to see how it scales with number of databases/schemas/sequences?
basically, in the setup, it can create multiple entities, and the Stmt to test can be something like
SELECT has_database_privilege(name, 'CREATE') FROM crdb_internal.databases;
for schemas, something like
SELECT has_schema_privilege(nspame, 'CREATE') FROM pg_catalog.pg_namespace
There was a problem hiding this comment.
Talked about the tests not supporting has_database_privilege offline. Updated the others.
|
|
ecwall
changed the title
| }, | ||
|
|
||
| { | ||
| Name: "has_schema_privilege multiple", |
There was a problem hiding this comment.
nit instead of multiple, state how many schemas are being created
we should probably test 1,3,5 schemas and see the relative increases
There was a problem hiding this comment.
Is there a way to create tables/schemas/etc using generate_series (or something similar) to avoid copying and pasting?
There was a problem hiding this comment.
I don't think we can use the sql language to iterate and do multiple DDLs but perhaps you could extend make a helper function to create a string for setup
has_table_privilege fixes #66173 These builtins have been changed to use HasPrivilege instead of executing SQL queries internally. Release note: None
| @@ -77,7 +77,7 @@ func runBenchmarkExpectationTests(t *testing.T, r *Registry) { | |||
|
|
|||
| var results resultSet | |||
| var wg sync.WaitGroup | |||
| concurrency := ((system.NumCPU()*4 - 1) / r.numNodes) + 1 // arbitrary | |||
| concurrency := ((system.NumCPU() - 1) / r.numNodes) + 1 // arbitrary | |||
There was a problem hiding this comment.
Spoke with @ajwerner on slack about this. Removing the *4 makes the tests run much faster with larger (>4) rewrite iterations.
| 1,ORMQueries/has_schema_privilege_5 | ||
| 2,ORMQueries/has_sequence_privilege_1 | ||
| 4,ORMQueries/has_sequence_privilege_3 | ||
| 6,ORMQueries/has_sequence_privilege_5 | ||
| 2,ORMQueries/has_table_privilege_1 | ||
| 4,ORMQueries/has_table_privilege_3 | ||
| 6,ORMQueries/has_table_privilege_5 |
|
bors r=RichardJCai |
|
Build failed (retrying...): |
|
Build succeeded: |
fixes #66173
These builtins have been changed to use HasPrivilege instead of executing SQL
queries internally.
Release notes: None