On May 23, 2012, at 7:34 PM, Adam Heath wrote: > On 05/23/2012 12:05 PM, Jacopo Cappellato wrote: >> >>> DBCPConnectionFactory is in framework/entity; having it require a >>> framework/entity/entitymodel seems the safest approach. >> >> I agree; what if we use: >> >> select count(testing_type_id) from testing_type where testing_type_id = 'ABC' > > Actually, no, TestingType seems like it is an entity that would never > be installed in a production environment; I certainly would never want > to install testing-only things. > > SequenceValueItem, EntityKeyStore always need to be in the *current* > datasource. Tenant* might be in another datasource, so you can't test > for those. > > count() also seems wrong, as it might be a heavy-weight operation. Is > this particular thing going to run often? That's an important thing > to consider if you switch the entity. > > What about using DatabaseMetadata? But that might have performance > issues. What does this check actually hope to accomplish? I haven't > read the code(sorry), and don't have time now to fully study it all.
Yeah, it is executed quite often (as far as I understand it) and we should use an efficient SQL that returns at least one record: the sql is used by DBCP to check if the connection is good and (I think) it is executed everytime the connection is borrowed/returned (but I don't remember exactly, it could be less often). I would also prefer to use one of SequenceValueItem or EntityKeyStore but I am worried that SequenceValueItem is a very used entity... EntityKeyStore may be a better choice. Using count() was simply a trick to be sure to get a record back even if the entity is empty... but if we use an entity that we are sure is populated we don't have to use it, of course. Of course if we use count(*) we have to use it in an entity with a very low number of rows... but I too would prefer to avoid it. Jacopo > >> that should always return one row with the 0 count; the entity is empty ootb >> (it is only used for automated tests) but the constraint on the pk should >> add an additional protection for efficiency. > > Well, that proves it, it really shouldn't create the entity except > during a test run. That's something I've been wanting to fix for > awhile now.
