Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make room for other database engines #1139

Closed
wants to merge 4 commits into from
Closed

Make room for other database engines #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2593e2
Compute user existence by count instead of by id
Apr 26, 2022
45fd9a2
Remove unneeded field
Mar 16, 2023
f972231
Use JOIN clause
May 3, 2022
0ec07de
Order using common field
May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions lib/Zonemaster/Backend/DB.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,16 +511,15 @@ sub get_test_history {
my @results;
my $query = q[
SELECT
(SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_critical,
(SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_error,
(SELECT count(*) FROM result_entries WHERE result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_warning,
id,
(SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_critical,
(SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_error,
(SELECT count(*) FROM result_entries JOIN test_results ON result_entries.hash_id = test_results.hash_id AND level = ?) AS nb_warning,
Comment on lines +514 to +516
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong to me but maybe it just me reading it wrong. Doesn't this joined test_results shadow the test_results in the outer select? If it is, doesn't that mean that the count includes all entries for all tests just as long as they have the right level?

hash_id,
created_at,
undelegated
FROM test_results
WHERE progress = 100 AND domain = ? AND ( ? IS NULL OR undelegated = ? )
ORDER BY id DESC
ORDER BY created_at DESC
LIMIT ?
OFFSET ?];

Expand Down Expand Up @@ -582,14 +581,14 @@ sub user_authorized {
my ( $self, $user, $api_key ) = @_;

my $dbh = $self->dbh;
my ( $id ) = $dbh->selectrow_array(
"SELECT id FROM users WHERE username = ? AND api_key = ?",
my ( $count ) = $dbh->selectrow_array(
"SELECT count(*) FROM users WHERE username = ? AND api_key = ?",
undef,
$user,
$api_key
);

return $id;
return $count;
}

sub batch_exists_in_db {
Expand Down Expand Up @@ -643,7 +642,7 @@ sub get_test_request {
WHERE progress = 0
AND queue = ?
ORDER BY priority DESC,
id ASC
created_at ASC
LIMIT 1
],
undef,
Expand All @@ -658,7 +657,7 @@ sub get_test_request {
FROM test_results
WHERE progress = 0
ORDER BY priority DESC,
id ASC
created_at ASC
LIMIT 1
],
);
Expand Down
10 changes: 5 additions & 5 deletions lib/Zonemaster/Backend/DB/MySQL.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ sub create_schema {
# retrieve all indexes by key name
my $indexes = $dbh->selectall_hashref( 'SHOW INDEXES FROM test_results', 'Key_name' );

if ( not exists($indexes->{test_results__hash_id}) ) {
if ( not exists($indexes->{test_results__hash_id_created_at}) ) {
$dbh->do(
'CREATE INDEX test_results__hash_id ON test_results (hash_id)'
'CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)'
);
}
if ( not exists($indexes->{test_results__fingerprint}) ) {
Expand Down Expand Up @@ -249,15 +249,15 @@ sub add_batch_job {

my $dbh = $self->dbh;

if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
$batch_id = $self->create_new_batch_job( $params->{username} );

my $test_params = $params->{test_params};
my $priority = $test_params->{priority};
my $queue_label = $test_params->{queue};

$dbh->{AutoCommit} = 0;
eval {$dbh->do( "DROP INDEX test_results__hash_id ON test_results" );};
eval {$dbh->do( "DROP INDEX test_results__hash_id_created_at ON test_results" );};
eval {$dbh->do( "DROP INDEX test_results__fingerprint ON test_results" );};
eval {$dbh->do( "DROP INDEX test_results__batch_id_progress ON test_results" );};
eval {$dbh->do( "DROP INDEX test_results__progress ON test_results" );};
Expand Down Expand Up @@ -298,7 +298,7 @@ sub add_batch_job {
$undelegated,
);
}
$dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" );
$dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" );
$dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" );
Expand Down
8 changes: 4 additions & 4 deletions lib/Zonemaster/Backend/DB/PostgreSQL.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ sub create_schema {
) or die Zonemaster::Backend::Error::Internal->new( reason => "PostgreSQL error, could not create 'test_results' table", data => $dbh->errstr() );

$dbh->do(
'CREATE INDEX IF NOT EXISTS test_results__hash_id ON test_results (hash_id)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still need an index for just hash_id? E.g. for the test_progress RPCAPI method.

'CREATE INDEX IF NOT EXISTS test_results__hash_id_created_at ON test_results (hash_id, created_at)'
);
$dbh->do(
'CREATE INDEX IF NOT EXISTS test_results__fingerprint ON test_results (fingerprint)'
Expand Down Expand Up @@ -222,7 +222,7 @@ sub add_batch_job {

my $dbh = $self->dbh;

if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
$batch_id = $self->create_new_batch_job( $params->{username} );

my $test_params = $params->{test_params};
Expand All @@ -234,7 +234,7 @@ sub add_batch_job {

$dbh->begin_work();
$dbh->do( "ALTER TABLE test_results DROP CONSTRAINT IF EXISTS test_results_pkey" );
$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id" );
$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id_created_at" );
$dbh->do( "DROP INDEX IF EXISTS test_results__fingerprint" );
$dbh->do( "DROP INDEX IF EXISTS test_results__batch_id_progress" );
$dbh->do( "DROP INDEX IF EXISTS test_results__progress" );
Expand Down Expand Up @@ -271,7 +271,7 @@ sub add_batch_job {
}
$dbh->pg_putcopyend();
$dbh->do( "ALTER TABLE test_results ADD PRIMARY KEY (id)" );
$dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" );
$dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" );
$dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" );
Expand Down
8 changes: 4 additions & 4 deletions lib/Zonemaster/Backend/DB/SQLite.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ sub create_schema {
) or die Zonemaster::Backend::Error::Internal->new( reason => "SQLite error, could not create 'test_results' table", data => $dbh->errstr() );

$dbh->do(
'CREATE INDEX IF NOT EXISTS test_results__hash_id ON test_results (hash_id)'
'CREATE INDEX IF NOT EXISTS test_results__hash_id_created_at ON test_results (hash_id, created_at)'
);
$self->dbh->do(
'CREATE INDEX IF NOT EXISTS test_results__fingerprint ON test_results (fingerprint)'
Expand Down Expand Up @@ -210,15 +210,15 @@ sub add_batch_job {

my $dbh = $self->dbh;

if ( $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
if ( 1 == $self->user_authorized( $params->{username}, $params->{api_key} ) ) {
$batch_id = $self->create_new_batch_job( $params->{username} );

my $test_params = $params->{test_params};
my $priority = $test_params->{priority};
my $queue_label = $test_params->{queue};

$dbh->{AutoCommit} = 0;
eval {$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id " );};
eval {$dbh->do( "DROP INDEX IF EXISTS test_results__hash_id_created_at " );};
eval {$dbh->do( "DROP INDEX IF EXISTS test_results__fingerprint " );};
eval {$dbh->do( "DROP INDEX IF EXISTS test_results__batch_id_progress " );};
eval {$dbh->do( "DROP INDEX IF EXISTS test_results__progress " );};
Expand Down Expand Up @@ -257,7 +257,7 @@ sub add_batch_job {
$undelegated,
);
}
$dbh->do( "CREATE INDEX test_results__hash_id ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__hash_id_created_at ON test_results (hash_id, created_at)" );
$dbh->do( "CREATE INDEX test_results__fingerprint ON test_results (fingerprint)" );
$dbh->do( "CREATE INDEX test_results__batch_id_progress ON test_results (batch_id, progress)" );
$dbh->do( "CREATE INDEX test_results__progress ON test_results (progress)" );
Expand Down