Skip to content

Commit

Permalink
Remove escape and escape_Table (and add indexes)
Browse files Browse the repository at this point in the history 
- try to simplify SQL statements
- create indexes
  • Loading branch information
@koppor
koppor committed Oct 4, 2024
1 parent 5c863dc commit 7e0ba3e
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 221 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We improved the performance when pasting and importing entries in an existing library. [#11843](https://github.com/JabRef/jabref/pull/11843)
- When fulltext search is selected but indexing is deactivated, a dialog is now shown asking if the user wants to enable indexing now [#9491](https://github.com/JabRef/jabref/issues/9491)
- We changed instances of 'Search Selected' to 'Search Pre-configured' in Web Search Preferences UI. [#11871](https://github.com/JabRef/jabref/pull/11871)
- We rewrote the [remote SQL database](https://docs.jabref.org/collaborative-work/sqldatabase) support. ⚠️database tables will be migrated. [#11879](https://github.com/JabRef/jabref/pull/11879)

### Fixed

Expand Down
198 changes: 69 additions & 129 deletions src/main/java/org/jabref/logic/shared/DBMSProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.jabref.model.entry.event.EntriesEventSource;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.types.EntryType;
import org.jabref.model.entry.types.EntryTypeFactory;
import org.jabref.model.metadata.MetaData;

Expand Down Expand Up @@ -129,19 +130,6 @@ public void setupSharedDatabase() throws SQLException {
*/
protected abstract void setUp() throws SQLException;

/**
* Escapes parts of SQL expressions such as a table name or a field name to match the conventions of the database
* system using the current dbmsType.
* <p>
* This method is package private, because of DBMSProcessorTest
*
* @param expression Table or field name
* @return Correctly escaped expression
*/
abstract String escape(String expression);

abstract String escape_Table(String expression);

abstract Integer getCURRENT_VERSION_DB_STRUCT();

/**
Expand Down Expand Up @@ -173,12 +161,11 @@ public void insertEntries(List<BibEntry> bibEntries) {
* @param bibEntries List of {@link BibEntry} to be inserted
*/
protected void insertIntoEntryTable(List<BibEntry> bibEntries) {
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape_Table("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");
if (bibEntries.isEmpty()) {
return;
}

StringBuilder insertIntoEntryQuery = new StringBuilder().append("INSERT INTO entry (entrytype) VALUES (?)");
// Number of commas is bibEntries.size() - 1
insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, (bibEntries.size() - 1))));

Expand All @@ -201,7 +188,7 @@ protected void insertIntoEntryTable(List<BibEntry> bibEntries) {
}
}
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
LOGGER.error("SQL Error", e);
}
}

Expand All @@ -222,8 +209,7 @@ private List<BibEntry> getNotYetExistingEntries(List<BibEntry> bibEntries) {
return bibEntries;
}
try {
String selectQuery = "SELECT * FROM " +
escape_Table("ENTRY");
String selectQuery = "SELECT * FROM ENTRY";

try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery)) {
while (resultSet.next()) {
Expand All @@ -245,22 +231,19 @@ private List<BibEntry> getNotYetExistingEntries(List<BibEntry> bibEntries) {
* @param bibEntries {@link BibEntry} to be inserted
*/
protected void insertIntoFieldTable(List<BibEntry> bibEntries) {
if (bibEntries.isEmpty()) {
return;
}

try {
// Inserting into FIELD table
// Coerce to ArrayList in order to use List.get()
List<List<Field>> fields = bibEntries.stream().map(bibEntry -> new ArrayList<>(bibEntry.getFields()))
List<List<Field>> fields = bibEntries.stream()
.map(bibEntry -> new ArrayList<>(bibEntry.getFields()))
.collect(Collectors.toList());

StringBuilder insertFieldQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape_Table("FIELD"))
.append("(")
.append(escape("ENTRY_SHARED_ID"))
.append(", ")
.append(escape("NAME"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES(?, ?, ?)");
.append("INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) VALUES(?, ?, ?)");
int numFields = 0;
for (List<Field> entryFields : fields) {
numFields += entryFields.size();
Expand All @@ -283,6 +266,7 @@ protected void insertIntoFieldTable(List<BibEntry> bibEntries) {
fieldsCompleted += 1;
}
}
// TODO: This could grow too large for a single query
preparedFieldStatement.executeUpdate();
}
} catch (SQLException e) {
Expand Down Expand Up @@ -317,18 +301,12 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL
.getVersion()) || localBibEntry.equals(sharedBibEntry)) {
insertOrUpdateFields(localBibEntry);

// updating entry type
String updateEntryTypeQuery = "UPDATE " +
escape_Table("ENTRY") +
" SET " +
escape("TYPE") +
" = ?, " +
escape("VERSION") +
" = " +
escape("VERSION") +
" + 1 WHERE " +
escape("SHARED_ID") +
" = ?";
String updateEntryTypeQuery = """
UPDATE entry
SET entrytype = ?,
version = version + 1
WHERE shared_id = ?
""";

try (PreparedStatement preparedUpdateEntryTypeStatement = connection.prepareStatement(updateEntryTypeQuery)) {
preparedUpdateEntryTypeStatement.setString(1, localBibEntry.getType().getName());
Expand All @@ -355,13 +333,10 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha
Set<Field> nullFields = new HashSet<>(sharedBibEntry.getFields());
nullFields.removeAll(localBibEntry.getFields());
for (Field nullField : nullFields) {
String deleteFieldQuery = "DELETE FROM " +
escape_Table("FIELD") +
" WHERE " +
escape("NAME") +
" = ? AND " +
escape("ENTRY_SHARED_ID") +
" = ?";
String deleteFieldQuery = """
DELETE FROM FIELD
WHERE NAME = ? AND ENTRY_SHARED_ID = ?
""";

try (PreparedStatement preparedDeleteFieldStatement = connection
.prepareStatement(deleteFieldQuery)) {
Expand All @@ -385,13 +360,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {
value = valueOptional.get();
}

String selectFieldQuery = "SELECT * FROM " +
escape_Table("FIELD") +
" WHERE " +
escape("NAME") +
" = ? AND " +
escape("ENTRY_SHARED_ID") +
" = ?";
String selectFieldQuery = """
SELECT * FROM FIELD
WHERE NAME = ? AND ENTRY_SHARED_ID = ?
""";

try (PreparedStatement preparedSelectFieldStatement = connection
.prepareStatement(selectFieldQuery)) {
Expand All @@ -400,15 +372,11 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {

try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) {
if (selectFieldResultSet.next()) { // check if field already exists
String updateFieldQuery = "UPDATE " +
escape_Table("FIELD") +
" SET " +
escape("VALUE") +
" = ? WHERE " +
escape("NAME") +
" = ? AND " +
escape("ENTRY_SHARED_ID") +
" = ?";
String updateFieldQuery = """
UPDATE FIELD
SET VALUE = ?
WHERE NAME = ? AND ENTRY_SHARED_ID = ?
""";

try (PreparedStatement preparedUpdateFieldStatement = connection
.prepareStatement(updateFieldQuery)) {
Expand All @@ -418,15 +386,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {
preparedUpdateFieldStatement.executeUpdate();
}
} else {
String insertFieldQuery = "INSERT INTO " +
escape_Table("FIELD") +
"(" +
escape("ENTRY_SHARED_ID") +
", " +
escape("NAME") +
", " +
escape("VALUE") +
") VALUES(?, ?, ?)";
String insertFieldQuery = """
INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE)
VALUES (?, ?, ?)
""";

try (PreparedStatement preparedFieldStatement = connection
.prepareStatement(insertFieldQuery)) {
Expand All @@ -451,12 +414,7 @@ public void removeEntries(List<BibEntry> bibEntries) {
if (bibEntries.isEmpty()) {
return;
}
StringBuilder query = new StringBuilder()
.append("DELETE FROM ")
.append(escape_Table("ENTRY"))
.append(" WHERE ")
.append(escape("SHARED_ID"))
.append(" IN (");
StringBuilder query = new StringBuilder().append("DELETE FROM ENTRY WHERE SHARED_ID IN (");
query.append("?, ".repeat(bibEntries.size() - 1));
query.append("?)");

Expand Down Expand Up @@ -509,31 +467,19 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {

List<BibEntry> sharedEntries = new ArrayList<>();

StringBuilder query = new StringBuilder();
query.append("SELECT ")
.append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID")).append(", ")
.append(escape_Table("ENTRY")).append(".").append(escape("TYPE")).append(", ")
.append(escape_Table("ENTRY")).append(".").append(escape("VERSION")).append(", ")
.append("F.").append(escape("ENTRY_SHARED_ID")).append(", ")
.append("F.").append(escape("NAME")).append(", ")
.append("F.").append(escape("VALUE"))
.append(" FROM ")
.append(escape_Table("ENTRY"))
// Handle special case if entry does not have any fields (yet)
.append(" left outer join ")
.append(escape_Table("FIELD"))
.append(" F on ")
.append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID"))
.append(" = F.").append(escape("ENTRY_SHARED_ID"));
StringBuilder query = new StringBuilder()
.append("SELECT entry.shared_id, entry.entrytype, entry.version, ")
.append("F.entry_shared_id, F.name, F.value ")
.append("FROM entry ")
.append("LEFT OUTER JOIN field F ON entry.shared_id = F.entry_shared_id");

if (!sharedIDs.isEmpty()) {
query.append(" where ")
.append(escape("SHARED_ID")).append(" in (")
query.append(" WHERE entry.shared_id IN (")
.append("?, ".repeat(sharedIDs.size() - 1))
.append("?)");
}
query.append(" order by ")
.append(escape("SHARED_ID"));

query.append(" ORDER BY shared_id");

try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) {
for (int i = 0; i < sharedIDs.size(); i++) {
Expand All @@ -547,12 +493,16 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
// We get a list of field values of bib entries "grouped" by bib entries
// Thus, the first change in the shared id leads to a new BibEntry
if (selectEntryResultSet.getInt("SHARED_ID") > lastId) {
bibEntry = new BibEntry();
bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID"));
bibEntry.setType(EntryTypeFactory.parse(selectEntryResultSet.getString("TYPE")));
bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION"));
int sharedId = selectEntryResultSet.getInt("shared_id");
int version = selectEntryResultSet.getInt("version");
EntryType entrytype = EntryTypeFactory.parse(selectEntryResultSet.getString("entrytype"));

bibEntry = new BibEntry(entrytype);
bibEntry.getSharedBibEntryData().setSharedID(sharedId);
bibEntry.getSharedBibEntryData().setVersion(version);

sharedEntries.add(bibEntry);
lastId = selectEntryResultSet.getInt("SHARED_ID");
lastId = sharedId;
}

// In all cases, we set the field value of the newly created BibEntry object
Expand All @@ -563,8 +513,7 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
}
}
} catch (SQLException e) {
LOGGER.error("Executed >{}<", query);
LOGGER.error("SQL Error", e);
LOGGER.error("Executed >{}< and got an error", query, e);
return Collections.emptyList();
}

Expand All @@ -580,10 +529,10 @@ public List<BibEntry> getSharedEntries() {
*/
public Map<Integer, Integer> getSharedIDVersionMapping() {
Map<Integer, Integer> sharedIDVersionMapping = new HashMap<>();
String selectEntryQuery = "SELECT * FROM " +
escape_Table("ENTRY") +
" ORDER BY " +
escape("SHARED_ID");
String selectEntryQuery = """
SELECT * FROM ENTRY
ORDER BY SHARED_ID
""";

try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(selectEntryQuery)) {
while (selectEntryResultSet.next()) {
Expand All @@ -602,7 +551,7 @@ public Map<Integer, Integer> getSharedIDVersionMapping() {
public Map<String, String> getSharedMetaData() {
Map<String, String> data = new HashMap<>();

try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM " + escape_Table("METADATA"))) {
try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM METADATA")) {
while (resultSet.next()) {
data.put(resultSet.getString("KEY"), resultSet.getString("VALUE"));
}
Expand All @@ -619,21 +568,12 @@ public Map<String, String> getSharedMetaData() {
* @param data JabRef meta data as map
*/
public void setSharedMetaData(Map<String, String> data) throws SQLException {
String insertOrUpdateQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape_Table("METADATA"))
.append(" (")
.append(escape("KEY"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES (?, ?) ")
.append("ON CONFLICT (")
.append(escape("KEY"))
.append(") DO UPDATE SET ")
.append(escape("VALUE"))
.append(" = EXCLUDED.")
.append(escape("VALUE"))
.toString();
String insertOrUpdateQuery = """
INSERT INTO METADATA (KEY, VALUE)
VALUES (?, ?)
ON CONFLICT (KEY) DO UPDATE
SET VALUE = EXCLUDED.VALUE
""";

try (PreparedStatement statement = connection.prepareStatement(insertOrUpdateQuery)) {
for (Map.Entry<String, String> metaEntry : data.entrySet()) {
Expand Down
Loading

0 comments on commit 7e0ba3e

Please sign in to comment.