You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Robert Huffman <ro...@gmail.com> on 2016/06/22 15:10:30 UTC
Re: [dbutils] Would it be possible to have parameters passed to
QueryRunner as an Iterable?
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 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