RFC: I'm continuing to enforce that you must pass at least
one SQLExpression, but if we didn't care this could be a bit
more straightforward in the form:
public Where (SQLExpression<?>... andConditions)
It could then use that array directly for the Ops.and(), but also
people would be able to make a "where true" by just constructing
a Where with no args and I'm not sure mdb is down with that.
When we saw the problem, I thought "there's no way!" because
the Logger is hardened and battle-tested.
But no. Since depot no longer depends on samskivert, it has
reimplemented Logger and throws away a lot of the goodness that
has evolved over the years. Goodness like:
- warning if there are an odd number of args and the last one
isn't a Throwable.
- catching problems toString()ing any of the objects in the message.
Let's go ride a fixie.
Things worked without this because the DepotMarshaller ends up
checking for the existence of the table using its own code...
(TODO?)
Also JDBCUtil checks the DatabaseMetaData too but the liaison
isn't passed to that, would require some refactoring...
Actually it seems there are mountains of dead code in JDBCUtil and
they could all be removed...
If only Java didn't make it so damned hard to create a simple named class with
two or more values in it. Naming is generally a good thing, especially when the
alternative is Tuple2<SQLExpression<?>,SQLExpression<?>>.
It was a constant source of annoyance that part of the Depot implementation
lived in samskivert.jdbc. Now it all lives in Depot and samskivert can go back
to being a long obsolete library of utility methods.
Depot clients will have to make three renaming changes:
1. ConnectionProvider &c are now in com.samskivert.depot.
2. ByteEnum is now in com.samskivert.util.
3. Methods that returned index information used to return samskivert Tuple if
you wanted an expression and an order. Now they must return Depot Tuple2. This
should fail at app startup if you miss a spot, so this should not result in any
ticking timebombs.
NB: the next commit is probably going to change Tuple2 into a proper named
class because using a Tuple2 for this situation turns out to suck.
NB2: I moved DropTableMigration out of impl and put the impl details into
DepotMetaData. I should have done that separately, but it was already in
progress when I undertook to revamp.
Strings were always being shoved into a Comparable array.
Does the array type matter? Apparently not. We should just
rip out these tests and always store in a Comparable[].
We've had JDBC array support in Postgres for 5 years now, so I think we're
probably safe to assume it's working everywhere. Ha ha, I'm such an optimist.
It's not clear what the right thing to do is if the database connection fails
during the middle of a transaction. In general, one can't talk to the database
after that, so I'm assuming the transaction is aborted (or eventually aborted)
and rolled back, and everything has to be retried.
I'm not currently tracking state inside a Transaction, which means that I don't
freak out if you call commit() then call rollback(), I just ignore the latter
call, and it's also possible to do something stupid like:
Transaction tx = Transaction.start(ctx);
try {
_somRepo.doOp(..);
} catch (DatabaseException de) {
// no problem!, keep on keepin' on
}
_someRepo.doAnotherOp(...);
tx.commit();
If the first repo op fails due to a database connection failure, the
transaction will get a new database connection for the second op and do the
last half of the op in a new connection.
I guess I'll have to do some state tracking, because stupid as the above code
is, I'm sure some circumstance will arise where it seems like a reasonable
thing to do, and hilarity will ensue. Blah.
Michael Bayne