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.

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

feat: improve DB query performance #803

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Copy link
Member

Choose a reason for hiding this comment

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

Would we have issues with name_lowercase not being populated when inserting new rows on H2?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume that name_lowercase is indeed being populated when inserting new rows. Otherwise our tests under H2 mode wouldn't be passing, since the code is searching by name_lowercase now. E.g. this test inserts a new row and then search for that (the underlying query is searching by the name_lowercase column).

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE credential
ADD COLUMN name_lowercase VARCHAR(1024) AS LOWER(name);

CREATE INDEX credential_name_lowercase
ON credential(name_lowercase);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE credential
ADD COLUMN name_lowercase VARCHAR(1024) GENERATED ALWAYS AS (lower(name)) STORED;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CREATE INDEX credential_name_lowercase
ON credential(name_lowercase);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE credential
ADD COLUMN name_lowercase VARCHAR(1024) GENERATED ALWAYS AS (lower(name)) STORED;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CREATE INDEX CONCURRENTLY IF NOT EXISTS credential_name_lowercase
ON credential(name_lowercase);
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public CredentialDataService(final CredentialRepository credentialRepository, fi
}

public Credential find(final String name) {
return credentialRepository.findOneByNameIgnoreCase(name);
return credentialRepository.findOneByNameLowercase(name.toLowerCase());
}

public Credential findByUUID(final UUID uuid) {
Expand All @@ -37,7 +37,7 @@ public Credential save(final Credential credential) {
public boolean delete(final String credentialName) {
final Credential cred = this.find(credentialName);
auditRecord.setResource(cred);
return credentialRepository.deleteByNameIgnoreCase(credentialName) > 0;
return credentialRepository.deleteByNameLowercase(credentialName.toLowerCase()) > 0;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ class Credential : AuditableCredential {
}
}

@Column(name = "name_lowercase", nullable = false, insertable = false)
var nameLowercase: String? = null

@Column(unique = true, nullable = false)
var checksum: String? = null

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@ import java.util.UUID

interface CredentialRepository : JpaRepository<Credential?, UUID?> {
@Transactional
fun deleteByNameIgnoreCase(name: String?): Long
fun deleteByNameLowercase(name: String?): Long

fun findOneByUuid(uuid: UUID?): Credential?

@Query(
value = "select credential.uuid, credential.name, credential.checksum from certificate_credential " +
value = "select credential.uuid, credential.name, credential.name_lowercase, credential.checksum from certificate_credential " +
"left join credential_version on certificate_credential.uuid = credential_version.uuid " +
"join credential on credential.uuid = credential_version.credential_uuid " +
"where credential.uuid = ?1",
nativeQuery = true,
)
fun findCertificateByUuid(uuid: UUID?): Credential?

fun findOneByNameIgnoreCase(name: String?): Credential?
fun findOneByNameLowercase(name: String?): Credential?

@Query(
value = "select credential.uuid, credential.name, credential.checksum from certificate_credential " +
value = "select credential.uuid, credential.name, credential.name_lowercase, credential.checksum from certificate_credential " +
"left join credential_version on certificate_credential.uuid = credential_version.uuid " +
"join credential on credential.uuid = credential_version.credential_uuid " +
"group by credential.uuid",
Expand All @@ -33,7 +33,7 @@ interface CredentialRepository : JpaRepository<Credential?, UUID?> {
fun findAllCertificates(): List<Credential>

@Query(
value = "select credential.uuid, credential.name, credential.checksum from certificate_credential " +
value = "select credential.uuid, credential.name, credential.name_lowercase, credential.checksum from certificate_credential " +
"left join credential_version on certificate_credential.uuid = credential_version.uuid " +
"join credential on credential.uuid = credential_version.credential_uuid " +
"where credential.name = ?1 limit 1 ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ constructor(
(select version_created_at, credential_uuid from credential_version LEFT OUTER JOIN
certificate_credential on credential_version.uuid = certificate_credential.uuid
WHERE (transitional is false or transitional IS NULL)
and credential_uuid in (select uuid from credential where lower(name) like ?))
and credential_uuid in (select uuid from credential where name_lowercase like ?))
as credential_version
group by credential_uuid ) as credential_version
inner join
(select * from credential where lower(name) like ? )
(select * from credential where name_lowercase like ? )
as name on credential_version.credential_uuid = name.uuid
order by version_created_at desc
""".trimMargin()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CredentialDataServiceTest {
credentialRepository.save(credential)
MatcherAssert.assertThat(credentialRepository.count(), IsEqual.equalTo(1L))
MatcherAssert.assertThat(
credentialRepository.findOneByNameIgnoreCase(CREDENTIAL_NAME)!!.name,
credentialRepository.findOneByNameLowercase(CREDENTIAL_NAME.lowercase())!!.name,
IsEqual.equalTo(CREDENTIAL_NAME),
)
}
Expand Down