On Thu, 13 Dec 2012 07:53:24 +0100
Lionel Elie Mamane <lio...@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. The intention of this is that
> e.g.:
>
> [Table_1].[name] < 'some value'
>
> gets displayed as
>
> < 'some value'
>
> Since the [Table_1].[name] is given by the rows above in the view.
>
> 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.
>
> Small incremental improvement: use rString.isEmpty() instead of
> rString.getLength()==0.
OK, sorry. Hope there is no fine for awkwardness :-)
>
> 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. The iterator does not recurse inside the tree up to
the leafs, it merely enumerates direct children. If you add:
if (i == m_aChildren.begin()) rString.append("*");
at the beginning of the loop, several stars may appear in the output.
To avoid testing emptiness of buffer, I thought about adding a bool (or
sal_Bool ?):
void OSQLParseNode::impl_parseNodeToString_throw (bool& atStart, otherargs)
{
if (atStart) {atStart = false; bFilter .... }
}
void OSQLParseNode::impl_parseNodeToString_throw (otherargs)
{
bool atStart = true;
impl_parseNodeToString_throw (atSart, otherargs)
}
but I didn't dare coding it.
>
> 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 tried the attached patch, but it does not work, because it refers to
> the original "real" table, and *not* to the *table* *alias* within the
> query.
>
> Forget the reference to schema and/or catalog in my patch, that was
> borne from the confusion between "real table" and "table instance in a
> query"...
>
> But currently we don't have property to refer to the "query table
> name", that is the "table alias" in the query. My first thought was to
> add such a property and populate it.
>
> However,
>
> The rParam.xField ultimately comes from
> OQueryDesignView::getPredicateTreeFromEntry
>
> if (pWin)
> {
> Reference<XNameAccess> xColumns =
> pWin->GetOriginalColumns();
> if (xColumns.is() && xColumns->hasByName(pEntry->GetField()))
> xColumns->getByName(pEntry->GetField()) >>= _rxColumn;
> }
>
> pWin is a OQueryTableWindow; getOriginalColumns() is implemented by the
> base class OTableWindow, which has no notion of Table Alias. So it
> seems that the "getOriginalColumns()" could be shared among all
> instances (aliases) of the same table in query.
>
> 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.
--
Olivier.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
