Re: Patch for bug 20227: msi: Table Join on two tables that have same (non-join) column name...

[Hans Leidekker]

Hi Nate,

Your patch applies with offsets, please rebase to current git.
And get rid of this warning:

db.c: In function ‘test_where_not_in_selected’:
db.c:1576: warning: unused variable ‘count’
> - query = /*"Select IESTable.Action, IESTable.Condition from CATable, IESTable where "*/
> -
>        "Select IESTable.Condition from CATable, IESTable where "
> -
>                  //"CATable.Action = IESTable.Action and (CATable.Type = 51 or CATable.Type = 32) "
> -
>                  "CATable.Action = IESTable.Action and CATable.Type = 32 "
> -
>                  /*"order by IESTable.Sequence"*/;

Please remove the commented strings.
> static UINT ALTER_get_column_info( struct tagMSIVIEW *view,
> -
>            UINT n, LPWSTR *name, UINT *type, BOOL *temporary )
> -
>            UINT n, LPWSTR *name, UINT *type, BOOL *temporary,
> -
>            LPWSTR *tableName)

'table_name' or just 'table' would be more in keeping with the existing style.
> - printf("Returned entry: %s\n", buf);

trace() is usually used for debug printing but you might want to turn this
into a test instead, or just leave it out. And you should split off the tests
to keep the patch size down.

-Hans

Patch for bug 20227: msi: Table Join on two tables that have same (non-join) column name...

[Nathan Gallaher]

Resubmit of patch.

Patch separated into test and fix components.
Patch rebased.
tableName variable --> table_name
Commented code removed.
Unused variables removed
Unnecessary debug printf's removed.

Thanks for the review,

~Nate

On Thu, 1 Oct 2009, Hans Leidekker wrote:
> Hi Nate,
>
> Your patch applies with offsets, please rebase to current git.
> And get rid of this warning:
>
> db.c: In function ‘test_where_not_in_selected’:
> db.c:1576: warning: unused variable ‘count’
> > - query = /*"Select IESTable.Action, IESTable.Condition from CATable, IESTable where "*/
> > -
> >        "Select IESTable.Condition from CATable, IESTable where "
> > -
> >                  //"CATable.Action = IESTable.Action and (CATable.Type = 51 or CATable.Type = 32) "
> > -
> >                  "CATable.Action = IESTable.Action and CATable.Type = 32 "
> > -
> >                  /*"order by IESTable.Sequence"*/;
>
> Please remove the commented strings.
> > static UINT ALTER_get_column_info( struct tagMSIVIEW *view,
> > -
> >            UINT n, LPWSTR *name, UINT *type, BOOL *temporary )
> > -
> >            UINT n, LPWSTR *name, UINT *type, BOOL *temporary,
> > -
> >            LPWSTR *tableName)
>
> 'table_name' or just 'table' would be more in keeping with the existing style.
> > - printf("Returned entry: %s\n", buf);
>
> trace() is usually used for debug printing but you might want to turn this
> into a test instead, or just leave it out. And you should split off the tests
> to keep the patch size down.
>
> -Hans

Patch for bug 20227: msi: Table Join on two tables that have same (non-join) column name...

[Hans Leidekker]

On Monday 19 October 2009 04:19:07 Nathan Gallaher wrote:
> Resubmit of patch.
>
> Patch separated into test and fix components.
> Patch rebased.
> tableName variable --> table_name
> Commented code removed.
> Unused variables removed
> Unnecessary debug printf's removed.

Looks good, please submit the patches to [e-mail],
one patch per email.

-Hans