You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2016/05/19 13:40:32 UTC
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
<moving this to the dev ML>
Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016 um
15:32 Uhr:
> Actually, Iterable doesn't work: in fillStatement you need to know the size
> and that's not easily obtainable from an Iterable. So I'm using Collection
> instead.
>
> I cloned the project and have QueryRunner.query working already. Basically
> I changed the private query method to take a Collection instead of varargs
> for the parameters. A public method is added that takes a Collection, and
> the public methods that take arrays simply use Arrays.asList on the
> arrays. You can check out the approach here:
>
>
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
>
> I will take the same approach on update, insert and batch, create a JIRA
> and attach a patch.
>
I've added a few comments. Looks good to me overall.
>
>
>
>
> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <br...@apache.org>
> wrote:
>
> > Hello Robert,
> >
> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18. Mai 2016
> um
> > 19:00 Uhr:
> >
> > > If a prepared statement is built dynamically, with a variable number of
> > > parameters, and parameters are collected in a Collection of some sort
> > > instead of an array, usage QueryRunner requires that the collection be
> > > converted to an array first. This means the parameters are iterated
> > twice:
> > > once to convert to an array and once again in
> QueryRunner.fillStatement.
> > >
> > > Would it violate a design decision if methods were added to QueryRunner
> > > that took the parameters as an Iterable instead of as varags? It should
> > be
> > > straightforward to add such methods and use an Iterable wrapper around
> an
> > > array to have the varargs methods invoke the new methods that take
> > > Iterables.
> > >
> > > I would be happy to submit a patch if this does not violate some sort
> of
> > > design decision I am not aware of and if the implementation approach
> > sounds
> > > reasonable.
> > >
> >
> > Sounds like a reasonable addition, although I'm not sure I understand
> your
> > proposal with the "Iterable wrapper around an array". Feel free to
> create a
> > JIRA and provide a patch/github PR for adding this functionality. Further
> > design discussions about this addition should go to the dev mailing list.
> >
> > Benedikt
> >
>
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Matt Sicker <bo...@gmail.com>.
GitHub mirrors are mainly available to make it easier for users to
contribute pull requests instead of patches.
On 22 June 2016 at 10:59, Robert Huffman <ro...@gmail.com> wrote:
> Got it. Thanks.
>
> On Wed, Jun 22, 2016 at 8:12 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
> > Nothing is moving to GitHub. Some projects are moving from Svn to Git
> > within Apache and then being _mirrored_ to GitHub.
> >
> > Gary
> >
> > On Wed, Jun 22, 2016 at 8:10 AM, Robert Huffman <
> robert.huffman@gmail.com>
> > wrote:
> >
> > > Some time ago I opened an issue for this and attached a patch. I see
> that
> > > some of the other Commons projects are moving to GitHub. Any chance
> that
> > > will happen for dbutils?
> > >
> > >
> > >
> > > On Fri, May 20, 2016 at 5:36 AM, Benedikt Ritter <br...@apache.org>
> > > wrote:
> > >
> > > > Hello Robert,
> > > >
> > > > Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai
> 2016
> > > um
> > > > 17:00 Uhr:
> > > >
> > > > > Thanks for the comments. Yes, it is in progress.
> > > > >
> > > > > I generally keep imports collapsed and never look at them. I hadn't
> > > even
> > > > > realized that was the style here. Thanks for pointing it out.
> > > > >
> > > > > I already changed ITERABLE to COLLECTION. (Originally I thought I
> > would
> > > > be
> > > > > using Iterable) and removed the parameterType from the signature.
> > > > >
> > > > > I agree with you: I'm not so sure using JUnit parameterized tests
> is
> > a
> > > > > great idea here. It is unfortunate JUnit doesn't give you more
> > control.
> > > > > Other frameworks (like Spock or TestNG) let you parameterize
> specific
> > > > test
> > > > > methods intead of the entire class. I am thinking that once I fix
> > > execute
> > > > > and update tests to also use the parameter type, most tests in the
> > > > > QueryRunnerTest will use it, so perhaps it won't be too bad.
> > > > >
> > > > > If you don't like it, what alternative would you suggest? I'm not
> > crazy
> > > > > about the idea of just copying the test methods. I suppose I could
> > just
> > > > add
> > > > > calls with collections to the existing test methods, but I'm not
> > crazy
> > > > > about that, either.
> > > > >
> > > >
> > > > It's okay for me if in the end all (or most of) the test methods
> > actually
> > > > use the test parameters. If not, I suggest splitting the test up into
> > to
> > > > test classes.
> > > >
> > > > Regards,
> > > > Benedikt
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <
> britter@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > >> <moving this to the dev ML>
> > > > >>
> > > > >> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai
> > > 2016
> > > > >> um 15:32 Uhr:
> > > > >>
> > > > >>> Actually, Iterable doesn't work: in fillStatement you need to
> know
> > > the
> > > > >>> size
> > > > >>> and that's not easily obtainable from an Iterable. So I'm using
> > > > >>> Collection
> > > > >>> instead.
> > > > >>>
> > > > >>> I cloned the project and have QueryRunner.query working already.
> > > > >>> Basically
> > > > >>> I changed the private query method to take a Collection instead
> of
> > > > >>> varargs
> > > > >>> for the parameters. A public method is added that takes a
> > Collection,
> > > > and
> > > > >>> the public methods that take arrays simply use Arrays.asList on
> the
> > > > >>> arrays. You can check out the approach here:
> > > > >>>
> > > > >>>
> > > > >>>
> > > >
> > >
> >
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
> > > > >>>
> > > > >>> I will take the same approach on update, insert and batch,
> create a
> > > > JIRA
> > > > >>> and attach a patch.
> > > > >>>
> > > > >>
> > > > >> I've added a few comments. Looks good to me overall.
> > > > >>
> > > > >>
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <
> > britter@apache.org
> > > >
> > > > >>> wrote:
> > > > >>>
> > > > >>> > Hello Robert,
> > > > >>> >
> > > > >>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18.
> > Mai
> > > > >>> 2016 um
> > > > >>> > 19:00 Uhr:
> > > > >>> >
> > > > >>> > > If a prepared statement is built dynamically, with a variable
> > > > number
> > > > >>> of
> > > > >>> > > parameters, and parameters are collected in a Collection of
> > some
> > > > sort
> > > > >>> > > instead of an array, usage QueryRunner requires that the
> > > collection
> > > > >>> be
> > > > >>> > > converted to an array first. This means the parameters are
> > > iterated
> > > > >>> > twice:
> > > > >>> > > once to convert to an array and once again in
> > > > >>> QueryRunner.fillStatement.
> > > > >>> > >
> > > > >>> > > Would it violate a design decision if methods were added to
> > > > >>> QueryRunner
> > > > >>> > > that took the parameters as an Iterable instead of as varags?
> > It
> > > > >>> should
> > > > >>> > be
> > > > >>> > > straightforward to add such methods and use an Iterable
> wrapper
> > > > >>> around an
> > > > >>> > > array to have the varargs methods invoke the new methods that
> > > take
> > > > >>> > > Iterables.
> > > > >>> > >
> > > > >>> > > I would be happy to submit a patch if this does not violate
> > some
> > > > >>> sort of
> > > > >>> > > design decision I am not aware of and if the implementation
> > > > approach
> > > > >>> > sounds
> > > > >>> > > reasonable.
> > > > >>> > >
> > > > >>> >
> > > > >>> > Sounds like a reasonable addition, although I'm not sure I
> > > understand
> > > > >>> your
> > > > >>> > proposal with the "Iterable wrapper around an array". Feel free
> > to
> > > > >>> create a
> > > > >>> > JIRA and provide a patch/github PR for adding this
> functionality.
> > > > >>> Further
> > > > >>> > design discussions about this addition should go to the dev
> > mailing
> > > > >>> list.
> > > > >>> >
> > > > >>> > Benedikt
> > > > >>> >
> > > > >>>
> > > > >>
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > Java Persistence with Hibernate, Second Edition
> > <http://www.manning.com/bauer3/>
> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> > Spring Batch in Action <http://www.manning.com/templier/>
> > Blog: http://garygregory.wordpress.com
> > Home: http://garygregory.com/
> > Tweet! http://twitter.com/GaryGregory
> >
>
--
Matt Sicker <bo...@gmail.com>
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Robert Huffman <ro...@gmail.com>.
Got it. Thanks.
On Wed, Jun 22, 2016 at 8:12 AM, Gary Gregory <ga...@gmail.com>
wrote:
> Nothing is moving to GitHub. Some projects are moving from Svn to Git
> within Apache and then being _mirrored_ to GitHub.
>
> Gary
>
> On Wed, Jun 22, 2016 at 8:10 AM, Robert Huffman <ro...@gmail.com>
> wrote:
>
> > Some time ago I opened an issue for this and attached a patch. I see that
> > some of the other Commons projects are moving to GitHub. Any chance that
> > will happen for dbutils?
> >
> >
> >
> > On Fri, May 20, 2016 at 5:36 AM, Benedikt Ritter <br...@apache.org>
> > wrote:
> >
> > > Hello Robert,
> > >
> > > Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016
> > um
> > > 17:00 Uhr:
> > >
> > > > Thanks for the comments. Yes, it is in progress.
> > > >
> > > > I generally keep imports collapsed and never look at them. I hadn't
> > even
> > > > realized that was the style here. Thanks for pointing it out.
> > > >
> > > > I already changed ITERABLE to COLLECTION. (Originally I thought I
> would
> > > be
> > > > using Iterable) and removed the parameterType from the signature.
> > > >
> > > > I agree with you: I'm not so sure using JUnit parameterized tests is
> a
> > > > great idea here. It is unfortunate JUnit doesn't give you more
> control.
> > > > Other frameworks (like Spock or TestNG) let you parameterize specific
> > > test
> > > > methods intead of the entire class. I am thinking that once I fix
> > execute
> > > > and update tests to also use the parameter type, most tests in the
> > > > QueryRunnerTest will use it, so perhaps it won't be too bad.
> > > >
> > > > If you don't like it, what alternative would you suggest? I'm not
> crazy
> > > > about the idea of just copying the test methods. I suppose I could
> just
> > > add
> > > > calls with collections to the existing test methods, but I'm not
> crazy
> > > > about that, either.
> > > >
> > >
> > > It's okay for me if in the end all (or most of) the test methods
> actually
> > > use the test parameters. If not, I suggest splitting the test up into
> to
> > > test classes.
> > >
> > > Regards,
> > > Benedikt
> > >
> > >
> > > >
> > > >
> > > >
> > > > On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <britter@apache.org
> >
> > > > wrote:
> > > >
> > > >> <moving this to the dev ML>
> > > >>
> > > >> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai
> > 2016
> > > >> um 15:32 Uhr:
> > > >>
> > > >>> Actually, Iterable doesn't work: in fillStatement you need to know
> > the
> > > >>> size
> > > >>> and that's not easily obtainable from an Iterable. So I'm using
> > > >>> Collection
> > > >>> instead.
> > > >>>
> > > >>> I cloned the project and have QueryRunner.query working already.
> > > >>> Basically
> > > >>> I changed the private query method to take a Collection instead of
> > > >>> varargs
> > > >>> for the parameters. A public method is added that takes a
> Collection,
> > > and
> > > >>> the public methods that take arrays simply use Arrays.asList on the
> > > >>> arrays. You can check out the approach here:
> > > >>>
> > > >>>
> > > >>>
> > >
> >
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
> > > >>>
> > > >>> I will take the same approach on update, insert and batch, create a
> > > JIRA
> > > >>> and attach a patch.
> > > >>>
> > > >>
> > > >> I've added a few comments. Looks good to me overall.
> > > >>
> > > >>
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <
> britter@apache.org
> > >
> > > >>> wrote:
> > > >>>
> > > >>> > Hello Robert,
> > > >>> >
> > > >>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18.
> Mai
> > > >>> 2016 um
> > > >>> > 19:00 Uhr:
> > > >>> >
> > > >>> > > If a prepared statement is built dynamically, with a variable
> > > number
> > > >>> of
> > > >>> > > parameters, and parameters are collected in a Collection of
> some
> > > sort
> > > >>> > > instead of an array, usage QueryRunner requires that the
> > collection
> > > >>> be
> > > >>> > > converted to an array first. This means the parameters are
> > iterated
> > > >>> > twice:
> > > >>> > > once to convert to an array and once again in
> > > >>> QueryRunner.fillStatement.
> > > >>> > >
> > > >>> > > Would it violate a design decision if methods were added to
> > > >>> QueryRunner
> > > >>> > > that took the parameters as an Iterable instead of as varags?
> It
> > > >>> should
> > > >>> > be
> > > >>> > > straightforward to add such methods and use an Iterable wrapper
> > > >>> around an
> > > >>> > > array to have the varargs methods invoke the new methods that
> > take
> > > >>> > > Iterables.
> > > >>> > >
> > > >>> > > I would be happy to submit a patch if this does not violate
> some
> > > >>> sort of
> > > >>> > > design decision I am not aware of and if the implementation
> > > approach
> > > >>> > sounds
> > > >>> > > reasonable.
> > > >>> > >
> > > >>> >
> > > >>> > Sounds like a reasonable addition, although I'm not sure I
> > understand
> > > >>> your
> > > >>> > proposal with the "Iterable wrapper around an array". Feel free
> to
> > > >>> create a
> > > >>> > JIRA and provide a patch/github PR for adding this functionality.
> > > >>> Further
> > > >>> > design discussions about this addition should go to the dev
> mailing
> > > >>> list.
> > > >>> >
> > > >>> > Benedikt
> > > >>> >
> > > >>>
> > > >>
> > > >
> > >
> >
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Gary Gregory <ga...@gmail.com>.
Nothing is moving to GitHub. Some projects are moving from Svn to Git
within Apache and then being _mirrored_ to GitHub.
Gary
On Wed, Jun 22, 2016 at 8:10 AM, Robert Huffman <ro...@gmail.com>
wrote:
> Some time ago I opened an issue for this and attached a patch. I see that
> some of the other Commons projects are moving to GitHub. Any chance that
> will happen for dbutils?
>
>
>
> On Fri, May 20, 2016 at 5:36 AM, Benedikt Ritter <br...@apache.org>
> wrote:
>
> > Hello Robert,
> >
> > Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016
> um
> > 17:00 Uhr:
> >
> > > Thanks for the comments. Yes, it is in progress.
> > >
> > > I generally keep imports collapsed and never look at them. I hadn't
> even
> > > realized that was the style here. Thanks for pointing it out.
> > >
> > > I already changed ITERABLE to COLLECTION. (Originally I thought I would
> > be
> > > using Iterable) and removed the parameterType from the signature.
> > >
> > > I agree with you: I'm not so sure using JUnit parameterized tests is a
> > > great idea here. It is unfortunate JUnit doesn't give you more control.
> > > Other frameworks (like Spock or TestNG) let you parameterize specific
> > test
> > > methods intead of the entire class. I am thinking that once I fix
> execute
> > > and update tests to also use the parameter type, most tests in the
> > > QueryRunnerTest will use it, so perhaps it won't be too bad.
> > >
> > > If you don't like it, what alternative would you suggest? I'm not crazy
> > > about the idea of just copying the test methods. I suppose I could just
> > add
> > > calls with collections to the existing test methods, but I'm not crazy
> > > about that, either.
> > >
> >
> > It's okay for me if in the end all (or most of) the test methods actually
> > use the test parameters. If not, I suggest splitting the test up into to
> > test classes.
> >
> > Regards,
> > Benedikt
> >
> >
> > >
> > >
> > >
> > > On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <br...@apache.org>
> > > wrote:
> > >
> > >> <moving this to the dev ML>
> > >>
> > >> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai
> 2016
> > >> um 15:32 Uhr:
> > >>
> > >>> Actually, Iterable doesn't work: in fillStatement you need to know
> the
> > >>> size
> > >>> and that's not easily obtainable from an Iterable. So I'm using
> > >>> Collection
> > >>> instead.
> > >>>
> > >>> I cloned the project and have QueryRunner.query working already.
> > >>> Basically
> > >>> I changed the private query method to take a Collection instead of
> > >>> varargs
> > >>> for the parameters. A public method is added that takes a Collection,
> > and
> > >>> the public methods that take arrays simply use Arrays.asList on the
> > >>> arrays. You can check out the approach here:
> > >>>
> > >>>
> > >>>
> >
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
> > >>>
> > >>> I will take the same approach on update, insert and batch, create a
> > JIRA
> > >>> and attach a patch.
> > >>>
> > >>
> > >> I've added a few comments. Looks good to me overall.
> > >>
> > >>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <britter@apache.org
> >
> > >>> wrote:
> > >>>
> > >>> > Hello Robert,
> > >>> >
> > >>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18. Mai
> > >>> 2016 um
> > >>> > 19:00 Uhr:
> > >>> >
> > >>> > > If a prepared statement is built dynamically, with a variable
> > number
> > >>> of
> > >>> > > parameters, and parameters are collected in a Collection of some
> > sort
> > >>> > > instead of an array, usage QueryRunner requires that the
> collection
> > >>> be
> > >>> > > converted to an array first. This means the parameters are
> iterated
> > >>> > twice:
> > >>> > > once to convert to an array and once again in
> > >>> QueryRunner.fillStatement.
> > >>> > >
> > >>> > > Would it violate a design decision if methods were added to
> > >>> QueryRunner
> > >>> > > that took the parameters as an Iterable instead of as varags? It
> > >>> should
> > >>> > be
> > >>> > > straightforward to add such methods and use an Iterable wrapper
> > >>> around an
> > >>> > > array to have the varargs methods invoke the new methods that
> take
> > >>> > > Iterables.
> > >>> > >
> > >>> > > I would be happy to submit a patch if this does not violate some
> > >>> sort of
> > >>> > > design decision I am not aware of and if the implementation
> > approach
> > >>> > sounds
> > >>> > > reasonable.
> > >>> > >
> > >>> >
> > >>> > Sounds like a reasonable addition, although I'm not sure I
> understand
> > >>> your
> > >>> > proposal with the "Iterable wrapper around an array". Feel free to
> > >>> create a
> > >>> > JIRA and provide a patch/github PR for adding this functionality.
> > >>> Further
> > >>> > design discussions about this addition should go to the dev mailing
> > >>> list.
> > >>> >
> > >>> > Benedikt
> > >>> >
> > >>>
> > >>
> > >
> >
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Robert Huffman <ro...@gmail.com>.
Some time ago I opened an issue for this and attached a patch. I see that
some of the other Commons projects are moving to GitHub. Any chance that
will happen for dbutils?
On Fri, May 20, 2016 at 5:36 AM, Benedikt Ritter <br...@apache.org> wrote:
> Hello Robert,
>
> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016 um
> 17:00 Uhr:
>
> > Thanks for the comments. Yes, it is in progress.
> >
> > I generally keep imports collapsed and never look at them. I hadn't even
> > realized that was the style here. Thanks for pointing it out.
> >
> > I already changed ITERABLE to COLLECTION. (Originally I thought I would
> be
> > using Iterable) and removed the parameterType from the signature.
> >
> > I agree with you: I'm not so sure using JUnit parameterized tests is a
> > great idea here. It is unfortunate JUnit doesn't give you more control.
> > Other frameworks (like Spock or TestNG) let you parameterize specific
> test
> > methods intead of the entire class. I am thinking that once I fix execute
> > and update tests to also use the parameter type, most tests in the
> > QueryRunnerTest will use it, so perhaps it won't be too bad.
> >
> > If you don't like it, what alternative would you suggest? I'm not crazy
> > about the idea of just copying the test methods. I suppose I could just
> add
> > calls with collections to the existing test methods, but I'm not crazy
> > about that, either.
> >
>
> It's okay for me if in the end all (or most of) the test methods actually
> use the test parameters. If not, I suggest splitting the test up into to
> test classes.
>
> Regards,
> Benedikt
>
>
> >
> >
> >
> > On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <br...@apache.org>
> > wrote:
> >
> >> <moving this to the dev ML>
> >>
> >> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016
> >> um 15:32 Uhr:
> >>
> >>> Actually, Iterable doesn't work: in fillStatement you need to know the
> >>> size
> >>> and that's not easily obtainable from an Iterable. So I'm using
> >>> Collection
> >>> instead.
> >>>
> >>> I cloned the project and have QueryRunner.query working already.
> >>> Basically
> >>> I changed the private query method to take a Collection instead of
> >>> varargs
> >>> for the parameters. A public method is added that takes a Collection,
> and
> >>> the public methods that take arrays simply use Arrays.asList on the
> >>> arrays. You can check out the approach here:
> >>>
> >>>
> >>>
> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
> >>>
> >>> I will take the same approach on update, insert and batch, create a
> JIRA
> >>> and attach a patch.
> >>>
> >>
> >> I've added a few comments. Looks good to me overall.
> >>
> >>
> >>>
> >>>
> >>>
> >>>
> >>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <br...@apache.org>
> >>> wrote:
> >>>
> >>> > Hello Robert,
> >>> >
> >>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18. Mai
> >>> 2016 um
> >>> > 19:00 Uhr:
> >>> >
> >>> > > If a prepared statement is built dynamically, with a variable
> number
> >>> of
> >>> > > parameters, and parameters are collected in a Collection of some
> sort
> >>> > > instead of an array, usage QueryRunner requires that the collection
> >>> be
> >>> > > converted to an array first. This means the parameters are iterated
> >>> > twice:
> >>> > > once to convert to an array and once again in
> >>> QueryRunner.fillStatement.
> >>> > >
> >>> > > Would it violate a design decision if methods were added to
> >>> QueryRunner
> >>> > > that took the parameters as an Iterable instead of as varags? It
> >>> should
> >>> > be
> >>> > > straightforward to add such methods and use an Iterable wrapper
> >>> around an
> >>> > > array to have the varargs methods invoke the new methods that take
> >>> > > Iterables.
> >>> > >
> >>> > > I would be happy to submit a patch if this does not violate some
> >>> sort of
> >>> > > design decision I am not aware of and if the implementation
> approach
> >>> > sounds
> >>> > > reasonable.
> >>> > >
> >>> >
> >>> > Sounds like a reasonable addition, although I'm not sure I understand
> >>> your
> >>> > proposal with the "Iterable wrapper around an array". Feel free to
> >>> create a
> >>> > JIRA and provide a patch/github PR for adding this functionality.
> >>> Further
> >>> > design discussions about this addition should go to the dev mailing
> >>> list.
> >>> >
> >>> > Benedikt
> >>> >
> >>>
> >>
> >
>
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Benedikt Ritter <br...@apache.org>.
Hello Robert,
Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016 um
17:00 Uhr:
> Thanks for the comments. Yes, it is in progress.
>
> I generally keep imports collapsed and never look at them. I hadn't even
> realized that was the style here. Thanks for pointing it out.
>
> I already changed ITERABLE to COLLECTION. (Originally I thought I would be
> using Iterable) and removed the parameterType from the signature.
>
> I agree with you: I'm not so sure using JUnit parameterized tests is a
> great idea here. It is unfortunate JUnit doesn't give you more control.
> Other frameworks (like Spock or TestNG) let you parameterize specific test
> methods intead of the entire class. I am thinking that once I fix execute
> and update tests to also use the parameter type, most tests in the
> QueryRunnerTest will use it, so perhaps it won't be too bad.
>
> If you don't like it, what alternative would you suggest? I'm not crazy
> about the idea of just copying the test methods. I suppose I could just add
> calls with collections to the existing test methods, but I'm not crazy
> about that, either.
>
It's okay for me if in the end all (or most of) the test methods actually
use the test parameters. If not, I suggest splitting the test up into to
test classes.
Regards,
Benedikt
>
>
>
> On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <br...@apache.org>
> wrote:
>
>> <moving this to the dev ML>
>>
>> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016
>> um 15:32 Uhr:
>>
>>> Actually, Iterable doesn't work: in fillStatement you need to know the
>>> size
>>> and that's not easily obtainable from an Iterable. So I'm using
>>> Collection
>>> instead.
>>>
>>> I cloned the project and have QueryRunner.query working already.
>>> Basically
>>> I changed the private query method to take a Collection instead of
>>> varargs
>>> for the parameters. A public method is added that takes a Collection, and
>>> the public methods that take arrays simply use Arrays.asList on the
>>> arrays. You can check out the approach here:
>>>
>>>
>>> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
>>>
>>> I will take the same approach on update, insert and batch, create a JIRA
>>> and attach a patch.
>>>
>>
>> I've added a few comments. Looks good to me overall.
>>
>>
>>>
>>>
>>>
>>>
>>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <br...@apache.org>
>>> wrote:
>>>
>>> > Hello Robert,
>>> >
>>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18. Mai
>>> 2016 um
>>> > 19:00 Uhr:
>>> >
>>> > > If a prepared statement is built dynamically, with a variable number
>>> of
>>> > > parameters, and parameters are collected in a Collection of some sort
>>> > > instead of an array, usage QueryRunner requires that the collection
>>> be
>>> > > converted to an array first. This means the parameters are iterated
>>> > twice:
>>> > > once to convert to an array and once again in
>>> QueryRunner.fillStatement.
>>> > >
>>> > > Would it violate a design decision if methods were added to
>>> QueryRunner
>>> > > that took the parameters as an Iterable instead of as varags? It
>>> should
>>> > be
>>> > > straightforward to add such methods and use an Iterable wrapper
>>> around an
>>> > > array to have the varargs methods invoke the new methods that take
>>> > > Iterables.
>>> > >
>>> > > I would be happy to submit a patch if this does not violate some
>>> sort of
>>> > > design decision I am not aware of and if the implementation approach
>>> > sounds
>>> > > reasonable.
>>> > >
>>> >
>>> > Sounds like a reasonable addition, although I'm not sure I understand
>>> your
>>> > proposal with the "Iterable wrapper around an array". Feel free to
>>> create a
>>> > JIRA and provide a patch/github PR for adding this functionality.
>>> Further
>>> > design discussions about this addition should go to the dev mailing
>>> list.
>>> >
>>> > Benedikt
>>> >
>>>
>>
>
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
Posted by Robert Huffman <ro...@gmail.com>.
Thanks for the comments. Yes, it is in progress.
I generally keep imports collapsed and never look at them. I hadn't even
realized that was the style here. Thanks for pointing it out.
I already changed ITERABLE to COLLECTION. (Originally I thought I would be
using Iterable) and removed the parameterType from the signature.
I agree with you: I'm not so sure using JUnit parameterized tests is a
great idea here. It is unfortunate JUnit doesn't give you more control.
Other frameworks (like Spock or TestNG) let you parameterize specific test
methods intead of the entire class. I am thinking that once I fix execute
and update tests to also use the parameter type, most tests in the
QueryRunnerTest will use it, so perhaps it won't be too bad.
If you don't like it, what alternative would you suggest? I'm not crazy
about the idea of just copying the test methods. I suppose I could just add
calls with collections to the existing test methods, but I'm not crazy
about that, either.
On Thu, May 19, 2016 at 6:40 AM, Benedikt Ritter <br...@apache.org> wrote:
> <moving this to the dev ML>
>
> Robert Huffman <ro...@gmail.com> schrieb am Do., 19. Mai 2016 um
> 15:32 Uhr:
>
>> Actually, Iterable doesn't work: in fillStatement you need to know the
>> size
>> and that's not easily obtainable from an Iterable. So I'm using Collection
>> instead.
>>
>> I cloned the project and have QueryRunner.query working already. Basically
>> I changed the private query method to take a Collection instead of varargs
>> for the parameters. A public method is added that takes a Collection, and
>> the public methods that take arrays simply use Arrays.asList on the
>> arrays. You can check out the approach here:
>>
>>
>> https://github.com/rhuffman/commons-dbutils/tree/feature/allow-collections-of-parameters-in-QueryRunner
>>
>> I will take the same approach on update, insert and batch, create a JIRA
>> and attach a patch.
>>
>
> I've added a few comments. Looks good to me overall.
>
>
>>
>>
>>
>>
>> On Thu, May 19, 2016 at 6:02 AM, Benedikt Ritter <br...@apache.org>
>> wrote:
>>
>> > Hello Robert,
>> >
>> > Robert Huffman <ro...@gmail.com> schrieb am Mi., 18. Mai 2016
>> um
>> > 19:00 Uhr:
>> >
>> > > If a prepared statement is built dynamically, with a variable number
>> of
>> > > parameters, and parameters are collected in a Collection of some sort
>> > > instead of an array, usage QueryRunner requires that the collection be
>> > > converted to an array first. This means the parameters are iterated
>> > twice:
>> > > once to convert to an array and once again in
>> QueryRunner.fillStatement.
>> > >
>> > > Would it violate a design decision if methods were added to
>> QueryRunner
>> > > that took the parameters as an Iterable instead of as varags? It
>> should
>> > be
>> > > straightforward to add such methods and use an Iterable wrapper
>> around an
>> > > array to have the varargs methods invoke the new methods that take
>> > > Iterables.
>> > >
>> > > I would be happy to submit a patch if this does not violate some sort
>> of
>> > > design decision I am not aware of and if the implementation approach
>> > sounds
>> > > reasonable.
>> > >
>> >
>> > Sounds like a reasonable addition, although I'm not sure I understand
>> your
>> > proposal with the "Iterable wrapper around an array". Feel free to
>> create a
>> > JIRA and provide a patch/github PR for adding this functionality.
>> Further
>> > design discussions about this addition should go to the dev mailing
>> list.
>> >
>> > Benedikt
>> >
>>
>