My initial version of the AddScanColumn assumed that the table should
already be registered in the statement cache during the parseTable call by
the time this method is invoked. In this case, there were no need to call
the  addTableToStmtCache at all to get the table index, which was obtained
through the alias map (ELSE branch). The assumption proved to be wrong when
the DDL compiler compiles the schema. For example, addIndexToCatalog calls
AbstractParsedStmt.addTableToStmtCache directly bypassing the parseTable
altogether.  It sets the table via the setTable call instead. This is why I
added the addTableToStmtCache call (the IF branch).

I  guess, the better solution would be to update setTable to add table to
the cache  right here, and don't call the addTableToStmtCache from the
addScanColumn at all. All the addScanColumn is needed is the table(or
sub-query) index to the cache


On Tue, Nov 12, 2013 at 10:23 PM, Paul A Martel <notifications@github.com>wrote:

> In src/frontend/org/voltdb/planner/AbstractParsedStmt.java:
>
> >          }
> >          assert(tableName != null);
> > +
> > +        // @TODO Is it safe to assume that the sub-query table name is always SYSTEM_SUBQUERY
> > +        // or is there a better way to identify the sub-query?
> > +        boolean isSubQuery = "SYSTEM_SUBQUERY".equalsIgnoreCase(tableName);
> > +
>
> This is definitely not needed here and I even doubt whether it is needed
> in addScanColumn.
> Why can't the TVE construction add the column to the existing cached scan
> without regard to whether it is a subquery scan?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/alexeevm/voltdb/pull/6/files#r7616077>
> .
>