osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: calcite-avatica git commit: [CALCITE-2412] Add appveyor.yml to have tests on Windows against jdk1.8, jdk9, jdk10 Add Appveyor badge Add -DskipDockerCheck because of CALCITE-2385 and to make it sync with travis.yml


> On Nov 19, 2018, at 11:19 AM, Vladimir Sitnikov <sitnikov.vladimir@xxxxxxxxx> wrote:
> 
> Well, a rule of "first line should be separated by a blank line" seems to
> be automatable.
> The rule of "CALCITE-XXX should be in [...]" seems to be automatable.
> And so on.

Yes, we should do that.

However there are things that automation could never achieve, so let’s continue to talk about those, also.

> setDynamicParam did not look good enough to me
> https://git-wip-us.apache.org/repos/asf?p=calcite.git;a=blobdiff;f=core/src/main/java/org/apache/calcite/runtime/ResultSetEnumerable.java;h=52c11f9aaf752cea367ae921c04a20e5e6da5488;hp=771772f1ea39d74eee6e9f889ae8d6500f129c7f;hb=53e15af6c5e8e782b2edcd7f5bf4f5f32225d110;hpb=02ca9bc995cac5b4b97855a4d06df46e632d7c22
> 
> I'm inclined to incline Avatica to expose
> TypedValue.setToPreparedStatement(PreparedStatement ps, int index) kind of
> API, so ResultSetEnumerable.setDynamicParam could be removed altogether.

I agree. The commit didn’t seem quite perfect to me either. However, it seemed to be progress. Log an Avatica JIRA if you have ideas for how to improve it further. Since it will be in Avatica it will take a while to bubble through the release cycle.

Julian