ADBA feedback

[Lukas Eder]

Lance, thanks a lot for your installation hints. That's greatly
appreciated, and I will be happy to use them on the next iteration.

Douglas, thanks a lot for looking through my comments. I agree that
renaming things isn't important - I couldn't come up with better names
either. Just thought I'd drop the concern now. Perhaps there's going to be
an epiphany some days later, or perhaps not.

Further comments inline

2018-04-26 22:44 GMT+02:00 Douglas Surber <douglas.surber at oracle.com>:

> Two things constrain what we can do with AoJ. First we have very limited
> resources we can expend on AoJ. To an extent we have spent all the time on
> it we can. Second, we can’t do much more without involving Oracle lawyers
> which would bring progress to a halt.
>

That's what I thought :)


> We believe the best way to address both of these is for the community to
> fork AoJ. All of the problems you have identified, things that we can’t
> address for one reason or another, can be fixed in a community supported
> fork. We absolutely will cooperate with a community project though we can’t
> promise to contribute.
>

Since the existing version is ASL 2.0 licensed, I don't see any issues with
forking - except perhaps for the name. As/if the fork evolves, it shouldn't
be called AoJ as that is your original name.

Thanks for offering help with such a potential community effort.

Speaking of which, I also agree with Chad's feedback here:
http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2018-April/000267.html

Another way to get feedback would be an early OSS or closed source
implementation for e.g. Oracle or MySQL databases, to better understand the
API. We'll be looking forward to this as soon as it's ready.

> 1) Result.Row should have better methods akin to what JDBC's
> > ResultSetMetaData (RSMD) has. For instance:
> >  o It's currently not possible to discover individual column types
> >  o The getIdentifiers() array corresponds to the RSMD.getColumnName()
> > values, but what about qualification of columns? What about
> > RSMD.getColumnLabel()?
> >  o Nullability flags may be cool too. Not sure about the other info in
> RSMD
>
> This is an intentional design choice for version 1. ADBA is targeted at
> high throughput apps. Metadata is slow. We intentionally omitted almost all
> metadata at least from V1 as we did not think it was an absolute
> requirement for the target use cases. If there are target use cases where
> metadata is critical we can revisit that decision, but we would prefer to
> defer that to version 2.
>

Cool, I think that's fine. Perhaps in this case, I'd simply clarify on the
getIdentifiers() Javadoc whether the semantics is that of
RSMD.getColumnName() (current AoJ implementation) or RSMD.getColumnLabel()
(I think that would be more useful)


>
> >
> > 2) A Result.Row.get(int, ...) overload to Result.Row.get(String, ...)
> > should be added in my opinion. E.g. what if I run this query? SELECT 'x'
> AS
> > "5", 'y' AS "1" FROM dual, and now I'm accessing Row.get("1"). Which row
> > should be returned? The row with index 1, or the row with name "1"? I'm
> not
> > happy with the existing "lazy" API design approach that prevents this
> > overload. The overload would remove any ambiguity.
>
> The intent is that the argument be a column name so your example should
> return ‘y’. I’ll try to clarify the JavaDoc. A vendor could choose to
> support indexes by adding get(int).
>

OK, another example, then:

SELECT 'x' AS "1", 'y' AS "1" FROM dual

Most SQL dialects allow for ambiguous column names on top-level SQL
statements (but not in nested queries). How to handle this with name based
column access? It wouldn't be possible to access both columns by name. Would
you want to rely on vendor-specific extensions in those cases?

I think you won't get around index based access eventually, maybe in
version 2, then :) (which should mostly be preferred for performance
reasons, as the internal record representation can be a more efficient
array rather than a much less efficient hashmap...)


> >
> > 4) I think the Result type should have a reference to its "parent" /
> > producing type, e.g. Connection. The example where the countOperation()
> > rolls back a transaction by referencing it from the lambda's outer scope
> > shows a flaw (IMO an important one) of the current design. The
> transaction
> > (and connection) should be available from the lambda's Result.Count
> > argument as a contextual parameter, for a much more functional design,
> not
> > in a globalish-variable kind of style. This is especially important in an
> > asynchronous API, where the statement submission may happen entirely
> > elsewhere than the rollback decision.
>
> I played with that approach and it adds a lot of parameters to the lambdas.


I'm not sure how it would. There's a single parameter, and that's
the Result subtype. Result could have getters for contextual information,
such as the Connection. From an API perspective, that would appear cheap at
first sight. Perhaps I'm missing something?


> >  o What is the utility of the getter "isRollbackOnly()”?
>
> Subsequent code may branch depending on whether the transaction will
> commit or rollback. Seemed like a good idea. I have no objection to
> removing it, but also don’t see any benefit to removing it.
>

I guess my doubt here stems from similar doubts as yours in the area of
metadata. If it's definitely useful, keep it. But if it just might be
useful, perhaps postpone to version 2? My reasoning here stems from the
idea that this "unit-of-work-global" transaction reference doesn't seem to
match my API expectation of an asynchronous API, so each thing that can be
removed on this type might make it easier to remove the type itself - if
that is really an option.

Anyway, I'll look through your other answers and play around with the
transaction API a bit more. I certainly haven't fully understood it yet.


> > 6) Why is Row.rowNumber() zero based? That's very confusing. E.g.
> Row.get()
> > is still 1-based, and all SQL databases that support either ROWNUM or
> > ROW_NUMBER() OVER () functions will produce 1-based row numbers
>
> Because everything else in Java is zero based. If the consensus is to
> change this, that’s fine with me.
>

My confusion stems from the asymmetry within ADBA. Row.get()
and ParameterizedRowOperation.set() are 1-based, for instance. Also
1-based: SqlBlob.setPosition(long offset). Just like in JDBC. I don't have
a preference for either approach, but I like consistency across an API.

JDBC's ResultSet.getRow(), which seems equivalent, is 1-based

Thanks,
Lukas