You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Daniel Barclay <db...@maprtech.com> on 2015/04/17 19:08:58 UTC
Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/
-----------------------------------------------------------
Review request for drill, Mehant Baid and Parth Chandra.
Repository: drill-git
Description
-------
- Added unit test.
- Added implementations of transaction-related methods:
- setAutoCommit - reject attempt to turn auto-commit off
- commit - reject when in auto-commit mode (which is always)
- rollback - reject when in auto-commit mode (which is always)
- other mode and metadata methods - roughly, report "no transactions"
- Added method declarations with doc. comments in Drill-specific interface.
Diffs
-----
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/33291/diff/
Testing
-------
Ran new specific tests.
Ran existing tests; no new problems.
Thanks,
Daniel Barclay
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Parth Chandra <pc...@maprtech.com>.
> On April 21, 2015, 4:36 a.m., Parth Chandra wrote:
> > What should our option be to handle clients that do not handle the exceptions thrown by these methods? We could check an environment variable and if it is set, then silently return from any of these calls instead of throwing an exception?
>
> Daniel Barclay wrote:
> Yes, we likely need to have multiple modes in order to handle multiple clients, e.g., this change's fail-fast/reliable mode and the mode you describe (the "Yeah, sure, whatever" mode? :-) ).
>
> I don't think the configuration mechanism should be an environment variable (unless Drill already uses environment variables for configuration flags).
>
> It should probably be a Java system property plus a JDBC URL parameter (something to allow both modes to exist in the same JVM at the same time (for testing, client testing, etc.)).
>
> Do you think we need to implement "Yeah, sure, whatever" mode and the flag now (for DRILL-2782)?
Yes I think we should implement it. Using an env variable has the advantage that it fits in nicely and simply with the script that launches sqlline.
- Parth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/#review80903
-----------------------------------------------------------
On April 20, 2015, 3:03 a.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33291/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 3:03 a.m.)
>
>
> Review request for drill, Mehant Baid and Parth Chandra.
>
>
> Bugs: DRILL-2782
> https://issues.apache.org/jira/browse/DRILL-2782
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> - Added unit test.
> - Added implementations of transaction-related methods:
> - setAutoCommit - reject attempt to turn auto-commit off
> - commit - reject when in auto-commit mode (which is always)
> - rollback - reject when in auto-commit mode (which is always)
> - other mode and metadata methods - roughly, report "no transactions"
> - Added method declarations with doc. comments in Drill-specific interface.
>
>
> Diffs
> -----
>
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
> exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33291/diff/
>
>
> Testing
> -------
>
> Ran new specific tests.
>
> Ran existing tests; no new problems.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Daniel Barclay <db...@maprtech.com>.
> On April 21, 2015, 4:36 a.m., Parth Chandra wrote:
> > What should our option be to handle clients that do not handle the exceptions thrown by these methods? We could check an environment variable and if it is set, then silently return from any of these calls instead of throwing an exception?
Yes, we likely need to have multiple modes in order to handle multiple clients, e.g., this change's fail-fast/reliable mode and the mode you describe (the "Yeah, sure, whatever" mode? :-) ).
I don't think the configuration mechanism should be an environment variable (unless Drill already uses environment variables for configuration flags).
It should probably be a Java system property plus a JDBC URL parameter (something to allow both modes to exist in the same JVM at the same time (for testing, client testing, etc.)).
Do you think we need to implement "Yeah, sure, whatever" mode and the flag now (for DRILL-2782)?
> On April 21, 2015, 4:36 a.m., Parth Chandra wrote:
> > exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java, line 68
> > <https://reviews.apache.org/r/33291/diff/2/?file=935091#file935091line68>
> >
> > Per the spec, one cannot call setTransactionLevel with TRANSACTION_NONE. Doesn't the parent method throw an exception?
No, AvaticaConnection.getTransactionIsolation(...) does not throw an exception when that executes.
- Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/#review80903
-----------------------------------------------------------
On April 20, 2015, 3:03 a.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33291/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 3:03 a.m.)
>
>
> Review request for drill, Mehant Baid and Parth Chandra.
>
>
> Bugs: DRILL-2782
> https://issues.apache.org/jira/browse/DRILL-2782
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> - Added unit test.
> - Added implementations of transaction-related methods:
> - setAutoCommit - reject attempt to turn auto-commit off
> - commit - reject when in auto-commit mode (which is always)
> - rollback - reject when in auto-commit mode (which is always)
> - other mode and metadata methods - roughly, report "no transactions"
> - Added method declarations with doc. comments in Drill-specific interface.
>
>
> Diffs
> -----
>
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
> exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33291/diff/
>
>
> Testing
> -------
>
> Ran new specific tests.
>
> Ran existing tests; no new problems.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/#review80903
-----------------------------------------------------------
What should our option be to handle clients that do not handle the exceptions thrown by these methods? We could check an environment variable and if it is set, then silently return from any of these calls instead of throwing an exception?
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java
<https://reviews.apache.org/r/33291/#comment131070>
Per the spec, one cannot call setTransactionLevel with TRANSACTION_NONE. Doesn't the parent method throw an exception?
- Parth Chandra
On April 20, 2015, 3:03 a.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33291/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 3:03 a.m.)
>
>
> Review request for drill, Mehant Baid and Parth Chandra.
>
>
> Bugs: DRILL-2782
> https://issues.apache.org/jira/browse/DRILL-2782
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> - Added unit test.
> - Added implementations of transaction-related methods:
> - setAutoCommit - reject attempt to turn auto-commit off
> - commit - reject when in auto-commit mode (which is always)
> - rollback - reject when in auto-commit mode (which is always)
> - other mode and metadata methods - roughly, report "no transactions"
> - Added method declarations with doc. comments in Drill-specific interface.
>
>
> Diffs
> -----
>
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
> exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33291/diff/
>
>
> Testing
> -------
>
> Ran new specific tests.
>
> Ran existing tests; no new problems.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/#review81411
-----------------------------------------------------------
Ship it!
LGTM
- Parth Chandra
On April 23, 2015, 9:16 p.m., Daniel Barclay wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33291/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 9:16 p.m.)
>
>
> Review request for drill, Mehant Baid and Parth Chandra.
>
>
> Bugs: DRILL-2782
> https://issues.apache.org/jira/browse/DRILL-2782
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> - Added unit test.
> - Added implementations of transaction-related methods:
> - setAutoCommit - reject attempt to turn auto-commit off
> - commit - reject when in auto-commit mode (which is always)
> - rollback - reject when in auto-commit mode (which is always)
> - other mode and metadata methods - roughly, report "no transactions"
> - Added method declarations with doc. comments in Drill-specific interface.
> - Overrode SQLLine's default transaction isolation level to Drill's TRANSACTION_NONE.
>
>
> Diffs
> -----
>
> distribution/src/resources/sqlline 0852fba
> distribution/src/resources/sqlline.bat 755526c
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
> exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
> exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33291/diff/
>
>
> Testing
> -------
>
> Ran new specific tests.
>
> Ran existing tests; no new problems.
>
>
> Thanks,
>
> Daniel Barclay
>
>
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/
-----------------------------------------------------------
(Updated April 23, 2015, 9:16 p.m.)
Review request for drill, Mehant Baid and Parth Chandra.
Changes
-------
Adjusted SQLLine scripts to avoid error message; updated a few missed comments.
Bugs: DRILL-2782
https://issues.apache.org/jira/browse/DRILL-2782
Repository: drill-git
Description (updated)
-------
- Added unit test.
- Added implementations of transaction-related methods:
- setAutoCommit - reject attempt to turn auto-commit off
- commit - reject when in auto-commit mode (which is always)
- rollback - reject when in auto-commit mode (which is always)
- other mode and metadata methods - roughly, report "no transactions"
- Added method declarations with doc. comments in Drill-specific interface.
- Overrode SQLLine's default transaction isolation level to Drill's TRANSACTION_NONE.
Diffs (updated)
-----
distribution/src/resources/sqlline 0852fba
distribution/src/resources/sqlline.bat 755526c
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/33291/diff/
Testing
-------
Ran new specific tests.
Ran existing tests; no new problems.
Thanks,
Daniel Barclay
Re: Review Request 33291: DRILL-2782: 2-Core: Decide,
implement behavior for transaction-related JDBC methods.
Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33291/
-----------------------------------------------------------
(Updated April 20, 2015, 3:03 a.m.)
Review request for drill, Mehant Baid and Parth Chandra.
Bugs: DRILL-2782
https://issues.apache.org/jira/browse/DRILL-2782
Repository: drill-git
Description
-------
- Added unit test.
- Added implementations of transaction-related methods:
- setAutoCommit - reject attempt to turn auto-commit off
- commit - reject when in auto-commit mode (which is always)
- rollback - reject when in auto-commit mode (which is always)
- other mode and metadata methods - roughly, report "no transactions"
- Added method declarations with doc. comments in Drill-specific interface.
Diffs (updated)
-----
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnection.java a52644d
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 3fdbf84
exec/jdbc/src/test/java/org/apache/drill/jdbc/ConnectionTransactionMethodsTest.java PRE-CREATION
exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/33291/diff/
Testing
-------
Ran new specific tests.
Ran existing tests; no new problems.
Thanks,
Daniel Barclay