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

James Taylor commented on PHOENIX-476:
--------------------------------------

Thanks for the quick turnaround, [~kliew]. I think we're almost there. Do the 
tests pass now?

A few changes are needed, though.
- To maintain backwards compatibility (as your change would break usages of USE 
SCHEMA DEFAULT), you can make this change to PhoenixSQL.g:
{code}
use_schema_node returns [UseSchemaStatement ret]
        :   USE (DEFAULT | s=identifier)
        {ret = factory.useSchema(s); }
    ;
{code}
And then in UseSchemaStatement:
{code}
public class UseSchemaStatement extends MutableStatement {
    private final String schemaName;

    public UseSchemaStatement(String schemaName) {
        this.schemaName = schemaName == null ? StringUtil.EMPTY_STRING : 
schemaName;
    }
{code}
And then finally in SchemaUtil, make this change:
{code}
    public MutationState useSchema(UseSchemaStatement useSchemaStatement) 
throws SQLException {
        // As we allow default namespace mapped to empty schema, so this is to 
reset schema in connection
        if (useSchemaStatement.getSchemaName().equals(StringUtil.EMPTY_STRING) {
            connection.setSchema(null);
        } else {
{code}
- Using null in UngroupedAggregateRegionObserver will be a problem. Add a test 
for a table with a default value for one of the PK columns and do an UPSERT 
SELECT into it.
{code}
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
index d8d313d..c30a44b 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
@@ -435,7 +435,7 @@ public class UngroupedAggregateRegionObserver extends 
BaseScannerRegionObserver
                                     }
                                     values[i] = ptr.copyBytes();
                                 }
-                                writeToTable.newKey(ptr, values);
+                                writeToTable.newKey(null, ptr, values);
{code}
Rather than requiring a StatementContext, let's do the following:
- 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 9). 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.
- Then modify the usage of the default value back to the way you had it where 
you didn't need a StatementContext since you'll know you have a literal.
We'll may have to tweak this once we want to support NEXT VALUE FOR and 
CURRENT_DATE expressions (or we may not need to as these values are generated 
on the client and turned into literals anyway). But for now, this will work 
fine.
- Also, instead of deriving DefaultColumnValueIT from BaseClientManagedTimeIT, 
derive it from ParallelStatsDisabledIT. Then just change hardcoded references 
of T_DEFAULT to a new fullTableName member variable which you set in an @Before 
method using the static generateUniqueName() method. This allows your tests to 
run in parallel (since the table names are independent) and enables our tests 
to run much faster. The tests in BaseClientManagedTimeIT are the only ones 
waiting to be converted like this as well, so it's essentially deprecated. You 
also don't need to use nextTimestamp() and open new connections every time - 
just use the same connection throughout your test.


> 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 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)

Reply via email to