You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2009/02/23 05:30:36 UTC

Re: svn commit: r746849 - in /commons/proper/dbcp/trunk: src/java/org/apache/commons/dbcp/ src/java/org/apache/commons/jocl/ src/test/ src/test/org/apache/commons/dbcp/ src/test/org/apache/commons/jocl/ xdocs/

sebb wrote:
> On 22/02/2009, psteitz@apache.org <ps...@apache.org> wrote:
>   
>> Author: psteitz
>>  Date: Sun Feb 22 23:43:52 2009
>>  New Revision: 746849
>>
>>  URL: http://svn.apache.org/viewvc?rev=746849&view=rev
>>  Log:
>>  Added a connectionInitSqls configuration parameter to BasicDataSource allowing
>>  the user to specify a collection of SQL statements to execute one time when
>>  a physical database connection is first opened.
>>  JIRA: DBCP-175
>>  Thanks to Jiri Melichna and Jerome Lacoste.
>>
>>
>>  Modified:
>>     commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
>>     commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSourceFactory.java
>>     commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnectionFactory.java
>>     commons/proper/dbcp/trunk/src/java/org/apache/commons/jocl/JOCLContentHandler.java
>>     commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java
>>     commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSourceFactory.java
>>     commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java
>>     commons/proper/dbcp/trunk/src/test/org/apache/commons/jocl/TestJOCLContentHandler.java
>>     commons/proper/dbcp/trunk/src/test/testpool.jocl
>>     commons/proper/dbcp/trunk/xdocs/changes.xml
>>     commons/proper/dbcp/trunk/xdocs/configuration.xml
>>
>>  Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
>>  URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java?rev=746849&r1=746848&r2=746849&view=diff
>>  ==============================================================================
>>  --- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java (original)
>>  +++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java Sun Feb 22 23:43:52 2009
>>  @@ -19,6 +19,11 @@
>>
>>   import java.io.PrintWriter;
>>   import java.util.Properties;
>>  +import java.util.Collection;
>>  +import java.util.List;
>>  +import java.util.ArrayList;
>>  +import java.util.Iterator;
>>  +import java.util.Collections;
>>   import java.sql.Connection;
>>   import java.sql.Driver;
>>   import java.sql.DriverManager;
>>  @@ -801,6 +806,60 @@
>>          }
>>          this.restartNeeded = true;
>>      }
>>  +
>>  +    /**
>>  +     * These SQL statements run once after a Connection is created.
>>  +     * <p>
>>  +     * This property can be used for example to run ALTER SESSION SET
>>  +     * NLS_SORT=XCYECH in an Oracle Database only once after connection
>>  +     * creation.
>>  +     * </p>
>>  +     *
>>  +     * @since 1.3
>>  +     */
>>  +    protected List connectionInitSqls;
>>  +
>>     
>
> Why protected and not private?
>
> This allows sub-classes to break the thread-safety provided by the
> synch. get and set methods.
>   
Good point.   All of BasicDataSource's properties are set up like 
this.   I was not there at the beginning, but if you look at the earlier 
revisions, you can see that BasicDataSource was not initially set up to 
support "dynamic reconfiguration" so the property getters/setters were 
not synchronized.  Support for dynamic reconfiguration was started in 
r132132 , but never really completed (the "restartNeeded" field is never 
read).  It is not clear to me how exactly this would ever work, to be 
honest.  The current setup is a little funny in that some setters have 
no effect once the pool has been initialized.  See comments in the 
javadoc for e.g. setDriverClassName.  None of this really answers your 
question though.  For backward compatability, the existing fields need 
to remain protected, but I guess we could make this new one private.

Phil 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>   


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r746849 - in /commons/proper/dbcp/trunk: src/java/org/apache/commons/dbcp/ src/java/org/apache/commons/jocl/ src/test/ src/test/org/apache/commons/dbcp/ src/test/org/apache/commons/jocl/ xdocs/

Posted by sebb <se...@gmail.com>.
On 23/02/2009, Phil Steitz <ph...@gmail.com> wrote:
> sebb wrote:
>
> > On 22/02/2009, psteitz@apache.org <ps...@apache.org> wrote:
> >
> >
> > > Author: psteitz
> > >  Date: Sun Feb 22 23:43:52 2009
> > >  New Revision: 746849
> > >
> > >  URL: http://svn.apache.org/viewvc?rev=746849&view=rev
> > >  Log:
> > >  Added a connectionInitSqls configuration parameter to BasicDataSource
> allowing
> > >  the user to specify a collection of SQL statements to execute one time
> when
> > >  a physical database connection is first opened.
> > >  JIRA: DBCP-175
> > >  Thanks to Jiri Melichna and Jerome Lacoste.
> > >
> > >
> > >  Modified:
> > >
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
> > >
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSourceFactory.java
> > >
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolableConnectionFactory.java
> > >
> commons/proper/dbcp/trunk/src/java/org/apache/commons/jocl/JOCLContentHandler.java
> > >
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSource.java
> > >
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestBasicDataSourceFactory.java
> > >
> commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java
> > >
> commons/proper/dbcp/trunk/src/test/org/apache/commons/jocl/TestJOCLContentHandler.java
> > >    commons/proper/dbcp/trunk/src/test/testpool.jocl
> > >    commons/proper/dbcp/trunk/xdocs/changes.xml
> > >    commons/proper/dbcp/trunk/xdocs/configuration.xml
> > >
> > >  Modified:
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
> > >  URL:
> http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java?rev=746849&r1=746848&r2=746849&view=diff
> > >
> ==============================================================================
> > >  ---
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
> (original)
> > >  +++
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/BasicDataSource.java
> Sun Feb 22 23:43:52 2009
> > >  @@ -19,6 +19,11 @@
> > >
> > >  import java.io.PrintWriter;
> > >  import java.util.Properties;
> > >  +import java.util.Collection;
> > >  +import java.util.List;
> > >  +import java.util.ArrayList;
> > >  +import java.util.Iterator;
> > >  +import java.util.Collections;
> > >  import java.sql.Connection;
> > >  import java.sql.Driver;
> > >  import java.sql.DriverManager;
> > >  @@ -801,6 +806,60 @@
> > >         }
> > >         this.restartNeeded = true;
> > >     }
> > >  +
> > >  +    /**
> > >  +     * These SQL statements run once after a Connection is created.
> > >  +     * <p>
> > >  +     * This property can be used for example to run ALTER SESSION SET
> > >  +     * NLS_SORT=XCYECH in an Oracle Database only once after
> connection
> > >  +     * creation.
> > >  +     * </p>
> > >  +     *
> > >  +     * @since 1.3
> > >  +     */
> > >  +    protected List connectionInitSqls;
> > >  +
> > >
> > >
> >
> > Why protected and not private?
> >
> > This allows sub-classes to break the thread-safety provided by the
> > synch. get and set methods.
> >
> >
>  Good point.   All of BasicDataSource's properties are set up like this.   I
> was not there at the beginning, but if you look at the earlier revisions,
> you can see that BasicDataSource was not initially set up to support
> "dynamic reconfiguration" so the property getters/setters were not
> synchronized.  Support for dynamic reconfiguration was started in r132132 ,
> but never really completed (the "restartNeeded" field is never read).  It is
> not clear to me how exactly this would ever work, to be honest.  The current
> setup is a little funny in that some setters have no effect once the pool
> has been initialized.  See comments in the javadoc for e.g.
> setDriverClassName.  None of this really answers your question though.  For
> backward compatability, the existing fields need to remain protected, but I
> guess we could make this new one private.

Sounds like a major overhaul is needed at some point ...

Given that DBCP is presumably intended for use with multiple threads,
the thread-safety of each class should be documented in some way.
There are no doubt some classes which are only intended for use from a
single thread, and there will be others that are intended for use by
many threads. Having this documented would be helpful to both user and
developer...

>  Phil
>
> >
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
> >
>
>
> ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org