You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Mamta Satoor <ms...@gmail.com> on 2005/10/05 06:20:06 UTC
[PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
This patch tries to support the type setting of unary-/unary+ parameter
similar to what is done for ? parameter ie the type of the -?/+? will be
dependent on the context in which it is used. And hence the type setting
rules for -?/+? will be same as for a regular ? parameter.
In order to achieve this, I have changed UnaryOperatorNode to extend
ParameterNode. In addition, it has the method isParameterNode which will
return true only if its operand is an instance of ParameterNode and it's
method isParameterNode returns true AND the unary operator is of the type
unary-/unary+. What this means is that just because a class is instance of
ParameterNode doesn't automatically mean it indeed is a ParameterNode. An
additional check of the isParameterNode is required to make sure there is a
parameter underneath. This change in rule has required adding a new class
called HasParameterNodeVisitor which gets used by the sqlgrammar to make
sure there are no parameters allowed in views and triggers.
HasParameterNodeVisitor checks if the node is of the type ParameterNode AND
checks if the isParameterNode returns true. If yes, then the
HasParameterNodeVisitor's visit method will return that node and grammar
will throw an exception for parameter usage in views/triggers.
One additional change is to not do any binding of -?/+? if the type of the
parameter is not yet determined. When the type does get set using
setDescriptor method, it will call the binding code on the -?/+?. An example
here will make things clearer select * from t1 where c1 = -?
During the bind phase of compilation, BinaryOperatorNode's bindExpression
method gets called. This method first calls bindExpression on c1 and then it
calls bindExpression on -?. But at this point, we don't know the type of -?
and hence the bindExpression of UnaryOperatorNode simply returns w/o trying
to do any binding. After the binding calls on the 2 operands,
bindExpression in BinaryOperatorNode checks if the right operand is a
parameter and if so, it calls setDescriptor method on it to set it's type to
the type of the left operand. This is when the actual binding of -? will
happen and this is accomplished by overwriting the setDescriptor method in
the UnaryArithmeticOperatorNode. This method after setting the type of -?
calls the binding code on it which among other checks will make sure that
the type is getting set to a numeric type.
I hope this explanation, the comments in the code and the new test makes it
easier to understand the patch.
svn stat
M java\engine\org\apache\derby\impl\sql\compile\ParameterNode.java
M
java\engine\org\apache\derby\impl\sql\compile\UnaryDateTimestampOperatorNode.java
M
java\engine\org\apache\derby\impl\sql\compile\UnaryArithmeticOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\CollectNodesVisitor.java
M java\engine\org\apache\derby\impl\sql\compile\HasNodeVisitor.java
M java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
A java\engine\org\apache\derby\impl\sql\compile\HasParameterNodeVisitor.java
M java\engine\org\apache\derby\impl\sql\compile\UnaryOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\ValueNode.java
M java\testing\org\apache\derbyTesting\functionTests\tests\lang\build.xml
A
java\testing\org\apache\derbyTesting\functionTests\tests\lang\unaryArithmeticDynamicParameter.java
A
java\testing\org\apache\derbyTesting\functionTests\master\unaryArithmeticDynamicParameter.out
M java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
M
java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
I have run all the tests and no new failures. I have added a new test which
is part of this patch. The patch is attached to the JIRA entry. Any
comments, feedback would be great.
thanks,
Mamta
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be
the operand of unary operator "-".
Posted by Jeffrey Lichtman <sw...@rcn.com>.
>I'm a little unclear why this making UnaryOperatorNode extend
>ParameterNode and the subsequent changes are needed. Currently
>triggers/views can check to see if the statement has a parameter node
>anywhere in the tree without having every node implement ParameterNode,
>so what is different about UnaryOperatorNode now?
I agree with Dan that it's not a good idea to make UnaryOperatorNode
extend ParameterNode, but I disagree about what we should do about it.
Here's some context --- when it comes to figuring out the type of a
node, in almost all cases the type is determined by the node itself,
or the node figures it out based on its children. For example, all
comparison operators have a type of boolean, while an arithmetic
operator node (like +) figures out its type based on its operands
(e.g. adding two ints gives an int).
ParameterNode is an exception. It gets its type from the context in
which it is used. For example, in:
t1.int_col = ?
the type of the ? is derived from what it is being compared to.
From a coding point of view, most type resolution is done bottom-up.
Each node resolves its children, then figures out what its own type
is. Parameters are different in that the type resolution must be done
top-down. In the "=" operator in the above example, the children of
the equals node are bound first, after which it checks to see whether
either child is a parameter node. If so, it takes the type from the
non-parameter side and sets the parameter to the same type.
There are something like 13 different cases in which a parameter node
can be used, and each case requires some special code of the type I
just described. Mamta is trying to avoid introducing even more
special-case code. One way to do this is to make UnaryOperatorNode
extend ParameterNode.
The biggest problem I have with this is that a unary operator isn't a
type of parameter. I think that changing the class hierarchy in this
way will only confuse people in the future.
Another problem is that allowing "- ?" is only one possible extension
to the cases parameters are allowed. If we add more extensions, will
we have to make more node types into parameters? We could eventually
end up with a large proportion of ValueNodes as extensions of ParameterNode.
It seems to me that, to determine whether a node needs this special
top-down type processing, the code shouldn't be asking whether it's a
parameter node. It should ask whether the node (or expression) has to
derive its type from context. For example, BinaryOperatorNode could
be changed to something like this:
if (leftOperand.getsTypeFromContext())
{
if (rightOperand.getsTypeFromContext())
{
throw StandardException.newException(. . .);
}
leftOperand.setDescriptor(rightOperand.getTypeServices());
}
if (rightOperand.getsTypeFromContext())
{
rightOperand.setDescriptor(leftOperand.getTypeServices());
}
In ValueNode, getsTypeFromContext() would return false. In
ParameterNode it would return true. In UnaryArithmeticOperatorNode it
would return true if its operand is a ParameterNode and false
otherwise (one way to do this is to make it return
operand.getsTypeFromContext()).
As for why there are two methods for setting types (setType() and
setDescriptor()), they do different things for ParameterNode. All
that setType() does is set the type for the node itself.
ParameterNode.setDescriptor() also sets the type in the array of
parameter types in the StatementNode. There's also some stuff in
there having to do with replication/synchronization that could be
obsolete (depending on whether this feature is ever resurrected). I
agree that it's confusing to have two different methods do similar
things. Perhaps the two methods could be collapsed into one.
- Jeff Lichtman
swazoo@rcn.com
Check out Swazoo Koolak's Web Jukebox at
http://swazoo.com/
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Satheesh Bandaram <ba...@gmail.com>.
I will review the patch on Monday... Sorry for the delay.
Satheesh
On 10/23/05, Mamta Satoor <ms...@gmail.com> wrote:
>
> Pretty soon, I will have no recollection of why I did what I did in this
> patch :) so, can someone review this patch or commit it?
> thanks,
> Mamta
>
> On 10/16/05, Mamta Satoor <ms...@gmail.com> wrote:
> >
> > I submitted a new patch about 5 days back. Wondered if anyone can look
> > at it and send comments.
> > thanks,
> > Mamta
> >
> > On 10/11/05, Mamta Satoor <msatoor@gmail.com > wrote:
> > >
> > > Hi,
> > > I have a new patch to allow dynamic parameters for unary minus and
> > > unary plus operator. This is based on the feedback from Jeff and Dan.
> > > The unary minus/plus parameters will determine their types depending on the
> > > context. For this, I have added requiresTypeFromContext to ValueNode which
> > > always returns false. ParameterNode always returns true for this method.
> > > UnaryOperatorNode's requiresTypeFromContext calls the operand's
> > > requiresTypeFromContext if operand is not null.
> > > SimpleStringOperatorNode(subclass of UnaryOperatorNode) ends up overriding
> > > this method and returns false because functions lower and upper are always
> > > typed to String and do not need to get their type from the context.
> > > I have added 2 methods to UnaryOperatorNode.java, namely
> > > getParameterOperand() and isUnaryMinusOrPlusWithParameter(). There are few
> > > places in engine, where we need to access the underlying parameter and that
> > > is when getParameterOperand() gets used. This is required to directly call
> > > the ParameterNode methods such as getDefaultValue(), getParameterNumber()
> > > etc isUnaryMinusOrPlusWithParameter() is required so that engine can catch
> > > edge cases like select * from t1 where -? and c11=c11 or +? ie such a use of
> > > -?/+? should be disallowed, same as what we do for ? parameters.
> > > Also, as mentioned in the earlier review packages, the binding code
> > > for unary minus/unary plus dynamic parameters will not be executed until the
> > > type of these parameters can be determined. The type gets detemined when the
> > > setType method is called. For this reason, setType method is overridden in
> > > UnaryArithmeticOperatorNode. After setting the type, this method calls the
> > > binding code to make sure all the bind time rules are enforced.
> > > I have also consolidated the 2 type setting methods into one, namely
> > > setType. This setType is overwritten in ParameterNode so it can also set the
> > > type in the array of parameter types in StatementNode. Hopefully, this will
> > > make the type setting code much cleaner.
> > > I have added several new tests to check this functionality and these
> > > tests are in unaryArithmeticDynamicParameter.java. The test will get
> > > run in both embedded and network server modes.
> > > svn stat
> > > M java\engine\org\apache\derby\impl\sql\compile\CastNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\ResultSetNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\TernaryOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\HasVariantValueNodeVisitor.java
> > > M java\engine\org\apache\derby\impl\sql\compile\ParameterNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\LikeEscapeOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\Predicate.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\BinaryOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\StaticClassFieldReferenceNode.java
> > >
> > > M java\engine\org\apache\derby\impl\sql\compile\MethodCallNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\SelectNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\SubqueryNode.java
> > > 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\ColumnDefinitionNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\UnaryArithmeticOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\ConditionalNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\TimestampOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\SimpleStringOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\JoinNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\SQLToJavaValueNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\BinaryRelationalOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\HashJoinStrategy.java
> > > M java\engine\org\apache\derby\impl\sql\compile\FromBaseTable.java
> > > M java\engine\org\apache\derby\impl\sql\compile\IsNullNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\SetOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\ValueNodeList.java
> > > M java\engine\org\apache\derby\impl\sql\compile\JavaValueNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\CoalesceFunctionNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\BaseColumnNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\BinaryLogicalOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\LengthOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\ConcatenationOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\UnaryOperatorNode.java
> > > M java\engine\org\apache\derby\impl\sql\compile\PredicateList.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\BinaryListOperatorNode.java
> > > M
> > > java\engine\org\apache\derby\impl\sql\compile\NonStaticMethodCallNode.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\compile\StaticMethodCallNode.java
> > > M
> > > java\engine\org\apache\derby\iapi\sql\compile\OptimizablePredicate.java
> > > M java\engine\org\apache\derby\iapi\sql\compile\JoinStrategy.java
> > > M
> > > java\testing\org\apache\derbyTesting\functionTests\tests\lang\build.xml
> > > A
> > > java\testing\org\apache\derbyTesting\functionTests\tests\lang\unaryArithmeticDynamicParameter.java
> > > A
> > > java\testing\org\apache\derbyTesting\functionTests\master\unaryArithmeticDynamicParameter.out
> > > M
> > > java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
> > > M
> > > java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
> > >
> > > The patch is attached to the JIRA entry. As always, any feedback will
> > > be very appreciated.
> > > Mamta
> > > On 10/6/05, Daniel John Debrunner <djd@debrunners.com > wrote:
> > > >
> > > > Mamta Satoor wrote:
> > > > > Jeff, I just read your emails about adding a new method
> > > > > getsTypeFromContext() to avoid jumping hoola hoops with
> > > > ParameterNode.
> > > > > It certainly sounds much cleaner than having to implement
> > > > ParameterNode
> > > > > specific methods in ValueNode. I would be interested in knowing
> > > > what Dan
> > > > > thinks of it too?
> > > >
> > > > Sounds good, the method now (almost) matches the way it is used,
> > > > rather
> > > > than overloading isParameterNode for multiple meanings.
> > > >
> > > > One small change might be to call the method
> > > > 'requiresTypeFromContext()'
> > > > as 'getsTypeFromContext()' could read that the node itself fetches
> > > > its
> > > > type from its context.
> > > >
> > > > Dan.
> > > >
> > > >
> > > >
> > >
> > >
> >
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Pretty soon, I will have no recollection of why I did what I did in this
patch :) so, can someone review this patch or commit it?
thanks,
Mamta
On 10/16/05, Mamta Satoor <ms...@gmail.com> wrote:
>
> I submitted a new patch about 5 days back. Wondered if anyone can look at
> it and send comments.
> thanks,
> Mamta
>
> On 10/11/05, Mamta Satoor <ms...@gmail.com> wrote:
> >
> > Hi,
> > I have a new patch to allow dynamic parameters for unary minus and
> > unary plus operator. This is based on the feedback from Jeff and Dan.
> > The unary minus/plus parameters will determine their types depending on the
> > context. For this, I have added requiresTypeFromContext to ValueNode which
> > always returns false. ParameterNode always returns true for this method.
> > UnaryOperatorNode's requiresTypeFromContext calls the operand's
> > requiresTypeFromContext if operand is not null.
> > SimpleStringOperatorNode(subclass of UnaryOperatorNode) ends up overriding
> > this method and returns false because functions lower and upper are always
> > typed to String and do not need to get their type from the context.
> > I have added 2 methods to UnaryOperatorNode.java, namely
> > getParameterOperand() and isUnaryMinusOrPlusWithParameter(). There are few
> > places in engine, where we need to access the underlying parameter and that
> > is when getParameterOperand() gets used. This is required to directly call
> > the ParameterNode methods such as getDefaultValue(), getParameterNumber()
> > etc isUnaryMinusOrPlusWithParameter() is required so that engine can catch
> > edge cases like select * from t1 where -? and c11=c11 or +? ie such a use of
> > -?/+? should be disallowed, same as what we do for ? parameters.
> > Also, as mentioned in the earlier review packages, the binding code for
> > unary minus/unary plus dynamic parameters will not be executed until the
> > type of these parameters can be determined. The type gets detemined when the
> > setType method is called. For this reason, setType method is overridden in
> > UnaryArithmeticOperatorNode. After setting the type, this method calls the
> > binding code to make sure all the bind time rules are enforced.
> > I have also consolidated the 2 type setting methods into one, namely
> > setType. This setType is overwritten in ParameterNode so it can also set the
> > type in the array of parameter types in StatementNode. Hopefully, this will
> > make the type setting code much cleaner.
> > I have added several new tests to check this functionality and these
> > tests are in unaryArithmeticDynamicParameter.java. The test will get run
> > in both embedded and network server modes.
> > svn stat
> > M java\engine\org\apache\derby\impl\sql\compile\CastNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\ResultSetNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\TernaryOperatorNode.java
> >
> > M
> > java\engine\org\apache\derby\impl\sql\compile\HasVariantValueNodeVisitor.java
> > M java\engine\org\apache\derby\impl\sql\compile\ParameterNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\LikeEscapeOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\Predicate.java
> > M java\engine\org\apache\derby\impl\sql\compile\BinaryOperatorNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\StaticClassFieldReferenceNode.java
> >
> > M java\engine\org\apache\derby\impl\sql\compile\MethodCallNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\SelectNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\SubqueryNode.java
> > 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\ColumnDefinitionNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\UnaryArithmeticOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\ConditionalNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\TimestampOperatorNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\SimpleStringOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\JoinNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\SQLToJavaValueNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\BinaryRelationalOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\HashJoinStrategy.java
> > M java\engine\org\apache\derby\impl\sql\compile\FromBaseTable.java
> > M java\engine\org\apache\derby\impl\sql\compile\IsNullNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\SetOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\ValueNodeList.java
> > M java\engine\org\apache\derby\impl\sql\compile\JavaValueNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
> > M
> > java\engine\org\apache\derby\impl\sql\compile\CoalesceFunctionNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\BaseColumnNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\BinaryLogicalOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\LengthOperatorNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\ConcatenationOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\UnaryOperatorNode.java
> > M java\engine\org\apache\derby\impl\sql\compile\PredicateList.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\BinaryListOperatorNode.java
> > M
> > java\engine\org\apache\derby\impl\sql\compile\NonStaticMethodCallNode.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\compile\StaticMethodCallNode.java
> > M
> > java\engine\org\apache\derby\iapi\sql\compile\OptimizablePredicate.java
> > M java\engine\org\apache\derby\iapi\sql\compile\JoinStrategy.java
> > M
> > java\testing\org\apache\derbyTesting\functionTests\tests\lang\build.xml
> > A
> > java\testing\org\apache\derbyTesting\functionTests\tests\lang\unaryArithmeticDynamicParameter.java
> > A
> > java\testing\org\apache\derbyTesting\functionTests\master\unaryArithmeticDynamicParameter.out
> > M
> > java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
> > M
> > java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
> >
> > The patch is attached to the JIRA entry. As always, any feedback will be
> > very appreciated.
> > Mamta
> > On 10/6/05, Daniel John Debrunner <djd@debrunners.com > wrote:
> > >
> > > Mamta Satoor wrote:
> > > > Jeff, I just read your emails about adding a new method
> > > > getsTypeFromContext() to avoid jumping hoola hoops with
> > > ParameterNode.
> > > > It certainly sounds much cleaner than having to implement
> > > ParameterNode
> > > > specific methods in ValueNode. I would be interested in knowing what
> > > Dan
> > > > thinks of it too?
> > >
> > > Sounds good, the method now (almost) matches the way it is used,
> > > rather
> > > than overloading isParameterNode for multiple meanings.
> > >
> > > One small change might be to call the method
> > > 'requiresTypeFromContext()'
> > > as 'getsTypeFromContext()' could read that the node itself fetches its
> > > type from its context.
> > >
> > > Dan.
> > >
> > >
> > >
> >
> >
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
I submitted a new patch about 5 days back. Wondered if anyone can look at it
and send comments.
thanks,
Mamta
On 10/11/05, Mamta Satoor <ms...@gmail.com> wrote:
>
> Hi,
> I have a new patch to allow dynamic parameters for unary minus and unary
> plus operator. This is based on the feedback from Jeff and Dan. The unary
> minus/plus parameters will determine their types depending on the context.
> For this, I have added requiresTypeFromContext to ValueNode which always
> returns false. ParameterNode always returns true for this method.
> UnaryOperatorNode's requiresTypeFromContext calls the operand's
> requiresTypeFromContext if operand is not null.
> SimpleStringOperatorNode(subclass of UnaryOperatorNode) ends up overriding
> this method and returns false because functions lower and upper are always
> typed to String and do not need to get their type from the context.
> I have added 2 methods to UnaryOperatorNode.java, namely
> getParameterOperand() and isUnaryMinusOrPlusWithParameter(). There are few
> places in engine, where we need to access the underlying parameter and that
> is when getParameterOperand() gets used. This is required to directly call
> the ParameterNode methods such as getDefaultValue(), getParameterNumber()
> etc isUnaryMinusOrPlusWithParameter() is required so that engine can catch
> edge cases like select * from t1 where -? and c11=c11 or +? ie such a use of
> -?/+? should be disallowed, same as what we do for ? parameters.
> Also, as mentioned in the earlier review packages, the binding code for
> unary minus/unary plus dynamic parameters will not be executed until the
> type of these parameters can be determined. The type gets detemined when the
> setType method is called. For this reason, setType method is overridden in
> UnaryArithmeticOperatorNode. After setting the type, this method calls the
> binding code to make sure all the bind time rules are enforced.
> I have also consolidated the 2 type setting methods into one, namely
> setType. This setType is overwritten in ParameterNode so it can also set the
> type in the array of parameter types in StatementNode. Hopefully, this will
> make the type setting code much cleaner.
> I have added several new tests to check this functionality and these
> tests are in unaryArithmeticDynamicParameter.java. The test will get run
> in both embedded and network server modes.
> svn stat
> M java\engine\org\apache\derby\impl\sql\compile\CastNode.java
> M java\engine\org\apache\derby\impl\sql\compile\ResultSetNode.java
> M java\engine\org\apache\derby\impl\sql\compile\TernaryOperatorNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\HasVariantValueNodeVisitor.java
> M java\engine\org\apache\derby\impl\sql\compile\ParameterNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\LikeEscapeOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\Predicate.java
> M java\engine\org\apache\derby\impl\sql\compile\BinaryOperatorNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\StaticClassFieldReferenceNode.java
>
> M java\engine\org\apache\derby\impl\sql\compile\MethodCallNode.java
> M java\engine\org\apache\derby\impl\sql\compile\SelectNode.java
> M java\engine\org\apache\derby\impl\sql\compile\SubqueryNode.java
> 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\ColumnDefinitionNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\UnaryArithmeticOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\ConditionalNode.java
> M java\engine\org\apache\derby\impl\sql\compile\TimestampOperatorNode.java
>
> M
> java\engine\org\apache\derby\impl\sql\compile\SimpleStringOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\JoinNode.java
> M java\engine\org\apache\derby\impl\sql\compile\SQLToJavaValueNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\BinaryRelationalOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\HashJoinStrategy.java
> M java\engine\org\apache\derby\impl\sql\compile\FromBaseTable.java
> M java\engine\org\apache\derby\impl\sql\compile\IsNullNode.java
> M java\engine\org\apache\derby\impl\sql\compile\SetOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\ValueNodeList.java
> M java\engine\org\apache\derby\impl\sql\compile\JavaValueNode.java
> M java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
> M java\engine\org\apache\derby\impl\sql\compile\CoalesceFunctionNode.java
> M java\engine\org\apache\derby\impl\sql\compile\BaseColumnNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\BinaryLogicalOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\LengthOperatorNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\ConcatenationOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\UnaryOperatorNode.java
> M java\engine\org\apache\derby\impl\sql\compile\PredicateList.java
> M
> java\engine\org\apache\derby\impl\sql\compile\BinaryListOperatorNode.java
> M
> java\engine\org\apache\derby\impl\sql\compile\NonStaticMethodCallNode.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\compile\StaticMethodCallNode.java
> M java\engine\org\apache\derby\iapi\sql\compile\OptimizablePredicate.java
> M java\engine\org\apache\derby\iapi\sql\compile\JoinStrategy.java
> M java\testing\org\apache\derbyTesting\functionTests\tests\lang\build.xml
> A
> java\testing\org\apache\derbyTesting\functionTests\tests\lang\unaryArithmeticDynamicParameter.java
> A
> java\testing\org\apache\derbyTesting\functionTests\master\unaryArithmeticDynamicParameter.out
> M
> java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
> M
> java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
>
> The patch is attached to the JIRA entry. As always, any feedback will be
> very appreciated.
> Mamta
> On 10/6/05, Daniel John Debrunner <dj...@debrunners.com> wrote:
> >
> > Mamta Satoor wrote:
> > > Jeff, I just read your emails about adding a new method
> > > getsTypeFromContext() to avoid jumping hoola hoops with ParameterNode.
> >
> > > It certainly sounds much cleaner than having to implement
> > ParameterNode
> > > specific methods in ValueNode. I would be interested in knowing what
> > Dan
> > > thinks of it too?
> >
> > Sounds good, the method now (almost) matches the way it is used, rather
> > than overloading isParameterNode for multiple meanings.
> >
> > One small change might be to call the method 'requiresTypeFromContext()'
> > as 'getsTypeFromContext()' could read that the node itself fetches its
> > type from its context.
> >
> > Dan.
> >
> >
> >
>
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Hi,
I have a new patch to allow dynamic parameters for unary minus and unary
plus operator. This is based on the feedback from Jeff and Dan. The unary
minus/plus parameters will determine their types depending on the context.
For this, I have added requiresTypeFromContext to ValueNode which always
returns false. ParameterNode always returns true for this method.
UnaryOperatorNode's requiresTypeFromContext calls the operand's
requiresTypeFromContext if operand is not null.
SimpleStringOperatorNode(subclass of UnaryOperatorNode) ends up overriding
this method and returns false because functions lower and upper are always
typed to String and do not need to get their type from the context.
I have added 2 methods to UnaryOperatorNode.java, namely
getParameterOperand() and isUnaryMinusOrPlusWithParameter(). There are few
places in engine, where we need to access the underlying parameter and that
is when getParameterOperand() gets used. This is required to directly call
the ParameterNode methods such as getDefaultValue(), getParameterNumber()
etc isUnaryMinusOrPlusWithParameter() is required so that engine can catch
edge cases like select * from t1 where -? and c11=c11 or +? ie such a use of
-?/+? should be disallowed, same as what we do for ? parameters.
Also, as mentioned in the earlier review packages, the binding code for
unary minus/unary plus dynamic parameters will not be executed until the
type of these parameters can be determined. The type gets detemined when the
setType method is called. For this reason, setType method is overridden in
UnaryArithmeticOperatorNode. After setting the type, this method calls the
binding code to make sure all the bind time rules are enforced.
I have also consolidated the 2 type setting methods into one, namely
setType. This setType is overwritten in ParameterNode so it can also set the
type in the array of parameter types in StatementNode. Hopefully, this will
make the type setting code much cleaner.
I have added several new tests to check this functionality and these tests
are in unaryArithmeticDynamicParameter.java. The test will get run in both
embedded and network server modes.
svn stat
M java\engine\org\apache\derby\impl\sql\compile\CastNode.java
M java\engine\org\apache\derby\impl\sql\compile\ResultSetNode.java
M java\engine\org\apache\derby\impl\sql\compile\TernaryOperatorNode.java
M
java\engine\org\apache\derby\impl\sql\compile\HasVariantValueNodeVisitor.java
M java\engine\org\apache\derby\impl\sql\compile\ParameterNode.java
M java\engine\org\apache\derby\impl\sql\compile\LikeEscapeOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\Predicate.java
M java\engine\org\apache\derby\impl\sql\compile\BinaryOperatorNode.java
M
java\engine\org\apache\derby\impl\sql\compile\StaticClassFieldReferenceNode.java
M java\engine\org\apache\derby\impl\sql\compile\MethodCallNode.java
M java\engine\org\apache\derby\impl\sql\compile\SelectNode.java
M java\engine\org\apache\derby\impl\sql\compile\SubqueryNode.java
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\ColumnDefinitionNode.java
M
java\engine\org\apache\derby\impl\sql\compile\UnaryArithmeticOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\ConditionalNode.java
M java\engine\org\apache\derby\impl\sql\compile\TimestampOperatorNode.java
M
java\engine\org\apache\derby\impl\sql\compile\SimpleStringOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\JoinNode.java
M java\engine\org\apache\derby\impl\sql\compile\SQLToJavaValueNode.java
M
java\engine\org\apache\derby\impl\sql\compile\BinaryRelationalOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\HashJoinStrategy.java
M java\engine\org\apache\derby\impl\sql\compile\FromBaseTable.java
M java\engine\org\apache\derby\impl\sql\compile\IsNullNode.java
M java\engine\org\apache\derby\impl\sql\compile\SetOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\ValueNodeList.java
M java\engine\org\apache\derby\impl\sql\compile\JavaValueNode.java
M java\engine\org\apache\derby\impl\sql\compile\sqlgrammar.jj
M java\engine\org\apache\derby\impl\sql\compile\CoalesceFunctionNode.java
M java\engine\org\apache\derby\impl\sql\compile\BaseColumnNode.java
M
java\engine\org\apache\derby\impl\sql\compile\BinaryLogicalOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\LengthOperatorNode.java
M
java\engine\org\apache\derby\impl\sql\compile\ConcatenationOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\UnaryOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\PredicateList.java
M java\engine\org\apache\derby\impl\sql\compile\BinaryListOperatorNode.java
M java\engine\org\apache\derby\impl\sql\compile\NonStaticMethodCallNode.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\compile\StaticMethodCallNode.java
M java\engine\org\apache\derby\iapi\sql\compile\OptimizablePredicate.java
M java\engine\org\apache\derby\iapi\sql\compile\JoinStrategy.java
M java\testing\org\apache\derbyTesting\functionTests\tests\lang\build.xml
A
java\testing\org\apache\derbyTesting\functionTests\tests\lang\unaryArithmeticDynamicParameter.java
A
java\testing\org\apache\derbyTesting\functionTests\master\unaryArithmeticDynamicParameter.out
M java\testing\org\apache\derbyTesting\functionTests\suites\derbylang.runall
M
java\testing\org\apache\derbyTesting\functionTests\suites\derbynetmats.runall
The patch is attached to the JIRA entry. As always, any feedback will be
very appreciated.
Mamta
On 10/6/05, Daniel John Debrunner <dj...@debrunners.com> wrote:
>
> Mamta Satoor wrote:
> > Jeff, I just read your emails about adding a new method
> > getsTypeFromContext() to avoid jumping hoola hoops with ParameterNode.
> > It certainly sounds much cleaner than having to implement ParameterNode
> > specific methods in ValueNode. I would be interested in knowing what Dan
> > thinks of it too?
>
> Sounds good, the method now (almost) matches the way it is used, rather
> than overloading isParameterNode for multiple meanings.
>
> One small change might be to call the method 'requiresTypeFromContext()'
> as 'getsTypeFromContext()' could read that the node itself fetches its
> type from its context.
>
> Dan.
>
>
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the
operand of unary operator "-".
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Mamta Satoor wrote:
> Jeff, I just read your emails about adding a new method
> getsTypeFromContext() to avoid jumping hoola hoops with ParameterNode.
> It certainly sounds much cleaner than having to implement ParameterNode
> specific methods in ValueNode. I would be interested in knowing what Dan
> thinks of it too?
Sounds good, the method now (almost) matches the way it is used, rather
than overloading isParameterNode for multiple meanings.
One small change might be to call the method 'requiresTypeFromContext()'
as 'getsTypeFromContext()' could read that the node itself fetches its
type from its context.
Dan.
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Jeff, I just read your emails about adding a new method
getsTypeFromContext() to avoid jumping hoola hoops with ParameterNode. It
certainly sounds much cleaner than having to implement ParameterNode
specific methods in ValueNode. I would be interested in knowing what Dan
thinks of it too?
Mamta
On 10/5/05, Mamta Satoor <ms...@gmail.com> wrote:
>
> Other than the ParameterNode casting for setDescriptor method, I found 3
> additional places where an object gets casted to ParameterNode to get to
> methods getJSQLType, getParameterNumber and getDefaultValue. These happen in
> SQLToJavaValueNode, StaticMethodCallNod and BinaryRelationOperator. Just to
> see how the tests will run, I have removed the Parameter casting in these 3
> instances and have added the 3 methods to ValueNode with the following
> implementations
> public JSQLType getJSQLType()
> {
> if (dataTypeServices != null)
> return(new JSQLType( dataTypeServices ));
> else
> return null;
> }
> public int getParameterNumber()
> {
> return -1;
> }
> DataValueDescriptor getDefaultValue()
> {
> return null;
> }
> My intention behind this is to see how the existing tests and my unary
> arithmetic test would run without making UnaryOperatorNode a superclass of
> ParameterNode. I have added methods isParameterNode & setDescriptor to
> UnaryOperatorNode and I don't do parameter binding for -?/+? until the type
> of the underneath parameter gets set. (I haven't yet looked at consolidating
> the 2 type setter methods, but I feel uneasy about putting the 3
> ParameterNode specific methods into ValueNode.) Any thoughts?
> Mamta
>
> On 10/5/05, Mamta Satoor <ms...@gmail.com> wrote:
> >
> > Hi Dan,
> > I will take you up on your offer to "I'm willing to help out on any
> > issues you discover on this new path." :)
> > While trying out the implementation you suggested (still not completely
> > done), I noticed that when I try to sue +?/-? in function calls, the code
> > flow is as follows
> > at org.apache.derby.impl.sql.compile.SQLToJavaValueNode.getJSQLType(
> > SQLToJavaValueNode.java:154)
> > at org.apache.derby.impl.sql.compile.MethodCallNode.bindParameters(
> > MethodCallNode.java:331)
> > at org.apache.derby.impl.sql.compile.StaticMethodCallNode.bindExpression(
> > StaticMethodCallNode.java:170)
> > at org.apache.derby.impl.sql.compile.JavaToSQLValueNode.bindExpression(
> > JavaToSQLValueNode.java:250)
> > at org.apache.derby.impl.sql.compile.BinaryOperatorNode.bindExpression(
> > BinaryOperatorNode.java :309)
> > at
> > org.apache.derby.impl.sql.compile.BinaryComparisonOperatorNode.bindExpression
> > (BinaryComparisonOperatorNode.java:137)
> > at org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(
> > SelectNode.java:512)
> > at org.apache.derby.impl.sql.compile.DMLStatementNode.bindExpressions(
> > DMLStatementNode.java:247)
> > at org.apache.derby.impl.sql.compile.DMLStatementNode.bind(
> > DMLStatementNode.java:161)
> > at org.apache.derby.impl.sql.compile.ReadCursorNode.bind (
> > ReadCursorNode.java:74)
> > at org.apache.derby.impl.sql.compile.CursorNode.bind(CursorNode.java
> > :250)
> > at org.apache.derby.impl.sql.GenericStatement.prepMinion(
> > GenericStatement.java:333)
> > at org.apache.derby.impl.sql.GenericStatement.prepare (
> > GenericStatement.java:107)
> > at
> > org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement
> > (GenericLanguageConnectionContext.java:704)
> > at org.apache.derby.impl.jdbc.EmbedPreparedStatement .<init>(
> > EmbedPreparedStatement.java:118)
> > at org.apache.derby.impl.jdbc.EmbedPreparedStatement20.<init>(
> > EmbedPreparedStatement20.java:82)
> > at org.apache.derby.impl.jdbc.EmbedPreparedStatement30.<init>(
> > EmbedPreparedStatement30.java:62)
> > at org.apache.derby.jdbc.Driver30.newEmbedPreparedStatement(
> > Driver30.java:92)
> > at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(
> > EmbedConnection.java:678)
> > at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement (
> > EmbedConnection.java:522)
> > at
> > org.apache.derbyTesting.functionTests.tests.lang.unaryArithmeticDynamicParameter.main
> > (unaryArithmeticDynamicParameter.java:90)
> > SQLToJavaValueNode.getJSQLType after checking isParameterNode does
> > ParameterNode casting so it can get to getJSQLType method. But since
> > UnaryOperatorNode is not of the type ParameterNode, the casting fails, as
> > expected. I can remove the casting but that would mean to implement
> > getJSQLType method in ValueNode as well. But does this method make a sense
> > for ValueNode? If we do decide to add this method, UnaryOperatorNode will
> > simply call the same method on it's operand.
> > thanks,
> > Mamta
> >
> > On 10/5/05, Daniel John Debrunner < djd@debrunners.com> wrote:
> > >
> > > Daniel John Debrunner wrote:
> > >
> > >
> > > > On this it seems that there is some current confusion in the code,
> > > > ValueNode has two methods to set the type, setType and
> > > setDescriptor.
> > > > So why does ParameterNode need a special set type method
> > > setDescriptor,
> > > > and the associated required casting?
> > >
> > > I got confused here, ParameterNode.setDescriptor is overriding
> > > ValueNode.setDescriptor, so I'm not sure why the casting to
> > > ParameterNode is required.
> > >
> > > Still a move to a single method would be a good thing.
> > >
> > > Dan.
> > >
> > >
> >
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Other than the ParameterNode casting for setDescriptor method, I found 3
additional places where an object gets casted to ParameterNode to get to
methods getJSQLType, getParameterNumber and getDefaultValue. These happen in
SQLToJavaValueNode, StaticMethodCallNod and BinaryRelationOperator. Just to
see how the tests will run, I have removed the Parameter casting in these 3
instances and have added the 3 methods to ValueNode with the following
implementations
public JSQLType getJSQLType()
{
if (dataTypeServices != null)
return(new JSQLType( dataTypeServices ));
else
return null;
}
public int getParameterNumber()
{
return -1;
}
DataValueDescriptor getDefaultValue()
{
return null;
}
My intention behind this is to see how the existing tests and my unary
arithmetic test would run without making UnaryOperatorNode a superclass of
ParameterNode. I have added methods isParameterNode & setDescriptor to
UnaryOperatorNode and I don't do parameter binding for -?/+? until the type
of the underneath parameter gets set. (I haven't yet looked at consolidating
the 2 type setter methods, but I feel uneasy about putting the 3
ParameterNode specific methods into ValueNode.) Any thoughts?
Mamta
On 10/5/05, Mamta Satoor <ms...@gmail.com> wrote:
>
> Hi Dan,
> I will take you up on your offer to "I'm willing to help out on any
> issues you discover on this new path." :)
> While trying out the implementation you suggested (still not completely
> done), I noticed that when I try to sue +?/-? in function calls, the code
> flow is as follows
> at org.apache.derby.impl.sql.compile.SQLToJavaValueNode.getJSQLType(
> SQLToJavaValueNode.java:154)
> at org.apache.derby.impl.sql.compile.MethodCallNode.bindParameters(
> MethodCallNode.java:331)
> at org.apache.derby.impl.sql.compile.StaticMethodCallNode.bindExpression (
> StaticMethodCallNode.java:170)
> at org.apache.derby.impl.sql.compile.JavaToSQLValueNode.bindExpression(
> JavaToSQLValueNode.java:250)
> at org.apache.derby.impl.sql.compile.BinaryOperatorNode.bindExpression(
> BinaryOperatorNode.java :309)
> at
> org.apache.derby.impl.sql.compile.BinaryComparisonOperatorNode.bindExpression
> (BinaryComparisonOperatorNode.java:137)
> at org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(
> SelectNode.java:512)
> at org.apache.derby.impl.sql.compile.DMLStatementNode.bindExpressions(
> DMLStatementNode.java:247)
> at org.apache.derby.impl.sql.compile.DMLStatementNode.bind(
> DMLStatementNode.java:161)
> at org.apache.derby.impl.sql.compile.ReadCursorNode.bind (
> ReadCursorNode.java:74)
> at org.apache.derby.impl.sql.compile.CursorNode.bind(CursorNode.java:250)
> at org.apache.derby.impl.sql.GenericStatement.prepMinion(
> GenericStatement.java:333)
> at org.apache.derby.impl.sql.GenericStatement.prepare (
> GenericStatement.java:107)
> at
> org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement
> (GenericLanguageConnectionContext.java:704)
> at org.apache.derby.impl.jdbc.EmbedPreparedStatement .<init>(
> EmbedPreparedStatement.java:118)
> at org.apache.derby.impl.jdbc.EmbedPreparedStatement20.<init>(
> EmbedPreparedStatement20.java:82)
> at org.apache.derby.impl.jdbc.EmbedPreparedStatement30.<init>(
> EmbedPreparedStatement30.java:62)
> at org.apache.derby.jdbc.Driver30.newEmbedPreparedStatement(Driver30.java
> :92)
> at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(
> EmbedConnection.java:678)
> at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement (
> EmbedConnection.java:522)
> at
> org.apache.derbyTesting.functionTests.tests.lang.unaryArithmeticDynamicParameter.main
> (unaryArithmeticDynamicParameter.java:90)
> SQLToJavaValueNode.getJSQLType after checking isParameterNode does
> ParameterNode casting so it can get to getJSQLType method. But since
> UnaryOperatorNode is not of the type ParameterNode, the casting fails, as
> expected. I can remove the casting but that would mean to implement
> getJSQLType method in ValueNode as well. But does this method make a sense
> for ValueNode? If we do decide to add this method, UnaryOperatorNode will
> simply call the same method on it's operand.
> thanks,
> Mamta
>
> On 10/5/05, Daniel John Debrunner <dj...@debrunners.com> wrote:
> >
> > Daniel John Debrunner wrote:
> >
> >
> > > On this it seems that there is some current confusion in the code,
> > > ValueNode has two methods to set the type, setType and setDescriptor.
> > > So why does ParameterNode need a special set type method
> > setDescriptor,
> > > and the associated required casting?
> >
> > I got confused here, ParameterNode.setDescriptor is overriding
> > ValueNode.setDescriptor, so I'm not sure why the casting to
> > ParameterNode is required.
> >
> > Still a move to a single method would be a good thing.
> >
> > Dan.
> >
> >
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be
the operand of unary operator "-".
Posted by Jeffrey Lichtman <sw...@rcn.com>.
>SQLToJavaValueNode.getJSQLType after checking isParameterNode does
>ParameterNode casting so it can get to getJSQLType method. But since
>UnaryOperatorNode is not of the type ParameterNode, the casting
>fails, as expected. I can remove the casting but that would mean to
>implement getJSQLType method in ValueNode as well. But does this
>method make a sense for ValueNode? If we do decide to add this
>method, UnaryOperatorNode will simply call the same method on it's operand.
I'll try to help. The bindParameters() method in MethodCallNode gets
the signature of the method call from the parameters to the method.
This signature is in the form of an array of JSQLTypes.
A JSQLType is a sort of universal type descriptor that can describe
any of three different catagories: a SQL type (like VARCHAR), a Java
primitive type (like byte) or a Java class (like java.lang.String).
It was introduced by the synchronization/replication team for some
reason I don't clearly remember - it had something to do with the
need to log and replicate method invocations on any of the three
categories. Originally a method signature was represented as an array
of Java types.
Although any value node can have a JSQLType associated with it, it
looks to me like no attempt is made to maintain this association for
most types of ValueNodes. ParameterNodes do maintain the association
- this is one of the special things done by setDescriptor() in
ParameterNode. I believe this was done for the sake of
synchronization/replication. We could push this up into ValueNode,
but the idea makes me nervous - I don't want the entire ValueNode
hierarchy to know about JSQLTypes, which are special-purpose types
that most of the nodes shouldn't have to deal with.
Remember the mail I sent earlier on the question of making
UnaryOperatorNode extend ParameterNode? I suggested that, rather than
do this, we introduce a method called getsTypeFromContext() to
determine whether the type information should be pushed down from the
node above. If we used this method for this one purpose, and
continued to use isParameterNode() to determine whether the node is a
parameter, we wouldn't run into the problem you're encountering here.
- Jeff Lichtman
swazoo@rcn.com
Check out Swazoo Koolak's Web Jukebox at
http://swazoo.com/
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Hi Dan,
I will take you up on your offer to "I'm willing to help out on any issues
you discover on this new path." :)
While trying out the implementation you suggested (still not completely
done), I noticed that when I try to sue +?/-? in function calls, the code
flow is as follows
at org.apache.derby.impl.sql.compile.SQLToJavaValueNode.getJSQLType(
SQLToJavaValueNode.java:154)
at org.apache.derby.impl.sql.compile.MethodCallNode.bindParameters(
MethodCallNode.java:331)
at org.apache.derby.impl.sql.compile.StaticMethodCallNode.bindExpression(
StaticMethodCallNode.java:170)
at org.apache.derby.impl.sql.compile.JavaToSQLValueNode.bindExpression(
JavaToSQLValueNode.java:250)
at org.apache.derby.impl.sql.compile.BinaryOperatorNode.bindExpression(
BinaryOperatorNode.java:309)
at
org.apache.derby.impl.sql.compile.BinaryComparisonOperatorNode.bindExpression
(BinaryComparisonOperatorNode.java:137)
at org.apache.derby.impl.sql.compile.SelectNode.bindExpressions(
SelectNode.java:512)
at org.apache.derby.impl.sql.compile.DMLStatementNode.bindExpressions(
DMLStatementNode.java:247)
at org.apache.derby.impl.sql.compile.DMLStatementNode.bind(
DMLStatementNode.java:161)
at org.apache.derby.impl.sql.compile.ReadCursorNode.bind(ReadCursorNode.java
:74)
at org.apache.derby.impl.sql.compile.CursorNode.bind(CursorNode.java:250)
at org.apache.derby.impl.sql.GenericStatement.prepMinion(
GenericStatement.java:333)
at org.apache.derby.impl.sql.GenericStatement.prepare(GenericStatement.java
:107)
at
org.apache.derby.impl.sql.conn.GenericLanguageConnectionContext.prepareInternalStatement
(GenericLanguageConnectionContext.java:704)
at org.apache.derby.impl.jdbc.EmbedPreparedStatement.<init>(
EmbedPreparedStatement.java:118)
at org.apache.derby.impl.jdbc.EmbedPreparedStatement20.<init>(
EmbedPreparedStatement20.java:82)
at org.apache.derby.impl.jdbc.EmbedPreparedStatement30.<init>(
EmbedPreparedStatement30.java:62)
at org.apache.derby.jdbc.Driver30.newEmbedPreparedStatement(Driver30.java
:92)
at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(
EmbedConnection.java:678)
at org.apache.derby.impl.jdbc.EmbedConnection.prepareStatement(
EmbedConnection.java:522)
at
org.apache.derbyTesting.functionTests.tests.lang.unaryArithmeticDynamicParameter.main
(unaryArithmeticDynamicParameter.java:90)
SQLToJavaValueNode.getJSQLType after checking isParameterNode does
ParameterNode casting so it can get to getJSQLType method. But since
UnaryOperatorNode is not of the type ParameterNode, the casting fails, as
expected. I can remove the casting but that would mean to implement
getJSQLType method in ValueNode as well. But does this method make a sense
for ValueNode? If we do decide to add this method, UnaryOperatorNode will
simply call the same method on it's operand.
thanks,
Mamta
On 10/5/05, Daniel John Debrunner <dj...@debrunners.com> wrote:
>
> Daniel John Debrunner wrote:
>
>
> > On this it seems that there is some current confusion in the code,
> > ValueNode has two methods to set the type, setType and setDescriptor.
> > So why does ParameterNode need a special set type method setDescriptor,
> > and the associated required casting?
>
> I got confused here, ParameterNode.setDescriptor is overriding
> ValueNode.setDescriptor, so I'm not sure why the casting to
> ParameterNode is required.
>
> Still a move to a single method would be a good thing.
>
> Dan.
>
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the
operand of unary operator "-".
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Daniel John Debrunner wrote:
> On this it seems that there is some current confusion in the code,
> ValueNode has two methods to set the type, setType and setDescriptor.
> So why does ParameterNode need a special set type method setDescriptor,
> and the associated required casting?
I got confused here, ParameterNode.setDescriptor is overriding
ValueNode.setDescriptor, so I'm not sure why the casting to
ParameterNode is required.
Still a move to a single method would be a good thing.
Dan.
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the
operand of unary operator "-".
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Mamta Satoor wrote:
> Let me go back to the example sql
> select * from t1 where c1 = -?
> BinaryOperatorNode.bindExpression checks if right operand is a parameter
> node and if yes, it CASTS the right operand to ParameterNode and calls
> setDescriptor method on it.
> /* Is there a ? parameter on the right? */
> if (rightOperand.isParameterNode())
> {
> /* Set the right operand to the type of the left parameter. */
> (*(ParameterNode)* rightOperand).setDescriptor(
> leftOperand.getTypeServices());
> }
> This casting of a parameter operand to ParameterNode and setting the
> descriptor on it is spread all over Derby engine code and rather than
> having to change them all, I decided to make UnaryOperatorNode extend
> ParameterNode so the rest of the type setting code can work without any
> change.
>
> It's possible that I have been thinking single track, so if anyone have
> any suggestions, do let me know.
Unfortunately I think you have been driven down that track due to the
current code. :-(
One of my credo's is that it doesn't matter how much code needs to be
changed to make a fix, if it makes the code better then it's a good
thing to change all that code.
On this it seems that there is some current confusion in the code,
ValueNode has two methods to set the type, setType and setDescriptor.
So why does ParameterNode need a special set type method setDescriptor,
and the associated required casting?
A possible direction for the fix is:
- Have an isParameterNode method in UnaryOperatorNode that returns
its operand's isParameterNode return value.
- Have a single type setting method setType in ValueNode
- Have ParameterNode override setType with any logic it needs.
The the code above becomes:
if (rightOperand.isParameterNode())
rightOperand.setType(leftOperand.getTypeServices())
Maybe I'm missing something in the above that will come out in the
details, but it may be better to address that, rather than the somewhat
unnatural forcing of a UnaryOperatorNode to be a ParameterNode, when
logically it is not a ParameterNode.
I'm willing to help out on any issues you discover on this new path.
Dan.
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the operand of unary operator "-".
Posted by Mamta Satoor <ms...@gmail.com>.
Let me go back to the example sql
select * from t1 where c1 = -?
BinaryOperatorNode.bindExpression checks if right operand is a parameter
node and if yes, it CASTS the right operand to ParameterNode and calls
setDescriptor method on it.
/* Is there a ? parameter on the right? */
if (rightOperand.isParameterNode())
{
/* Set the right operand to the type of the left parameter. */
(*(ParameterNode)* rightOperand).setDescriptor(leftOperand.getTypeServices
());
}
This casting of a parameter operand to ParameterNode and setting the
descriptor on it is spread all over Derby engine code and rather than having
to change them all, I decided to make UnaryOperatorNode extend ParameterNode
so the rest of the type setting code can work without any change.
It's possible that I have been thinking single track, so if anyone have any
suggestions, do let me know.
Mamta
On 10/5/05, Daniel John Debrunner <dj...@debrunners.com> wrote:
>
> Mamta Satoor wrote:
>
> > This patch tries to support the type setting of unary-/unary+ parameter
> > similar to what is done for ? parameter ie the type of the -?/+? will be
> > dependent on the context in which it is used. And hence the type setting
> > rules for -?/+? will be same as for a regular ? parameter.
> >
> > In order to achieve this, I have changed UnaryOperatorNode to extend
> > ParameterNode. In addition, it has the method isParameterNode which will
> > return true only if its operand is an instance of ParameterNode and it's
> > method isParameterNode returns true AND the unary operator is of the
> > type unary-/unary+. What this means is that just because a class is
> > instance of ParameterNode doesn't automatically mean it indeed is a
> > ParameterNode. An additional check of the isParameterNode is required to
> > make sure there is a parameter underneath. This change in rule has
> > required adding a new class called HasParameterNodeVisitor which gets
> > used by the sqlgrammar to make sure there are no parameters allowed in
> > views and triggers. HasParameterNodeVisitor checks if the node is of the
> > type ParameterNode AND checks if the isParameterNode returns true. If
> > yes, then the HasParameterNodeVisitor's visit method will return that
> > node and grammar will throw an exception for parameter usage in
> > views/triggers.
>
> Thanks for the detailed explanations.
>
> I'm a little unclear why this making UnaryOperatorNode extend
> ParameterNode and the subsequent changes are needed. Currently
> triggers/views can check to see if the statement has a parameter node
> anywhere in the tree without having every node implement ParameterNode,
> so what is different about UnaryOperatorNode now?
>
> Or maybe to ask a different way, why does the current searching for
> parameters break if UnaryOperatorNode has a parameter as its argument?
>
> Dan.
>
>
>
Re: [PATCH]Derby-582 Dynamic parameter should be allowed to be the
operand of unary operator "-".
Posted by Daniel John Debrunner <dj...@debrunners.com>.
Mamta Satoor wrote:
> This patch tries to support the type setting of unary-/unary+ parameter
> similar to what is done for ? parameter ie the type of the -?/+? will be
> dependent on the context in which it is used. And hence the type setting
> rules for -?/+? will be same as for a regular ? parameter.
>
> In order to achieve this, I have changed UnaryOperatorNode to extend
> ParameterNode. In addition, it has the method isParameterNode which will
> return true only if its operand is an instance of ParameterNode and it's
> method isParameterNode returns true AND the unary operator is of the
> type unary-/unary+. What this means is that just because a class is
> instance of ParameterNode doesn't automatically mean it indeed is a
> ParameterNode. An additional check of the isParameterNode is required to
> make sure there is a parameter underneath. This change in rule has
> required adding a new class called HasParameterNodeVisitor which gets
> used by the sqlgrammar to make sure there are no parameters allowed in
> views and triggers. HasParameterNodeVisitor checks if the node is of the
> type ParameterNode AND checks if the isParameterNode returns true. If
> yes, then the HasParameterNodeVisitor's visit method will return that
> node and grammar will throw an exception for parameter usage in
> views/triggers.
Thanks for the detailed explanations.
I'm a little unclear why this making UnaryOperatorNode extend
ParameterNode and the subsequent changes are needed. Currently
triggers/views can check to see if the statement has a parameter node
anywhere in the tree without having every node implement ParameterNode,
so what is different about UnaryOperatorNode now?
Or maybe to ask a different way, why does the current searching for
parameters break if UnaryOperatorNode has a parameter as its argument?
Dan.