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

[WIP] Initial work on DBMSProcessor batch entry insertion into ENTRY table #5814

Merged
merged 31 commits into from
Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e701c62
Initial work on DBMSProcessor entry insertion into ENTRY table
abepolk Jan 2, 2020
a5bfc87
Change syntax for Oracle multi-row insert SQL statement
abepolk Jan 17, 2020
3027c54
Merge branch 'master' into batch_DBMSProcessor_entries
abepolk Jan 17, 2020
ae9d53c
Run tests also when source files changed
tobiasdiez Jan 17, 2020
ed675a2
Add to comment about Oracle
abepolk Jan 21, 2020
4e80030
Assume ResultSet is in order for setting shared IDs
abepolk Jan 22, 2020
67fdf27
Add insertEntry for DBMSProcessor tests and fix PostgresSQLProcessor
abepolk Jan 22, 2020
8aaf774
Fix SQL typo
abepolk Jan 25, 2020
bbc81ab
Separate table drops in Oracle tests
abepolk Jan 27, 2020
74ab816
Merge remote-tracking branch 'fork/fix_fields_sql' into fix_fields_sql
abepolk Jan 27, 2020
17de560
Remove CI tests that were added in branch
abepolk Jan 27, 2020
5f23e03
Work on unit test for DBMSProcessor insertEntries
abepolk Jan 30, 2020
402a9cc
Fix bug in DBMSProcessorTest and simplify DBMSProcessor.FilterForBibE…
abepolk Jan 31, 2020
a37129a
Remove Oracle connection bug with wrong port
abepolk Jan 31, 2020
94a75cd
Add Oracle insertIntoEntryTable
abepolk Jan 31, 2020
fbf1d33
Oracle connection fix - taken from fix_fields_sql branch
abepolk Jan 31, 2020
dbbfe00
Fix typo bug
abepolk Jan 31, 2020
a5ab4e5
Clean up code
abepolk Jan 31, 2020
2279464
Remove commented blocks
abepolk Jan 31, 2020
926473a
Remove comment about needing a test that probably isn't necessary
abepolk Jan 31, 2020
9d5960f
Manually merge fix_fields_sql OracleProcessor (just add method)
abepolk Jan 31, 2020
a2fcb77
Merge fix_fields_sql
abepolk Jan 31, 2020
3bdf2d8
Emphasize todo
abepolk Jan 31, 2020
85f9196
setSharedID into OracleProcessor entry table method
abepolk Feb 1, 2020
245c48e
Add shared id to preparedEntryStatement
abepolk Feb 2, 2020
79d8354
Make Oracle insertIntoEntryTable iterative - pasted from master - not…
abepolk Feb 19, 2020
c315838
Add fields to fields table in parallel
abepolk Feb 19, 2020
c073b8f
Merge master
abepolk Feb 19, 2020
934acb2
Reset test trace length
abepolk Feb 19, 2020
bd32ecd
Fix checkStyle
abepolk Feb 19, 2020
99191ff
Revert port setting
tobiasdiez Feb 19, 2020
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
113 changes: 70 additions & 43 deletions src/main/java/org/jabref/logic/shared/DBMSProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;

import org.jabref.logic.shared.exception.OfflineLockException;
import org.jabref.model.database.shared.DBMSType;
import org.jabref.model.database.shared.DatabaseConnection;
import org.jabref.model.database.shared.DatabaseConnectionProperties;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.SharedBibEntryData;
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.slf4j.Logger;
Expand Down Expand Up @@ -131,40 +134,61 @@ public void setupSharedDatabase() throws SQLException {
abstract String escape(String expression);

/**
* Inserts the given bibEntry into shared database.
* For use in test only. Inserts the BibEntry into the shared database.
*
* @param bibEntry {@link BibEntry} to be inserted
* @param bibEntry {@link BibEntry} to be inserted.
* */
public void insertEntry(BibEntry bibEntry) {
insertEntries(Collections.singletonList(bibEntry));
}

/**
* Inserts the List of BibEntry into the shared database.
*
* @param bibEntries List of {@link BibEntry} to be inserted
*/
public void insertEntry(BibEntry bibEntry) {
if (!checkForBibEntryExistence(bibEntry)) {
insertIntoEntryTable(bibEntry);
public void insertEntries(List<BibEntry> bibEntries) {
List<BibEntry> notYetExistingEntries = getNotYetExistingEntries(bibEntries);
insertIntoEntryTable(notYetExistingEntries);
// Temporary fix - THIS MUST BE CHANGED BEFORE MERGING TO MASTER
for (BibEntry bibEntry : notYetExistingEntries) {
insertIntoFieldTable(bibEntry);
}
}

/**
* Inserts the given bibEntry into ENTRY table.
* Inserts the given List of BibEntry into the ENTRY table.
*
* @param bibEntry {@link BibEntry} to be inserted
* @param bibEntries List of {@link BibEntry} to be inserted
*/
protected void insertIntoEntryTable(BibEntry bibEntry) {
// This is the only method to get generated keys which is accepted by MySQL, PostgreSQL and Oracle.
String insertIntoEntryQuery =
"INSERT INTO " +
escape("ENTRY") +
"(" +
escape("TYPE") +
") VALUES(?)";

try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery,
new String[]{"SHARED_ID"})) {
protected void insertIntoEntryTable(List<BibEntry> bibEntries) {
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");
// Number of commas is bibEntries.size() - 1
for (int i = 0; i < bibEntries.size() - 1; i++) {
insertIntoEntryQuery.append(", (?)");
}

preparedEntryStatement.setString(1, bibEntry.getType().getName());
try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(),
new String[]{"SHARED_ID"})) {
for (int i = 0; i < bibEntries.size(); i++) {
preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName());
}
preparedEntryStatement.executeUpdate();

try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) {
// The following assumes that we get the generated keys in the order the entries were inserted
// This should be the case
for (BibEntry bibEntry : bibEntries) {
generatedKeys.next();
bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1));
}
if (generatedKeys.next()) {
bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally
LOGGER.error("Error: Some shared IDs left unassigned");
}
}
} catch (SQLException e) {
Expand All @@ -173,44 +197,47 @@ protected void insertIntoEntryTable(BibEntry bibEntry) {
}

/**
* Checks whether the given bibEntry already exists on shared database.
* Filters a list of BibEntry to and returns those which do not exist in the database
*
* @param bibEntry {@link BibEntry} to be checked
* @param bibEntries {@link BibEntry} to be checked
* @return <code>true</code> if existent, else <code>false</code>
*/
private boolean checkForBibEntryExistence(BibEntry bibEntry) {
private List<BibEntry> getNotYetExistingEntries(List<BibEntry> bibEntries) {

List<Integer> remoteIds = new ArrayList<>();
List<Integer> localIds = bibEntries.stream()
.map(BibEntry::getSharedBibEntryData)
.map(SharedBibEntryData::getSharedID)
.filter((id) -> id != -1)
.collect(Collectors.toList());
if (localIds.isEmpty()) {
return bibEntries;
}
try {
// Check if already exists
int sharedID = bibEntry.getSharedBibEntryData().getSharedID();
if (sharedID != -1) {
String selectQuery =
"SELECT * FROM " +
escape("ENTRY") +
" WHERE " +
escape("SHARED_ID") +
" = ?";

try (PreparedStatement preparedSelectStatement = connection.prepareStatement(selectQuery)) {
preparedSelectStatement.setInt(1, sharedID);
try (ResultSet resultSet = preparedSelectStatement.executeQuery()) {
if (resultSet.next()) {
return true;
}
}
StringBuilder selectQuery = new StringBuilder()
.append("SELECT * FROM ")
.append(escape("ENTRY"));

try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery.toString())) {
while (resultSet.next()) {
int id = resultSet.getInt("SHARED_ID");
remoteIds.add(id);
}
}
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
return false;
}
return bibEntries.stream().filter((entry) ->
!remoteIds.contains(entry.getSharedBibEntryData().getSharedID()))
.collect(Collectors.toList());
}

/**
* Inserts the given bibEntry into FIELD table.
*
* @param bibEntry {@link BibEntry} to be inserted
*/
private void insertIntoFieldTable(BibEntry bibEntry) {
protected void insertIntoFieldTable(BibEntry bibEntry) {
try {
// Inserting into FIELD table
// Coerce to ArrayList in order to use List.get()
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,9 @@ public void listen(EntriesAddedEvent event) {
if (isEventSourceAccepted(event) && checkCurrentConnection()) {
synchronizeLocalMetaData();
synchronizeLocalDatabase(); // Pull changes for the case that there were some
List<BibEntry> entries = event.getBibEntries();
for (BibEntry entry : entries) {
dbmsProcessor.insertEntry(entry);
dbmsProcessor.insertEntries(event.getBibEntries());
}
}
}

/**
* Listening method. Updates an existing shared {@link BibEntry}.
Expand Down
79 changes: 79 additions & 0 deletions src/main/java/org/jabref/logic/shared/OracleProcessor.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
package org.jabref.logic.shared;

import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;

import org.jabref.logic.shared.listener.OracleNotificationListener;
import org.jabref.model.database.shared.DatabaseConnection;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;

import oracle.jdbc.OracleConnection;
import oracle.jdbc.OracleStatement;
Expand Down Expand Up @@ -97,6 +103,79 @@ public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) {

}

@Override
protected void insertIntoEntryTable(List<BibEntry> entries) {
try {
// Inserting into ENTRY table
StringBuilder insertEntryQuery = new StringBuilder()
.append("INSERT ALL");
for (BibEntry entry : entries) {
insertEntryQuery.append(" INTO ")
.append(escape("ENTRY"))
.append(" (")
.append(escape("TYPE"))
.append(") VALUES (?)");
}
insertEntryQuery.append(" SELECT * FROM DUAL");
LOGGER.info(insertEntryQuery.toString());
try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertEntryQuery.toString(),
new String[]{"SHARED_ID"})) {
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure, but this looks odd to me with the String[]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just indicates the column where we want to put the auto-generated keys. See https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#prepareStatement(java.lang.String,%20int[])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into Stack Overflow and the Oracle JDBC driver source code and it looks like the Oracle JDBC doesn't support getGeneratedKeys on INSERT ALL statements. So I'll have to find another way of getting the shared IDs.

Copy link
Member

Choose a reason for hiding this comment

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

OMG. This sounds like we should really drop Oracle support. There is no need for Oracle in 2020, is it? - Postgres is the way to go, isn't it?

We should IMHO also drop MySQL support as it does not offer automatic updates on changes on the server.

for (int i = 0; i < entries.size(); i++) {
// columnIndex starts with 1
preparedEntryStatement.setString(i + 1, entries.get(i).getType().getName());
}
preparedEntryStatement.executeUpdate();
try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) {
// The following assumes that we get the generated keys in the order the entries were inserted
// This should be the case
for (BibEntry entry : entries) {
generatedKeys.next();
entry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1));
}
if (generatedKeys.next()) {
LOGGER.error("Error: Some shared IDs left unassigned");
}
}
}
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
}

@Override
protected void insertIntoFieldTable(BibEntry bibEntry) {
try {
// Inserting into FIELD table
// Coerce to ArrayList in order to use List.get()
List<Field> fields = new ArrayList<>(bibEntry.getFields());
StringBuilder insertFieldQuery = new StringBuilder()
.append("INSERT ALL");
for (Field field : fields) {
insertFieldQuery.append(" INTO ")
.append(escape("FIELD"))
.append(" (")
.append(escape("ENTRY_SHARED_ID"))
.append(", ")
.append(escape("NAME"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES (?, ?, ?)");
}
insertFieldQuery.append(" SELECT * FROM DUAL");
try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) {
for (int i = 0; i < fields.size(); i++) {
// columnIndex starts with 1
preparedFieldStatement.setInt((3 * i) + 1, bibEntry.getSharedBibEntryData().getSharedID());
preparedFieldStatement.setString((3 * i) + 2, fields.get(i).getName());
preparedFieldStatement.setString((3 * i) + 3, bibEntry.getField(fields.get(i)).get());
}
preparedFieldStatement.executeUpdate();
}
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
}

@Override
public void stopNotificationListener() {
try {
Expand Down
36 changes: 23 additions & 13 deletions src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.List;

import org.jabref.JabRefExecutorService;
import org.jabref.logic.shared.listener.PostgresSQLNotificationListener;
Expand Down Expand Up @@ -49,25 +50,34 @@ public void setUp() throws SQLException {
}

@Override
protected void insertIntoEntryTable(BibEntry bibEntry) {
// Inserting into ENTRY table
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");

protected void insertIntoEntryTable(List<BibEntry> bibEntries) {
// This is the only method to get generated keys which is accepted by MySQL, PostgreSQL and Oracle.
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");
// Number of commas is bibEntries.size() - 1
for (int i = 0; i < bibEntries.size() - 1; i++) {
insertIntoEntryQuery.append(", (?)");
}
try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(),
Statement.RETURN_GENERATED_KEYS)) {

preparedEntryStatement.setString(1, bibEntry.getType().getName());
Statement.RETURN_GENERATED_KEYS)) {
for (int i = 0; i < bibEntries.size(); i++) {
preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName());
}
preparedEntryStatement.executeUpdate();

try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) {
// The following assumes that we get the generated keys in the order the entries were inserted
// This should be the case
for (BibEntry bibEntry : bibEntries) {
generatedKeys.next();
bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1));
}
if (generatedKeys.next()) {
bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally
LOGGER.error("Error: Some shared IDs left unassigned");
}
}
} catch (SQLException e) {
Expand Down
Loading