Cayenne
  1. Cayenne
  2. CAY-1470

Oracle: Problem with bind CHAR in select

    Details

      Description

      I have Table tUser

      desc tUser
      Name Null Type
      ------------------------------ --------
      USERID NOT NULL NUMBER(15)
      BRIEF NOT NULL CHAR(30)

      And wish to find user with BRIEF 'credit' :

      INFO QueryLogger: Detected and installed adapter: org.apache.cayenne.dba.oracle.OracleAdapter
      INFO QueryLogger: SELECT * FROM tUser WHERE Brief = ? [bind: 'credit']
      INFO QueryLogger: === returned 0 rows. - took 125 ms.

      But if I try whitespaces at the end I would get:

      INFO QueryLogger: SELECT * FROM tUser WHERE Brief = ? [bind: 'credit ']
      INFO QueryLogger: === returned 1 row. - took 109 ms.

      It's absolutely not CROSS DB behavior!

      1. CAY-1470-Test.patch
        1 kB
        Evgeny Ryabitskiy
      2. CAY-1470.patch
        4 kB
        Evgeny Ryabitskiy
      3. CAY-1470.patch
        6 kB
        Evgeny Ryabitskiy
      4. CAY-1470.patch
        6 kB
        Evgeny Ryabitskiy

        Activity

        Hide
        Andrus Adamchik added a comment - - edited

        I will remove a unit test that we happened to have in the code and that was testing this issue. Until the issue is fixed, we shouldn't keep a failing test. See r1566009

           public void testBindCHARInWHERE() {
                // add 2 Artists
                DataMap testDataMap = context.getEntityResolver().getDataMap("testmap");
                SQLTemplate q1 = new SQLTemplate(
                        testDataMap,
                        "INSERT INTO ARTIST VALUES (1, 'Surikov', null)",
                        true);
                SQLTemplate q2 = new SQLTemplate(
                        testDataMap,
                        "INSERT INTO ARTIST VALUES (2, 'Shishkin', null)",
                        true);
                QueryChain chain = new QueryChain();
                chain.addQuery(q1);
                chain.addQuery(q2);
                context.performNonSelectingQuery(chain);
                // now select one Artist by Name, It's matter that ARTIST_NAME is CHAR not VARCHAR
                SQLTemplate s1 = new SQLTemplate(
                        testDataMap,
                        "SELECT * FROM ARTIST WHERE ARTIST_NAME = #bind($ARTIST_NAME)",
                        true);
                // whitespace after name is for reason
                s1.setParameters(Collections.singletonMap("ARTIST_NAME", "Surikov "));
                List<DataRow> result = context.performQuery(s1);
                assertEquals(1, result.size());
            }
        
        Show
        Andrus Adamchik added a comment - - edited I will remove a unit test that we happened to have in the code and that was testing this issue. Until the issue is fixed, we shouldn't keep a failing test. See r1566009 public void testBindCHARInWHERE() { // add 2 Artists DataMap testDataMap = context.getEntityResolver().getDataMap("testmap"); SQLTemplate q1 = new SQLTemplate( testDataMap, "INSERT INTO ARTIST VALUES (1, 'Surikov', null)", true); SQLTemplate q2 = new SQLTemplate( testDataMap, "INSERT INTO ARTIST VALUES (2, 'Shishkin', null)", true); QueryChain chain = new QueryChain(); chain.addQuery(q1); chain.addQuery(q2); context.performNonSelectingQuery(chain); // now select one Artist by Name, It's matter that ARTIST_NAME is CHAR not VARCHAR SQLTemplate s1 = new SQLTemplate( testDataMap, "SELECT * FROM ARTIST WHERE ARTIST_NAME = #bind($ARTIST_NAME)", true); // whitespace after name is for reason s1.setParameters(Collections.singletonMap("ARTIST_NAME", "Surikov ")); List<DataRow> result = context.performQuery(s1); assertEquals(1, result.size()); }
        Hide
        Ari Maniatis added a comment -

        Have these patches been committed already?

        Show
        Ari Maniatis added a comment - Have these patches been committed already?
        Hide
        Andrus Adamchik added a comment -

        > In CAY-1470-Test.patch some change to DriverDataSource which will help to fix tests, if you don't mind I will commit it.

        This is pretty heavy-handed... Now of course Oracle won't let you pass properties as JDBC URL parameters (and this is supposed to be the most robust DB out there!)... I suggest maybe adding support for JDBC properties to be read per-profile from ~/.cayenne/connection.properties , as this is the file where we configure specific DBs without affecting others. E.g.:

        oracle.jdbc.driver = oracle.jdbc.driver.OracleDriver
        oracle.jdbc.driver.fixedString = true

        Show
        Andrus Adamchik added a comment - > In CAY-1470 -Test.patch some change to DriverDataSource which will help to fix tests, if you don't mind I will commit it. This is pretty heavy-handed... Now of course Oracle won't let you pass properties as JDBC URL parameters (and this is supposed to be the most robust DB out there!)... I suggest maybe adding support for JDBC properties to be read per-profile from ~/.cayenne/connection.properties , as this is the file where we configure specific DBs without affecting others. E.g.: oracle.jdbc.driver = oracle.jdbc.driver.OracleDriver oracle.jdbc.driver.fixedString = true
        Hide
        Evgeny Ryabitskiy added a comment -

        I have add fix for this problem + some test case.

        But test will fail on Oracle since fixedString property not set.

        In CAY-1470-Test.patch some change to DriverDataSource which will help to fix tests, if you don't mind I will commit it.

        Show
        Evgeny Ryabitskiy added a comment - I have add fix for this problem + some test case. But test will fail on Oracle since fixedString property not set. In CAY-1470 -Test.patch some change to DriverDataSource which will help to fix tests, if you don't mind I will commit it.
        Hide
        Andrus Adamchik added a comment -

        > fixedString String (containing boolean value) "true" causes JDBC to use FIXED CHAR semantics when setObject() is called with a String argument.
        > ...
        > So it solves this problem in best way.

        Awesome. That's the ideal way to fix such things.

        > Now I'm wandering it we could build-in setting of this property somewhere in Cayenne?

        Actually no. We intentionally never attempt to reconfigure DataSource from within Cayenne, as DataSource is an external subsystem to Cayenne, that should be independently configurable in a JEE environment. If this can possibly be done per PreparedStatement, then we can probably try it (although not sure how that would affect internal driver caching of PreparedStatements). Doing it globally per DataSource (or even per Connection), is going to affect other users of that DataSource/Connection.

        Show
        Andrus Adamchik added a comment - > fixedString String (containing boolean value) "true" causes JDBC to use FIXED CHAR semantics when setObject() is called with a String argument. > ... > So it solves this problem in best way. Awesome. That's the ideal way to fix such things. > Now I'm wandering it we could build-in setting of this property somewhere in Cayenne? Actually no. We intentionally never attempt to reconfigure DataSource from within Cayenne, as DataSource is an external subsystem to Cayenne, that should be independently configurable in a JEE environment. If this can possibly be done per PreparedStatement, then we can probably try it (although not sure how that would affect internal driver caching of PreparedStatements). Doing it globally per DataSource (or even per Connection), is going to affect other users of that DataSource/Connection.
        Hide
        Evgeny Ryabitskiy added a comment -

        To be honest I don't like solution with some inner methods.
        It's not working with DBCP since PreparedStatements are wrapped.

        Good news.
        I have found better solution.

        In Connection Properties for Oracle JDBC Drivers
        http://download.oracle.com/docs/cd/B14117_01/java.101/b10979/basic.htm#g1028323

        fixedString String (containing boolean value)
        "true" causes JDBC to use FIXED CHAR semantics when setObject() is called with a String argument. By default JDBC uses VARCHAR semantics. The difference is in blank padding. By default there is no blank padding. For example, 'a' does not equal 'a ' in a CHAR(4) unless fixedString is "true".

        So it solves this problem in best way.

        Now I'm wandering it we could build-in setting of this property somewhere in Cayenne?

        Show
        Evgeny Ryabitskiy added a comment - To be honest I don't like solution with some inner methods. It's not working with DBCP since PreparedStatements are wrapped. Good news. I have found better solution. In Connection Properties for Oracle JDBC Drivers http://download.oracle.com/docs/cd/B14117_01/java.101/b10979/basic.htm#g1028323 fixedString String (containing boolean value) "true" causes JDBC to use FIXED CHAR semantics when setObject() is called with a String argument. By default JDBC uses VARCHAR semantics. The difference is in blank padding. By default there is no blank padding. For example, 'a' does not equal 'a ' in a CHAR(4) unless fixedString is "true". So it solves this problem in best way. Now I'm wandering it we could build-in setting of this property somewhere in Cayenne?
        Hide
        Evgeny Ryabitskiy added a comment -

        Tests are done.

        this method works fine with VARCHAR.

        No performance problems also, even no problems with reflection. We don't need driver.

        Show
        Evgeny Ryabitskiy added a comment - Tests are done. this method works fine with VARCHAR. No performance problems also, even no problems with reflection. We don't need driver.
        Hide
        Evgeny Ryabitskiy added a comment -

        It's very good question. I will examine it.
        Behavior tests + performance tests

        While I ma studding this problem... could you say.. if it's possible to use Oracle Driver class in Cayenne?

        We could set scope "provided" for this dependency
        http://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope
        so it will be not transitive.

        One little problem is with not Oracle JDBC driver for Oracle... not sure if it's exists....?

        This question is because using native Java code is faster then Reflection....

        Show
        Evgeny Ryabitskiy added a comment - It's very good question. I will examine it. Behavior tests + performance tests While I ma studding this problem... could you say.. if it's possible to use Oracle Driver class in Cayenne? We could set scope "provided" for this dependency http://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope so it will be not transitive. One little problem is with not Oracle JDBC driver for Oracle... not sure if it's exists....? This question is because using native Java code is faster then Reflection....
        Hide
        Andrus Adamchik added a comment -

        So what happens if this is used with a VARCHAR column? Will there be any disadvantage over "setString"?

        Show
        Andrus Adamchik added a comment - So what happens if this is used with a VARCHAR column? Will there be any disadvantage over "setString"?
        Hide
        Evgeny Ryabitskiy added a comment -

        sorry.. redeploy patch

        Show
        Evgeny Ryabitskiy added a comment - sorry.. redeploy patch
        Hide
        Evgeny Ryabitskiy added a comment -

        One more patch... tested... it works anyway
        on Cayenne ConnectionPool / DBCP... Finally got what I want...

        Unfortunately solution become more complicated....
        Hope there are no more PreparedStatements then
        OraclePreparedStatement
        OraclePreparedStatementWrapper

        in Oracle Driver

        Show
        Evgeny Ryabitskiy added a comment - One more patch... tested... it works anyway on Cayenne ConnectionPool / DBCP... Finally got what I want... Unfortunately solution become more complicated.... Hope there are no more PreparedStatements then OraclePreparedStatement OraclePreparedStatementWrapper in Oracle Driver
        Hide
        Evgeny Ryabitskiy added a comment -

        >> This seems wrong. SET_FIXED_CHAR_METHOD.getClass() is "java.lang.refelect.Method"

        Yes! it's wrong, thx

        Show
        Evgeny Ryabitskiy added a comment - >> This seems wrong. SET_FIXED_CHAR_METHOD.getClass() is "java.lang.refelect.Method" Yes! it's wrong, thx
        Hide
        Evgeny Ryabitskiy added a comment -

        PreparedStatement is decorated by OraclePreparedStatementWrapper By Oracle driver... If I am using ConnectionPool

        Show
        Evgeny Ryabitskiy added a comment - PreparedStatement is decorated by OraclePreparedStatementWrapper By Oracle driver... If I am using ConnectionPool
        Hide
        Andrus Adamchik added a comment -

        > SET_FIXED_CHAR_METHOD.getClass().equals(value.getClass())

        This seems wrong. SET_FIXED_CHAR_METHOD.getClass() is "java.lang.refelect.Method"

        Show
        Andrus Adamchik added a comment - > SET_FIXED_CHAR_METHOD.getClass().equals(value.getClass()) This seems wrong. SET_FIXED_CHAR_METHOD.getClass() is "java.lang.refelect.Method"
        Hide
        Evgeny Ryabitskiy added a comment -

        Patch + JUnit test

        Show
        Evgeny Ryabitskiy added a comment - Patch + JUnit test
        Hide
        Andrus Adamchik added a comment -

        Nice.

        + if (value == null || SET_FIXED_CHAR_METHOD == null || type == Types.CLOB)

        { + // TODO should we check (st instance of OraclePreparedStatement) here? + super.setJdbcObject(st, value, pos, type, scale); + }

        Yeah, one possible issue is when PreparedStatement is decorated (e.g. by DBCP connection pool), and the decorator interface is simple PreparedStatement with no Oracle methods.

        Show
        Andrus Adamchik added a comment - Nice. + if (value == null || SET_FIXED_CHAR_METHOD == null || type == Types.CLOB) { + // TODO should we check (st instance of OraclePreparedStatement) here? + super.setJdbcObject(st, value, pos, type, scale); + } Yeah, one possible issue is when PreparedStatement is decorated (e.g. by DBCP connection pool), and the decorator interface is simple PreparedStatement with no Oracle methods.
        Hide
        Evgeny Ryabitskiy added a comment -

        small draft patch

        Show
        Evgeny Ryabitskiy added a comment - small draft patch
        Hide
        Andrus Adamchik added a comment -

        > Will be small patch for revision soon (today-tomorrow).

        That'll be cool as long as there is a failover to the current operation mode if reflection fails (e.g. if Oracle decides to change their API, or if there are driver versions that don't have it).

        Show
        Andrus Adamchik added a comment - > Will be small patch for revision soon (today-tomorrow). That'll be cool as long as there is a failover to the current operation mode if reflection fails (e.g. if Oracle decides to change their API, or if there are driver versions that don't have it).
        Hide
        Evgeny Ryabitskiy added a comment -

        Yes, RTRIM kills indexes as well.

        I would suggest to overwrite org.apache.cayenne.access.types.CharType.setJdbcObject(...), that is used while bind params, to some Oracle Extended type and add there Oracle Driver API ussage (it's not so private... but it's some Oracle specific extension of JDBC, don't like the way Oracle does it's Drivers but there is nothing better).
        Since we don't have Oracle Driver in dependencies we could use reflection.

        Already try it, it works

        Will be small patch for revision soon (today-tomorrow).

        Show
        Evgeny Ryabitskiy added a comment - Yes, RTRIM kills indexes as well. I would suggest to overwrite org.apache.cayenne.access.types.CharType.setJdbcObject(...), that is used while bind params, to some Oracle Extended type and add there Oracle Driver API ussage (it's not so private... but it's some Oracle specific extension of JDBC, don't like the way Oracle does it's Drivers but there is nothing better). Since we don't have Oracle Driver in dependencies we could use reflection. Already try it, it works Will be small patch for revision soon (today-tomorrow).
        Hide
        Andrus Adamchik added a comment -

        To be exact OracleAdapter is using RTRIM , but I suspect that will kill indexes use as well (something to check though)..

        Anyways, what are you suggesting? There is no JDBC solution ... We may potentially use oracle driver private APIs (with some driver version detection ... quoted forum dates back to 2003) for SelectQuery. SQLTemplate is a black box to Cayenne in a way... Maybe experiment using ParameterMetaData for binding type detection? I vaguely remember ParameterMetaData not providing any meaningful data on some other DBs, but that may work on Oracle.

        Show
        Andrus Adamchik added a comment - To be exact OracleAdapter is using RTRIM , but I suspect that will kill indexes use as well (something to check though).. Anyways, what are you suggesting? There is no JDBC solution ... We may potentially use oracle driver private APIs (with some driver version detection ... quoted forum dates back to 2003) for SelectQuery. SQLTemplate is a black box to Cayenne in a way... Maybe experiment using ParameterMetaData for binding type detection? I vaguely remember ParameterMetaData not providing any meaningful data on some other DBs, but that may work on Oracle.
        Hide
        Evgeny Ryabitskiy added a comment -

        related topic about this problem:

        http://forums.oracle.com/forums/thread.jspa?threadID=183252

        there is some solution for this problem via JDBC

        Show
        Evgeny Ryabitskiy added a comment - related topic about this problem: http://forums.oracle.com/forums/thread.jspa?threadID=183252 there is some solution for this problem via JDBC
        Hide
        Evgeny Ryabitskiy added a comment - - edited

        In this case I will lose performance since indexes are not used:

        SELECT * FROM tUser WHERE TRIM(Brief) = 'credit'

        TABLE ACCESS FULL

        SELECT * FROM tUser WHERE Brief = 'credit'

        TABLE ACCESS BY INDEX ROWID
        RANGE SCAN

        Show
        Evgeny Ryabitskiy added a comment - - edited In this case I will lose performance since indexes are not used: SELECT * FROM tUser WHERE TRIM(Brief) = 'credit' TABLE ACCESS FULL SELECT * FROM tUser WHERE Brief = 'credit' TABLE ACCESS BY INDEX ROWID RANGE SCAN
        Hide
        Andrus Adamchik added a comment -

        You haven't said that explcitly, but I assume you are using SQLTemplate. SQLTemplate is not cross-db by definition. E.g. SelectQuery would have generated SQL like WHERE TRIM(brief) = ? ... In SQLTemplate you must do it. If you also want to make it cross-db, use different SQL strings for different DB's within the same query (SQLTemplate allows that).

        I.e. I don't think this is a bug.

        Show
        Andrus Adamchik added a comment - You haven't said that explcitly, but I assume you are using SQLTemplate. SQLTemplate is not cross-db by definition. E.g. SelectQuery would have generated SQL like WHERE TRIM(brief) = ? ... In SQLTemplate you must do it. If you also want to make it cross-db, use different SQL strings for different DB's within the same query (SQLTemplate allows that). I.e. I don't think this is a bug.

          People

          • Assignee:
            Evgeny Ryabitskiy
            Reporter:
            Evgeny Ryabitskiy
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development