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

Thomas D'Silva commented on PHOENIX-1016:
-----------------------------------------

[~jamestaylor], thanks for the feedback.

I modified CreateSequenceStatement to set startWith to LiteralParseNode.ONE 
if both maxValue and minValue are null in CreateSequenceStatement, I think this 
will maintain the existing behavior (startValue is 1 when minValue, maxValue 
and startValue are not specified). If either minValue or maxValue are specified 
then 
startWith is set to minValue (if the sequence is increasing) or maxValue (if 
the sequence is decreasing).  
The default values for minValue and maxValue are Long.MIN_VALUE and 
Long.MAX_VALUE . 
I attached a second version of the patches with these changes.

{code}
protected CreateSequenceStatement(TableName sequenceName, ParseNode startWith,
            ParseNode incrementBy, ParseNode cacheSize, ParseNode minValue, 
ParseNode maxValue,
            boolean cycle, boolean ifNotExists, int bindCount) {
        this.sequenceName = sequenceName;
        // if MINVALUE, MAXVALUE and START WITH are not specified, set START 
WITH to 1 in order to
        // maintain backward compatibility
        this.startWith =
                (minValue == null && maxValue == null && startWith == null) ? 
LiteralParseNode.ONE
                        : startWith;
        this.minValue = minValue == null ? new LiteralParseNode(Long.MIN_VALUE) 
: minValue;
        this.maxValue = maxValue == null ? new LiteralParseNode(Long.MAX_VALUE) 
: maxValue;
        this.incrementBy = incrementBy == null ? LiteralParseNode.ONE : 
incrementBy;
        this.cacheSize = cacheSize;
        this.cycle = cycle;
        this.ifNotExists = ifNotExists;
        this.bindCount = bindCount;
    }
{code}

I had to store the current value in SYSTEM.SEQUENCE instead of the next value 
to handle the case when cycle is set to true. e.g If we have a sequence with 
startWith 11, minValue 1, maxValue 12, and cacheSize is 5. If SYSTEM.SEQUENCE 
stores the nextValue we would not ever return the sequence values 11 and 12, 
since SequenceRegionObserver would return 6 as the nextValue and  the 
currentValue of the sequence on the client side in SequenceValue (in 
Sequence.java) would be set as
{code}
           currentValue = nextValue - incrementBy * cacheSize;
{code}

If we don't change the CURRENT_VALUE for existing sequences in SYSTEM.SEQUENCE 
we would skip cacheSize sequence values. Is this OK?
 
Also, I had to change QueryConstants.java to add MINVALUE, MAXVALUE and CYCLE 
columns to SYSTEM.SEQUENCE, so  would I need a script to add these columns to 
any existing deployments?

Thanks,
Thomas


> Support MINVALUE, MAXVALUE, and CYCLE options in CREATE SEQUENCE
> ----------------------------------------------------------------
>
>                 Key: PHOENIX-1016
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1016
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: James Taylor
>            Assignee: Thomas D'Silva
>         Attachments: PHOENIX-1016.3.0.patch, PHOENIX-1016.patch, 
> PHOENIX-1016.v2.3.0.patch, PHOENIX-1016.v2.patch
>
>
> We currently don't support MINVALUE, MAXVALUE, and CYCLE options in CREATE 
> SEQUENCE, but we should. See 
> http://msdn.microsoft.com/en-us/library/ff878091.aspx for the syntax.
> I believe MINVALUE applies if the INCREMENT is negative while MAXVALUE 
> applies otherwise. If the value of a sequence goes beyond MINVALUE/MAXVALUE, 
> then:
> - if CYCLE is true, then the sequence value should start again at the START 
> WITH value (or the MINVALUE if specified too? Not sure about this).
> - if CYCLE is false, then an exception should be thrown.
> To implement this:
> - make the grammar changes in PhoenixSQL.g
> - add member variables for MINVALUE, MAXVALUE, and CYCLE to 
> CreateSequenceStatement
> - add the appropriate error checking and handle bind variables for these new 
> options in CreateSequenceCompiler
> - modify the MetaDataClient.createSequence() call by passing along these new 
> parameters.
> - same for ConnectionQueryServices.createSequence() call
> - same for Sequence.createSequence().
> - pass along these parameters as new KeyValues in the Append that constitutes 
> the RPC call
> - act on these in the SequenceRegionObserver coprocessor as indicated above.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to