[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 2/18/17 6:59 AM:
-

Hi [~tdsilva], support for sequence expressions is planned for PHOENIX-3425, 
which will likely be picked up again after the Calcite integration.
We do expect this error when parsing sequence values as default expression:
{code:java}
CANNOT_CREATE_DEFAULT(1031, "42Y90", "Cannot create column with a stateful 
default value.")
{code}


was (Author: kliew):
Hi [~tdsilva], support for sequence expressions is planned for PHOENIX-3425.
We do expect this error when parsing sequence values as default expression:
{code:java}
CANNOT_CREATE_DEFAULT(1031, "42Y90", "Cannot create column with a stateful 
default value.")
{code}

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: New Feature
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.10.patch, PHOENIX-476.11.patch, 
> PHOENIX-476.12.patch, PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.8.patch, PHOENIX-476.9.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 11:08 PM:
---

I got it working. With the recent changes, we access the context again during 
the UPSERT so we can compile arbitrary isConstant expressions. I just needed to 
revert the older changes where we were compiling the expression to a literal in 
CreateTableCompiler and storing the expression string obtained from that. 

For scalar date-time, the expression was untouched. For arrays, it was being 
modified below:
{code:java}
public Expression visitLeave(ArrayConstructorNode node, List 
children) throws SQLException {
...
ArrayConstructorExpression arrayExpression = new 
ArrayConstructorExpression(children, arrayElemDataType, rowKeyOrderOptimizable);
if (ExpressionUtil.isConstant(arrayExpression)) {
for (int i = 0; i < children.size(); i++) {
Expression child = children.get(i);
child.evaluate(null, ptr);
Object value = null;
if (child.getDataType() == null) {
value = arrayElemDataType.toObject(ptr, 
theArrayElemDataType, child.getSortOrder());
} else {
value = arrayElemDataType.toObject(ptr, 
child.getDataType(), child.getSortOrder());
}
elements[i] = LiteralExpression.newConstant(value, 
theArrayElemDataType, child.getDeterminism()).getValue();
}
Object value = 
PArrayDataType.instantiatePhoenixArray(arrayElemDataType, elements);
return LiteralExpression.newConstant(value,
PDataType.fromTypeId(arrayElemDataType.getSqlType() + 
PDataType.ARRAY_TYPE_BASE), null, null, arrayExpression.getSortOrder(), 
Determinism.ALWAYS, rowKeyOrderOptimizable);
}
{code}
It looks that behavior was intended for some other use case but the issue might 
show up again.

Its just testing and cleanup left now.


was (Author: kliew):
I got it working. With the recent changes, we access the context again during 
the UPSERT so we can compile arbitrary isConstant expressions. I just needed to 
revert the older changes where we were compiling the expression to a literal in 
CreateTableCompiler and storing the expression string obtained from that. 

For scalar date-time, the expression was untouched. For arrays, it was being 
modified below:
{code:java}
public Expression visitLeave(ArrayConstructorNode node, List 
children) throws SQLException {
...
ArrayConstructorExpression arrayExpression = new 
ArrayConstructorExpression(children, arrayElemDataType, rowKeyOrderOptimizable);
if (ExpressionUtil.isConstant(arrayExpression)) {
for (int i = 0; i < children.size(); i++) {
Expression child = children.get(i);
child.evaluate(null, ptr);
Object value = null;
if (child.getDataType() == null) {
value = arrayElemDataType.toObject(ptr, 
theArrayElemDataType, child.getSortOrder());
} else {
value = arrayElemDataType.toObject(ptr, 
child.getDataType(), child.getSortOrder());
}
elements[i] = LiteralExpression.newConstant(value, 
theArrayElemDataType, child.getDeterminism()).getValue();
}
Object value = 
PArrayDataType.instantiatePhoenixArray(arrayElemDataType, elements);
return LiteralExpression.newConstant(value,
PDataType.fromTypeId(arrayElemDataType.getSqlType() + 
PDataType.ARRAY_TYPE_BASE), null, null, arrayExpression.getSortOrder(), 
Determinism.ALWAYS, rowKeyOrderOptimizable);
}
{code}

Its just testing and cleanup left now.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.8.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 7:52 PM:
--

We compile the string to an expression in CreateTableCompiler and save that to 
the metastore. For arrays: the date-time is represented as a string (without 
the type specifier) in the new expression string.

When we call `parseExpression` while doing UPSERT, it only sees the string 
without the type information.

{code:java}
ExpressionCompiler compiler = new ExpressionCompiler(context);
ParseNode defaultParseNode = new 
SQLParser(column.getExpressionStr()).parseExpression();
Expression defaultExpression = defaultParseNode.accept(compiler);
{code}

For scalar columns, the type specifier is not removed after compiling an 
Expression (ie. "DATE '1900-10-01 14:03:22.559'").

I don't think it is expected for the type specifier to be removed after 
compiling array values.


was (Author: kliew):
We compile the string to an expression contained in a ColumnDef. For arrays: 
the date-time is represented as a string (without the type specifier) in the 
new expression string.

When we call `parseExpression`, it only sees the string without the type 
information.

{code:java}
ExpressionCompiler compiler = new ExpressionCompiler(context);
ParseNode defaultParseNode = new 
SQLParser(column.getExpressionStr()).parseExpression();
Expression defaultExpression = defaultParseNode.accept(compiler);
{code}

For scalar columns, the type specifier is not removed after compiling an 
Expression (ie. "DATE '1900-10-01 14:03:22.559'").

I don't think it is expected for the type specifier to be removed after 
compiling array values.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.8.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 7:15 PM:
--

The byte coercion fix for data types also fixed byte coercion for arrays. 

Right now I'm running into a problem with default values for date-time types.

"time '1900-10-01 14:03:22.559'" worked for non array types.

I tried a few different variations of:
{code:sql}
tim TIME[5] DEFAULT ARRAY[time '1900-10-01 14:03:22.559', time '1901-10-01 
14:03:22.559']
{code}

{code}

org.apache.phoenix.schema.TypeMismatchException: ERROR 203 (22005): Type 
mismatch. ERROR 203 (22005): Type mismatch. ERROR 203 (22005): Type mismatch. 
ERROR 203 (22005): Type mismatch. VARCHAR cannot be coerced to TIME

at 
org.apache.phoenix.exception.SQLExceptionCode$1.newException(SQLExceptionCode.java:74)
at 
org.apache.phoenix.exception.SQLExceptionInfo.buildException(SQLExceptionInfo.java:145)
at 
org.apache.phoenix.util.ServerUtil.parseRemoteException(ServerUtil.java:129)
at 
org.apache.phoenix.util.ServerUtil.parseServerExceptionOrNull(ServerUtil.java:118)
at 
org.apache.phoenix.util.ServerUtil.parseServerException(ServerUtil.java:107)
at 
org.apache.phoenix.iterate.BaseResultIterators.getIterators(BaseResultIterators.java:751)
at 
org.apache.phoenix.iterate.BaseResultIterators.getIterators(BaseResultIterators.java:695)
at 
org.apache.phoenix.iterate.ConcatResultIterator.getIterators(ConcatResultIterator.java:50)
at 
org.apache.phoenix.iterate.ConcatResultIterator.currentIterator(ConcatResultIterator.java:97)
at 
org.apache.phoenix.iterate.ConcatResultIterator.next(ConcatResultIterator.java:117)
at 
org.apache.phoenix.jdbc.PhoenixResultSet.next(PhoenixResultSet.java:778)
at 
org.apache.phoenix.end2end.DefaultColumnValueIT.testDefaultArrays(DefaultColumnValueIT.java:732)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at 
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at 
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at 
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at 
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at 
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:117)
at 
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
at 
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:262)
at 
com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:84)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
{code}

I'll try alternate syntax, but was the above syntactically correct?

I also tried the TO_TIME function.


was (Author: kliew):
The byte coercion fix for data types also fixed byte coercion for arrays. 

Right now I'm running into a problem with def

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 6:36 PM:
--

Thanks James, that works. I'll get started on array support and further 
revision and testing .


was (Author: kliew):
Thanks James, that works. 

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.8.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 6:10 PM:
--

Oops, sorry I had to reset my environment so the patch is messy due to IDE 
changes. I cleaned up and reattached the patch. 


was (Author: kliew):
Oops, sorry I had to reset my environment so the patch is messy due to IDE 
changes. I will clean up the patch and attach again. 

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.8.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[James Taylor (JIRA)]

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

James Taylor edited comment on PHOENIX-476 at 10/22/16 5:53 PM:


No parser changes are required:
- In CreateTableCompiler, when you compile the defaultExpression, you'll end up 
with a LiteralExpression (i.e. DEFAULT -2 + 7 would be turned into a 
LiteralExpression of 5). Create a new CreateTableStatement copy constructor 
that passes in a createTableStatement and an expressionStr. Use this to create 
a new CreateTableStatement using defaultExpression.toString() to form the 
literal expression string which you'll pass through to the MetaDataClient to 
guarantee that the default expression is always a literal.
- By evaluating the default expression at CREATE TABLE time, and storing the 
evaluated expression string value, you'll know you have literal. When reading 
and parsing the default value back in PTableImpl, you can assume it's already a 
literal and parse it as you were doing in your previous patch:
{code}
+LiteralParseNode defaultParseNode = new 
SQLParser(column.getExpressionStr()).parseLiteral();
+LiteralExpression defaultLiteral = 
LiteralExpression.newConstant(defaultParseNode.getValue(), 
column.getDataType(), column.getMaxLength(), column.getScale(), 
column.getSortOrder(), Determinism.ALWAYS);
+ImmutableBytesWritable valuePtr = new 
ImmutableBytesWritable();
+defaultLiteral.evaluate(null, valuePtr);
+byteValue = 
ByteUtil.copyKeyBytesIfNecessary(valuePtr);
{code}


was (Author: jamestaylor):
No parser changes are required:
-In CreateTableCompiler, when you compile the defaultExpression, you'll end up 
with a LiteralExpression (i.e. DEFAULT -2 + 7 would be turned into a 
LiteralExpression of 5). Create a new CreateTableStatement copy constructor 
that passes in a createTableStatement and an expressionStr. Use this to create 
a new CreateTableStatement using defaultExpression.toString() to form the 
literal expression string which you'll pass through to the MetaDataClient to 
guarantee that the default expression is always a literal.
- By evaluating the default expression at CREATE TABLE time, and storing the 
evaluated expression string value, you'll know you have literal. When reading 
and parsing the default value back in PTableImpl, you can assume it's already a 
literal and parse it as you were doing in your previous patch:
{code}
+LiteralParseNode defaultParseNode = new 
SQLParser(column.getExpressionStr()).parseLiteral();
+LiteralExpression defaultLiteral = 
LiteralExpression.newConstant(defaultParseNode.getValue(), 
column.getDataType(), column.getMaxLength(), column.getScale(), 
column.getSortOrder(), Determinism.ALWAYS);
+ImmutableBytesWritable valuePtr = new 
ImmutableBytesWritable();
+defaultLiteral.evaluate(null, valuePtr);
+byteValue = 
ByteUtil.copyKeyBytesIfNecessary(valuePtr);
{code}

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benef

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 5:30 PM:
--

Hi [~jamestaylor], I made the suggested changes to recreate the create-table 
statement with the literal value string that we get after compiling the 
statement.

Without a StatementContext, that takes us back to the issue with parsing 
negative literal numbers at `new 
SQLParser(column.getExpressionStr()).parseLiteral()`.

I tried defining a new parse function without adding new tokens so that other 
functions won't be affected but the parser throws an exception during the 
prediction stage.

{code}
// Get a string, integer, double, date, boolean, or NULL value.
// Able to parse negative numbers.
any_literal returns [LiteralParseNode ret]
:   s=STRING_LITERAL {
ret = factory.literal(s.getText()); 
}
|   n=NUMBER {
ret = factory.wholeNumber(n.getText());
}
|   nn=MINUS DIGIT+ {
ret = factory.wholeNumber(nn.getText());
}
|   d=DECIMAL  {
ret = factory.realNumber(d.getText());
}
|   nd=MINUS DIGIT+ DOT POSINTEGER  {
ret = factory.realNumber(nd.getText());
}
|   dbl=DOUBLE  {
ret = factory.literal(Double.valueOf(dbl.getText()));
}
|   ndbl='.' POSINTEGER Exponent | MINUS DIGIT+ DOT Exponent | MINUS DIGIT+ 
(DOT (POSINTEGER (Exponent)?)? | Exponent)  {
ret = factory.literal(Double.valueOf(ndbl.getText()));
}
|   NULL {ret = factory.literal(null);}
|   TRUE {ret = factory.literal(Boolean.TRUE);} 
|   FALSE {ret = factory.literal(Boolean.FALSE);}
|   dt=identifier t=STRING_LITERAL { 
try {
ret = factory.literal(t.getText(), dt);
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
;
{code}


was (Author: kliew):
Hi [~jamestaylor], I made the suggested changes to recreate the create-table 
statement with the literal value string that we get after compiling the 
statement.

Without a StatementContext, that takes us back to the issue with parsing 
negative literals at `new SQLParser(column.getExpressionStr()).parseLiteral()`.

I tried defining a new parse function without adding new tokens so that other 
functions won't be affected but the parser throws an exception during the 
prediction stage.

{code}
// Get a string, integer, double, date, boolean, or NULL value.
// Able to parse negative numbers.
any_literal returns [LiteralParseNode ret]
:   s=STRING_LITERAL {
ret = factory.literal(s.getText()); 
}
|   n=NUMBER {
ret = factory.wholeNumber(n.getText());
}
|   nn=MINUS DIGIT+ {
ret = factory.wholeNumber(nn.getText());
}
|   d=DECIMAL  {
ret = factory.realNumber(d.getText());
}
|   nd=MINUS DIGIT+ DOT POSINTEGER  {
ret = factory.realNumber(nd.getText());
}
|   dbl=DOUBLE  {
ret = factory.literal(Double.valueOf(dbl.getText()));
}
|   ndbl='.' POSINTEGER Exponent | MINUS DIGIT+ DOT Exponent | MINUS DIGIT+ 
(DOT (POSINTEGER (Exponent)?)? | Exponent)  {
ret = factory.literal(Double.valueOf(ndbl.getText()));
}
|   NULL {ret = factory.literal(null);}
|   TRUE {ret = factory.literal(Boolean.TRUE);} 
|   FALSE {ret = factory.literal(Boolean.FALSE);}
|   dt=identifier t=STRING_LITERAL { 
try {
ret = factory.literal(t.getText(), dt);
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
;
{code}

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created 

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/22/16 5:17 PM:
--

Hi [~jamestaylor], I made the suggested changes to recreate the create-table 
statement with the literal value string that we get after compiling the 
statement.

Without a StatementContext, that takes us back to the issue with parsing 
negative literals at `new SQLParser(column.getExpressionStr()).parseLiteral()`.

I tried defining a new parse function without adding new tokens so that other 
functions won't be affected but the parser throws an exception during the 
prediction stage.

{code}
// Get a string, integer, double, date, boolean, or NULL value.
// Able to parse negative numbers.
any_literal returns [LiteralParseNode ret]
:   s=STRING_LITERAL {
ret = factory.literal(s.getText()); 
}
|   n=NUMBER {
ret = factory.wholeNumber(n.getText());
}
|   nn=MINUS DIGIT+ {
ret = factory.wholeNumber(nn.getText());
}
|   d=DECIMAL  {
ret = factory.realNumber(d.getText());
}
|   nd=MINUS DIGIT+ DOT POSINTEGER  {
ret = factory.realNumber(nd.getText());
}
|   dbl=DOUBLE  {
ret = factory.literal(Double.valueOf(dbl.getText()));
}
|   ndbl='.' POSINTEGER Exponent | MINUS DIGIT+ DOT Exponent | MINUS DIGIT+ 
(DOT (POSINTEGER (Exponent)?)? | Exponent)  {
ret = factory.literal(Double.valueOf(ndbl.getText()));
}
|   NULL {ret = factory.literal(null);}
|   TRUE {ret = factory.literal(Boolean.TRUE);} 
|   FALSE {ret = factory.literal(Boolean.FALSE);}
|   dt=identifier t=STRING_LITERAL { 
try {
ret = factory.literal(t.getText(), dt);
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
;
{code}


was (Author: kliew):
Hi [~jamestaylor], I made the suggested changes to recreate the create-table 
statement with the literal value string that we get after compiling the 
statement.

Without a StatementContext, that takes us back to the issue with parsing 
negative literals at `new SQLParser(column.getExpressionStr()).parseLiteral()`.

I tried defining a new parse function without adding new tokens so that other 
functions won't be affected but the parser throws an exception.

{code}
// Get a string, integer, double, date, boolean, or NULL value.
// Able to parse negative numbers.
any_literal returns [LiteralParseNode ret]
:   s=STRING_LITERAL {
ret = factory.literal(s.getText()); 
}
|   n=NUMBER {
ret = factory.wholeNumber(n.getText());
}
|   nn=MINUS DIGIT+ {
ret = factory.wholeNumber(nn.getText());
}
|   d=DECIMAL  {
ret = factory.realNumber(d.getText());
}
|   nd=MINUS DIGIT+ DOT POSINTEGER  {
ret = factory.realNumber(nd.getText());
}
|   dbl=DOUBLE  {
ret = factory.literal(Double.valueOf(dbl.getText()));
}
|   ndbl='.' POSINTEGER Exponent | MINUS DIGIT+ DOT Exponent | MINUS DIGIT+ 
(DOT (POSINTEGER (Exponent)?)? | Exponent)  {
ret = factory.literal(Double.valueOf(ndbl.getText()));
}
|   NULL {ret = factory.literal(null);}
|   TRUE {ret = factory.literal(Boolean.TRUE);} 
|   FALSE {ret = factory.literal(Boolean.FALSE);}
|   dt=identifier t=STRING_LITERAL { 
try {
ret = factory.literal(t.getText(), dt);
} catch (SQLException e) {
throw new RuntimeException(e);
}
}
;
{code}

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Fix For: 4.9.0
>
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.6.patch, 
> PHOENIX-476.7.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set 

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 10/9/16 7:03 AM:
-

I attached a patch with additional test coverage and all test cases passing.
Expressions are now cached outside of PColumn and ColumnDef. The driver sends 
the default value expression as part of the projector scan and the coprocessors 
reply with the result sets with default values substituted in (previously I was 
trying to get the driver to differentiate between nulls and default values at 
the client side to reduce the resultset size, but it was causing issues).
Let me know your feedback.


was (Author: kliew):
I attached a patch with additional test coverage and all test cases passing.
Expressions are now cached outside of PColumn and ColumnDef. The driver sends 
the default value expression as part of the projector scan and the coprocessors 
reply with the result sets with default values substituted in (previously I was 
trying to get the driver to differentiate between nulls and default values at 
the client side, which was causing issues).
Let me know your feedback.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.4.patch, PHOENIX-476.5.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/27/16 8:08 PM:
-

Thanks James, will do. I didn’t have the right context to understand the 
interactions between the coprocessor and driver; I was too focused on the 
individual components but I’m starting to see how it fits together.


was (Author: kliew):
Thanks James, will do. I didn’t have the right context; I was too focused on 
the individual parts but I’m starting to see how it fits together.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/27/16 2:41 AM:
-

I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
for (int j = 0; j < field.getCount(); j++) {
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // 
Skip null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + 
getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
index++;
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these to evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.


was (Author: kliew):
I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
for (int j = 0; j < field.getCount(); j++) {
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() 
> 0) { // Skip null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + 
getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, 
nBytes);
offset += nBytes;
}
}
index++;
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these to evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
>  

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/27/16 2:40 AM:
-

I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
for (int j = 0; j < field.getCount(); j++) {
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() 
> 0) { // Skip null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + 
getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, 
nBytes);
offset += nBytes;
}
}
index++;
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these to evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.


was (Author: kliew):
I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // Skip 
null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these to evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a def

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/27/16 2:38 AM:
-

I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // Skip 
null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these to evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.


was (Author: kliew):
I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // Skip 
null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, w

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/27/16 2:38 AM:
-

I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // Skip 
null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. _The 
driver receives only non-null values and a value bitmask from the coprocessor_, 
using these evaluate the Expression tree.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.


was (Author: kliew):
I did take a look at `PRowImpl.setValue` but that only determines how rows are 
serialized to disk. `KeyValueSchema.toBytes` determines how rows are serialized 
from the coprocessor to be sent to the driver. 

{code:java}
if (expressions[index].evaluate(tuple, ptr) && ptr.getLength() > 0) { // Skip 
null values
if (index >= minNullableIndex) {
valueSet.set(index - minNullableIndex);
}
if (!type.isFixedWidth()) {
b = ensureSize(b, offset, offset + getVarLengthBytes(ptr.getLength()));
offset = writeVarLengthField(ptr, b, offset);
} else {
int nBytes = ptr.getLength();
b = ensureSize(b, offset, offset + nBytes);
System.arraycopy(ptr.get(), ptr.getOffset(), b, offset, nBytes);
offset += nBytes;
}
}
{code}

In the code above (for `KeyValueSchema.toBytes`), regardless of whether we 
store the null value on disk, nulls are not set in the value bitmask. The 
driver receives only non-null values and a value bitmask from the coprocessor.

We could decide to store nulls but not default values on disk, which is what I 
think you were suggesting. Then we could differentiate nulls from default 
values and set default values in the value bitmask so that the driver will not 
see them as null values. 
But the driver also uses the value bitmask to determine the offset used to 
deserialize each cell, and now the driver will not be able to differentiate 
between non-null values that were serialized (and produce additional offset) 
and default values that were not serialized (and throws an exception because 
the expected byte array size is smaller than expected).

The previous test cases were only passing before due to a quirk in the 
`KeyValueSchema.next` -> `Expression.evaluate` logic where non-trailing nulls 
evaluate false -> true while trailing nulls evaluate null -> false.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key va

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[James Taylor (JIRA)]

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

James Taylor edited comment on PHOENIX-476 at 9/26/16 3:15 PM:
---

bq. I think that given the constraints that we have now, we might need to 
serialize another bitmask indicating default values. Does that sound like a 
good solution?
There's no need to make any changes to this because the DefaultValueExpression 
would have already been executed and correctly determined the correct value 
(whether null or the default value). The ProjectedColumnExpression is evaluated 
on the client side, *after* the DefaultValueExpression(KeyValueExpression) 
would already have been evaluated, so if it's null at that point, it really 
should stay null.

I think the remaining work is here[1]. Have you tried this yet, [~kliew] to see 
if there are any more issues once these are done?

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


was (Author: jamestaylor):
bq. I think that given the constraints that we have now, we might need to 
serialize another bitmask indicating default values.
Does that sound like a good solution?
There's no need to make any changes to this because the DefaultValueExpression 
would have already been executed and correctly determined the correct value 
(whether null or the default value). The ProjectedColumnExpression is evaluated 
on the client side, *after* the DefaultValueExpression(KeyValueExpression) 
would already have been evaluated, so if it's null at that point, it really 
should stay null.

I think the remaining work is here[1]. Have you tried this yet, [~kliew] to see 
if there are any more issues once these are done?

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

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/26/16 6:43 AM:
-

Thanks for the feedback guys. I investigated some more and I think I understand 
the issue more clearly now.

We’ve set (in MetadataClient) tables with default value columns to store nulls 
in HBase and we can detect whether a cell is null or default in 
`PRowImpl.setValue`. Next, we’d need to communicate that from the coprocessor 
to the driver but the rows serialized on the coprocessors contain only the 
non-null values followed by a bitmask indicating set/null values. Currently 
both nulls and default values are indicated as nulls in the bitmask.

We can’t change the value bitmask to indicate that default values are set and 
not null, because that would affect the size of the expected bytes and the 
offsets used for deserialization at the driver. 

I think that given the constraints that we have now, we might need to serialize 
another bitmask indicating default values. 
Does that sound like a good solution?
I think we’d run into the same constraints after migrating to Calcite because 
the root of the issue lies in the communication between the coprocessor and the 
driver.

Regarding `isConstant`: I think we can borrow the term ‘pure’ from functional 
programmers :). The current implementation for `isConstant` describes the 
criteria for a pure expression.

Thanks for pointing out the parens [~julianhyde]. I’ll remove them so that the 
tests will pass after migrating to Calcite.


was (Author: kliew):
Thanks for the feedback guys. I investigated some more and I think I understand 
the issue more clearly now.

We’ve set (in MetadataClient) tables with default value columns to store nulls 
in HBase and we can detect whether a cell is null or default in 
`PRowImpl.setValue`. Next, we’d need to communicate that from the coprocessor 
to the driver but the rows serialized on the coprocessors contain only the 
non-null values followed by a bitmask indicating set/null values. Currently 
both nulls and default values are indicated as nulls in the bitmask.

We can’t change the value bitmask to indicate that default values are set and 
not null, because that would affect the size of the expected bytes and the 
offsets used for deserialization at the driver. 

I think that given the constraints that we have now, we might need to serialize 
another bitmask indicating default values. 
Does that sound like a good solution?
I think we’d run into the same constraints after migrating to Calcite because 
the root of the issue lies in the communication between the coprocessor and the 
driver.

Regarding `isConstant`: I think we can borrow the term ‘pure’ from functional 
programmers :). The current implementation for `isConstant` describes the 
criteria for a pure expression.

Thanks for pointing out the parens [~julianhyde]. I’ll removed them so that the 
tests will pass after migrating to Calcite.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.3.patch, 
> PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doin

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/20/16 7:51 AM:
-

I attached a new patch implementing default values for both primary and 
row-value columns. 

There are a few issues though.

* After compiling a default value ParseNode into an Expression, it becomes a 
LiteralExpression. For `CURRENT_DATE()`, the ParseNode is stateless and the 
compiled `Expression.isConstant` is true so we aren’t able to throw an 
exception. Is there a existing expression compiler or a flag that will not 
convert the default value ParseNode to LiteralExpression?
Setting `requiresFinalEvaluation` did not change the result.

* DefaultValueExpression wraps a ProjectedColumnExpression in 
ExpressionCompiler. 
I modified `ProjectedColumnExpression.evaluate`  so that it returns true if the 
cell contains a null value.
But `KeyValueSchema.next` returns `null` for trailing nulls in a row so 
`ProjectedColumnExpression.evaluate` cannot differentiate between a trailing 
null and trailing default value.
If we change the return value contract of `KeyValueSchema.next`, it causes 
failures in other test cases (ie. SortMergeIT).
I’m not familiar with the serialization scheme for rows returned by the region 
servers, but I suspect that trailing nulls are trimmed, which might be why 
`KeyValueSchema.next` does not account for trailing nulls.
To fix this, we might have to provide an option to serialize trailing nulls 
from region servers, change the return contract of `KeyValueSchema.next`, and 
modify the affected components.

The following test case (which isn't in the patch) fails due to a trailing 
default value that is overwritten by a null value:
{code:java}

@Test
public void testCreateTableTrailingNullOverwritingDefault() throws 
Exception {
String table = generateRandomString();
String ddl = "CREATE TABLE " + table + " (" +
"pk INTEGER PRIMARY KEY, " +
"def INTEGER DEFAULT 1 + 9)";

long ts = nextTimestamp();
Properties props = new Properties();
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts));
Connection conn = DriverManager.getConnection(getUrl(), props);
conn.createStatement().execute(ddl);

String dml = "UPSERT INTO " + table + " VALUES (1, null)";
conn.createStatement().execute(dml);
conn.commit();

props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 
10));
conn = DriverManager.getConnection(getUrl(), props);

ResultSet rs = conn.createStatement().executeQuery("SELECT * FROM " + 
table + " WHERE pk = 1");
assertTrue(rs.next());
assertEquals(1, rs.getInt(1));
assertEquals(0, rs.getInt(2));
assertTrue(rs.wasNull());
assertFalse(rs.next());
}
{code}


was (Author: kliew):
I attached a new patch implementing default values for both primary and 
row-value columns. 

There are a few issues though.

* After compiling a default value ParseNode into an Expression, it becomes a 
LiteralExpression. For `CURRENT_DATE()`, the ParseNode is stateless and the 
compiled `Expression.isConstant` is true so we aren’t able to throw an 
exception. Is there a existing expression compiler or a flag that will not 
convert the default value ParseNode to LiteralExpression?
Setting `requiresFinalEvaluation` did not change the result.

* DefaultValueExpression wraps a ProjectedColumnExpression in 
ExpressionCompiler. 
I modified `ProjectedColumnExpression.evaluate`  so that it returns true if the 
cell contains a null value.
But `KeyValueSchema.next` returns `null` for trailing nulls in a row so 
`ProjectedColumnExpression.evaluate` cannot differentiate between a trailing 
null and trailing default value.
If we change the return value contract of `KeyValueSchema.next`, it causes 
failures in other test cases (ie. SortMergeIT).
I’m not familiar with the serialization scheme for rows returned by the region 
servers, but I suspect that trailing nulls are trimmed, which might be why 
`KeyValueSchema.next` does not account for trailing nulls.

The following test case (which isn't in the patch) fails due to a trailing 
default value that is overwritten by a null value:
{code:java}

@Test
public void testCreateTableTrailingNullOverwritingDefault() throws 
Exception {
String table = generateRandomString();
String ddl = "CREATE TABLE " + table + " (" +
"pk INTEGER PRIMARY KEY, " +
"def INTEGER DEFAULT 1 + 9)";

long ts = nextTimestamp();
Properties props = new Properties();
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts));
Connection conn = DriverManager.getConnection(getUrl(), props);
conn

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/20/16 7:47 AM:
-

I attached a new patch implementing default values for both primary and 
row-value columns. 

There are a few issues though.

* After compiling a default value ParseNode into an Expression, it becomes a 
LiteralExpression. For `CURRENT_DATE()`, the ParseNode is stateless and the 
compiled `Expression.isConstant` is true so we aren’t able to throw an 
exception. Is there a existing expression compiler or a flag that will not 
convert the default value ParseNode to LiteralExpression?
Setting `requiresFinalEvaluation` did not change the result.

* DefaultValueExpression wraps a ProjectedColumnExpression in 
ExpressionCompiler. 
I modified `ProjectedColumnExpression.evaluate`  so that it returns true if the 
cell contains a null value.
But `KeyValueSchema.next` returns `null` for trailing nulls in a row so 
`ProjectedColumnExpression.evaluate` cannot differentiate between a trailing 
null and trailing default value.
If we change the return value contract of `KeyValueSchema.next`, it causes 
failures in other test cases (ie. SortMergeIT).
I’m not familiar with the serialization scheme for rows returned by the region 
servers, but I suspect that trailing nulls are trimmed, which might be why 
`KeyValueSchema.next` does not account for trailing nulls.

The following test case (which isn't in the patch) fails due to a trailing 
default value that is overwritten by a null value:
{code:java}

@Test
public void testCreateTableTrailingNullOverwritingDefault() throws 
Exception {
String table = generateRandomString();
String ddl = "CREATE TABLE " + table + " (" +
"pk INTEGER PRIMARY KEY, " +
"def INTEGER DEFAULT 1 + 9)";

long ts = nextTimestamp();
Properties props = new Properties();
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts));
Connection conn = DriverManager.getConnection(getUrl(), props);
conn.createStatement().execute(ddl);

String dml = "UPSERT INTO " + table + " VALUES (1, null)";
conn.createStatement().execute(dml);
conn.commit();

props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 
10));
conn = DriverManager.getConnection(getUrl(), props);

ResultSet rs = conn.createStatement().executeQuery("SELECT * FROM " + 
table + " WHERE pk = 1");
assertTrue(rs.next());
assertEquals(1, rs.getInt(1));
assertEquals(0, rs.getInt(2));
assertTrue(rs.wasNull());
assertFalse(rs.next());
}
{code}


was (Author: kliew):
I attached a new patch implementing default values for both primary and 
row-value columns. 

There are a few issues though.

* After compiling a default value ParseNode into an Expression, it becomes a 
LiteralExpression. For `CURRENT_DATE()`, the ParseNode is stateless and the 
compiled `Expression.isConstant` is true so we aren’t able to throw an 
exception. Is there a existing expression compiler or a flag that will not 
convert the default value ParseNode to LiteralExpression?
Setting `requiresFinalEvaluation` did not change the result.

* DefaultValueExpression wraps a ProjectedColumnExpression in 
ExpressionCompiler. 
I modified `ProjectedColumnExpression.evaluate`  so that it returns true if the 
cell contains a null value.
But `KeyValueSchema.next` returns `null` for trailing nulls in a row 
`ProjectedColumnExpression.evaluate` cannot differentiate between a trailing 
null and trailing default value.
If we change the return value contract of `KeyValueSchema.next`, it causes 
failures in other test cases (ie. SortMergeIT).
I’m not familiar with the serialization scheme for rows returned by the region 
servers, but I suspect that trailing nulls are trimmed, which might be why 
`KeyValueSchema.next` does not account for trailing nulls.

The following test case (which isn't in the patch) fails due to a trailing 
default value that is overwritten by a null value:
{code:java}

@Test
public void testCreateTableTrailingNullOverwritingDefault() throws 
Exception {
String table = generateRandomString();
String ddl = "CREATE TABLE " + table + " (" +
"pk INTEGER PRIMARY KEY, " +
"def INTEGER DEFAULT 1 + 9)";

long ts = nextTimestamp();
Properties props = new Properties();
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts));
Connection conn = DriverManager.getConnection(getUrl(), props);
conn.createStatement().execute(ddl);

String dml = "UPSERT INTO " + table + " VALUES (1, null)";
conn.createStatement().execute(dml);
conn.commit();

props.setProp

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/16/16 7:15 PM:
-

If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`? 

I suppose CoalesceFunction will differentiate between null parse nodes and null 
values and that would give us the expected result?


was (Author: kliew):
If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`? 

I suppose CoalesceFunction will differentiate between null parse nodes and null 
values.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/16/16 7:16 PM:
-

If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`? 

I suppose CoalesceFunction will differentiate between null `Expression` and 
null values and that would give us the expected result?


was (Author: kliew):
If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`? 

I suppose CoalesceFunction will differentiate between null parse nodes and null 
values and that would give us the expected result?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/16/16 7:15 PM:
-

If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`? 

I suppose CoalesceFunction will differentiate between null parse nodes and null 
values.


was (Author: kliew):
If we consider the following case:

{noformat}
create table testdefault (pk integer primary key, test integer default 5);

upsert into testdefault values (1, null);

select * from testdefault;
{noformat}

We expect `1, ` but if we use `COALESCE` while retrieving the second 
column, would we get `1, 5`?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/16/16 7:05 PM:
-

Thanks for the feedback [~jamestaylor].

I attached a patch with  values implemented for primary keys. The  expression 
node is compiled and cached in CreateTableCompiler and accessed in PTableImpl. 
Will the compiled expression persist between server restarts? Do I need to 
recompile the  expression in MetadataClient so that the  values of existing 
tables are cached on server-restart?

* For non-pk columns, if we evaluate the  expression once on  and wrap the 
column in `COALESCE` to replace `NULL` values with the  expression, the user 
can explicitly set values (for columns with default values) to `NULL` but the 
values will always be  as the  value. We can only make this optimization for 
columns that have a `NOT NULL` constraint.

* For nullable columns and stateful default expressions (or non-literal 
expressions), we will probably have to  the value during the `UPSERT` 
execution. In my previous patch, did I make changes in the right locations 
(UpsertCompiler) for these specific cases?


was (Author: kliew):
Thanks for the feedback [~jamestaylor].

I attached a patch with  values implemented for primary keys. The  expression 
node is compiled and cached in CreateTableCompiler and accessed in PTableImpl. 
Will the compiled expression persist between server restarts? Do I need to 
recompile the  expression in MetadataClient so that the  values of existing 
tables are cached on server-restart?

* For non-pk columns, if we evaluate the  expression once on  and wrap the 
column in `COALESCE` to replace `NULL` values with the  expression, the user 
can explicitly set values (for columns with  values) to `NULL` but the values 
will always be  as the  value. We can only make this optimization for columns 
that have a `NOT NULL` constraint.

* For nullable columns and stateful default expressions (or non-literal 
expressions), we will probably have to  the value during the `UPSERT` 
execution. In my previous patch, did I make changes in the right locations 
(UpsertCompiler) for these specific cases?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



--
This 

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/16/16 6:21 PM:
-

Thanks for the feedback [~jamestaylor].

I attached a patch with  values implemented for primary keys. The  expression 
node is compiled and cached in CreateTableCompiler and accessed in PTableImpl. 
Will the compiled expression persist between server restarts? Do I need to 
recompile the  expression in MetadataClient so that the  values of existing 
tables are cached on server-restart?

* For non-pk columns, if we evaluate the  expression once on  and wrap the 
column in `COALESCE` to replace `NULL` values with the  expression, the user 
can explicitly set values (for columns with  values) to `NULL` but the values 
will always be  as the  value. We can only make this optimization for columns 
that have a `NOT NULL` constraint.

* For nullable columns and stateful default expressions (or non-literal 
expressions), we will probably have to  the value during the `UPSERT` 
execution. In my previous patch, did I make changes in the right locations 
(UpsertCompiler) for these specific cases?


was (Author: kliew):
Thanks for the feedback [~jamestaylor].

I attached a patch with default values implemented for primary keys. The 
default expression node is compiled and cached in CreateTableCompiler and 
accessed in PTableImpl. Will the compiled expression persist between server 
restarts? Do I need to recompile the default expression in MetadataClient so 
that the default values of existing tables are cached on server-restart?

* For non-pk columns, if we evaluate the default expression once on read and 
wrap the column in `COALESCE` to replace `NULL` values with the default 
expression, the user can explicitly set values (for columns with default 
values) to `NULL` but the values will always be read as the default value. We 
can only make this optimization for columns that have a `NOT NULL` constraint.

* For nullable columns and stateful default expressions, we will probably have 
to store the value during the `UPSERT` execution. In my previous patch, did I 
make changes in the right locations (UpsertCompiler) for these specific cases?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.2.patch, PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use t

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/8/16 6:24 PM:


I attached a patch implementing support for  in the CREATE statement. Is this 
the right approach? I will work on ALTER, DROP support, site documentation, and 
support for expressions in the  definition.

Should we save the  value in the SYSTEM.CATALOG table?

edit: I hadn't read the discussion above. I will take a look at that.


was (Author: kliew):
I attached a patch implementing support for  in the CREATE statement. Is this 
the right approach? I will work on ALTER, DROP support, site documentation, and 
support for expressions in the  definition.

Should we save the default value in the SYSTEM.CATALOG table?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/7/16 10:21 PM:
-

I attached a patch implementing support for  in the CREATE statement. Is this 
the right approach? I will work on ALTER, DROP support, site documentation, and 
support for expressions in the  definition.

Should we save the default value in the SYSTEM.CATALOG table?


was (Author: kliew):
I attached a patch implementing support for  in the CREATE statement. Is this 
the right approach? I will work on ALTER, DROP support and support for 
expressions in the  definition.

Should we  the  value in the SYSTEM.CATALOG table?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[Kevin Liew (JIRA)]

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

Kevin Liew edited comment on PHOENIX-476 at 9/7/16 10:11 PM:
-

I attached a patch implementing support for  in the CREATE statement. Is this 
the right approach? I will work on ALTER, DROP support and support for 
expressions in the  definition.

Should we  the  value in the SYSTEM.CATALOG table?


was (Author: kliew):
I attached a patch implementing support for DEFAULT in the CREATE statement. I 
will work on ALTER, DROP support and support for expressions in the DEFAULT 
definition.

Should we store the default value in the SYSTEM.CATALOG table?

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>Assignee: Kevin Liew
>  Labels: enhancement
> Attachments: PHOENIX-476.patch
>
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[James Taylor (JIRA)]

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

James Taylor edited comment on PHOENIX-476 at 9/2/16 4:50 PM:
--

[~kliew] - would you be interested in this one? We could do it in two stages:
# Allow only constants (i.e. ParseNode.isStateless() of true), like 5, 'foo', 
1.23.
# Allow things like CURRENT_DATE and NEXT VALUE FOR expressions which evaluate 
to constants, but must be evaluated per statement so they're a bit trickier.


was (Author: jamestaylor):
[~kliew] - would you be interested in this one? We could do it in two stages:
1. Allow only constants (i.e. ParseNode.isStateless() of true).
2. Allow things like CURRENT_DATE and NEXT VALUE FOR expressions which evaluate 
to constants, but must be evaluated per statement so they're a bit trickier.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>  Labels: enhancement
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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

[jira] [Comment Edited] (PHOENIX-476) Support declaration of DEFAULT in CREATE statement

[James Taylor (JIRA)]

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

James Taylor edited comment on PHOENIX-476 at 9/2/16 4:49 PM:
--

[~kliew] - would you be interested in this one? We could do it in two stages:
1. Allow only constants (i.e. ParseNode.isStateless() of true).
2. Allow things like CURRENT_DATE and NEXT VALUE FOR expressions which evaluate 
to constants, but must be evaluated per statement so they're a bit trickier.


was (Author: jamestaylor):
[~kliew] - would you be interested in this one? We could do it in two stages:
#. Allow only constants (i.e. ParseNode.isStateless() of true).
#. Allow things like CURRENT_DATE and NEXT VALUE FOR expressions which evaluate 
to constants, but must be evaluated per statement so they're a bit trickier.

> Support declaration of DEFAULT in CREATE statement
> --
>
> Key: PHOENIX-476
> URL: https://issues.apache.org/jira/browse/PHOENIX-476
> Project: Phoenix
>  Issue Type: Task
>Affects Versions: 3.0-Release
>Reporter: James Taylor
>  Labels: enhancement
>
> Support the declaration of a default value in the CREATE TABLE/VIEW statement 
> like this:
> CREATE TABLE Persons (
> Pid int NOT NULL PRIMARY KEY,
> LastName varchar(255) NOT NULL,
> FirstName varchar(255),
> Address varchar(255),
> City varchar(255) DEFAULT 'Sandnes'
> )
> To implement this, we'd need to:
> 1. add a new DEFAULT_VALUE key value column in SYSTEM.TABLE and pass through 
> the value when the table is created (in MetaDataClient).
> 2. always set NULLABLE to ResultSetMetaData.columnNoNulls if a default value 
> is present, since the column will never be null.
> 3. add a getDefaultValue() accessor in PColumn
> 4.  for a row key column, during UPSERT use the default value if no value was 
> specified for that column. This could be done in the PTableImpl.newKey method.
> 5.  for a key value column with a default value, we can get away without 
> incurring any storage cost. Although a little bit of extra effort than if we 
> persisted the default value on an UPSERT for key value columns, this approach 
> has the benefit of not incurring any storage cost for a default value.
> * serialize any default value into KeyValueColumnExpression
> * in the evaluate method of KeyValueColumnExpression, conditionally use 
> the default value if the column value is not present. If doing partial 
> evaluation, you should not yet return the default value, as we may not have 
> encountered the the KeyValue for the column yet (since a filter evaluates 
> each time it sees each KeyValue, and there may be more than one KeyValue 
> referenced in the expression). Partial evaluation is determined by calling 
> Tuple.isImmutable(), where false means it is NOT doing partial evaluation, 
> while true means it is.
> * modify EvaluateOnCompletionVisitor by adding a visitor method for 
> RowKeyColumnExpression and KeyValueColumnExpression to set 
> evaluateOnCompletion to true if they have a default value specified. This 
> will cause filter evaluation to execute one final time after all KeyValues 
> for a row have been seen, since it's at this time we know we should use the 
> default value.



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