Native OO API implementations #19

vasiliy-yashkov · 2018-10-29T14:38:28Z

New OO API interfaces implementations and tests for them.
Classes are in a separate package org.firebirdsql.nativeoo.gds.ng by analogy with existing JNA implementations. Some batch interfaces (FbBatch, etc.) have been added to the wire layer and can be used for wire implementations. Connection through new interfaces is carried out through its own protocols: jdbc:firebirdsql:fboo: [embedded:|locale:|native:].
Interface FbInterface extends FbClientLibrary, while the library has only the definition of the interface IMaster.
FbInterface was generated by modified cloop.

New OO API interfaces implementations and tests for them. Classes are in a separate package org.firebirdsql.nativeoo.gds.ng by analogy with existing JNA implementations. Some batch interfaces (FbBatch, etc.) have been added to the wire layer and can be used for wire implementations. Connection through new interfaces is carried out through its own protocols: jdbc:firebirdsql:fboo: [embedded:|locale:|native:]. Interface FbInterface extends FbClientLibrary, while the library has only the definition of the interface IMaster. FbInterface was generated by modified cloop.

mrotteveel · 2018-10-29T19:29:32Z

Thanks, as I mentioned on the mailing list, I'll try to review it this weekend.

mrotteveel · 2018-10-29T19:37:14Z

src/jna-client/org/firebirdsql/gds/ng/jna/AbstractNativeDatabaseFactory.java

@@ -35,14 +32,14 @@
 public abstract class AbstractNativeDatabaseFactory implements FbDatabaseFactory {

 @Override
- public JnaDatabase connect(IConnectionProperties connectionProperties) throws SQLException {
+ public FbDatabase connect(IConnectionProperties connectionProperties) throws SQLException {


Why did you remove the covariant inheritance here? This also introduces casts in TestJnaDatabase and TestJnaBlob (and other tests) that were previously unnecessary.

I forgot. It was a rough initial version, need to reverse it.

mrotteveel

I think that this should be a separate library entirely: both the fboo bindings, and this batch API.

My main concern is that I don't think the proposed batch API (and related interface/classes) belongs as part of the gds-ng API: it doesn't mesh well with the JDBC part of the driver and it will need significant changes to be usable from JDBC. It also deviates significantly from the normal metadata usage (field descriptor, etc) in gds-ng.

My secondary concern is the additional burden of taking on the maintenance of the OO API bindings as part of Jaybird. I hadn't realized earlier the amount of code involved.

mrotteveel · 2018-10-29T19:39:51Z

src/nativeoo/org/firebirdsql/nativeoo/gds/ng/FbException.java

+ *
+ * @since 4.0
+ */
+public class FbException extends SQLException {


Does this exception really need to exist; the SQLException hierarchy is pretty complex, and this flattens it back to a single exception given the implementation in checkException

Yes, I'll reconsider the importance of class. This class was created to satisfy Cloop requirements when generating interfaces. I have to change the definition of interfaces, maybe.

mrotteveel · 2018-10-29T19:42:56Z

src/nativeoo/org/firebirdsql/nativeoo/gds/ng/FbException.java

+
+ if (!builder.isEmpty()) {
+ SQLException exception = builder.toSQLException();
+ throw new FbException(exception.getMessage(), exception.getSQLState(), exception.getErrorCode(),


This removes the subclasses that FbExceptionBuilder may generate depending on the error code and sql state, in addition it loses other exception information that the builder produces. That is not a desirable solution. If possible this class should be removed, and otherwise it should not extend SQLException and contain the SQLException as its cause.

Here I'll try to convert IStatus message to ISC_STATUS in order to use processStatusVector method to generate SQLException. Right?

mrotteveel · 2018-10-29T19:50:47Z

src/main/org/firebirdsql/gds/BatchParameterBuffer.java

+ * @since 4.0
+ */
+public interface BatchParameterBuffer extends ParameterBuffer {
+


Remove methods already defined in ParameterBuffer, or revise method documentation to be specific to batch parameter usage.

mrotteveel · 2018-11-04T10:57:22Z

src/jna-client/org/firebirdsql/gds/ng/jna/AbstractNativeDatabaseFactory.java

@@ -35,14 +32,14 @@
 public abstract class AbstractNativeDatabaseFactory implements FbDatabaseFactory {

 @Override
- public JnaDatabase connect(IConnectionProperties connectionProperties) throws SQLException {
+ public FbDatabase connect(IConnectionProperties connectionProperties) throws SQLException {


As discussed, revert back to covariant inheritance

mrotteveel · 2018-11-04T10:58:40Z

src/jna-client/org/firebirdsql/gds/ng/jna/JnaDatabase.java

@@ -471,6 +471,21 @@ public void cancelEvent(EventHandle eventHandle) throws SQLException {
 }
 }

+ @Override
+ public FbBatch createBatch(FbTransaction transaction, String statement, FbMessageMetadata metadata, BatchParameterBuffer parameters) throws SQLException {
+ throw new SQLException("Not implemented");


Use FBDriverNotCapableException (also for the other two methods)

mrotteveel · 2018-11-04T11:54:03Z

src/main/org/firebirdsql/gds/ng/FbBatch.java

+ /**
+ * Register existing blob.
+ */
+ void registerBlob(long existingBlob, long blobId) throws SQLException;


What is the meaning and difference between existingBlob and blobId?

existingBlob has ID assigned by engine (when GDSHelper's createBlob method is called). blobId is user defined. registerBlob is used when BLOB_STREAM parameter is set as TAG_BLOB_POLICY.

mrotteveel · 2018-11-04T11:54:28Z

src/main/org/firebirdsql/gds/ng/FbBatch.java

+ /**
+ * Add blob in this batch.
+ */
+ FbBlob addBlob(byte[] inBuffer, long blobId, BlobParameterBuffer buffer) throws SQLException;


How can a caller decide on a blobId?

If TAG_BLOB_POLICY is set to BLOB_ID_ENGINE, then caller can not specify an blobId, otherwise, can specify existing blobId.

mrotteveel · 2018-11-04T12:11:28Z

src/main/org/firebirdsql/gds/ng/FbDatabase.java

+ * @return Instance of {@link FbBatch}
+ * @throws SQLException
+ */
+ FbBatch createBatch(FbTransaction transaction, String statement, FbMessageMetadata metadata, BatchParameterBuffer parameters) throws SQLException;


I don't think this belongs in FbDatabase, it should be part of FbStatement if anything.

However, in its current form, I don't think that these methods should be part of the gds-ng API. I see a lot of mismatches with the requirements for JDBC batch execution and the normal usage of the gds-ng API.

Making this part of the gds-ng API now, will only lead to a lot of rearchitecturing and breaking API changes later when I will try to make a uniform API that can be used from the JDBC part of the driver.

I'll try to think about it. Are there any templates of uniform interface?

mrotteveel · 2018-11-04T12:12:40Z

src/test/org/firebirdsql/gds/ng/wire/version10/TestV10Database.java

@@ -434,7 +434,8 @@ private FbTransaction getTransaction(FbDatabase db) throws SQLException {
 private static void safelyClose(FbDatabase db) {
 if (db == null) return;
 try {
- db.close();
+ if (db.isAttached())


Why this change?

To avoid 'is not attached to a database' message.

mrotteveel · 2018-11-04T12:15:16Z

src/test/org/firebirdsql/gds/impl/jni/TestServicesAPI.java

-
- @ClassRule
- public static final GdsTypeRule testType = GdsTypeRule.supports(EmbeddedGDSFactoryPlugin.EMBEDDED_TYPE_NAME);
+public abstract class TestServicesAPI {


I think the changes to this class (and the creation of its subclasses) can be avoided by making proper use of the GdsTypeRule and FBTestProperties.

I'll try to do it.

1. Removed FbException handling class. Now, to form an exception, the status vector is processed after native methods calls and an SQLException is generated. 2. Exception handling works with the specified database encoding. 3. Methods that do not have batches implementations throw FBDriverNotCapableException instead of SQLException. 4. The batch generation mechanisms is similar to the implementation of prepared statements: 4.1. The batch generates field descriptors based on the prepared statement. 4.2 If the batch is created from external metadata, then it makes field descriptors like a prepared statement. 4.3 The batch uses methods to set the request fields like a prepared statement. The conversion of data of these fields in this case is carried out by FBField class, which uses database encoding and other parameters. 5. Changed test class names. Instead of TestXXX, to XXXTest. 6. Using GdsTypeRule for native tests. 7. Minor bugs fixed.

1. Updated interfaces `FbInterface.java` in accordance with the current firebird 4 implementation. Removed old and unnecessary interface implementations. `fb_get_master_interface()` method moved from FbClientLibrary to separate old and new implementations. 2. Fixed setting `null` flag in message data of `IStatement` if the field data was `null`. 3. Removed excess imports of `org.firebirdsql.nativeoo.gds.ng.FbInterface` from `org.firebirdsql.gds` layer. 4. The batch parameter buffer now contains constants instead of the `FbBatch` interface. 5. Reimplemented event handling with old `isc_*` calls. 6. JNA deprecated function `loadLibrary` replaced by `load`. 7. Other corrections and additions related to main code of the driver.

… implementations Additional changes: 1. Fix preparation of metadata when executing statement. 2. Add GDS type rule for new native support. 3. Replace SQL_DEC_FIXED with SQL_INT128. 4. Update gradle build config. 5. Add small comments.

1. Update interfaces for `fbclient` library. 2. Fix memory allocation for error message. 3. Fix parsing of warnings from status vector. 4. Fix `free` call when closing statement. 5. Update and fix tests.

…tation

…implementations

Move implementation to `jaybird-native` dependency.

…o_batch

… of getting field

…o_batch

…h blobs in native batch Also fix creation of `org.firebirdsql.jdbc.FBBlob` instances.

mrotteveel reviewed Oct 29, 2018

View reviewed changes

mrotteveel reviewed Nov 4, 2018

View reviewed changes

mrotteveel added the CLA Signed label Jan 23, 2019

vasiliy-yashkov and others added 19 commits January 28, 2020 14:50

Merge branch 'master' into native_oo_batch

f117647

Merge remote-tracking branch 'origin/master' into native_oo_batch

15b0855

Merge remote-tracking branch 'origin/master' into native_oo_batch

e0d14ad

Update native OO API implementations

7b1f4ac

Merge branch 'master' into native_oo_batch

826f698

Update native OO API implementations

d79b0a5

1. Update interfaces for `fbclient` library. 2. Fix memory allocation for error message. 3. Fix parsing of warnings from status vector. 4. Fix `free` call when closing statement. 5. Update and fix tests.

Merge branch 'master' into native_oo_batch

bf59522

Add support for batch updates to the native OO API statement implemen…

f8f18e5

…tation

Merge remote-tracking branch 'origin/master' into native_oo_batch

5bb1b93

Use ReentrantLock instead of synchronized object for native OO API …

a57ca08

…implementations

Update implementation of native OO API

c122329

Move implementation to `jaybird-native` dependency.

Merge commit '7c3652071766feba1c3e356a292632d1f4c1c1f9' into native_o…

e874a6f

…o_batch

Use correct invalid descriptor field identifier for exception in case…

b4e6ade

… of getting field

Minor cleanup and comments

a6fdaba

Merge commit '7a493a89a79e058b171b8dde8e7e37aa0cd4d3d6' into native_o…

63cd757

…o_batch

Use org.firebirdsql.jdbc.FirebirdBlob as arguments when working wit…

11ee8f0

…h blobs in native batch Also fix creation of `org.firebirdsql.jdbc.FBBlob` instances.

Do not check connection when cancel operation in case of forced abort

0d55f0d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native OO API implementations #19

Native OO API implementations #19

vasiliy-yashkov commented Oct 29, 2018

mrotteveel commented Oct 29, 2018

mrotteveel Oct 29, 2018 •

edited

Loading

vasiliy-yashkov Oct 30, 2018

mrotteveel left a comment

mrotteveel Oct 29, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Oct 29, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Oct 29, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

mrotteveel Nov 4, 2018

vasiliy-yashkov Nov 5, 2018

Native OO API implementations #19

Are you sure you want to change the base?

Native OO API implementations #19

Conversation

vasiliy-yashkov commented Oct 29, 2018

mrotteveel commented Oct 29, 2018

mrotteveel Oct 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrotteveel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrotteveel Oct 29, 2018 •

edited

Loading