perf(spanner-jdbc): cache JDBC metadata query strings - #14041
Conversation
Cache the SQL strings that are loaded from disk for the standard JDBC metadata queries.
There was a problem hiding this comment.
Code Review
This pull request introduces caching for SQL queries loaded from files in JdbcDatabaseMetaData using a ConcurrentHashMap to improve performance, and adds a corresponding unit test to verify the caching behavior. It also updates the file reading logic to use UTF-8 explicitly and adds a null check for the resource stream. The reviewer suggests managing all closeable resources (InputStream, InputStreamReader, and Scanner) within a try-with-resources block to prevent potential resource leaks and notes that BufferedReader is redundant.
| BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8)); | ||
| StringBuilder builder = new StringBuilder(); | ||
| try (Scanner scanner = new Scanner(reader)) { |
There was a problem hiding this comment.
To prevent potential resource leaks and ensure exception safety, all closeable resources (InputStream, InputStreamReader, and Scanner) should be managed within the try-with-resources block. If an exception occurs during the initialization of BufferedReader or Scanner, the underlying InputStream might not be closed. Additionally, BufferedReader is redundant here because Scanner performs its own buffering.
StringBuilder builder = new StringBuilder();
try (InputStream input = in;
InputStreamReader reader = new InputStreamReader(input, StandardCharsets.UTF_8);
Scanner scanner = new Scanner(reader)) {References
- When managing a collection of closeable resources, ensure they are closed in the reverse order of their creation (LIFO). The implementation must be exception-safe to prevent resource leaks, meaning all opened resources should be closed even if exceptions occur during their creation or closing.
Cache the SQL strings that are loaded from disk for the standard JDBC metadata queries.