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

improve sql querying of tags, fixes #221 #224

Merged
merged 2 commits into from
Apr 7, 2020
Merged

improve sql querying of tags, fixes #221 #224

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66d0b9c
improve sql querying of tags, fixes #221
tyrasd Apr 4, 2020
cc14920
synchronize on prepared statement usage
tyrasd Apr 6, 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
144 changes: 95 additions & 49 deletions ...-util/src/main/java/org/heigit/bigspatialdata/oshdb/util/tagtranslator/TagTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.EnumSet;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand All @@ -20,7 +21,7 @@
* Easily translate your textual tags and roles to OSHDB's internal
* representation (encoded as integers) and vice versa.
*
* This class handles missing/not-found data in the following ways:
* <p>This class handles missing/not-found data in the following ways:</p>
* <ul>
* <li>
* for (tag/role) strings that cannot be found in a keytable, the tagtranslator will generate a
Expand All @@ -33,9 +34,16 @@
* </li>
* </ul>
*/
public class TagTranslator {
public class TagTranslator implements AutoCloseable {
private static final Logger LOG = LoggerFactory.getLogger(TagTranslator.class);

private final PreparedStatement keyIdQuery;
private final PreparedStatement keyTxtQuery;
private final PreparedStatement valueIdQuery;
private final PreparedStatement valueTxtQuery;
private final PreparedStatement roleIdQuery;
private final PreparedStatement roleTxtQuery;

private final Map<OSMTagKey, OSHDBTagKey> keyToInt;
private final Map<OSHDBTagKey, OSMTagKey> keyToString;
private final Map<OSMTag, OSHDBTag> tagToInt;
Expand Down Expand Up @@ -66,12 +74,44 @@ public TagTranslator(Connection conn) throws OSHDBKeytablesNotFoundException {
EnumSet<TableNames> keyTables =
EnumSet.of(TableNames.E_KEY, TableNames.E_KEYVALUE, TableNames.E_ROLE);
for (TableNames table : keyTables) {
try {
this.conn.prepareStatement("select 1 from " + table.toString() + " LIMIT 1").execute();
try (Statement testTablePresentQuery = this.conn.createStatement()) {
testTablePresentQuery.execute("select 1 from " + table.toString() + " LIMIT 1");
} catch (SQLException e) {
throw new OSHDBKeytablesNotFoundException();
}
}

// create prepared statements for querying tags from keytables
try {
keyIdQuery = conn.prepareStatement(
"select ID from " + TableNames.E_KEY.toString() + " where KEY.TXT = ?;");
keyTxtQuery = conn.prepareStatement(
"select TXT from " + TableNames.E_KEY.toString() + " where KEY.ID = ?;");
valueIdQuery = conn.prepareStatement("select k.ID as KEYID,kv.VALUEID as VALUEID"
+ " from " + TableNames.E_KEYVALUE.toString() + " kv"
+ " inner join " + TableNames.E_KEY.toString() + " k on k.ID = kv.KEYID"
+ " where k.TXT = ? and kv.TXT = ?;");
valueTxtQuery = conn.prepareStatement("select k.TXT as KEYTXT,kv.TXT as VALUETXT"
+ " from " + TableNames.E_KEYVALUE.toString() + " kv"
+ " inner join " + TableNames.E_KEY.toString() + " k on k.ID = kv.KEYID"
+ " where k.ID = ? and kv.VALUEID = ?;");
roleIdQuery = conn
.prepareStatement("select ID from " + TableNames.E_ROLE.toString() + " where TXT = ?;");
roleTxtQuery = conn
.prepareStatement("select TXT from " + TableNames.E_ROLE.toString() + " where ID = ?;");
} catch (SQLException e) {
throw new OSHDBKeytablesNotFoundException();
Copy link
Contributor

Choose a reason for hiding this comment

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

We tested for the existence of the tables above, right? As a nitpicker I would prefer a different error or at least some explanatory string passed to the exception?

Copy link
Member

Choose a reason for hiding this comment

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

How is the TagTranslator used with the ohsome-api, does it create a new Translator for every request or does it reuse an existing one?

Copy link
Member Author

Choose a reason for hiding this comment

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

We tested for the existence of the tables above, right? As a nitpicker I would prefer a different error or at least some explanatory string passed to the exception?

True, this would trigger most likely in the case where the keytables database and tables are present, but would not have the correct columns. In which they could be called "corrupted" maybe…. It's a very unlikely error case in normal use, though.

Anyway, it's not really related to the issue this PR is trying to resolve. Can you perhaps open a separate ticket?

Copy link
Member Author

Choose a reason for hiding this comment

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

How is the TagTranslator used with the ohsome-api, does it create a new Translator for every request or does it reuse an existing one?

I'm not sure how this relates to the comment by @SlowMo24… Did you mean to post it as a separate question, @rtroilo ?

In any case: If I'm not mistaken, the ohsome-api ends up having one TagTranslator for "normal" use (i.e. translating incoming tags from request parameters), and many TagTranslators for data extraction (translating tags for the GeoJSON output): one per thread per request. The "normal" tag translator is reused for all requests, if I remember correctly, while the ones used for data extraction requests are thrown away after the respective "parent" request is completed.

Does that matter for this PR?

}
}

@Override
public void close() throws Exception {
keyIdQuery.close();
keyTxtQuery.close();
valueIdQuery.close();
valueTxtQuery.close();
roleIdQuery.close();
roleTxtQuery.close();
}

/**
Expand All @@ -95,12 +135,14 @@ public OSHDBTagKey getOSHDBTagKeyOf(OSMTagKey key) {
return this.keyToInt.get(key);
}
OSHDBTagKey keyInt;
try (PreparedStatement keystmt = this.conn.prepareStatement(
"select ID from " + TableNames.E_KEY.toString() + " where KEY.TXT = ?;")) {
keystmt.setString(1, key.toString());
ResultSet keys = keystmt.executeQuery();
keys.next();
keyInt = new OSHDBTagKey(keys.getInt("ID"));
try {
synchronized (keyIdQuery) {
keyIdQuery.setString(1, key.toString());
try (ResultSet keys = keyIdQuery.executeQuery()) {
keys.next();
keyInt = new OSHDBTagKey(keys.getInt("ID"));
}
}
} catch (SQLException ex) {
LOG.info("Unable to find tag key {} in keytables.", key);
keyInt = new OSHDBTagKey(getFakeId(key.toString()));
Expand Down Expand Up @@ -133,13 +175,15 @@ public OSMTagKey getOSMTagKeyOf(OSHDBTagKey key) {
return this.keyToString.get(key);
}
OSMTagKey keyString;
try (PreparedStatement keystmt =
this.conn.prepareStatement("select TXT from KEY where KEY.ID = ?;")) {
keystmt.setInt(1, key.toInt());
ResultSet keys = keystmt.executeQuery();
keys.next();
keyString = new OSMTagKey(keys.getString("TXT"));
this.keyToInt.put(keyString, key);
try {
synchronized (keyTxtQuery) {
keyTxtQuery.setInt(1, key.toInt());
try (ResultSet keys = keyTxtQuery.executeQuery()) {
keys.next();
keyString = new OSMTagKey(keys.getString("TXT"));
}
this.keyToInt.put(keyString, key);
}
} catch (SQLException ex) {
throw new OSHDBTagOrRoleNotFoundException(String.format(
"Unable to find tag key id %d in keytables.", key.toInt()
Expand Down Expand Up @@ -173,16 +217,15 @@ public OSHDBTag getOSHDBTagOf(OSMTag tag) {
}
OSHDBTag tagInt;
// key or value is not in cache so let's go toInt them
try (PreparedStatement valstmt =
this.conn.prepareStatement("select k.ID as KEYID,kv.VALUEID as VALUEID "
+ "from " + TableNames.E_KEYVALUE.toString() + " kv "
+ "inner join " + TableNames.E_KEY.toString() + " k on k.ID = kv.KEYID "
+ "WHERE k.TXT = ? and kv.TXT = ?;")) {
valstmt.setString(1, tag.getKey());
valstmt.setString(2, tag.getValue());
ResultSet values = valstmt.executeQuery();
values.next();
tagInt = new OSHDBTag(values.getInt("KEYID"), values.getInt("VALUEID"));
try {
synchronized (valueIdQuery) {
valueIdQuery.setString(1, tag.getKey());
valueIdQuery.setString(2, tag.getValue());
try (ResultSet values = valueIdQuery.executeQuery()) {
values.next();
tagInt = new OSHDBTag(values.getInt("KEYID"), values.getInt("VALUEID"));
}
}
} catch (SQLException ex) {
LOG.info("Unable to find tag {}={} in keytables.", tag.getKey(), tag.getValue());
tagInt = new OSHDBTag(this.getOSHDBTagKeyOf(tag.getKey()).toInt(), getFakeId(tag.getValue()));
Expand Down Expand Up @@ -219,15 +262,15 @@ public OSMTag getOSMTagOf(OSHDBTag tag) {
OSMTag tagString;

// key or value is not in cache so let's go toInt them
try (PreparedStatement valstmt =
this.conn.prepareStatement("select k.TXT as KEYTXT,kv.TXT as VALUETXT from "
+ TableNames.E_KEYVALUE.toString() + " kv inner join " + TableNames.E_KEY.toString()
+ " k on k.ID = kv.KEYID WHERE k.ID = ? and kv.VALUEID = ?;")) {
valstmt.setInt(1, tag.getKey());
valstmt.setInt(2, tag.getValue());
ResultSet values = valstmt.executeQuery();
values.next();
tagString = new OSMTag(values.getString("KEYTXT"), values.getString("VALUETXT"));
try {
synchronized (valueTxtQuery) {
valueTxtQuery.setInt(1, tag.getKey());
valueTxtQuery.setInt(2, tag.getValue());
try (ResultSet values = valueTxtQuery.executeQuery()) {
values.next();
tagString = new OSMTag(values.getString("KEYTXT"), values.getString("VALUETXT"));
}
}
} catch (SQLException ex) {
throw new OSHDBTagOrRoleNotFoundException(String.format(
"Unable to find tag id %d=%d in keytables.",
Expand Down Expand Up @@ -261,12 +304,14 @@ public OSHDBRole getOSHDBRoleOf(OSMRole role) {
return this.roleToInt.get(role);
}
OSHDBRole roleInt;
try (PreparedStatement rolestmt = conn
.prepareStatement("select ID from " + TableNames.E_ROLE.toString() + " WHERE txt = ?;")) {
rolestmt.setString(1, role.toString());
ResultSet roles = rolestmt.executeQuery();
roles.next();
roleInt = new OSHDBRole(roles.getInt("ID"));
try {
synchronized (roleIdQuery) {
roleIdQuery.setString(1, role.toString());
try (ResultSet roles = roleIdQuery.executeQuery()) {
roles.next();
roleInt = new OSHDBRole(roles.getInt("ID"));
}
}
} catch (SQLException ex) {
LOG.info("Unable to find role {} in keytables.", role);
roleInt = new OSHDBRole(getFakeId(role.toString()));
Expand Down Expand Up @@ -299,13 +344,14 @@ public OSMRole getOSMRoleOf(OSHDBRole role) {
return this.roleToString.get(role);
}
OSMRole roleString;
try (PreparedStatement Rolestmt = conn.prepareStatement(
"select TXT from " + TableNames.E_ROLE.toString() + " WHERE ID = ?;"
)) {
Rolestmt.setInt(1, role.toInt());
ResultSet Roles = Rolestmt.executeQuery();
Roles.next();
roleString = new OSMRole(Roles.getString("TXT"));
try {
synchronized (roleTxtQuery) {
roleTxtQuery.setInt(1, role.toInt());
try (ResultSet roles = roleTxtQuery.executeQuery()) {
roles.next();
roleString = new OSMRole(roles.getString("TXT"));
}
}
} catch (SQLException ex) {
throw new OSHDBTagOrRoleNotFoundException(String.format(
"Unable to find role id %d in keytables.", role.toInt()
Expand Down