[nhibernate-development] NH-1353 Possibility to deactivate row count checking

[mikedoherty]

Hi,

I'm not sure if this is the correct place to be asking this question
but I would like to submit a patch for NH-1353 and have some questions
about my proposed solution and whether or not it would be suitable.

Basically our team need to be able to turn off the expectation that
the number of rows returned from the query matches the number expected
by NHibernate as we have no control over the triggers on our 3rd party
database. It would appear that we are not alone in this requirement.
NH-1353 proposes a potential solution to this problem along with a
patch but it appears that this patch was never accepted. As an
alternative then I would like to propose an alternative, and hopefully
more acceptable, solution.

Firstly what I would like to propose is that this expectation can be
turned off for an individual entity via the class or collection
mapping. As a result this approach would require a new attribute in
the hbm schema. Is it acceptable to add new attributes or do you need
to keep compatibility with the Java configuration schema?

Secondly, assuming a new attribute would be acceptable, what name
should I give to the new attribute? Looking at the schema the sql-
insert, sql-update and sql-delete elements allow control over this
using the "check" attribute. However class already has an attribute
called check for constraints. Within the code there are two ways of
referring to this check either as an Expectation or as
ExecuteUpdateResultCheckStyle so maybe the attribute could be called
"expectation" or "check-style"? I would appreciate guidance on this
too.

Thanks in advance,

Mike Doherty


--
Subscription settings: http://groups.google.com/group/nhibernate-development/subscribe?hl=en

[Fabio Maulo]

The check of the expectation is a fundamental check.

I have a proposal for you: change the stored procedure.

You can't touch it ? well... to work with untouchable code is an hard living; Elliot Ness is not a guy of this century.

--
Fabio Maulo

[John Davidson]

Is this not an even more fundamental issue, in that NHibernate is not a suitable tool to be used when multiple applications are accessing the same database at the same time? Or if it is not multiple accesses then the triggers are activated by some interaction in the NHibernate application and then there needs to be a refresh of data from the database, before the next interaction - also probably a not very desirable use of NHibernate.

John Davidson

[Diego Jancic]

Why is it 'fundamental' ? Can't NH work without it? It looks like a validation that could be easily removed.
I don't need it, and I think Fabio's solution is the best one, however I see no reason to don't add a new feature to NH if the patch is submitted.

Maybe there's another solution: make the validations injectable, that way validations can be extended or (although it's not recommended) removed.
Came on Fabio, be good with contributors =)

[Fabio Maulo]

- The JIRA is there

- I'm not the only one contributor

- NH is OSS and you can fork it <=== touchable code of this century; NO Eliot Ness

My point is:

Because you can't touch your code you create an issue in NH.

Because your RDBMS has a very poor pagination you create an issue in NH to have a workaround.

Because your framework does not create a good XYZ you ask more work in NH that should be done by NH team but following your rules.

and my point about about untouchable stuff is public:

http://fabiomaulo.blogspot.com/2009/06/database-eliot-ness-of-it.html

If you will remove the "expectation" check anything may happen during an insert/update/delete and you must pray that everything is under your control (from optimistic-lock to a simple value of column) inside and outside the batcher.

--
Fabio Maulo

[Diego Jancic]

Ohh.. I see, those are good points ;-)

[mikedoherty]

Fabio,

I'm sorry if my question didn't come across very well. I didn't mean
to offend.

In answer to your points above:

1) I didn't raise the defect. I commented on an existing defect in
jira.
2) I'm not asking you to do the work. I'm volunteering to submit a
patch. In fact I already have a prototype working and wanted to ask
your opinion as I am new to the NHibernate codebase.
3) I've already forked it. I'm offering to do the additional work to
make this a feature in Nibernate IF that is something you would like
included.

I fully understand all your points about the fundamental nature of
this check but why prevent people from removing that check if they
know what they are doing. The rest of the framework is really
extensible so why does this part have to be fixed? Also what about
Diego's point about making the validations injectable? NH is really
flexible in other areas.

At the end of the day I'm volunteering my effort to work with you to
create a solution for something that is a pain point for a few of your
users. If my contribution doesn't fit with your vision of where
NHibernate is going then fair enough.

Regards,

Mike

On Apr 22, 2:12 pm, Fabio Maulo <fabioma...@gmail.com> wrote:
> - The JIRA is there
> - I'm not the only one contributor
> - NH is OSS and you can fork it <=== touchable code of this century; NO
> Eliot Ness
>
> My point is:
> Because you can't touch your code you create an issue in NH.
> Because your RDBMS has a very poor pagination you create an issue in NH to
> have a workaround.
> Because your framework does not create a good XYZ you ask more work in NH
> that should be done by NH team but following your rules.
>
> and my point about about untouchable stuff is public:http://fabiomaulo.blogspot.com/2009/06/database-eliot-ness-of-it.html
>
> If you will remove the "expectation" check anything may happen during an
> insert/update/delete and you must pray that everything is under your control
> (from optimistic-lock to a simple value of column) inside and outside the
> batcher.
>
>
>
>
>
> On Thu, Apr 22, 2010 at 9:33 AM, Diego Jancic <jan...@gmail.com> wrote:
> > Why is it 'fundamental' ? Can't NH work without it? It looks like a
> > validation that could be easily removed.
> > I don't need it, and I think Fabio's solution is the best one, however I
> > see no reason to don't add a new feature to NH if the patch is submitted.
>
> > Maybe there's another solution: make the validations injectable, that way
> > validations can be extended or (although it's not recommended) removed.
> > Came on Fabio, be good with contributors =)
>

> > On Thu, Apr 22, 2010 at 09:09, Fabio Maulo <fabioma...@gmail.com> wrote:
>
> >> The check of the expectation is a fundamental check.
> >> I have a proposal for you: change the stored procedure.
> >> You can't touch it ? well... to work with untouchable code is an hard
> >> living; Elliot Ness is not a guy of this century.
>

[Fabio Maulo]

Mike,

I'm not feeling offended at all.

My opinion is only just my opinion.

Inject-ability:

The feature is there. You can inject your IBatcherFactory implementation. Inside your batcher you can override DoExecuteBatch and avoid the call of VerifyOutcomeBatched.

--
Fabio Maulo

[mikedoherty]

Thanks I'll have a look at injecting my own IBatcherFactory
implementation instead.

Mike

On Apr 22, 4:25 pm, Fabio Maulo <fabioma...@gmail.com> wrote:
> Mike,
> I'm not feeling offended at all.
> My opinion is only just my opinion.
>
> Inject-ability:
> The feature is there. You can inject your IBatcherFactory implementation.
> Inside your batcher you can override DoExecuteBatch and avoid the call
> of VerifyOutcomeBatched.
>

[Fabio Maulo]

What we should do with the issue NH-1353 ?

Close it because already supported ?

--
Fabio Maulo

[Fabio Maulo]

Dieguito querido!! ;) my friend...

NH is so injectable that you can change the whole behavior without touch the core.

But I know... there are so many injection points that many times it is really difficult to understand which point choose and in many cases what I saw is an addition of a "more granular" injection making understandable the responsibility of each class.

Taking this case as example:

- You must inject the Dialect

- through the dialect you can inject the Drive (you can do the same with a configuration property)

- through the Drive you can inject the batch-factory (you can do the same with a configuration property)

- through the batch-factory you can inject a real or a fake batcher or a "partially fake batcher"

Instead use all already available injections the proposal is add another configuration-property to avoid the call of one method that is represented by just one code line:

Expectations.VerifyOutcomeBatched(totalExpectedRowsAffected, rowsAffected);

Perhaps the right direction would be:

"Hi friend. I have this need..... the solution I have found is ..... is there another behavior-injection-point ?"

Believe me that, in general, the answer is "Yes" but to find it inside +1400 classes is not so easy, I know.

--
Fabio Maulo

[Diego Jancic]

I know Fabio. You didn't surprised me with that answer, you make me just love you more ;-) jajajaj
As you said, there're many injection points and it's really difficult to understand them.

[mikedoherty]

Thanks for your help. It all worked beautifully. I wrote up my
solution on stack overflow to hopefully help other people what
encounter the same issues:
http://stackoverflow.com/questions/1354362/toomanyrowsaffectedexception-with-encrypted-triggers/

Mike