prevent wrong SQL generation in ERXExistsQualifier

[sgaertner]

As the destination attributes are already selected properly before, this string relacement is not needed anymore and even leads to invalid SQL in some cases.

Example: We have three tables: book, chapter and page. The two relationships between them are 1:n. We want to find all books which have a page with number 26.

One way to write the qualifier is:
new ERXExistsQualifier(Page.NUMBER.is(26), Book.CHAPTERS.dot(Chapter.PAGES).key(), true)

But this leads to the following invalid SQL:

SELECT ... FROM book t0 WHERE t0.book_id IN
( SELECT exists0.book_id FROM chapter exists0 WHERE exists0.chapter_id IN
( SELECT exists0.book_id FROM page exists0 WHERE exists0.number = ?::int4 ) )

Proper SQL should be:

SELECT ... FROM book t0 WHERE t0.book_id IN
( SELECT exists0.book_id FROM chapter exists0 WHERE exists0.chapter_id IN
( SELECT exists0.chapter_id FROM page exists0 WHERE exists0.number = ?::int4 ) )

I have tested this change with a few combinations of constructors and relationships. However there will be edge cases, which I have missed. So additional tests by someone else would be good.

[darkv]

@recurve can you have a look at this? The part that is removed had been added by you 2013. Can you check if that part is really obsolete or what was the purpose?

[recurve]

Hi Johann and Stefan,

We are asking EOF to generate "subExprStr" and then we muck with it...

EOF is trying to select the primary key ID but we want to replace that with the foreign key.

Basically... EOF doesn't know anything other than making a select statement so we trick it afterwards with a string replacement... which is not as good as a pattern replacement.

Stefan - sorry it's not working for you and thank you for looking into it. Perhaps someone has fixed "subExprStr" to where this code is no longer relavent and even wrong as you have pointed out.

The offending code - the code that feels like an "appendix" of the human body - we don't know what it does today and sometimes it causes us trouble so we aim to remove it... that code came from this commit: c119866

That old code was from 2013 and actually is older than that... and if you look at that commit it was doing some really brittle string replacements.

Today's code uses a "pattern matcher" and is much more robust and easier to read.

Stefan was right to delete the code. I deleted it on my local disk and tried some queries and looked at the SQL.

Stefan hit it on the head when he said that it often replaces the same string with the same value but sometimes does the wrong thing... so why have it? He's right, let us remove it.

[darkv]

Thanks for your reply Aaron!