db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mamta Satoor <msat...@gmail.com>
Subject Re: [PATCH] Jira-189 ResultSetMetaData.getSchemaName and ResultSetMetaData.isWritable donot return correct values
Date Mon, 02 May 2005 18:08:49 GMT
Thanks for looking into it, David. I just wanted to be sure that patch
has not been forgotten.

Mamta

On 5/2/05, David Van Couvering <David.Vancouvering@sun.com> wrote:
> Hi, Mamta, I have actually been trying to review this patch, but this is
>  my first patch review, and I am learning *a lot* about how to set up a
> clean "drop" of Derby at the revision level you are at, applying and
> reviewing a patch.  I do hope in the future my response time will be better.
> 
> Thanks,
> 
> David
> 
> Mamta Satoor wrote:
> 
> > Hi,
> >
> > I am sorry if I sound like a broken record, but please, will someone
> > review this? And if no concerns from anyone, will a commiter commit
> > it?
> >
> > thanks,
> > Mamta
> >
> > On 4/27/05, Mamta Satoor <msatoor@gmail.com> wrote:
> >
> >>Hi,
> >>
> >>I wondered if anyone got a chance to review this patch?
> >>
> >>thanks,
> >>Mamta
> >>
> >>On 4/24/05, Mamta Satoor <msatoor@gmail.com> wrote:
> >>
> >>>Hi Dan,
> >>>
> >>>I have a new patch for this bug which also fixes the problem you
> >>>brought up with sql select * from a.t as X. The fix for this required
> >>>change in impl.sql.compile.FromBaseTable's method genResultColList().
> >>>I changed the code such that we set the TableDescriptor on the
> >>>ColumnDescriptor instance. This TableDescriptor is later used by
> >>>ResultColumn.getTableName to get the base table name of the column. In
> >>>addition to that, I changed ColumnReference.getSourceTableName and
> >>>ColumnReference.getSourceSchemaName so that they don't look at the
> >>>user supplied correlation name (if any) to fetch the base table/schema
> >>>name.
> >>>
> >>>I have also added some test cases for the code changes above in
> >>>jdbcapi/resultset.java
> >>>
> >>>Other than this, the rest of the patch stays the same as what you
> >>>tried applying last week.
> >>>
> >>>I have run the tests and there is no new failure because of my changes.
> >>>
> >>>svn stat output
> >>>M      java\engine\org\apache\derby\impl\sql\compile\ResultColumn.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\VirtualColumnNode.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\CursorNode.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\FromBaseTable.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\BaseColumnNode.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\ColumnReference.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\ValueNode.java
> >>>M      java\engine\org\apache\derby\impl\sql\compile\ResultColumnList.java
> >>>M      java\engine\org\apache\derby\impl\sql\GenericColumnDescriptor.java
> >>>M      java\engine\org\apache\derby\impl\jdbc\EmbedResultSet.java
> >>>M      java\engine\org\apache\derby\impl\jdbc\EmbedResultSetMetaData.java
> >>>M      java\engine\org\apache\derby\iapi\sql\dictionary\ColumnDescriptor.java
> >>>M      java\engine\org\apache\derby\iapi\sql\ResultColumnDescriptor.java
> >>>M      java\testing\org\apache\derbyTesting\functionTests\tests\lang\updatableResultSet.java
> >>>M      java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\resultset.java
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\updatableResultSet.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\resultset.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\updatableResultSet.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\resultset.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\updatableResultSet.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\jdk14\updatableResultSet.out
> >>>M      java\testing\org\apache\derbyTesting\functionTests\master\resultset.out
> >>>
> >>>Please commit the patch if there are no further issues,
> >>>Mamta
> >>>
> >>>On 4/22/05, Mamta Satoor <msatoor@gmail.com> wrote:
> >>>
> >>>>Hi Dan,
> >>>>
> >>>>I did some research into this. You are right that for the sql
> >>>>select * from a.t as X
> >>>>the existing code will return ABC using both getTableName and
> >>>>getSourceTableName. Looking at the history of ColumnReference before
> >>>>the code was
> >>>>contributed, the getSourceTableName was added to Cloudscape so that
> >>>>ResultSetMetaData.getTableName will return the correct value which is
> >>>>base table
> >>>>name. Seems like over the time, this functionality got broken again.
> >>>>
> >>>>In my code line, I tried changing ColumnReference.getSourceTableName
> >>>>to following
> >>>>public String getSourceTableName()
> >>>>{
> >>>>return ((source != null) ? source.getTableName() : null);
> >>>>}
> >>>>
> >>>>But, that did not solve the problem. The source.getTableName() invokes
> >>>>ResultColumn.getTableName which is currently coded as follows
> >>>>public String getTableName()
> >>>>{
> >>>> if (tableName != null)
> >>>> {
> >>>>   return tableName;
> >>>> }
> >>>> if ((columnDescriptor!=null) &&
> >>>>   (columnDescriptor.getTableDescriptor() != null))
> >>>> {
> >>>>   return columnDescriptor.getTableDescriptor().getName();
> >>>> }
> >>>> else
> >>>> {
> >>>>   return expression.getTableName();
> >>>> }
> >>>>}
> >>>>
> >>>>In the code above, the first 2 if conditions return false and hence
> >>>>expression.getTableName() get called which returns ABC for the sql
> >>>>above. And this
> >>>>is the problem in my opinion. The 2nd if condition needs to succeed in
> >>>>order to get the correct value for table name(currently, the 2nd if
> >>>>condition
> >>>>returns false because columnDescriptor.getTableDescriptor() returns
> >>>>null). columnDescriptor has its table descriptor set to null since it
> >>>>got
> >>>>instantiated via SYSCOLUMNSRowFactory which passes the uuid for the
> >>>>table but not the table descriptor. I tried changing
> >>>>ColumnDescriptor's
> >>>>2nd constructor(the one which doesn't get table descriptor passed to
> >>>>it) to try to get the table descriptor from the uuid by calling
> >>>>getDataDictionary.getTableDescriptor(uuid), but it gives me Raw Store
> >>>>internal error. I will continue to research but does someone looking
> >>>>at this
> >>>>explanation have any thoughts on the program flow or issue in general?
> >>>>
> >>>>thanks,
> >>>>Mamta
> >>>>
> >>>>
> >>>>On 4/20/05, Daniel John Debrunner <djd@debrunners.com> wrote:
> >>>>
> >>>>>The patch applies fine and passes the tests but I need some
> >>>>>clarification on some methods in ColumnReference.
> >>>>>
> >>>>>ColumnReference has these class specific methods
> >>>>>
> >>>>>getSourceTableName()
> >>>>>getSourceSchemaName() [added by this contribution]
> >>>>>
> >>>>>and because it is a ValueNode it also has
> >>>>>
> >>>>>getTableName
> >>>>>getSchemaName
> >>>>>
> >>>>>Mamta, can you clarify the difference between the getSource*Name
methods
> >>>>>and get*Name methods? Eventually as comments in the javadoc for these
> >>>>>methods, but some discussion on the list may be useful.
> >>>>>
> >>>>>I'm worried because most of the bugs around correlation names I think
> >>>>>were due to having multiple methods with similar and maybe misleading
> >>>>>names but no clear definition of what they returned.
> >>>>>
> >>>>>In this specific case, my gut reaction from the names of the
> >>>>>getSource*Name methods is that they return the actual name of the
> >>>>>underlying table the column comes from. But looking the implementation
&
> >>>>>comments of getTableName and getSourceTableName in ColumnReference.java
> >>>>>it seems in this case
> >>>>>
> >>>>>select * from a.t as X
> >>>>>
> >>>>>that both methods will return X.
> >>>>>
> >>>>>Dan.
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>
>

Mime
View raw message