Remove synchronization of executeStatement()

To resolve the synchronization at SQLStatementStore we not only need to care on the perpared statemnt but also on the counter and the resultset. While the best solution for the prepared statement again seams to be a ThreadLocal variable I don't think this would be the right solution for counter and resultset. I would pass the result set as parameter from executeQuery to processData method or merge both methods into one.

The only reason to have a global counter is that we need to pass the inital index between the 3 bind methods. In my opinion we could evaluate the inital counters for each bind when building the SQL statement as these initial indizes will never change for an SQLStatementStore instance. With the global inital counters the counter itself can be moved into each of the bind methods. Does that make any sense to you?

I looked in to this matter.

1) I agree that prepared statement should preferably by ThreadLocal.

2) As only the executeQeury is the producer of resultSet and processData is the consumer of ResultSet, I agree with you that it doesn't make sense to keep it as global class attribute. But I am not in favor of merging executeQuery and processData together because I think structurally and functionally they are different methods. So I would prefer to return resultSet from executeQuery and pass it as a parameter to processData.

3) As far as global counter is concerned. It is a bit complex issue. The global counter sets the offset for binding the parameters in the prepared Statement in three methods bindNewEntity, bindIdentity, bindOldEntity. Now the calculation of the these offset breakpoint for each of these methods is different. For example

bindNewEntity - It involved SQLColumnInfo[] columns, SQLFieldInfo[] _fields, newentity to loop through the binding parameters
bindIdentity - It involves SQLColumnInfo[] _ids
bindOldEntity - It involved SQLColumnInfo[] columns, SQLFieldInfo[] _fields, oldentity to loop through the binding parameters

I am not sure but I guess we have to run same kind of loop for all the three offsets required above inside build statement in order to calculate their offsets separately. Isn't it an overhead?

I wouldn't mind to make the existing counter as ThreadLocal though because it will save us extra processing and also because the counter is being used by multiple methods.

Regards, Ahmad

I looked into the possibility of pre-calculating the offsets in the build statement. I came up with the following code for calculating the offsets. Just sharing it for the review.

        int _offsetNewEntity; //class attribute
        int _offsetIdentity; //class attribute
        int _offsetOldEntity; //class attribute

        //buildStatement Method
        int storedFields = 0;
        for (int i = 0; i < _fields.length; ++i) {
            if (_fields[i].isStore()) {
                SQLColumnInfo[] columns = _fields[i].getColumnInfo();

                for (int j = 0; j < columns.length; j++) {
                    ++storedFields;
                }
            }
        }
        
        _offsetNewEntity = 1;
        _offsetIdentity = 1 + storedFields;
        _offsetOldEntity = _offsetIdentity + _ids.length;

Regards, Ahmad

P.S I agree with you that this will be much more efficient because this will only be executed once at the time of buildling the statement.

Regards, Ahmad

Patch Attached .

One thing to review is the calculating of offsets. I ran the test cases and also try to see assigned values in debug mode. The tests run successfully.

One other thing to note that, I am passing offset parameter explicitly in the bindIdentity method. Thid is so because this method is called for storeStatement and then for loadStatement. Both these Statement needs different offsets to starts with.

Regards, Ahmad

Great work. Commited patch as is.