Hi,

Looking at the code, it doesn't seem that the Transaction object will handle errors if the 'ne' bit is set in the connection. Is this right? How can I handle errors in the transaction other than enabling exceptions? Is checking for the last error number good enough? What if the connection is set to auto-reconnect?

Thanks in advance,

Bruno

[The `Transaction` object][0] exists because exceptions may be happening and you want your transactions to be safely rolled back in [RAII][1] fashion when that happens. In the absence of exceptions, I'm not sure what sort of value you're expecting it to provide.

If you use `Transaction` without MySQL++ exceptions enabled, then of course you must do your own error checking through other objects. (Most likely the `Connection` or `Result` objects.)

Here are a few observations that you may have missed:

1. `Transaction` doesn't even look at the `throw_exceptions` flag. It behaves the same way regardless, because MySQL++ exceptions might not be the only type being thrown in your program. (e.g. `runtime_error` from `std::vector`.)

2. Given that `Transaction` does not normally roll back exceptions in the face of a DB error until it dies (again, RAII) that means `Transaction` would be the wrong place to be looking for error checking methods. The object's dead; you can't call such methods on it. Thus, you're back to checking other longer-lived objects for error results.

Maybe you should try to patch `Transaction` to make it behave the way you expect, then post the code here for discussion. I suspect that if you try this, you'll get partway into the project and then realize that you're down a blind alley, but who knows, maybe you've got an enlightening idea that could improve MySQL++.

[0]: https://tangentsoft.com/mysqlpp/doc/html/refman/classmysqlpp_1_1Transaction.html
[1]: https://en.wikipedia.org/wiki/Resource_acquisition_is_initialization

> In the absence of exceptions, I'm not sure what sort of value you're expecting it to provide.

Any type of other application error, e.g.:

- Start transaction
- Read row 1
- Modify row 1
- Read row 2
- Parse row 2 contents and if value X isn't positive, return (i.e.: rollback)
- Commit

> because MySQL++ exceptions might not be the only type being thrown in your program. (e.g. runtime_error from std::vector.)

The code I am working at doesn't make use of exceptions, it uses error codes instead. Runtime exceptions would be treated as a crash in this case. The code creates the connections with 'ne' = true.

> Given that Transaction does not normally roll back exceptions in the face of a DB error until it dies (again, RAII) that means Transaction would be the wrong place to be looking for error checking methods. The object's dead; you can't call such methods on it. Thus, you're back to checking other longer-lived objects for error results.

I think that could be done explicitly by the Transaction object, since it contains the connection object. I was thinking on modifying it along the lines of:

    std::unique_ptr<Transaction> CreateTransaction(Connection* conn) {
        // Begin the transaction set
        Query q(conn_.query("START TRANSACTION"));
        if (consistent) {
            q << " WITH CONSISTENT SNAPSHOT";
        }
        if (!q.execute()) {  // error code could be extracted from conn
            return nullptr;
        }
        return std::make_unique<Transaction>(*conn);
    }

    Transaction::Transaction(Connection* conn, bool consistent) :
    conn_(*conn), finished_(false) {}

    Transaction::~Transaction() {
        if (!finished_) {
            try {
                // even if it fails, on error the transaction will be rollback
                rollback();
            } catch (...) {
                // eat all exceptions
            }
        }
    }

    bool Transaction::commit() {
        // error code could be extracted from conn
        if (!conn_.query("COMMIT").execute()) {
            return false;
        }
        finished_ = true;
        return true;
    }

This way both exception and exception-free usages would be supported. Wdyt?

Thanks Warren,

> The error return you're talking about is already a property of the Result object, thus the frequent use of res in bool context within the samples. For instance, in simple1

But I would also like to capture the error at the start of a transaction (i.e. the START TRANSACTION statement status).

> If the error you want to trap is reported by the Connection object, then ask the Connection object, don't make Transaction report it for you. That isn't its job.

I know that is how the lib exposes errors, but it is easier to me to think about the errors belonging to their entities. Example: a Transaction object error represents the error that happened during a transaction, a Query object error represents the error that happened during a query, and so on (instead of checking the connection object).

> I see no reason at all to use the heap here. Transaction is a 16-byte object on a typical 64-bit system. Adding smart pointers on top of that is even harder to justify here.

I was just trying to exemplify a factory function that could return some sort of error status (the nullptr in that case).

> This seems to be the only necessary part of your changes, but to accept that, we'd have go break the library ABI.

I understand your concern. In terms of functionality, do you think this is the right approach?

> That's quite unlikely to happen ever again, since our rules would require that we move to MySQL++ 4.0 for that, and all efforts to get anyone interested in a successor to MySQL++ 3.x have fizzled. (Primarily libtabula.) If you're willing to work on libtabula, then maybe this change goes there, but I think MySQL++'s ABI is frozen for all practical purposes now.

I think that if you move to a more common repository it could gain traction again and more people would contribute! (like GitHub, just my opinion)

Best regards

> Parse row 2 contents and if value X isn't positive, return (i.e.: rollback)

The error return you're talking about is already a property of the `Result` object, thus the frequent use of `res` in `bool` context within the samples. For instance, in `simple1`:

    if (mysqlpp::StoreQueryResult res = query.store()) {

> that could be done explicitly by the Transaction object, since it contains the connection object

If the error you want to trap is reported by the `Connection` object, then ask the `Connection` object, don't make `Transaction` report it for you. That isn't its job.

> `std::unique_ptr<Transaction> CreateTransaction()`

I see no reason at all to use the heap here. `Transaction` is a 16-byte object on a typical 64-bit system. Adding smart pointers on top of that is even harder to justify here.

What are you going to do with the smart pointer if you had it, anyway? Keep an instance of it past the end of the call frame so you can ask `Transaction` what `Connection` said about the error state of the library? Then all you're doing is delaying the transaction rollback until the final instance is destroyed.

`Transaction` *needs* to be single-instance on the stack in order to do its job properly! (For the same reason as for `unique_ptr`, incidentally.)

> `bool Transaction::commit()`

This seems to be the only necessary part of your changes, but to accept that, we'd have go break the library ABI. That's quite unlikely to happen ever again, since our rules would require that we move to MySQL++ 4.0 for that, and all efforts to get anyone interested in a successor to MySQL++ 3.x have fizzled.

If you're willing to begin work on a "MySQL++ 4.0" line of development, then maybe this change goes there, but I think MySQL++'s ABI is frozen for all practical purposes now.

> But I would also like to capture the error at the start of a transaction (i.e. the START TRANSACTION statement status).

You can probably achieve that by adding [a safe bool operator][1] instead, so you can test the `Transaction` object in `bool` context before making any queries on the connection.

I'm not sure whether you can do this without adding a data member to the object, again breaking the library's ABI, but it'd be a much better solution than wrapping an object that naturally lives on the stack in an object that must live on the stack which in turn holds the actual object on the heap. Keep it on the stack to begin with!

> I was just trying to exemplify a factory function that could return some sort of error status (the nullptr in that case).

Use of nulls to signify errors is [usually a bad design][2].

> do you think this is the right approach?

I have no objection to making `commit()` return `false` on error. The only question is whether you want to carry the patch locally or work with me to get this "MySQL++ 4.0" effort launched.

> I think that if you move to a more common repository it could gain traction again and more people would contribute! (like GitHub, just my opinion)

The MySQL++ source repo has been [mirrored to GitHub][3] for most of a year now. Note the complete lack of PRs.

Almost always when someone complains about the version control system as the reason why contributions don't exist, that is not the actual reason. FOSS predates widespread version control entirely; FOSS didn't begin in 2005 when Git was created.

[1]: https://tangentsoft.com/mysqlpp/artifact?udc=1&ln=51-55&name=45a3d9569eea3951
[2]: https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare/
[3]: https://github.com/tangentsoft/mysqlpp