Hi Joe, Thank you for taking time to help review. On Oct 27, 2014, at 3:27 PM, huizhe wang <[email protected]> wrote:
> Hi Lance, > > Does test37 needs to loop through all of the columns? Wanted a test which made sure that for the number of columns in the metadata that the fields can be set. Does it have to, not really, but it wanted to sanity check that they could > There doesn't seem to be a differentiator or different factor between the > columns, e.g if one column is set to auto_increment and another not, the > results would be different. Also in the test, some of the setters seem to be > invalid, for example, whether the column is auto_increment or not shall > affect setters such as setPrecision, setScale, setNullable, isReadOnly, > isWritable and etc., shouldn't it? For this abstract class it is just a matter of validating the methods can be set for the various columns. in an implementation of a RowSet, the data associated with a given column will have more actual meaning based on the column definition. So yes the values being set are somewhat artificial but that is OK in this instance. Also different databases for example can allow an auto-incrementable column to be writable, others do not so your milage varies as to which fields can and cannot have coresponding values. Best, Lance > > -Joe > > On 10/27/2014 11:06 AM, Lance Andersen wrote: >> Hi all, >> >> Need a reviewer for the RowSetMetaDataImpl tests which also includes a fix >> to isDefinitelyWritable so that it validates the column index ranges >> >> Included are some minor housekeeping changes to remove redundant classes >> >> The webrev can be found at >> http://cr.openjdk.java.net/~lancea/8062198/webrev.00/ >> >> Best >> Lance >> >> >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> [email protected] >> >> >> > Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [email protected]
