phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas D'Silva (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (PHOENIX-1030) Change Expression.isDeterministic() to return an ENUM with three values {DETERMINISTIC, UNDETERMINISTIC_ROW, UNDETERMINISTIC_STMT}
Date Thu, 25 Sep 2014 23:57:34 GMT

    [ https://issues.apache.org/jira/browse/PHOENIX-1030?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14148513#comment-14148513
] 

Thomas D'Silva edited comment on PHOENIX-1030 at 9/25/14 11:57 PM:
-------------------------------------------------------------------

[~jamestaylor] ,

Thanks a lot for the feedback. I changed DeterministicEnum and getDeterministicEnum() to Determinism
and getDeterminism() and added the combine method.

I used this combine method in all the places that were and-ing together isDeterministic()
except in four places in ExpressionCompiler.
The code was previously calling LiteralExpression.newConstant() with value set to null, and
so the determinism was being ignored. 
LiteralExpression.newConstant returns either a NULL_EXPRESSION or one of the TYPED_NULL_EXPRESSIONS
so I modified the code as follows:
{code}
-                    return LiteralExpression.newConstant(null, theType, isDeterministic);
+                    return LiteralExpression.newConstant(null, theType);
{code}

I have a question about the Non-Deterministic versions of NULL, TYPED_NULL, TRUE and FALSE
expressions. Previously the Non-Deterministic version of NULL etc were setting the deterministicEnum
to false. Since Determinism is now an enum, I have set it to Determinism.NEVER, is this approach
correct?

Also, how should I handle code that previously or-ed the isDeterministic flag (in ComparisonExpression.java)?
This is the change I made:
{code}
-        boolean isDeterministic = lhsExpr.isDeterministic() || rhsExpr.isDeterministic();
+        Determinism determinism = ( lhsExpr.getDeterminism() == Determinism.ALWAYS || rhsExpr.getDeterminism()
 == Determinism.ALWAYS ) 
+                       ? Determinism.ALWAYS : Determinism.NEVER;
{code}

I also changed SequenceValueExpression.getDeterminism to return PER_ROW for sequences. Currently
only  AggregateFunction.getDerterminism() returns Determinism.NEVER. The only other places
where it is used is 
for the Non-Deterministic versions of NULL, TYPED_NULL, TRUE and FALSE expressions and where
the deterministicEnum used to be or-ed.

I also modified LiteralExpression.readFields and LiteralExpression.write to store the determinism
enum in the same integer used to store the sortOrder so that a older client can still talk
to a server on the new version. I had to modify ExpressionUtil.isConstant to use OVERRIDE_LITERAL_FUNCTIONS
as before (in case the client is from an older version and doesn’t have the determinism
enum). 

I have attached a path for 3.2 and anther for 4.2/master
Thanks,
Thomas


was (Author: tdsilva):
James,

Thanks a lot for the feedback. I changed DeterministicEnum and getDeterministicEnum() to Determinism
and getDeterminism() and added the combine method.

I used this combine method in all the places that were and-ing together isDeterministic()
except in four places in ExpressionCompiler.
The code was previously calling LiteralExpression.newConstant() with value set to null, and
so the determinism was being ignored. 
LiteralExpression.newConstant returns either a NULL_EXPRESSION or one of the TYPED_NULL_EXPRESSIONS
so I modified the code as follows:
{code}
-                    return LiteralExpression.newConstant(null, theType, isDeterministic);
+                    return LiteralExpression.newConstant(null, theType);
{code}

I have a question about the Non-Deterministic versions of NULL, TYPED_NULL, TRUE and FALSE
expressions. Previously the Non-Deterministic version of NULL etc were setting the deterministicEnum
to false. Since Determinism is now an enum, I have set it to Determinism.NEVER, is this approach
correct?

Also, how should I handle code that previously or-ed the isDeterministic flag (in ComparisonExpression.java)?
This is the change I made:
{code}
-        boolean isDeterministic = lhsExpr.isDeterministic() || rhsExpr.isDeterministic();
+        Determinism determinism = ( lhsExpr.getDeterminism() == Determinism.ALWAYS || rhsExpr.getDeterminism()
 == Determinism.ALWAYS ) 
+                       ? Determinism.ALWAYS : Determinism.NEVER;
{code}

I also changed SequenceValueExpression.getDeterminism to return PER_ROW for sequences. Currently
only  AggregateFunction.getDerterminism() returns Determinism.NEVER. The only other places
where it is used is 
for the Non-Deterministic versions of NULL, TYPED_NULL, TRUE and FALSE expressions and where
the deterministicEnum used to be or-ed.

I also modified LiteralExpression.readFields and LiteralExpression.write to store the determinism
enum in the same integer used to store the sortOrder so that a older client can still talk
to a server on the new version. I had to modify ExpressionUtil.isConstant to use OVERRIDE_LITERAL_FUNCTIONS
as before (in case the client is from an older version and doesn’t have the determinism
enum). 

I have attached a path for 3.2 and anther for 4.2/master
Thanks,
Thomas

> Change Expression.isDeterministic() to return an ENUM with three values {DETERMINISTIC,
UNDETERMINISTIC_ROW, UNDETERMINISTIC_STMT}
> ----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-1030
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1030
>             Project: Phoenix
>          Issue Type: Improvement
>            Reporter: Thomas D'Silva
>            Assignee: Thomas D'Silva
>         Attachments: PHOENIX-1030-3.0.patch, PHOENIX-1030-3.0.patch, PHOENIX-1030-4.0.patch,
PHOENIX-1030-master.patch
>
>
> Change Expression.isDeterministic() to return an ENUM with three values
> DETERMINISTIC -  the expression returns the same output every time given the same input.
> UNDETERMINISTIC_ROW - the expression should be computed for every row
> UNDETERMINISTIC_STMT - the expression should be be computed for a given statement only
once
> See PHOENIX-1001



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message