incubator-empire-db-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Francis De Brabandere <franci...@gmail.com>
Subject Re: re: Prepared statement support?
Date Sun, 12 Dec 2010 20:02:54 GMT
Rainer,

I added the test (slightly modified), please check:
org.apache.empire.db.PreparedStatementTest

Cheers,
Francis

On Thu, Dec 2, 2010 at 3:03 PM, Rainer Döbele <doebele@esteam.de> wrote:
> Hi Francis,
>
> yes, I agree.
> Can you give me some advice where to best put the test procedure and how to call it?
>
> The test procedure itself should look somewhat like this: (CompanyDB and a connection
required)
>
>    void testPreparedStatement(CompanyDB db, Connection conn)
>    {
>        int empId = 1; // ID of test employee
>
>        // Define shortcuts for tables used - not necessary but convenient
>        SampleDB.Employees EMP = db.EMPLOYEES;
>        // Define the query
>        DBCommand cmd = db.createCommand();
>        // Create parameters
>        DBCmdParameter empIdParam  = cmd.addCmdParam(null);
>        // create statement
>        cmd.select(EMP.getColumns());
>        cmd.where(EMP.EMPLOYEE_ID.is(empIdParam));
>        // set param value
>        empIdParam.setValue(empId);
>        // check command
>        assertNotNull(cmd.getCmdParams());
>        assertTrue(cmd.getSelect().indexOf('?')>0);
>        // open reader
>        DBReader r = new DBReader();
>        try {
>            r.open(cmd, conn);
>            // must have one record
>            assertEquals(true, r.moveNext());
>            // Employee Id must be correct
>            assertEquals(empId, r.getValue(EMP.EMPLOYEE_ID));
>
>        } catch(Exception e) {
>            fail(r.getErrorMessage());
>        } finally {
>            r.close();
>        }
>    }
>
> Currently this would fail with an exception due to a bug in DBReader.open()
>
> Regards
> Rainer
>
> P.S. Shame that this comes to late for our current release candidate. Or do you think
we should cancel until we have resolved this (won't take long)
>
> Francis De Brabandere
>> from: Francis De Brabandere [mailto:francisdb@gmail.com]
>> to: empire-db-dev@incubator.apache.org
>> re: Re: re: Prepared statement support?
>>
>> We should create tests for this before fixing
>> On Dec 2, 2010 11:29 AM, "Rainer Döbele" <doebele@esteam.de> wrote:
>> > Hi everyone,
>> >
>> > thanks for your comment Matt.
>> >
>> > To my own surprise I have overlooked that there is already substantial
>> support for prepared statement generation in Empire-db now, but you have
>> to
>> explicitly declare the parameters.
>> > Here is an example of how to generate a prepared statement phrase and
>> execute it with the corresponding parameters:
>> >
>> > // Define the query
>> > DBCommand cmd = db.createCommand();
>> >
>> > // Create parameters
>> > DBCmdParameter depIdParam = cmd.addCmdParam(1);
>> > DBCmdParameter genderParam = cmd.addCmdParam('F');
>> >
>> > // create statement
>> > cmd.select(EMP.getColumns());
>> > cmd.where(EMP.DEPARTMENT_ID.is(depIdParam));
>> > cmd.where(EMP.GENDER.is(genderParam));
>> >
>> > // First execution
>> > String sql = cmd.getSelect();
>> > ResultSet r = db.executeQuery(sql, cmd.getCmdParams(), false, conn);
>> > // do something
>> > r.close();
>> >
>> > // Modify command parameters
>> > depIdParam.setValue(2);
>> > genderParam.setValue('M');
>> >
>> > // Second execution
>> > r = db.executeQuery(sql, cmd.getCmdParams(), false, conn);
>> > // do something
>> > r.close();
>> >
>> > This will result in the following SQL:
>> >
>> > SELECT t2.EMPLOYEE_ID, t2...
>> > FROM EMPLOYEES t2
>> > WHERE t2.DEPARTMENT_ID=? AND t2.GENDER=?
>> >
>> > And set the parameter to 1 and 'F' for the first query and to 2 and
>> 'M'
>> for the second.
>> >
>> > Unfortunately there is a bug in DBReader so that cmd params are not
>> properly set.
>> > This is the reason why I used db.executeQuery(..) instead of a
>> DBReader in
>> the example above.
>> > I will fix this bug as soon as possible.
>> >
>> > Another thing we should do is to use the prepared statements for
>> DBRecord.read (which in turn uses DBRowSet.readRecord(...)).
>> >
>> > As far as the pooling of prepared statements is concerned, if it's not
>> done by the data source already it can also be done by subclassing the
>> DBDatabaseDriver and overriding executeQuery() and / or executeSQL() and
>> do
>> it yourself. But it is not necessary for Empire-db to provide this.
>> >
>> > Kenji will this satisfy your needs?
>> >
>> > Regards,
>> > Rainer
>> >
>> >
>> >
>> > Matthew Bond wrote:
>> >> from: Matthew Bond [mailto:bond@bond-it.de]
>> >> to: empire-db-dev@incubator.apache.org; empire-db-
>> >> re: AW: Prepared statement support?
>> >>
>> >> Hi Rainer, Hi Kenji,
>> >>
>> >> Rainer's comments are true in a Web Application scenario where the
>> >> connection if got for a short time and then released again. Empire DB
>> >> can also be used in other scenarios, like a Fat Clients or Command
>> Line
>> >> Utility tools, where a connection will probably be held for the whole
>> >> duration of the application lifetime and PooledStatements could bring
>> >> more performance. So it really depends on what you application type
>> you
>> >> are programming.
>> >>
>> >> FYI: WebSphere too pools prepared statements (see page 2 of
>> http://www-
>> >>
>> 03.ibm.com/systems/resources/systems_i_advantages_perfmgmt_pdf_stmntcach
>> >> e.pdf "WebSphere, however, will do the caching automatically. When
>> you
>> >> execute a query, WebSphere determines if the SQL text is already in
>> the
>> >> cache and if so, it will use that cached statement instead of
>> preparing
>> >> a new one." ). So if EmpireDB was extended to make more use of
>> Prepared
>> >> Statements it would be advantageous.
>> >>
>> >> However as Rainer describes, the big benefit of using EmpireDB is
>> that
>> >> the selects are going to be way better than other ORM's as the
>> developer
>> >> hand crafts the "SQL" statement.
>> >>
>> >> The great thing is that it is Open Source so if you feel strongly
>> about
>> >> the use of PreparedStatements, you could submit a Patch adding this
>> >> functionality.
>> >>
>> >> Cheers
>> >> Matt
>> >>
>> >> -----Ursprüngliche Nachricht-----
>> >> Von: Rainer Döbele [mailto:doebele@esteam.de]
>> >> Gesendet: Donnerstag, 2. Dezember 2010 00:11
>> >> An: empire-db-user@incubator.apache.org; empire-db-
>> >> dev@incubator.apache.org
>> >> Betreff: re: Prepared statement support?
>> >>
>> >> Dear Kenji,
>> >>
>> >> I have reviewed our code and thought about this subject again.
>> >> As you mentioned there is both a performance and a security issue to
>> >> consider.
>> >> For the moment I would like to focus on the performance issue as
>> >> security can as well be established by other measures.
>> >>
>> >> It's pretty obvious to understand that creating a prepared statement
>> and
>> >> executing it multiple times with varying parameters is superior over
>> >> creating a normal statement each time. But as far as I understand it,
>> >> the advantage of a ps exists only as long as the statement lives, and
>> >> ends when you close it.
>> >>
>> >> The problem is, that a prepared statement is created for a particular
>> >> connection. In a web-application we usually use a connection pool and
>> >> the connection is fetched for a particular request. It is extremely
>> >> rare, that the same statement is executed multiple times within a
>> single
>> >> request - whereas it is very likely that the same statement needs to
>> be
>> >> executed by other users' requests. As those other users have
>> different
>> >> connections they cannot share the same prepared statement.
>> >>
>> >> Here is a thread discussing this issue:
>> >> http://www.velocityreviews.com/forums/t644638-jdbc-preparedstatement-
>> in-
>> >> a-multi-threaded-environment.html
>> >>
>> >> As Empire-db does not store or maintain a connection, it is not
>> sensible
>> >> for us to store the actual JDBC prepared statement object. But this
>> >> might not be necessary as it could be done on another level. Possibly
>> >> the solution lies just in another Apache Project: Apache Commons
>> DBCP.
>> >> http://commons.apache.org/dbcp/index.html
>> >>
>> >> From my understanding it should be possible to use a commons-dbcp
>> >> connection pool that will also pool prepared statements. The
>> connections
>> >> returned by the pool can be used with Empire db just like a normal
>> JDBC
>> >> connection.
>> >> Of course we still need to enforce and extend the generation of
>> prepared
>> >> statement phrases beyond the CUD operations.
>> >>
>> >> Still we must keep in mind, that probably for most real world
>> >> applications the performance benefit of prepared statements over
>> simple
>> >> statements is negligible, and it is our primary goal to maintain
>> >> simplicity and transparency.
>> >> It is IMO far more important to be able to create efficient
>> statements -
>> >> and avoid the problem of OR-Mappers that usually work with lots of
>> >> simple operations. After all, one clever statement with server side
>> db
>> >> logic will still execute a lot faster than 10 prepared statements
>> with
>> >> trailed Java logic.
>> >> (Still the gloal is to have it all of course)
>> >>
>> >> Any more suggestions or remarks on this topic?
>> >>
>> >> Regards
>> >> Rainer
>> >>
>> >>
>> >> Kenji Nakamura wrote:
>> >> > from: Kenji Nakamura [mailto:kenji_nakamura@diva-america.com]
>> >> > to: empire-db-user@incubator.apache.org
>> >> > re Re: Prepared statement support?
>> >> >
>> >> > Rainer,
>> >> >
>> >> > Thank you for your reply. My comment are inline.
>> >> >
>> >> > On Wed, Dec 1, 2010 at 2:14 AM, Rainer Döbele <doebele@esteam.de>
>> >> > wrote:
>> >> > > Hi Kenji,
>> >> > >
>> >> > > thanks for your interesting links about this subject.
>> >> > >
>> >> > > It is certainly true, that the performance of a prepared
>> statements
>> >> > is better when you execute it multiple times with varying parameter
>> >> > values.
>> >> > > This is not always possible when varying statements with
>> conditional
>> >> > joins are created at runtime.
>> >> > > For a one-time statement using a prepared statement does not
>> execute
>> >> > faster than a normal statement.
>> >> >
>> >> > I understand the issue that the use of PreparedStatement seems to
>> have
>> >> > overhead and actually it may take longer if we measure it with a
>> >> > single execution from application developer's point of view, but
>> the
>> >> > compiled result of the statement is kept added to Oracle's cache
>> and
>> >> > it flushes the compiled results of the PreparedStatement invoked
>> from
>> >> > different applications as the cache is managed per SID in Oracle.
>> So
>> >> > it has negative impact from the DBA's point of view. It is not an
>> >> > issue as long as the DB is used as the data storage of a web
>> >> > application server and the performance of the app is only concern,
>> but
>> >> > the assumption is not true when the DB is also used in data
>> >> > processing.
>> >> >
>> >> > > The inclusion of parameter values in the SQL text when assembling
>> >> > statements is an advantage when it comes to logging (logging of
>> >> > parameterized statements is not sufficient to track errors) or for
>> the
>> >> > creation of SQL scripts that are saved and executed later.
>> >> >
>> >> > I see your point.
>> >> >
>> >> > >
>> >> > > Currently Empire-db uses prepared statements by default only for
>> >> > statements with BLOB and CLOB fields.
>> >> > >
>> >> > > However at least as far as update and insert statements are
>> >> > > concerned
>> >> > you can override the method useCmdParam() in DBCommandOracle, but
>> you
>> >> > need to subclass the DBDatabaseDriverOracle and override
>> createCommand
>> >> > first. If you return true in useCmdParam(), then Empire-DB will use
>> a
>> >> > prepared statement and supply this value as a prepared statement
>> >> > parameter.
>> >> >
>> >> > From the point of view of Oracle administrator, the primary
>> interest
>> >> > is how to reduce the # of hard parse and increase the hit rate of
>> the
>> >> > cache, and using PreparedStatement only for CUD operation is not
>> >> > sufficient if the ratio of Select outweigh CUD operations. From
>> >> > security point of view, Select statement with parameters embedding
>> >> > user's input is as vulnerable as other DMLs, so the option to use
>> >> > PreparedStatement for CUD operation doesn't address those concerns,
>> >> > while it may be useful to improve the performance on iterative
>> >> > operations.
>> >> >
>> >> > >
>> >> > > Personally I have used Empire-DB in many projects and performance
>> or
>> >> > security have never been a problem. However, if you except to
>> execute
>> >> > 10.000 sql statements a minute then certainly this needs to be
>> >> > thoroughly checked.
>> >> >
>> >> > It is nice to know the framework has been proven in production
>> >> > environments. Our current performance test also doesn't show the
>> hard
>> >> > parse is the primary culprit of the performance bottleneck, so it
>> is
>> >> > not an urgent problem, but I'd like prepare to answer the questions
>> >> > from our DB engineers.
>> >> >
>> >> > >
>> >> > > I have created a new Jira (EMPIREDB-91) issue for us to check,
>> how
>> >> > and where we can increase and optimize the use of prepared
>> statements.
>> >> >
>> >> > Thank you for the reaction. I registered myself to the watch list.
>> Let
>> >> > me know if I can do something to make this forward.
>> >> >
>> >> > Lastly, I really thank you to share the framework in public. I have
>> >> > used Toplink, Hibernate, and iBatis, but I favor empire-db a lot
>> >> > because of the simplicity and type-safe coding. It is very
>> >> > straightforward to customize to fulfill our specific needs such as
>> the
>> >> > support of TableFunction in Oracle.
>> >> >
>> >> > Regards,
>> >> >
>> >> > Kenji
>> >> >
>> >> > >
>> >> > > Regards
>> >> > > Rainer
>> >> > >
>> >> > >
>> >> > > Kenji Nakamura wrote:
>> >> > >> from: Kenji Nakamura [mailto:kenji_nakamura@diva-america.com]
>> >> > >> to: empire-db-user@incubator.apache.org
>> >> > >> re: Prepared statement support?
>> >> > >>
>> >> > >> Hi,
>> >> > >>
>> >> > >> I got a question from one of our DB engineer about the use
of
>> >> > prepared
>> >> > >> statements.
>> >> > >> According to him, or a thread in AskTom, it is always preferred
>> to
>> >> > use
>> >> > >> PreparedStatement instead of Statement whenever possible.
>> >> > >>
>> >> >
>> http://asktom.oracle.com/pls/asktom/f?p=100:11:7607696421577136::::P11
>> >> > _
>> >> > Q
>> >> > >> UESTION_ID:1993620575194
>> >> > >>
>> >> > >> As far as I looked at the code, PreparedStatement is not used
>> other
>> >> > >> than DBDatabaseDriver class and the method is not used from
>> other
>> >> > >> code.
>> >> > >>
>> >> > >> My understanding is that creation of PreparedStatement has
>> certain
>> >> > >> overhead, but statement pooling introduced in JDBC 3.0 mitigates
>> >> > >> the impact especially from application server point of view.
>> >> > >> We use Oracle, and the DB engineer explained that the use
of
>> >> > statement
>> >> > >> floods the library cache in SGA and reduce the hit rate of
>> >> > >> pre-compiled statements so it has negative impact on entire
db,
>> and
>> >> > >> using PreparedStatement simply reduces the cost of hard parse.
>> >> > >>
>> >> > >> Another aspect is about SQL injection prevention. I noticed
>> single
>> >> > >> quotes are escaped at DBDatabaseDriver#getValueString() method,
>> but
>> >> > >> the preferred way to prevent SQL injection is to use
>> >> > PreparedStatement
>> >> > >> according to OWASP website.
>> >> > >>
>> http://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet
>> >> > >>
>> >> > >> Would you tell me the design philosophy or reasons not to
use or
>> >> > >> provide the option to use prepared statement? Is it possible,
or
>> >> > have
>> >> > >> a plan to support PreparedStatement?
>> >> > >>
>> >> > >> Thanks,
>> >> > >>
>> >> > >> Kenji Nakamura
>> >> > >
>



-- 
http://www.somatik.be
Microsoft gives you windows, Linux gives you the whole house.

Mime
View raw message