Yes.
It makes sense.
In addition, I would suggest to change constructor signature. Also to
prevent user confusing.
ublic SQLTemplate(DataMap rootMap, String defaultTemplate, boolean
isFetchingDataRows) {
setDefaultTemplate(defaultTemplate);
setRoot(rootMap);
+ setFetchingDataRows(isFetchingDataRows);
}
And make previous not changed but deprecated.
Evgeny.
2010/4/27 Andrus Adamchik <andru..bjectstyle.org>:
> Hi Evgeny,
>
> The failure was a result of the other part of that change:
>
>.. -105,6 +105,7 @@ public class SQLTemplate extends Abstrac
> public SQLTemplate(DataMap rootMap, String defaultTemplate) {
> setDefaultTemplate(defaultTemplate);
> setRoot(rootMap);
> + setFetchingDataRows(true); // ObjEntity not passed, so it's DataRow
> query
> }
>
> So, SQLTemplate property defaults... I actually agree with you that the
> proposed defaults per r935207 make more sense when viewed in isolation. But
> I'd still like to keep the old ones for consistency reasons:
>
> 1. All SQLTemplate constructors create a query that fetches objects, so
> users will have to manually change to DataRows if they need to, without
> having to think about the query creation history.
> 2. When there are 2 properties in an object "abc" and "xyz", calling setAbc
> should not change the value of xyz implicitly. Different behavior may be
> confusing to many users.
>
> So back to the original issue CAY-1328... Adding
> "q2.setFetchingDataRows(true)" to the test fixes it. So the Jira IMO is
> about better error reporting, not changing the defaults. E.g. consider
> this:
>
> SQLTemplate q2 = new SQLTemplate(testDataMap, "SELECT * FROM ARTIST");
> q2.setFetchingDataRows(false); // the user might override the new default by
> mistake,
> // causing that same NPE as the Jira mentions
>
> If instead of an NPE Cayenne would throw CayenneRuntimeException saying
> "SQLTemplate is not mapped to ObjEntity and no SQLResult is set, so
> 'fetchingDataRows' property must be set to 'false'", I think we'll solve the
> problem in the least confusing way for the end user. One possible place to
> perform this validation is in the Query.route(..) method.
>
> Does this make sense?
>
> Andrus
>
>
>
>
> On Apr 18, 2010, at 7:31 PM, Evgeny Ryabitskiy wrote:
>
>> This was add to fix another JUnit test (that was failing).
>> JUnit test was expecting that this flag is false while use
>> setFatchingDataRows.
>> So I add it here, to didn't change previous behavior
>> Maybe I didn't understand the meaning of SQLResult....
>>
>> Evgeny.
>>
>>
>> 2010/4/18 Andrus Adamchik <andru..bjectstyle.org>:
>>>
>>> On Apr 18, 2010, at 12:46 AM, evgen..pache.org wrote:
>>>
>>>> public void setResult(SQLResult resultSet) {
>>>> + setFetchingDataRows(false); // turn off mapping to DataRows,
>>>> use
>>>> explicit
>>>> this.result = resultSet;
>>>> }
>>>
>>> Implicit flipping of the DataRows flag outside constructor seems
>>> counterintuitive. Also SQLResult is not equal to an object fetch. So I
>>> think
>>> this is wrong.
>>>
>>> Andrus
>>
>
>
This archive was generated by hypermail 2.0.0 : Tue Apr 27 2010 - 13:41:07 EDT