[
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.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}
> 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)
