[
https://issues.apache.org/jira/browse/PHOENIX-476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15560493#comment-15560493
]
James Taylor commented on PHOENIX-476:
--------------------------------------
Looking really good, [~kliew]. Thanks for the updates! Here's some feedback:
- Don't bother caching the default Expression. It's not worth the trouble as
the parsing cost is minuscule. Your technique will break down if a column is
dropped as the position changes in that case. This could lead to a default
value of what was in a lower position to be used mistakenly. Just not caching
it is fine. If it ever shows up as a perf issue, we can deal with it then.
- Instead of forcing STORE_NULLS to be true for a table with default values
(and I should have thought about this before), just tweak the code in
PRowImpl.setValue() based on if the PColumn has a default value. Something like
this:
{code}
@Override
public void setValue(PColumn column, byte[] byteValue) {
deleteRow = null;
byte[] family = column.getFamilyName().getBytes();
byte[] qualifier = column.getName().getBytes();
PDataType<?> type = column.getDataType();
// Check null, since some types have no byte representation for null
boolean isNull = type.isNull(byteValue);
if (isNull && !column.isNullable()) {
throw new ConstraintViolationException(name.getString() + "." +
column.getName().getString() + " may not be null");
} else if (isNull && PTableImpl.this.isImmutableRows() &&
column.getExpressionStr() == null) { // Need to preserve explicit nulls for
immutable tables otherwise default value would be used
removeIfPresent(setValues, family, qualifier);
removeIfPresent(unsetValues, family, qualifier);
} else if (isNull && !getStoreNulls() && &&
column.getExpressionStr() == null) {
{code}
- Add some test cases where the table is immutable (i.e. set
IMMUTABLE_ROWS=true on DDL statement)
- Add a test for the case of a default value being set on a PK column in the
middle. For example: PK1, PK2 DEFAULT 'foo', PK3.
- Also, have a test that uses different types for default values (i.e. VARCHAR,
DECIMAL, TINYINT, and ARRAY would be interesting too).
- Make sure you've imported all of our default settings in terms of keeping
imports explicitly rather than wildcarding, import order, etc. For example, I
noticed that this was incorrectly change by your IDE:
{code}
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.sql.Connection;
-import java.sql.DriverManager;
-import java.sql.SQLException;
-import java.sql.Statement;
+import static org.junit.Assert.*;
{code}
See https://phoenix.apache.org/contributing.html#Code_conventions
> 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)
