[
https://issues.apache.org/jira/browse/DERBY-3155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13802201#comment-13802201
]
Rick Hillegas commented on DERBY-3155:
--------------------------------------
Hi Mike,
Here are responses to your later comments:
> would like to understand the includeLocations use in the code, if this
> is in progress or what is expected for whole project.
In a nutshell, includeRowLocations() == true iff the contents of the hash table
are LocatedRows. Conversely, includeRowLocations() == false iff the contents of
the hash table are DataValueDescriptor[]. Support for LocatedRows is provided
for scan-driven (rather than RowSource-driven) hashtables. If someone wants to
add LocatedRow support for RowSource-driven hash tables, they are welcome to do
so. But I don't recommend doing this until someone has an actual use-case which
would benefit from this feature and prove that it works. At this time, my MERGE
statement work hasn't created such a use-case.
> might be good to add an assert somewhere if code tries to ask for a
> RowLocation on a btree, or at least understand what
> happens in checked in code if that happened.
I will put the following assertion at the beginning of
BTreeForwardScan.fetchRows():
if (SanityManager.DEBUG)
{
// RowLocations in the BTree itself are unstable and should
// not be put in long-lived structures like persistent hash tables.
SanityManager.ASSERT
( (hash_table == null) || !hash_table.includeRowLocations() );
}
> Index:
> java/engine/org/apache/derby/iapi/store/access/BackingStoreHashtable.java
>
> psuedo code in BackingStoreHashTable javdoc is not right anymore, it shows row
> as being Object[]. Not sure if the hash description is right or not still.
I will change the row declaration in the pseudo-code to:
Object row; // is a DataValueDescriptor[] or a LocatedRow
The hash description looks right to me, but I am happy to correct it if you see
a problem.
> nit: comments added, over 80 char line
I don't see any added lines which bust this limit now. There are some original
lines which bust this limit, however. If you still think some lines should be
corrected, please give me the offending line numbers.
> this comment is confusing to me:
> Currently, if the RowSource is not null, then there is no su
> pport for including RowLocations in the returned rows. That functionality is
> only supported for scans from base tables.
>
> I think BackingStoreHashTable constructor action is now dependent on info
> provided from the passed in row source, some comment on that would help -
> which
> may be what the above is trying to say.
I will change the comment to the following. I hope this is less confusing...
* constructor. RowLocations are supported iff row_source is null.
* RowLocations in a non-null row_source can be added later
* if there is a use-case that stresses this behavior.
...and I will add the following sanity check at the beginning of the
constructor:
if (SanityManager.DEBUG)
{
// RowLocations are not currently supported if the
// hash table is being filled from a non-null
// row_source arg.
if ( row_source != null )
{
SanityManager.ASSERT( !includeRowLocations() );
}
}
> is it right that includeRowLocations is hard coded to return false?
That's correct. That method is overridden by the BackingStoreHashTableFromScan
subclass, based on its constructor args.
> nit: the following style still in code (this could just be my issue):
> + if ( !includeRowLocations() )
> +
> { return diskRow; }
>
> + else
> { return new LocatedRow( diskRow ); }
>
> I think the 2 acceptable forms are either:
> if ( !includeRowLocations() )
> { return diskRow; }
>
> else
> { return new LocatedRow( diskRow ); }
>
> if ( !includeRowLocations() )
> { return diskRow; }
>
> else
> { return new LocatedRow( diskRow ); }
I'm afraid I'm not following this one. I don't see any instances of a blank
line following "if ( !includeRowLocations() )" in the diff file. Can you give
me the line number of the problematic usage?
> Index:
> java/engine/org/apache/derby/iapi/store/access/TransactionController.java
>
> nit: over 80 char lines
Done.
> Index:
> java/engine/org/apache/derby/impl/store/access/conglomerate/GenericScanCo
> ntroller.java
> nit: over 80 char
Done.
Thanks,
-Rick
> Support for SQL:2003 MERGE statement
> ------------------------------------
>
> Key: DERBY-3155
> URL: https://issues.apache.org/jira/browse/DERBY-3155
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Reporter: Trejkaz
> Assignee: Rick Hillegas
> Labels: derby_triage10_10
> Attachments: derby-3155-01-ac-grammar.diff,
> derby-3155-02-ag-fixParserWarning.diff,
> derby-3155-03-ae-backingStoreHashtableWithRowLocation.diff,
> derby-3155-03-af-backingStoreHashtableWithRowLocation.diff,
> derby-3155-04-ae-deleteAction.diff, MergeStatement.html, MergeStatement.html,
> MergeStatement.html
>
>
> A relatively common piece of logic in a database application is to check for
> a row's existence and then either update or insert depending on its existence.
> SQL:2003 added a MERGE statement to perform this operation. It looks like
> this:
> MERGE INTO table_name USING table_name ON (condition)
> WHEN MATCHED THEN UPDATE SET column1 = value1 [, column2 = value2 ...]
> WHEN NOT MATCHED THEN INSERT column1 [, column2 ...] VALUES (value1 [,
> value2 ...])
> At the moment, the only workaround for this would be to write a stored
> procedure to do the same operation, or to implement the logic client-side.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
