You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Dag H. Wanvik (JIRA)" <ji...@apache.org> on 2008/11/27 03:52:46 UTC
[jira] Issue Comment Edited: (DERBY-481) implement SQL generated
columns
[ https://issues.apache.org/jira/browse/DERBY-481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12651231#action_12651231 ]
dagw edited comment on DERBY-481 at 11/26/08 6:52 PM:
---------------------------------------------------------------
Reviewed derby-481-14-ab-dropColumn. Thanks again Rick! The patch is
good to commit. My notes:
* AlterTableConstantAction.java:
- The following fragment threw me off at first:
> int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - cascadedDrops;
> // can NOT drop a column if it is the only one in the table
> if (sizeAfterCascadedDrops == 1)
Surely,, since we are talkign about size after cascaded drops, th test
should be against 0? But no, the quantity sizeAfterCascadedDrops does
not include the original dropped column, so the code works, but is a
bit misleading. Maybe do:
int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - 1 - cascadedDrops;
// Can not drop a column if it there would be no columns left:
if (sizeAfterCascadedDrops == 0)
- Maybe it would be informative to add a comment on the recursive call
that it never exceeds 2 in depth since generated columns don't
reference other generated columns.
- The new error message LANG_CASCADED_GEN_COL_DROP doesn't appear to be
used in the patch?
Is the intention that it be used in this exception?
throw StandardException.newException(
SQLState.LANG_PROVIDER_HAS_DEPENDENT_OBJECT,
dm.getActionString(DependencyManager.DROP_COLUMN),
"THE *LAST* COLUMN " + columnName,
"TABLE",
td.getQualifiedName() );
The new tests use LANG_PROVIDER_HAS_DEPENDENT_OBJECT (X0Y25), so
perhaps LANG_CASCADED_GEN_COL_DROP is cruft?
The wording of X0Y25 doesn't really precisely describe the problem,
though, so a new error message would be nice, I think...
- The string "THE *LAST* COLUMN " is not i18n safe.
- If generated column were to be dropped indirectly, columnName could
not *appear* to the user to be THE *LAST* COLUMN; I think the error
should point out in such cases that, after cascade action, we would
have 0 columns left. Perhaps the wording can be word-smithed to
handle both cases well.
- The error message wording of LANG_CASCADED_GEN_COL_DROP is also
confusing to me:
"Dropping column '{0}' would orphan the generated column '{1}' which
references it. You must drop the generated column first.
I interpret this to mean that if 0 is dropped, 1 would be the only
column left and a computed column can obviously not be alone in a
table. But the issue would be clearer explained by saying that after
1 is removed by cascade, there would be *no* columns left. I also
don't understand the point about dropping the generated column
first? Dropping 0 after having dropped 1 wouldn't improve things
unless another column were added in the meantime? Or maybe I am
misunderstanding here.. getting late ;-)
* GeneratedColumnsTest.java
- The verification that an index on a generated column is actually
dropped could be better, I think. Presently, you only verify that
an insert previosly blocked by the index is legal after the dropping
of the column. It would be good to inspect the dictionary to see
that it is actually gone physically as well.
- Under verification that constraints are dropped, I started
wondering.. I think this expected error is not according to the
standard:
:
create table t_dc_10( a int generated always as ( -b ) check ( a is not null ), b int, c int )"
alter table t_dc_10 drop column a restrict // gives X0Y25
:
According to section 11.18, we have:
"5) If RESTRICT is specified, then C shall not be referenced in any of the following:
a) The <query expression> of any view descriptor.
b) The <search condition> of any constraint descriptor other than a
table constraint descriptor that contains references to no other column and
*********************************************************************
that is included in the table descriptor of T.
c) The SQL routine body of any routine descriptor.
d) Either an explicit trigger column list or a triggered action
column set of any trigger descriptor.
e) The generation expression of any column descriptor."
It would seem to be that in this case we are covered under item 5b),
the check constraint only references the dropped column "a," so
RESTRICT should not block this dropping. This is not
behavior particular to generated columns, so if you agree with my interpretation, we should
file a JIRA for it.
was (Author: dagw):
Reviewed derby-481-14-ab-dropColumn. Thanks again Rick! The patch is
good to commit. My notes:
* AlterTableConstantAction.java:
- The following fragment threw me off at first:
> int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - cascadedDrops;
> // can NOT drop a column if it is the only one in the table
> if (sizeAfterCascadedDrops == 1)
Surely,, since we are talkign about size after cascaded drops, th test
should be against 0? But no, the quantity sizeAfterCascadedDrops does
not include the original dropped column, so the code works, but is a
bit misleading. Maybe do:
int sizeAfterCascadedDrops = td.getColumnDescriptorList().size() - 1 - cascadedDrops;
// Can not drop a column if it there would be no columns left:
if (sizeAfterCascadedDrops == 0)
- Maybe it would be informative to add a comment on the recursive call
that it never exceeds 2 in depth since generated columns don't
reference other generated columns.
- The new error message LANG_CASCADED_GEN_COL_DROP doesn't appear to be
used in the patch?
Is the intention that it be used in this exception?
throw StandardException.newException(
SQLState.LANG_PROVIDER_HAS_DEPENDENT_OBJECT,
dm.getActionString(DependencyManager.DROP_COLUMN),
"THE *LAST* COLUMN " + columnName,
"TABLE",
td.getQualifiedName() );
The new tests use LANG_PROVIDER_HAS_DEPENDENT_OBJECT (X0Y25), so
perhaps LANG_CASCADED_GEN_COL_DROP is cruft?
The wording of X0Y25 doesn't really precisely describe the problem,
though, so a new error message would be nice, I think...
- The string "THE *LAST* COLUMN " is not i18n safe.
- If generated column were to be dropped indirectly, columnName could
not *appear* to the user to be THE *LAST* COLUMN; I think the error
should point out in such cases that, after cascade action, we would
have 0 columns left. Perhaps the wording can be word-smithed to
handle both cases well.
- The error message wording of LANG_CASCADED_GEN_COL_DROP is also
confusing to me:
"Dropping column '{0}' would orphan the generated column '{1}' which
references it. You must drop the generated column first.
I interpret this to mean that if 0 is dropped, 1 would be the only
column left and a computed column can obviously not be alone in a
table. But the issue would be clearer explained by saying that after
1 is removed by cascade, there would be *no* columns left. I also
don't understand the point about dropping the generated column
first? Dropping 0 after having dropped 1 wouldn't improve things
unless another column were added in the meantime? Or maybe I am
misunderstanding here.. getting late ;-)
* GeneratedColumnsTest.java
- The verification that an index on a generated column is actually
dropped could be better, I think. Presently, you only verify that
an insert previosly blocked by the index is legal after the dropping
of the column. It would be good to inspect the dictionary to see
that it is actually gone physically as well.
- Under verification that constraints are dropped, I started
wondering.. I think this expected error is not according to the
standard:
:
create table t_dc_10( a int generated always as ( -b ) check ( a is not null ), b int, c int )"
alter table t_dc_10 drop column a restrict // gives X0Y25
:
According to section 11.19, we have:
"5) If RESTRICT is specified, then C shall not be referenced in any of the following:
a) The <query expression> of any view descriptor.
b) The <search condition> of any constraint descriptor other than a
table constraint descriptor that contains references to no other column and
******************
that is included in the table descriptor of T.
c) The SQL routine body of any routine descriptor.
d) Either an explicit trigger column list or a triggered action
column set of any trigger descriptor.
e) The generation expression of any column descriptor."
It would seem to be that in this case we are covered under item 5b),
the check constraint only references the dropped column "a," so
RESTRICT should not block this dropping. This is not
behavior particular to generated columns, so if you agree with my interpretation, we should
file a JIRA for it.
> implement SQL generated columns
> -------------------------------
>
> Key: DERBY-481
> URL: https://issues.apache.org/jira/browse/DERBY-481
> Project: Derby
> Issue Type: New Feature
> Components: SQL
> Affects Versions: 10.0.2.1
> Reporter: Rick Hillegas
> Assignee: Rick Hillegas
> Attachments: derby-481-00-aa-prototype.diff, derby-481-01-aa-catalog.diff, derby-481-02-aa-utilities.diff, derby-481-03-aa-grammar.diff, derby-481-04-aa-insert.diff, derby-481-05-aa-update.diff, derby-481-06-aa-genreferences.diff, derby-481-07-aa-noSQLinRoutines.diff, derby-481-07-ab-noSQLinRoutines.diff, derby-481-08-aa-castToDeclaredType.diff, derby-481-09-aa-dummyDefaults.diff, derby-481-10-aa-foreignKeyActions.diff, derby-481-11-aa-notNull.diff, derby-481-12-aa-padding.diff, derby-481-13-aa-alterDatatype.diff, derby-481-14-ab-dropColumn.diff, derby-481-15-aa-renameAndAddDefault.diff, derby-481-16-aa-dropFunction.diff, derby-481-17-aa-currentSchema.diff, derby-481-18-aa-updatableResultSets.diff, derby-481-19-aa-cleanup.diff, GeneratedColumns.html, GeneratedColumns.html, GeneratedColumns.html
>
>
> Satheesh has pointed out that generated columns, a SQL 2003 feature, would satisfy the performance requirements of Expression Indexes (bug 455). Generated columns may not be as elegant as Expression Indexes, but they are easier to implement. We would allow the following new kind of column definition in CREATE TABLE and ALTER TABLE statements:
> columnName GENERATED ALWAYS AS ( expression )
> If expression were an indexableExpression (as defined in bug 455), then we could create indexes on it. There is no work for the optimizer to do here. The Language merely has to compute the generated column at INSERT/UPDATE time.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.