Satheesh Bandaram wrote: > I am getting ready to submit this patch. > > Satheesh > > Mamta Satoor wrote: > >>Hi David, >> >>I have attached a new patch to take care of the comments from your review.
>>2)The change in ResultColumnDescriptor.java for getSourceSchemaName is >>for clarity and consistency reasons (similar to getSourceTableName). >>With this >>change, EmbedResultSetMetaData now calls getSourceXXXName for >>getTableName and getSchemaName. Similarly, in >>GenericColumnDescriptor(which implements ResultColumnDescriptor), we >>have getSourceXXXName to keep the source table and source schema name. I'm still confused by the various get methods for table name and schema names. Additional comments on getSourceTableName() explictly stating that it does get the name of the underlying table would be useful. Then its description matches the behaviour implied by its method name. Then with this patch ColumnReference has a getSchemaName() and a getSourceSchemaName(). I'm unclear on what the intended behaviour of each is, and why we need two methods. If getTableName() means return the "user" name or correlation name, then logically getSchemaName() would follow the same pattern, but user names or correlation names don't have schemas. This isn't helped by the fact that the comments for the ValueNode methods getTableName and getSchemaName do not have clear or correct comments, and that's the place where logically their behaviour is defined. Maybe this patch is ok without addressing these issues, but I'm worried that it might be making a confused area even more confused. Dan.
