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