[
https://issues.apache.org/jira/browse/PHOENIX-476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15558377#comment-15558377
]
James Taylor commented on PHOENIX-476:
--------------------------------------
Thanks for the patch, [~kliew]. Here's some feedback:
- Let's revert this change to ExpressionUtil as it's creating too many diffs in
other places. Plus, you'll only use the logic to check that an expression is a
constant in one place:
{code}
- public static boolean isConstant(Expression expression) {
+ public static boolean isConstant(Expression expression) {
+ return (expression.isStateless() && expression.getDeterminism() ==
Determinism.ALWAYS);
+ }
+
+ public static boolean isPureExpression(Expression expression) {
{code}
- This change in ExpressionCompiler can be much simpler. No need to recreate a
ColumnDef. Also, PColumn needs to stay immutable, so no setters there. Instead,
just parse the expressionStr using {{expressionNode = new
SQLParser(expressionStr).parseExpression()}} and call
expressionNode.accept(this) to get the Expression here (and minor nit, but just
check that {{column.getExpressionStr() != null}}. Shouldn't be any need to
check if empty too):
{code}
+ if (!SchemaUtil.isPKColumn(column)) {
+ if (column.getExpressionStr() != null &&
!column.getExpressionStr().isEmpty() && column.getDefaultExpression() == null) {
+ // We have the default expression but it has not been compiled
yet
+ ExpressionCompiler compiler = new ExpressionCompiler(context);
+ ColumnDef columnDef = new ParseNodeFactory().columnDef(
+
ColumnName.caseSensitiveColumnName(column.getFamilyName().getString(),
column.getName().getString()),
+ column.getDataType().getSqlTypeName(),
+ column.isNullable(),
+ column.getMaxLength(),
+ column.getScale(),
+ false,
+ column.getSortOrder(),
+ column.getExpressionStr(),
+ column.isRowTimestamp());
+ Expression defaultExpression =
columnDef.getDefaultExpressionNode().accept(compiler);
+ if (!columnDef.getDefaultExpressionNode().isStateless() ||
!ExpressionUtil.isConstant(defaultExpression)) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_CREATE_STATEFUL_DEFAULT)
+
.setColumnName(columnDef.getColumnDefName().getColumnName()).build().buildException();
+ }
+ column.setDefaultExpression(defaultExpression);
+ }
+ if (column.getDefaultExpression() != null) {
+ expression = new
DefaultValueExpression(Arrays.asList(expression,
column.getDefaultExpression()));
+ }
+ }
{code}
- Also, ColumnDef is immutable and needs to stay that way. Don't bother caching
the Expression on it here:
{code}
+ if (columnDef.getDefaultExpressionNode() != null) {
+ ExpressionCompiler compiler = new ExpressionCompiler(context);
+ Expression defaultExpression =
columnDef.getDefaultExpressionNode().accept(compiler);
+ if (!columnDef.getDefaultExpressionNode().isStateless() ||
!ExpressionUtil.isConstant(defaultExpression)) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_CREATE_STATEFUL_DEFAULT)
+
.setColumnName(columnDef.getColumnDefName().getColumnName()).build().buildException();
+ }
+ columnDef.setDefaultExpression(defaultExpression);
+ }
{code}
- Should be able to revert changes to PColumn and PColumnImpl (and sorry to
have led you down that path).
- There's a bit of code missing in PTableImpl.newKey() here (if this isn't the
case, let me know what you mean by the "trailing null issue"):
{code}
public int newKey(ImmutableBytesWritable key, byte[][] values) {
...
// If some non null pk values aren't set, then throw
if (i < nColumns) {
PColumn column = columns.get(i);
if (column.getDataType().isFixedWidth() ||
!column.isNullable()) {
throw new ConstraintViolationException(name.getString() +
"." + column.getName().getString() + " may not be null");
}
}
{code}
You'll need to loop until {{i == nColumns}} to check if the column is a PK
column with a default value and then evaluate and set the value. You'll need to
insert null values for PK columns which don't have values before that.
> 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.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)
