Add support for two additional datatypes (DB2)

[mikepatrick]

Add support for zoned decimal (NUMERIC) and single-precision floating point (REAL) data types (DB2Environment).

I develop primarily on an IBM iSeries. DB2 implementations vary a bit; there are even subtle differences between IBM's iSeries (midrange) implementation and their zSeries (mainframe) implementation.

Attached are diagrams from two of IBM's DB2 manuals, displaying the various data types supported by each DB2 implementation. The first diagram shows the mainframe implementation, the second shows the midrange implementation.

[benilovj]

Thanks for submitting this (and 👍 for attaching the manual pages).

I have a question and a request.

First the question: I'm not really clear on it yet, you mentioned the iSeries and zSeries being marginally different - are they incompatibly different? (ie could this change somehow adversely affect the zSeries implementation?)

And for the request: would you be able to extend the appropriate DataType tests (found here) and confirm that they run through fine, since I don't have a DB2 instance at hand?

[mikepatrick]

Re: Question.
I don't think the iSeries and zSeries are incompatibly different. Notice on the iSeries diagram, there are two types under decimal ("DECIMAL" and "NUMERIC"), whereas there is only one decimal type for the zSeries. I don't have a mainframe to test on, but I'm guessing that the zSeries (and most other implementations) always return "DECIMAL" for numeric integer values. The iSeries will return "NUMERIC" if the field is declared "zoned", or unpacked.

Re: Request.
I have looked at the DataType tests (and I made an effort at extending them), but I have run into a couple of obstacles. First, my shop does not have the latest iSeries upgrade yet, so our midrange does not support DECFLOAT (I have to remove that column from tests using it).

Second, my DB2 instance seems to have some trouble with INSERTs and DELETEs via DbFit. This is not a big problem for me, since I'm using DbFit only as a verification tool (SELECTs only). With INSERT I have trouble with getAllColumns( ), and with DELETEs the exception says my table is not eligible for the operation. I don't need to INSERT or DELETE for my own purposes, but I'll look into this some more in an effort to get all the tests running. Interestingly, I can INSERT and DELETE from my terminal emulator, but not from DbFit.

Finally, while our midrange has files containing single precision floating point and zoned decimal fields, I have not yet figured out how to use Create Table to create them. My floating point values always end up double precision, and my decimal values never end up zoned when I use Create Table. I can create these files using DDS on the iSeries, but have not yet been able to create them using Create Table and execute().

I'll take a closer look at java-to-DB2 data type equivalencies, and see if I can find a way to create a table (via DbFit) that will contain the single precision floating point and zoned decimal data types. I will also see about setting up a DB2 instance on a Windows or Linux box, and testing there. I will keep you posted on my progress, and let you know when I have something for you to look at (hopefully by the beginning of next week).

DbFit has come in quite handy for me, and I'm grateful for the effort you and others have expended to make it as good as it is. I'm available almost any time if you need things tested on IBM's DB2 for iSeries. And thanks very much for the prompt reply!

[benilovj]

Ok, no worries, if it's so much hassle, don't worry about it, I'm pretty sure this wont break anything. I'll merge it...

[benilovj]

you can fetch an edge build that contains your pull request here.

[mikepatrick]

I appreciate your diplomacy and applaud your judgement in merging, and I am grateful. I concur that the worst thing that could happen is somebody who would otherwise get a "data type NUMERIC not supported" exception, could conceivably get a different exception message. Even this seems very, very unlikely.

However, I must admit, it does bug me more than a little bit to request a change, but be unable to make corresponding changes to acceptance tests. Fortunately, I've got my puzzle about half figured out already, I think.

For example, I found I can easily create the appropriate fields (i.e. "NUMERIC") on my DB2 instance (iSeries), but I'm not sure every DB2 instance will support

CREATE TABLE TESTTABLE (n1 NUMERIC).

The zSeries, for example, probably doesn't. This isn't any great surprise, since (for example) my DB2 instance doesn't yet support the recently added DECFLOAT. I'd be curious to hear your feelings about a thorough Acceptance Test Suite that needs slightly tailored to each installation (i.e. I have to remove DECFLOAT to pass on my instance), vs. a less aggressive Suite that passes "out of the box" for most or all instances. Personally, I certainly don't mind tailoring the acceptance tests to my shop, but everybody might not feel that way.

I don't think my pull request will break any fixture functionality for anyone, but I'm wary about "breaking" acceptance tests. I've developed the confidence to say that I could write them, but I'll need to defer to you with respect to whether they should be introduced or not.

Sometime very soon (next week or so) I'll either send you some updated DataType tests or a good, concrete explanation regarding why I think the tests can't or shouldn't be changed. Perhaps both.

Thanks again for doing what I believe amounts to making DbFit work "out of the box" for iSeries developers, and for writing this in the first place. DbFit has made an incredibly powerful and positive impact on my unit testing of COBOL code on the iSeries. You have a great product here; great work by you and all other contributors to the project

[benilovj]

@mikepatrick, I would definitely welcome a pull request with changes that restructure the tests into a "core" part and a implementation-specific part. That seems like the most sensible solution.

If that's not possible for whatever reason, some kind of write-up (added to the root page of the DB2 tests) that gives some context to the situation would be the next best thing, just to warn the next contributor.

Thank you for your contribution!

[mikepatrick]

Jake,

I promised I would get back to you regarding my recent pull request that
didn't have any AcceptanceTests changes with it.

I have worked through my remaining iSeries issues with the DbFit acceptance
tests. Everything works okay, with the exception of the DECFLOAT data type
(which appears to be a problem with my JDBC driver, not my DB2 instance).
I do like your idea of splitting the tests into "core" vs.
"implementation-specific", but I'm still puzzling over how exactly such a
restructuring would look. I can set up a Windows test environment easily
enough, but do not have access to a zSeries.

With respect to my last pull request, I have a few ideas:

• Simply add the new data types. Just as I have to remove DECFLOAT to
  get NumbersTests to pass, others may have to remove NUMERIC. My reading of
  the DB2 manuals leads me to believe that Windows/*nix should not have a
  problem with these new types, but I will work on getting this tested to be
  sure. This would be the quickest, easiest way to get coverage of the new
  data types. At the risk of sounding lazy, this is what I would probably
  vote for. A write up could accompany this, outlining known
  implementation-specific issues (i.e. "If using the IBM Java Toolbox Driver
  for iSeries, DECFLOAT is not supported.")
• Restructure the NumbersTests into two pages: one with just basic data
  types ("core"), and one with all data types ("implementation-specific").
• Create a separate suite of AcceptanceTests specific to the iSeries.
  This would take care of the tests that require an AS clause on the iSeries
  (tests with UNIONs).

I'd be happy to issue a pull request for the first bullet point any time.
This is probably what I should have done initially, but I wasn't
comfortable doing so before I got all the acceptance tests to pass. The
second two options would require a little more analysis and effort, but
could be ready in another week or so. If you have thoughts on this, I'd
love to hear them.

It turns out, many of my problems with the acceptance tests are related to
the fact that DB2Environment has two queries hard-coded into it: one to get
column names, and one to get procedure parameters. These queries needed to
be changed in order to work on the iSeries; IBM didn't make this a
platform-independent feature. I've been thinking about how to make
DB2Environment more implementation-independent (these two queries, and
using different JDBC drivers), but haven't come up with much so far. If I
have a breakthrough idea, I'll be sure to float it by you.

I've attached a Word document containing the changes I made, which are in
the iSeriesDbFit branch of
mpatrick/dbfithttps://github.com/mikepatrick/dbfit/tree/iSeriesDbFit.
This is really provided just for curiosity's sake, as well as to provide a
starting point for others who may want to use DbFit on an iSeries.

Thanks,
Mike P.

On Thu, Aug 8, 2013 at 7:24 AM, Jake Benilov notifications@github.comwrote:

@mikepatrick https://github.com/mikepatrick, I would definitely welcome
a pull request with changes that restructure the tests into a "core" part
and a implementation-specific part. That seems like the most sensible
solution.

If that's not possible for whatever reason, some kind of write-up (added
to the root page of the DB2 tests) that gives some context to the situation
would be the next best thing, just to warn the next contributor.

Thank you for your contribution!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/158#issuecomment-22319703
.

Mike Patrick
mpatrick76@gmail.com

[benilovj]

Hi Mike,

First of all, thanks for putting in the time and effort to investigate this question and then code it up. It's very much appreciated!

The changes to DB2Environment needed to get everything working on the iSeries environment are pretty substantial, and I don't think it's worth extending that class to deal with both what it supports now and the iSeries. Ideally:

1. we should add another class alongside DB2Environment (say, DB2iSeriesEnvironment, or something more appropriate - I'd leave the naming up to you) which contains the appropriate datatype mappings, JDBC driver name, and queries.
2. we add some documentation which outlines which environment should be used when (if this could be in markdown format, that would be ideal - I would then find a home for it in our documentation somewhere)
3. (ideally, but optionally) updates to the tests. It seems that a lot of the tests will be shared across all the DB2 implementations (this can be done using the symlink feature of Fitnesse), and a couple datatype-specific ones.

What do you think?

[mikepatrick]

I like this idea quite a bit, and my own thoughts were drifting in this direction as well. I'll start looking into the details right away. I'll send you my implementation for review before issuing a pull request.

And it's my pleasure to investigate and contribute to the project. All of your work (and that of others who have contributed) is very much appreciated my me and my shop!

[mikepatrick]

Jake,

I have a pull request ready for a DB2iEnvironment adapter. The code is in
the main branch here https://github.com/mikepatrick/iSeriesDbFit. I am
now testing on both the iSeries and a Windows instance. The bullet points:

• Both adapters can share a set of unit tests.
• My problems with DECFLOAT were due to an outdated driver (Shame on me).
  • DB2 for LUW supports REAL and NUMERIC (from my last pull request)
    just fine.
  • Adding an alias to some SELECT statements is safe for LUW instances
    (required on iSeries, optional on LUW).

So I have incorporated some minor changes to some existing DB2 acceptance
tests, but they pass on Windows and iSeries both. These changes allow the
DB2i acceptance tests to consist purely of symlinks, rather than a mixture
of symlinks and tests with "iSeries specific" syntax. Separate tests are
always possible if you have strong feelings against changing the existing
AcceptanceTest suite.

Other than that, it's your standard fare; pretty much just a couple of
query tweaks and some parameter direction translation (iSeries uses words,
not letter codes in system tables). It was very easy to extend. Kudos to
your for that!

My version doesn't include the ClassIndex -> Reflections switch-over,
please let me know if that's something I should address. It looked like
that might require minor changes to some import statements in various
database adapters, but I'm not sure.

Please feel free to take a peek at the fork and offer any
suggestions/criticism. I went back and forth on the name
(DB2iSeriesEnvironment vs. DB2iEnvironment), and don't have strong feelings
either way.

If everything looks good to you, I'd love to issue a pull request.

I do have documentation (DB2 specific stuff) in fitnesse-style markup in
the works, and I'll get it to you in the next day or so.

Thanks,
Mike P.

On Thu, Aug 15, 2013 at 3:37 AM, Jake Benilov notifications@github.comwrote:

Hi Mike,

First of all, thanks for putting in the time and effort to investigate
this question and then code it up. It's very much appreciated!

The changes to DB2Environment needed to get everything working on the
iSeries environment are pretty substantial, and I don't think it's worth
extending that class to deal with both what it supports now and the
iSeries. Ideally:

1. we should add another class alongside DB2Environment (say,
   DB2iSeriesEnvironment, or something more appropriate - I'd leave the
   naming up to you) which contains the appropriate datatype mappings, JDBC
   driver name, and queries.
2. we add some documentation which outlines which environment should
   be used when (if this could be in markdown format, that would be ideal - I
   would then find a home for it in our documentation somewhere)
3. (ideally, but optionally) updates to the tests. It seems that a lot
   of the tests will be shared across all the DB2 implementations (this can be
   done using the symlink feature of Fitnesse), and a couple
   datatype-specific ones.

What do you think?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/158#issuecomment-22691254
.

Mike Patrick
mpatrick76@gmail.com

[benilovj]

Mike,
You should go ahead and just raise the PR. @javornikolov and I can then review and provide concrete feedback.
Thanks!