New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong result set for unions containing null-constant projections #136

Closed
nyc2 opened this Issue May 2, 2012 · 17 comments

Comments

Projects
None yet
2 participants
@nyc2

nyc2 commented May 2, 2012

Hi,

I've found an issue when trying to union queries that contain one or more constant projections. I'm not really sure if there are problems only with null-constants or if it is a general issue with constants.

I'm using it this way:

List<Object[]> rows = new JPASQLQuery().union(new SQLSubQuery().from(QTab1).where(...).distinct().list(QTab1.col1,QTab2.col2), new SQLSubQuery().from(QTab1).where(...).distinct().list(QTab1.col1,null)).list();

The result set does not contain any rows from the second sub query. On the DB however, it works properly.

I suppose that it has something to do with auto-generated column aliases.

Is there a workaround in the meanwhile to define explicit aliases for the constants?

Bye,
nyc2

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 3, 2012

Member

Which database are you using? Also how does the generated SQL query look like?

Member

timowest commented May 3, 2012

Which database are you using? Also how does the generated SQL query look like?

timowest added a commit that referenced this issue May 3, 2012

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 6, 2012

The DB in use is SQL Server 2008. At the moment I don't have access to the generated SQL. However, I can remember that exactly the same statement worked correctly on the DB. Perhaps there is a problem with the Hibernate persistence provider once again. There has been an issue with duplicate column names in the past. You therefore provided a workaround by explicitly introducing enumerated column names, remember this?

nyc2 commented May 6, 2012

The DB in use is SQL Server 2008. At the moment I don't have access to the generated SQL. However, I can remember that exactly the same statement worked correctly on the DB. Perhaps there is a problem with the Hibernate persistence provider once again. There has been an issue with duplicate column names in the past. You therefore provided a workaround by explicitly introducing enumerated column names, remember this?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 6, 2012

Member

Can you post the generated SQL when you have access again?

Member

timowest commented May 6, 2012

Can you post the generated SQL when you have access again?

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 7, 2012

I tried to construct a simple example that demonstrates the problem. In addition, I encountered a (possibly related) issue also dealing with wrong result sets.

Test schema and data:

create table tbl1 (
    pk int primary key identity(1,1),
    col1 nvarchar(10) not null,
    col2 nvarchar(10) not null
)
create table tbl2 (
    pk int primary key,
    col1 nvarchar(10) not null,
    col2 nvarchar(10) not null,
    constraint fk_tbl2_tbl1 foreign key(pk) references tbl1(pk)
)
insert tbl1 (col1,col2) values ('foo1','bar1')
insert tbl2 (pk,col1,col2) values (@@identity,'foo2','bar2')

TestNG test:

    @Test
    public void unionQuerydsl() {
        EntityManager em = null;
        try {
            em = Persistence.createEntityManagerFactory("TestPU").createEntityManager();
            em.getTransaction().begin();

            JPASQLQuery query = new JPASQLQuery(em, new SQLServerTemplates());
            @SuppressWarnings("unchecked")
            List<Object[]> rows = query.union(
                    new SQLSubQuery()
                            .from(QTbl1.tbl1)
                            .join(QTbl2.tbl2)
                            .on(QTbl2.tbl2.pk.eq(QTbl1.tbl1.pk))
                            .list(QTbl1.tbl1.col1, QTbl1.tbl1.col2, QTbl2.tbl2.col1, QTbl2.tbl2.col2),
                    new SQLSubQuery().from(QTbl1.tbl1).list(QTbl1.tbl1.col1, QTbl1.tbl1.col2, null, null)).list();//.orderBy(QTbl2.tbl2.col1.asc()).list();
            Assert.assertTrue(rows.get(0)[2] == null || rows.get(1)[2] == null);

            em.getTransaction().commit();
        }
        catch (Exception ex) {
            logger.error(ex);
            Assert.fail();
            em.getTransaction().rollback();
        }
        finally {
            try {
                em.close();
            }
            catch (Exception ex) {
            }
        }
    }

Generated SQL:

(select tbl1.col1, tbl1.col2, tbl2.col1, tbl2.col2
from tbl1 tbl1
join tbl2 tbl2
on tbl2.pk = tbl1.pk)
union
(select tbl1.col1, tbl1.col2, null, null
from tbl1 tbl1)

If you uncomment the orderBy-clause, you'll see another exception:

08:21:48,005  WARN JDBCExceptionReporter:77 - SQL Error: 209, SQLState: S1000
08:21:48,005 ERROR JDBCExceptionReporter:78 - Ambiguous column name 'col1'.

nyc2 commented May 7, 2012

I tried to construct a simple example that demonstrates the problem. In addition, I encountered a (possibly related) issue also dealing with wrong result sets.

Test schema and data:

create table tbl1 (
    pk int primary key identity(1,1),
    col1 nvarchar(10) not null,
    col2 nvarchar(10) not null
)
create table tbl2 (
    pk int primary key,
    col1 nvarchar(10) not null,
    col2 nvarchar(10) not null,
    constraint fk_tbl2_tbl1 foreign key(pk) references tbl1(pk)
)
insert tbl1 (col1,col2) values ('foo1','bar1')
insert tbl2 (pk,col1,col2) values (@@identity,'foo2','bar2')

TestNG test:

    @Test
    public void unionQuerydsl() {
        EntityManager em = null;
        try {
            em = Persistence.createEntityManagerFactory("TestPU").createEntityManager();
            em.getTransaction().begin();

            JPASQLQuery query = new JPASQLQuery(em, new SQLServerTemplates());
            @SuppressWarnings("unchecked")
            List<Object[]> rows = query.union(
                    new SQLSubQuery()
                            .from(QTbl1.tbl1)
                            .join(QTbl2.tbl2)
                            .on(QTbl2.tbl2.pk.eq(QTbl1.tbl1.pk))
                            .list(QTbl1.tbl1.col1, QTbl1.tbl1.col2, QTbl2.tbl2.col1, QTbl2.tbl2.col2),
                    new SQLSubQuery().from(QTbl1.tbl1).list(QTbl1.tbl1.col1, QTbl1.tbl1.col2, null, null)).list();//.orderBy(QTbl2.tbl2.col1.asc()).list();
            Assert.assertTrue(rows.get(0)[2] == null || rows.get(1)[2] == null);

            em.getTransaction().commit();
        }
        catch (Exception ex) {
            logger.error(ex);
            Assert.fail();
            em.getTransaction().rollback();
        }
        finally {
            try {
                em.close();
            }
            catch (Exception ex) {
            }
        }
    }

Generated SQL:

(select tbl1.col1, tbl1.col2, tbl2.col1, tbl2.col2
from tbl1 tbl1
join tbl2 tbl2
on tbl2.pk = tbl1.pk)
union
(select tbl1.col1, tbl1.col2, null, null
from tbl1 tbl1)

If you uncomment the orderBy-clause, you'll see another exception:

08:21:48,005  WARN JDBCExceptionReporter:77 - SQL Error: 209, SQLState: S1000
08:21:48,005 ERROR JDBCExceptionReporter:78 - Ambiguous column name 'col1'.
@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 7, 2012

This holds for the wrong result set: QTbl1.tbl1.col1==QTbl2.tbl2.col1 and QTbl1.tbl1.col2==QTbl2.tbl2.col2. I'd expect to see null values or the correct values from QTbl2, resp.

nyc2 commented May 7, 2012

This holds for the wrong result set: QTbl1.tbl1.col1==QTbl2.tbl2.col1 and QTbl1.tbl1.col2==QTbl2.tbl2.col2. I'd expect to see null values or the correct values from QTbl2, resp.

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 7, 2012

The issue is similar to #80.

nyc2 commented May 7, 2012

The issue is similar to #80.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 7, 2012

Member

I couldn't reproduce the issue. When using the SQL Select via JPA Native queries directly, are you able to get all the results?

The ambigious column name can be avoided by aliasing the columns via as(String) calls.

Member

timowest commented May 7, 2012

I couldn't reproduce the issue. When using the SQL Select via JPA Native queries directly, are you able to get all the results?

The ambigious column name can be avoided by aliasing the columns via as(String) calls.

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 8, 2012

Sorry, I stated that the result set does not contain any rows from the second sub query. That's not true, in fact the number of rows is correct but not the column values, however.

Expected:

foo1 bar1 NULL NULL
foo1 bar1 foo2 bar2

Got:

foo1 bar1 foo1 bar1
foo1 bar1 foo1 bar1

This happens with Hibernate JPA native queries as well. Could you introduce column aliases for path projections as well? How can I define aliases for null or constant value projections?

This works even though it restricts Querydsl and JPA native queries because it's not necessary to do this in plain SQL:

List<Object[]> rows = query.union(new SQLSubQuery().from(QTbl1.tbl1).join(QTbl2.tbl2).on(QTbl2.tbl2.pk.eq(QTbl1.tbl1.pk)).list(QTbl1.tbl1.col1.as("column1"), QTbl1.tbl1.col2.as("column2"), QTbl2.tbl2.col1, QTbl2.tbl2.col2),new SQLSubQuery().from(QTbl1.tbl1).list(QTbl1.tbl1.col1.as("column1"), QTbl1.tbl1.col2.as("column2"), null, null)).orderBy(QTbl2.tbl2.col1.asc()).list();

nyc2 commented May 8, 2012

Sorry, I stated that the result set does not contain any rows from the second sub query. That's not true, in fact the number of rows is correct but not the column values, however.

Expected:

foo1 bar1 NULL NULL
foo1 bar1 foo2 bar2

Got:

foo1 bar1 foo1 bar1
foo1 bar1 foo1 bar1

This happens with Hibernate JPA native queries as well. Could you introduce column aliases for path projections as well? How can I define aliases for null or constant value projections?

This works even though it restricts Querydsl and JPA native queries because it's not necessary to do this in plain SQL:

List<Object[]> rows = query.union(new SQLSubQuery().from(QTbl1.tbl1).join(QTbl2.tbl2).on(QTbl2.tbl2.pk.eq(QTbl1.tbl1.pk)).list(QTbl1.tbl1.col1.as("column1"), QTbl1.tbl1.col2.as("column2"), QTbl2.tbl2.col1, QTbl2.tbl2.col2),new SQLSubQuery().from(QTbl1.tbl1).list(QTbl1.tbl1.col1.as("column1"), QTbl1.tbl1.col2.as("column2"), null, null)).orderBy(QTbl2.tbl2.col1.asc()).list();
@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 8, 2012

Also, wouldn't it be clearer to further restrict the OrderIdentifiers for Union.orderBy by either only allowing string aliases or positional numbers?

At least in SQL Server it's allowed to do this:

select col1 foo from tbl1
union 
select col1 bar from tbl2
order by foo

However, this does not work:

select col1 foo from tbl1
union 
select col1 bar from tbl2
order by bar

Why should we use the name/alias of the first column? I think that's a bit odd. Wouldn't it be better to only allow

select col1 from tbl1
union 
select col2 from tbl2
order by 1

nyc2 commented May 8, 2012

Also, wouldn't it be clearer to further restrict the OrderIdentifiers for Union.orderBy by either only allowing string aliases or positional numbers?

At least in SQL Server it's allowed to do this:

select col1 foo from tbl1
union 
select col1 bar from tbl2
order by foo

However, this does not work:

select col1 foo from tbl1
union 
select col1 bar from tbl2
order by bar

Why should we use the name/alias of the first column? I think that's a bit odd. Wouldn't it be better to only allow

select col1 from tbl1
union 
select col2 from tbl2
order by 1
@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 8, 2012

Member

Also, wouldn't it be clearer to further restrict the OrderIdentifiers for Union.orderBy by either only allowing string aliases or positional numbers?

No, Strings and positional numbers shouldn't be used to refer to Paths in Querydsl queries.

Why should we use the name/alias of the first column?

Querydsl behaves like SQL here. I think it's quite transparent.

Member

timowest commented May 8, 2012

Also, wouldn't it be clearer to further restrict the OrderIdentifiers for Union.orderBy by either only allowing string aliases or positional numbers?

No, Strings and positional numbers shouldn't be used to refer to Paths in Querydsl queries.

Why should we use the name/alias of the first column?

Querydsl behaves like SQL here. I think it's quite transparent.

timowest added a commit that referenced this issue May 8, 2012

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 8, 2012

Member

Could you introduce column aliases for path projections as well?

Done

How can I define aliases for null or constant value projections?

I just added another method to the com.mysema.query.support.Expressions class for that.

Here are some examples from a unit test

    assertEquals("null as str", Expressions.as(null, str).toString());
    assertEquals("s as str", Expressions.as(new StringPath("s"), str).toString());
    assertEquals("str as str", Expressions.constantAs("str", str).toString());
Member

timowest commented May 8, 2012

Could you introduce column aliases for path projections as well?

Done

How can I define aliases for null or constant value projections?

I just added another method to the com.mysema.query.support.Expressions class for that.

Here are some examples from a unit test

    assertEquals("null as str", Expressions.as(null, str).toString());
    assertEquals("s as str", Expressions.as(new StringPath("s"), str).toString());
    assertEquals("str as str", Expressions.constantAs("str", str).toString());
@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 9, 2012

Is there a snapshot? Thank you for the improvements!

nyc2 commented May 9, 2012

Is there a snapshot? Thank you for the improvements!

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 9, 2012

Member

Hi.

At the moment not. But you can build it easily without any databases
locally.

Checkout, go to querydsl-root and run

mvn -Dtest= clean install

Br,
Timo

On Wed, May 9, 2012 at 8:55 AM, nyc2 <
reply@reply.github.com

wrote:

Is there a snapshot? Thank you for the improvements!


Reply to this email directly or view it on GitHub:
#136 (comment)

Timo Westkämper
Mysema Oy
+358 (0)40 591 2172
www.mysema.com

Member

timowest commented May 9, 2012

Hi.

At the moment not. But you can build it easily without any databases
locally.

Checkout, go to querydsl-root and run

mvn -Dtest= clean install

Br,
Timo

On Wed, May 9, 2012 at 8:55 AM, nyc2 <
reply@reply.github.com

wrote:

Is there a snapshot? Thank you for the improvements!


Reply to this email directly or view it on GitHub:
#136 (comment)

Timo Westkämper
Mysema Oy
+358 (0)40 591 2172
www.mysema.com

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 25, 2012

Member

Released in 2.6.0

Member

timowest commented May 25, 2012

Released in 2.6.0

@timowest timowest closed this May 25, 2012

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 29, 2012

Could you introduce column aliases for path projections as well?

Done

Hi Timo,

This doesn't work, at least not for SQLSubQueries. As a consequence, you may not use any projections containing paths with the same column names in it, unless you explicitly provide column aliases for those duplicates.
Should I open a separate issue?

Best regards,
nyc2

nyc2 commented May 29, 2012

Could you introduce column aliases for path projections as well?

Done

Hi Timo,

This doesn't work, at least not for SQLSubQueries. As a consequence, you may not use any projections containing paths with the same column names in it, unless you explicitly provide column aliases for those duplicates.
Should I open a separate issue?

Best regards,
nyc2

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 29, 2012

Member

Yes, please open a new ticket.

Member

timowest commented May 29, 2012

Yes, please open a new ticket.

@nyc2

This comment has been minimized.

Show comment
Hide comment
@nyc2

nyc2 May 29, 2012

Just opened a new issue: #157

nyc2 commented May 29, 2012

Just opened a new issue: #157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment