Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Received-SPF: pass (asf.osuosl.org: local policy)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
  s=s1024; d=yahoo.com;
  h=Message-ID:Received:Date:From:Subject:To:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding;
  b=JwzvOWer0GD8vQ/pYeM+/XBaqSFWUNnKevDjSffuXLC1SMw5zOdKHYKLu0etygmPgjlDTts5vjzcet8cozOrwq9sFsHe42indUWPqUN4AfYPcz8/mcj0OiOrgTEB9QfdANZThoOMGru2QZpD3BOxOZi4OyZ8rDXnMuCsYtyRnAg=
  ;
Message-ID: <20060615235500.83553.qmail@web34809.mail.mud.yahoo.com>
Date: Thu, 15 Jun 2006 16:55:00 -0700 (PDT)
From: Manish Khettry <manish_khettry@yahoo.com>
Subject: Re: [jira] Updated: (DERBY-578) Grouped select from temporary table
 raises null pointer exception in byte code generator
To: derby-dev@db.apache.org
In-Reply-To: <4491ECA1.2070908@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8bit


--- Army <qozinx@gmail.com> wrote:

>
> Did your changes actually make it so that the query
> no longer uses an index on 
> T1, or did it just change the logQueryPlan output so
> that the index is no longer 
> being printed? 

Actually, neither. The qeury plan was erroneusly
reporting an index being used when none was being
used. Try to do a distinct scan on a table without an
index.

---------
 maximumdisplaywidth 20000;
 create table noindex (x int);
 call SYSCS_UTIL.SYSCS_SET_RUNTIMESTATISTICS(1);
 select distinct x from noindex;
 values SYSCS_UTIL.SYSCS_GET_RUNTIMESTATISTICS();
---------


> As for the "None" vs "null", I personally prefer the
> former over the latter--so 

This is only an indirect result of my change-- Since
the indexname is now null, the code that generates the
query plan out put prints None instead of null (or the
other way round).

>
> As I said, I haven't had time to do a thorough
> review, but I did notice the 
> following changes in your patch:
> 	
> +        /* helper method used by
> generateMaxSpecialResultSet and
> +         * generateDistinctScan to return the name
> of the index if the
> +         * conglomerate is an index.
> +         */
> +        private void
> pushIndexName(ConglomerateDescriptor cd,
> MethodBuilder mb)
> +          throws StandardException
> +        {
> +            if (cd.isConstraint()) {
> +                DataDictionary dd =
> getDataDictionary();
> +                ConstraintDescriptor constraintDesc
> =
> +                   
> dd.getConstraintDescriptor(tableDescriptor,
> cd.getUUID());
> +               
> mb.push(constraintDesc.getConstraintName());
> +            } else if (cd.isIndex())  {
> +                mb.push(cd.getConglomerateName());
> +            } else {
> +              mb.pushNull("java.lang.String");
> +            }
> +        }
> +	
> 
> @@ -3128,7 +3148,7 @@
>   		 
>
mb.push(org.apache.derby.iapi.util.PropertyUtil.sortProperties(tableProperties));
>   		else
>   			mb.pushNull("java.lang.String");
> -		mb.push(cd.getConglomerateName());
> +                pushIndexName(cd, mb);
>   		mb.push(colRefItem);
>   	
> mb.push(getTrulyTheBestAccessPath().getLockMode());
>   		mb.push(tableLockGranularity);
> 
> If I'm reading this correctly, we used to _always_
> push 
> cd.getConglomerateName()--which, if we're using an
> index, is the name of the 
> index--but now, in order to avoid the NPE reported
> as part of this issue, we 
> will only push the conglomerate name if it (the
> conglomerate) is not a 
> constraint.  Logically, I'm not entirely sure that's
> the correct thing to do, 

Well if cd is a conglomerate that represents the base
table then why use the base table name as the index
name in the distict result set?! Let me know if this
clarifies or makes you (or any other commiter) feel
better (or worse) about the patch?

The method that you refer to here(pushIndexName) is
pretty much the code that is used
BaseJoinStrategy:fillInScanArgs2. 

Thanks,
Manish


> since I *think* that a conglomerate can be an index
> and a constraint at the same 
> time (I'd have to investigate more--if anyone out
> there knows for sure, please 
> chime in!).  If that's true, then in situations
> where we used to have a 
> conglomerate that was both an index and a
> constraint, we would push the index 
> name; but now for that same conglomerate we will
> push the constraint name 
> instead, and I'm guessing that the constraint name
> and the index name are not 
> always the same--which might account for the fact
> the index name is now missing 
> from the logQueryPlan?
> 
> That's just speculation, though--if anyone can
> confirm/deny, that'd be great. 
> In any event, in order to preserve the existing
> behavior in logQueryPlan, the 
> change might be as small as something like:
> 
> +        /* helper method used by
> generateMaxSpecialResultSet and
> +         * generateDistinctScan to return the name
> of the index if the
> +         * conglomerate is an index.
> +         */
> +        private void
> pushIndexName(ConglomerateDescriptor cd,
> MethodBuilder mb)
> +          throws StandardException
> +        {
> +            if (cd.getConglomerateName() != null)
> +                mb.push(cd.getConglomerateName());
> +            else if (cd.isConstraint()) {
> +                DataDictionary dd =
> getDataDictionary();
> +                ConstraintDescriptor constraintDesc
> =
> +                   
> dd.getConstraintDescriptor(tableDescriptor,
> cd.getUUID());
> +               
> mb.push(constraintDesc.getConstraintName());
> +            } else {
> +              mb.pushNull("java.lang.String");
> +            }
> +        }
> 
> Assuming, of course, that
> constraintDesc.getConstraintName() can't ever be 
> null... :)
> 
> I didn't test the above suggestion so it may need
> some tweaking--but if you 
> could look at the changes again and/or explain a bit
> more about what is causing 
> the diff in the master files--and hopefully post
> your findings--then I'd feel a 
> lot better about this patch...
> 
> For what it's worth,
> Army
> 
> 


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com