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

James Taylor commented on PHOENIX-1016:
---------------------------------------

Thanks for the updates, [~tdsilva]. Yes, that's ok to skip sequence values. For 
the new columns, you'll need to dynamically add them to SYSTEM.SEQUENCE if the 
table already exists when we attempt to create it at startup time. Take a look 
at this commit and just mimic it for SYSTEM.SEQUENCE: 
https://github.com/apache/phoenix/commit/aa2559fcfbbb41aecaaac1c067eabbb2632325d1

The way to test it is to create a sequence in Phoenix 3.0, then upgrade to the 
latest (i.e. just remove the old jar and copy in the snapshot jar), and bounce 
your local cluster. Then add a new sequence (using the new options) and make 
sure the old and new ones both work.

Here's some more additional comments:
1. For b/w compat, don't rename the START_WITH column here:
{code}
-    public static final String START_WITH = "START_WITH";
-    public static final byte[] START_WITH_BYTES = Bytes.toBytes(START_WITH);
+    public static final String START_VALUE = "START_VALUE";
+    public static final byte[] START_VALUE_BYTES = Bytes.toBytes(START_VALUE);
{code}

2. Can you limit test changes to SequenceIT to just adding new tests? Looks 
like you've modified some existing tests. Just rename those to new tests 
instead and keep the old tests as is, as we'll have more confidence about b/w 
compat then. If there are changes you need to make, can you explain why and 
then make the minimum changes necessary to them?

Looks great, though. One more update and I think we can declare victory! Nice 
job.


> 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