You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metamodel.apache.org by kaspersorensen <gi...@git.apache.org> on 2017/10/18 11:40:06 UTC
[GitHub] metamodel pull request #164: METAMODEL-86: Allow transactional querying in J...
GitHub user kaspersorensen opened a pull request:
https://github.com/apache/metamodel/pull/164
METAMODEL-86: Allow transactional querying in JDBC update scripts
Here's my attempt at solving METAMODEL-86. I wasn't sure how to test it so ... It's not really tested, except existing tests should pass obviously. Review accordingly, please :-)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kaspersorensen/metamodel METAMODEL-86-query-in-jdbc-transaction
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metamodel/pull/164.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #164
----
commit cd78f067ba32f684f78f533b6e842230315ed801
Author: Kasper Sørensen <i....@gmail.com>
Date: 2017-10-18T11:37:24Z
METAMODEL-86: Allow transactional querying in JDBC update scripts
----
---
[GitHub] metamodel pull request #164: METAMODEL-86: Allow transactional querying in J...
Posted by jakubneubauer <gi...@git.apache.org>.
Github user jakubneubauer commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/164#discussion_r147782884
--- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/JdbcUpdateCallback.java ---
@@ -156,20 +158,25 @@ public final TableCreationBuilder createTable(Schema schema, String name) throws
@Override
public final RowInsertionBuilder insertInto(Table table) throws IllegalArgumentException, IllegalStateException {
- return new JdbcInsertBuilder(this, table, getDataContext().getQueryRewriter());
+ return new JdbcInsertBuilder(this, table, getJdbcDataContext().getQueryRewriter());
+ }
+
+ protected final JdbcDataContext getJdbcDataContext() {
+ return (JdbcDataContext) super.getDataContext();
}
// override the return type to the more specific subtype.
@Override
- public final JdbcDataContext getDataContext() {
- return (JdbcDataContext) super.getDataContext();
+ public final DataContext getDataContext() {
+ final Connection connection = getConnection();
+ return new JdbcUpdateCallbackDataContext(getJdbcDataContext(), connection);
--- End diff --
it is fine I think, it was just neutral observation ;-)
---
[GitHub] metamodel pull request #164: METAMODEL-86: Allow transactional querying in J...
Posted by jakubneubauer <gi...@git.apache.org>.
Github user jakubneubauer commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/164#discussion_r147759008
--- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/JdbcUpdateCallback.java ---
@@ -156,20 +158,25 @@ public final TableCreationBuilder createTable(Schema schema, String name) throws
@Override
public final RowInsertionBuilder insertInto(Table table) throws IllegalArgumentException, IllegalStateException {
- return new JdbcInsertBuilder(this, table, getDataContext().getQueryRewriter());
+ return new JdbcInsertBuilder(this, table, getJdbcDataContext().getQueryRewriter());
+ }
+
+ protected final JdbcDataContext getJdbcDataContext() {
+ return (JdbcDataContext) super.getDataContext();
}
// override the return type to the more specific subtype.
@Override
- public final JdbcDataContext getDataContext() {
- return (JdbcDataContext) super.getDataContext();
+ public final DataContext getDataContext() {
+ final Connection connection = getConnection();
+ return new JdbcUpdateCallbackDataContext(getJdbcDataContext(), connection);
--- End diff --
Ok, this hack took me a while before I got the idea :-) Clever trick how to make the associated DataContext use our connection
---
[GitHub] metamodel pull request #164: METAMODEL-86: Allow transactional querying in J...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metamodel/pull/164
---
[GitHub] metamodel issue #164: METAMODEL-86: Allow transactional querying in JDBC upd...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on the issue:
https://github.com/apache/metamodel/pull/164
Update: I found a way to unit test this and a good thing I did too. The approach I had built wasn't really working since the query method I used closed connection. That's fixed now. Furthermore the unit test revealed that such update scripts can easily cause deadlocks at the DB level. These will be rolled back, but the client needs a good way to catch them, which is why I added the `RolledBackUpdateException`.
---
[GitHub] metamodel pull request #164: METAMODEL-86: Allow transactional querying in J...
Posted by kaspersorensen <gi...@git.apache.org>.
Github user kaspersorensen commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/164#discussion_r147773342
--- Diff: jdbc/src/main/java/org/apache/metamodel/jdbc/JdbcUpdateCallback.java ---
@@ -156,20 +158,25 @@ public final TableCreationBuilder createTable(Schema schema, String name) throws
@Override
public final RowInsertionBuilder insertInto(Table table) throws IllegalArgumentException, IllegalStateException {
- return new JdbcInsertBuilder(this, table, getDataContext().getQueryRewriter());
+ return new JdbcInsertBuilder(this, table, getJdbcDataContext().getQueryRewriter());
+ }
+
+ protected final JdbcDataContext getJdbcDataContext() {
+ return (JdbcDataContext) super.getDataContext();
}
// override the return type to the more specific subtype.
@Override
- public final JdbcDataContext getDataContext() {
- return (JdbcDataContext) super.getDataContext();
+ public final DataContext getDataContext() {
+ final Connection connection = getConnection();
+ return new JdbcUpdateCallbackDataContext(getJdbcDataContext(), connection);
--- End diff --
Clever good or clever bad? :-) Are we good to merge you think?
---
[GitHub] metamodel issue #164: METAMODEL-86: Allow transactional querying in JDBC upd...
Posted by jakubneubauer <gi...@git.apache.org>.
Github user jakubneubauer commented on the issue:
https://github.com/apache/metamodel/pull/164
Now I checked the rest. Also executed few tests on real database:
- The connection obtained from the UpdateScript used for queries is the same as used for updates - OK
- Transaction is not committed during the UpdateScript duration nor after updates/inserts/deletes nor after the queries.
- On the other getting connection on DataContext outside of UpdateScript returns new connection - OK
- Throwing any exception from the update script causes rollback.
Code looks fine, I recommend merge :-)
---