OFBiz › OFBiz - Dev

Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

taher

Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

Hi Shi,

I can see this is related to OFBIZ-8137 right?

Anyway, are you sure this is the correct fix? The original condition says
if the table name does not start with the schema name, then append the
schema name. This is used because Postgres 7.3 did not include the schema
name. Your fix is to add the schema name followed by "dot" right? What if
the table name contains the schema name followed by a dot (it is a legal
character) then you are back to the same issue no?

Maybe I missed something that you caught while debugging, but just wanted
to highlight because the fix seems a bit odd.

Taher Alkhateeb

On Sun, Sep 4, 2016 at 11:57 AM, <[hidden email]> wrote:

> Author: shijh
> Date: Sun Sep 4 08:57:44 2016
> New Revision: 1759155
>
> URL: http://svn.apache.org/viewvc?rev=1759155&view=rev
> Log:
> OFBIZ-8137 Entityname starts with schema name causes error in PostgreSQL
>
> Modified:
> ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java
>
> Modified: ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/
> src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java?rev=1759155&
> r1=1759154&r2=1759155&view=diff
> ============================================================
> ==================
> --- ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java (original)
> +++ ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java Sun Sep 4 08:57:44 2016
> @@ -1001,7 +1001,7 @@ public class DatabaseUtil {
> String tableName = tableSet.getString("TABLE_NAME");
> // for those databases which do not return the schema
> name with the table name (pgsql 7.3)
> boolean appendSchemaName = false;
> - if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName)) {
> + if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName + "\\.")) {
> appendSchemaName = true;
> }
> if (needsUpperCase && tableName != null) {
> @@ -3042,7 +3042,7 @@ public class DatabaseUtil {
> String tableName = rawTableName;
> // for those databases which do not return the schema name
> with the table name (pgsql 7.3)
> boolean appendSchemaName = false;
> - if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName)) {
> + if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName + "\\.")) {
> appendSchemaName = true;
> }
> if (needsUpperCase && tableName != null) {
>
>
>

Shi Jinghai-3

Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

Hi Taher,

Yes, it's for OFBIZ-8137.

Good questions on why adding a dot. Honest, I don't think it's in our entity name conventions, maybe it's safe to use it here.

Kind Regards,

Shi Jinghai

-----邮件原件-----
发件人: Taher Alkhateeb [mailto:[hidden email]]
发送时间: 2016年9月4日 17:15
收件人: OFBIZ Development Mailing List
主题: Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

Hi Shi,

I can see this is related to OFBIZ-8137 right?

Anyway, are you sure this is the correct fix? The original condition says if the table name does not start with the schema name, then append the schema name. This is used because Postgres 7.3 did not include the schema name. Your fix is to add the schema name followed by "dot" right? What if the table name contains the schema name followed by a dot (it is a legal
character) then you are back to the same issue no?

Maybe I missed something that you caught while debugging, but just wanted to highlight because the fix seems a bit odd.

Taher Alkhateeb

On Sun, Sep 4, 2016 at 11:57 AM, <[hidden email]> wrote:

> Author: shijh
> Date: Sun Sep 4 08:57:44 2016
> New Revision: 1759155
>
> URL: http://svn.apache.org/viewvc?rev=1759155&view=rev
> Log:
> OFBIZ-8137 Entityname starts with schema name causes error in
> PostgreSQL
>
> Modified:
> ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java
>
> Modified: ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/
> src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java?rev=17591
> 55&
> r1=1759154&r2=1759155&view=diff
> ============================================================
> ==================
> --- ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java (original)
> +++ ofbiz/trunk/framework/entity/src/main/java/org/apache/
> ofbiz/entity/jdbc/DatabaseUtil.java Sun Sep 4 08:57:44 2016 @@
> -1001,7 +1001,7 @@ public class DatabaseUtil {
> String tableName = tableSet.getString("TABLE_NAME");
> // for those databases which do not return the
> schema name with the table name (pgsql 7.3)
> boolean appendSchemaName = false;
> - if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName)) {
> + if (tableName != null && lookupSchemaName != null
> + &&
> !tableName.startsWith(lookupSchemaName + "\\.")) {
> appendSchemaName = true;
> }
> if (needsUpperCase && tableName != null) { @@
> -3042,7 +3042,7 @@ public class DatabaseUtil {
> String tableName = rawTableName;
> // for those databases which do not return the schema
> name with the table name (pgsql 7.3)
> boolean appendSchemaName = false;
> - if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName)) {
> + if (tableName != null && lookupSchemaName != null &&
> !tableName.startsWith(lookupSchemaName + "\\.")) {
> appendSchemaName = true;
> }
> if (needsUpperCase && tableName != null) {
>
>
>

taher

Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

I see Shi, you're right about the conventions.

This makes me feel like I wish to refactor the whole thing to make it
programmatic instead of just using String manipulation, but that's perhaps
for another day. Thank you for the feedback.

On Sun, Sep 4, 2016 at 12:43 PM, Shi Jinghai <[hidden email]> wrote:

> Hi Taher,
>
> Yes, it's for OFBIZ-8137.
>
> Good questions on why adding a dot. Honest, I don't think it's in our
> entity name conventions, maybe it's safe to use it here.
>
> Kind Regards,
>
> Shi Jinghai
>
>
> -----邮件原件-----
> 发件人: Taher Alkhateeb [mailto:[hidden email]]
> 发送时间: 2016年9月4日 17:15
> 收件人: OFBIZ Development Mailing List
> 主题: Re: svn commit: r1759155 - /ofbiz/trunk/framework/entity/
> src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java
>
> Hi Shi,
>
> I can see this is related to OFBIZ-8137 right?
>
> Anyway, are you sure this is the correct fix? The original condition says
> if the table name does not start with the schema name, then append the
> schema name. This is used because Postgres 7.3 did not include the schema
> name. Your fix is to add the schema name followed by "dot" right? What if
> the table name contains the schema name followed by a dot (it is a legal
> character) then you are back to the same issue no?
>
> Maybe I missed something that you caught while debugging, but just wanted
> to highlight because the fix seems a bit odd.
>
> Taher Alkhateeb
>
> On Sun, Sep 4, 2016 at 11:57 AM, <[hidden email]> wrote:
>
> > Author: shijh
> > Date: Sun Sep 4 08:57:44 2016
> > New Revision: 1759155
> >
> > URL: http://svn.apache.org/viewvc?rev=1759155&view=rev
> > Log:
> > OFBIZ-8137 Entityname starts with schema name causes error in
> > PostgreSQL
> >
> > Modified:
> > ofbiz/trunk/framework/entity/src/main/java/org/apache/
> > ofbiz/entity/jdbc/DatabaseUtil.java
> >
> > Modified: ofbiz/trunk/framework/entity/src/main/java/org/apache/
> > ofbiz/entity/jdbc/DatabaseUtil.java
> > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/
> > src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java?rev=17591
> > 55&
> > r1=1759154&r2=1759155&view=diff
> > ============================================================
> > ==================
> > --- ofbiz/trunk/framework/entity/src/main/java/org/apache/
> > ofbiz/entity/jdbc/DatabaseUtil.java (original)
> > +++ ofbiz/trunk/framework/entity/src/main/java/org/apache/
> > ofbiz/entity/jdbc/DatabaseUtil.java Sun Sep 4 08:57:44 2016 @@
> > -1001,7 +1001,7 @@ public class DatabaseUtil {
> > String tableName = tableSet.getString("TABLE_
> NAME");
> > // for those databases which do not return the
> > schema name with the table name (pgsql 7.3)
> > boolean appendSchemaName = false;
> > - if (tableName != null && lookupSchemaName != null &&
> > !tableName.startsWith(lookupSchemaName)) {
> > + if (tableName != null && lookupSchemaName != null
> > + &&
> > !tableName.startsWith(lookupSchemaName + "\\.")) {
> > appendSchemaName = true;
> > }
> > if (needsUpperCase && tableName != null) { @@
> > -3042,7 +3042,7 @@ public class DatabaseUtil {
> > String tableName = rawTableName;
> > // for those databases which do not return the schema
> > name with the table name (pgsql 7.3)
> > boolean appendSchemaName = false;
> > - if (tableName != null && lookupSchemaName != null &&
> > !tableName.startsWith(lookupSchemaName)) {
> > + if (tableName != null && lookupSchemaName != null &&
> > !tableName.startsWith(lookupSchemaName + "\\.")) {
> > appendSchemaName = true;
> > }
> > if (needsUpperCase && tableName != null) {
> >
> >
> >
>