Re: [PATCH] Base: fix incorrect field removal in criterion inside query design view

On Wed, Dec 19, 2012 at 06:34:18PM +0100, Olivier Ploton wrote:
> On Thu, 13 Dec 2012 07:53:24 +0100
> Lionel Elie Mamane <lionel@mamane.lu> wrote:
> > On Wed, Dec 12, 2012 at 10:53:07PM +0100, Olivier Ploton wrote:

> > > Problem: you cannot type a field name inside a criterion
> > > if the field name is also the column name, even if it is qualified.

> > The "[Table_1].[name]" gets deleted. I see.

> > There are actually two problems there:

> > 1) the [Table_1].[name] should be deleted ONLY at the beginning of
> >    the expression, not later.

> > That's what your patch handles.

> > Now, I'm uncertain if we should check "rString.isEmpty()" or
> > "i == m_aChildren.begin()". Maybe it is equivalent, but I'm not sure
> > yet. Since you have looked at this code more than I did at this point,
> > do you have an opinion?

> IMO it's not equivalent.

OK, not equivalent. But which one is the *right* test? Would testing
"i == m_aChildren.begin()" give a wrong result?

> To avoid testing emptiness of buffer, I thought about adding a bool (or sal_Bool ?):

Nah, if emptiness of buffer is the right test, it is the right test :)

> > 2) The [Table_1].[name] should be deleted ONLY if it refers to the
> >    current column.

> > Currently *only* the column name is checked, not the source table.

> IMO, throwing away column name at the very beginning of the
> statement is a consistent way of simplifying. Not high end but
> useful and far better than nothing.

I feel you didn't understand my remark. I mean this in the query
design:

Field:             name
Table:             Table_2
Condition:         [Table_1].[name] = 5

gets rewritten to:

Field:             name
Table:             Table_2
Condition:         5

which is *not* the same! Because "[Table_1].[name]" is *not* the
current column! The "[Table_1].[name]" should be deleted *only* if
"Table_1" is the current table (as entered in Table) *and* "name" is
the current field (as entered in Field).

> > So my current thought is that we should add a "sTableAlias" member to
> > rParam, populate it in the code above, and pass it along down to
> > impl_parseNodeToString_throw, where it can use it instead of
> > PROPERTY_TABLENAME in my patch.

> > Do you feel like you can look into this?

> I'm trying to do it but I'm having a hard time with aliases... Right
> now I'm still digging in the sources... Wait and see.

Let me know what comes out of it :)

-- 
Lionel