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

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.

> For example (see attached file: pairs.odb), 
> Query pairs_SQL can not be generated using query design view :
> Inside first column, named "name", typing in something like
>    < [Table_1].[name]
> or
>    < "Table_1"."name"
> inside a criterion leads to an error.

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.

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?


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.

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?


-- 
Lionel

diff --git a/connectivity/source/parse/sqlnode.cxx b/connectivity/source/parse/sqlnode.cxx
index 73fd510..60504e4 100644
--- a/connectivity/source/parse/sqlnode.cxx
+++ b/connectivity/source/parse/sqlnode.cxx
@@ -469,6 +469,9 @@ void OSQLParseNode::impl_parseNodeToString_throw(::rtl::OUStringBuffer& rString,
                 sal_Bool bFilter = sal_False;
                 // retrieve the fields name
                 ::rtl::OUString aFieldName;
+                ::rtl::OUString aTableName;
+                ::rtl::OUString aSchemaName;
+                ::rtl::OUString aCatalogName;
                 try
                 {
                     sal_Int32 nNamePropertyId = PROPERTY_ID_NAME;
@@ -480,7 +483,39 @@ void OSQLParseNode::impl_parseNodeToString_throw(::rtl::OUStringBuffer& 
rString,
                 {
                 }
 
-                if(pSubTree->count())
+                try
+                {
+                    sal_Int32 nNamePropertyId = PROPERTY_ID_TABLENAME;
+                    if ( rParam.xField->getPropertySetInfo()->hasPropertyByName( 
OMetaConnection::getPropMap().getNameByIndex( PROPERTY_ID_ALIASNAME ) ) )
+                        nNamePropertyId = PROPERTY_ID_ALIASNAME;
+                    rParam.xField->getPropertyValue( OMetaConnection::getPropMap().getNameByIndex( 
nNamePropertyId ) ) >>= aTableName;
+                }
+                catch ( Exception& )
+                {
+                }
+
+                try
+                {
+                    rParam.xField->getPropertyValue( OMetaConnection::getPropMap().getNameByIndex( 
PROPERTY_ID_SCHEMANAME ) ) >>= aSchemaName;
+                }
+                catch ( Exception& )
+                {
+                }
+
+                try
+                {
+                    rParam.xField->getPropertyValue( OMetaConnection::getPropMap().getNameByIndex( 
PROPERTY_ID_CATALOGNAME ) ) >>= aCatalogName;
+                }
+                catch ( Exception& )
+                {
+                }
+
+                SAL_DEBUG("FieldName: '" << aFieldName << "'" );
+                SAL_DEBUG("TableName: '" << aTableName << "'" );
+                SAL_DEBUG("SchemaName: '" << aSchemaName << "'" );
+                SAL_DEBUG("CatalogName: '" << aCatalogName << "'" );
+
+                if(pSubTree->count() > 0)
                 {
                     const OSQLParseNode* pCol = pSubTree->m_aChildren[pSubTree->count()-1];
                     if  (   (   SQL_ISRULE(pCol,column_val)
@@ -491,6 +526,28 @@ void OSQLParseNode::impl_parseNodeToString_throw(::rtl::OUStringBuffer& 
rString,
                         bFilter = sal_True;
                 }
 
+                if (bFilter && pSubTree->count() > 2)
+                {
+                    const OSQLParseNode* pTable = pSubTree->m_aChildren[pSubTree->count()-3];
+                    if  ( ! pTable->getTokenValue().equalsIgnoreAsciiCase(aTableName) )
+                        bFilter = sal_False;
+                }
+
+                if (bFilter && pSubTree->count() > 4)
+                {
+                    const OSQLParseNode* pSchema = pSubTree->m_aChildren[pSubTree->count()-5];
+                    if  ( ! pSchema->getTokenValue().equalsIgnoreAsciiCase(aSchemaName) )
+                        bFilter = sal_False;
+                }
+
+                if (bFilter && pSubTree->count() > 6)
+                {
+                    const OSQLParseNode* pCatalog = pSubTree->m_aChildren[pSubTree->count()-7];
+                    if  ( ! pCatalog->getTokenValue().equalsIgnoreAsciiCase(aCatalogName) )
+                        bFilter = sal_False;
+                }
+
+
                 // ok we found the field, if the following node is the
                 // comparision operator '=' we filter it as well
                 if (bFilter)