[
http://issues.apache.org/jira/browse/DERBY-717?page=comments#action_12412758 ]
Øystein Grøvlen commented on DERBY-717:
---------------------------------------
Good start, but you have not found all places in the documentation
that needs to be changed. I suggest doing the following Google
search, and go through each hit:
"updatable site:db.apache.org/derby/docs/dev"
I looked through the first hits and see my comments below for these.
You should probably do the same for "scroll" and for "FOR UPDATE" too.
Detailed comments:
Dev. guide:
--------------------
Scrolling insensitive ResultSets:
- "The materialized rows _may_ be backed to disk". I suggest
_will_.
- Is it possible to tune the amount of memory used by
scroll-insensitive Result Sets?
- I think this page should say that both read-only and updatable
are possible.
- "all rows _become_ materialized". I suggest _will be_
Updatable cursors and result set:
- I feel that the doc is not consistent with respect to definition
of cursors and result sets. The first page on result sets says:
"A cursor provides you with the ability to step through and
process the rows in a ResultSet one by one."
You say:
"Scrollable updatable result sets are based on cursors which
can both scroll and update rows."
This seems like opposite views or may be circular definition.
Is cursors based on result sets or result sets based on cursors?
Generally, it is not clear to me if you have any rules for when
to talk about result sets and when to talk about cursors.
- Spelling: "postioned"
- It would be a good with a reference to a definition of
"positioned updates and deletes"
- I am not sure everybody will understand the meaning of
"ResultSet.updateXXX() + ResultSet.updateRow()". Is this common
notation in the docs? I suggest you come up with another way to
express this. If you disagree, at least write the '+' in text
font and not code font.
- "Both scrollable and forward only cursors _can_ be updatable". I
suggest _may_. Why do you choose to talk about cursors here and
not result sets? I would suggest to focus on result sets and
only use cursors where you need to talk about SQL cursors (e.g.,
WHERE CURRENT OF).
- "query producing the cursor" sounds a bit strange to me. I also
wonder whether one should use "statement" instead of "query"
- It would be good with a reference to somewhere in the manual
where it is described how to handle warnings like the one
described here.
- "Query compilation fails if the query is not updatable, and
contains a FOR UPDATE clause." Is the comma correct here?
- A reference to the definition of "update lock" would be good.
- "if you need to actually update the rows". I suggest "if you
actually need to update the rows".
- "which can both scroll and update rows". Sounds a bit strange
to me.
Forward only updatable result sets
- Headings in the Dev. Guide are not consistent with respect to
"ResultSets" vs. "result sets"
- "cursors which can move forward and update rows". Would you
call it a cursor if could not move forward? :-)
- "On cursor held on commit" sounds strange. I suggest "cursors
held open after a commit". This phrase is used in the "Holdable
cursors" chapter. Or maybe you should use result set instead of
cursor here. Here you talk about operations on cursors, what
about operations on result sets?
Holdable cursors
- For scrollable result sets, I do not think it is true that
next() and close() are the only valid operations after a commit.
Inserting rows with updatable result sets
- "qualifies to the query condition". How about "satifies the
query predicate"
- "after the current row". Is it obvious what this means? What
defines the ordering here?
- ",i.e if the result set is not fully populated." I would drop
", i.e.". I also wonder whether the meaning of "is not fully
populated" will be meaningful to the reader.
Scrollable updatable result sets
- The term "scrollable" is not used in the manual prior to your
changes. I think that indicates that a job is needed to make
the terminology consistent. Either, you need to keep on using
the old terminology, or you need to update the old text to use
"scrollable".
- I think the introduction to the deleteRow example should say
that it deletes the 5th from last row.
- Visibility, 2nd bullet. I suggest making this note part of the
1st bullet.
- Visibility, 3rd bullet. I suggest "Changes caused by other
statements and triggers within the same transaction are considered
as other changes, and are not visible in scroll insensitive
result sets." I am not sure that you need to mention changes by
other transaction since that should be obvious. If you think
that is needed, I suggest adding another statement for that.
- Visibility, 4th bullet. "it has been on the last row" sounds a
bit strange. I also think you need to define "populated"
- Visibility, last bullet. "_may_ return true if ..." Is this not
deterministic?
- "after they were _populated_ to the result set". I would say it
is a result set that is populated, not the rows. Or can someone
be populated to a city? :-)
- "columns being changed _will_ be overwritten". I suggest "_may_
be overwritten"
- It would be good to give the SQLSTATE for the warning you get
when a row has been deleted.
- I still don't like "held over a commit"
- "If you use cursors held across commits,". I suggest "For a
cursor held open after a commit"
- "held cursors". I think it should be "holdable cursors"
Extended updatable cursor example
- Comment says auto-commit needs to be off. The section
"ResultSets and auto-commit" says the opposite. The section
"Using auto-commit" also says updatable cursors do not work with
auto-commit on. At least one of those must be wrong.
- I think this example should be reworked to use updatable
ResultSet instead of "WHERE CURRENT OF". It would also be good
if it used ResultSet.CONCUR_UPDATABLE instead of FOR UPDATE.
Update locks
- Is FOR UPDATE still required to get update locks?
Reference guide:
--------------------
For UPDATE clause:
- "ResultSet" => "JDBC ResultSet"
SELECT statement:
- Suggest rephrasing third paragraph to:
"In order to get an updatable JDBC ResultSet, you do not need
to include a FOR UPDATE clause. Instead, you can specify
concurrency mode ResultSet.CONCUR_UPDATABLE when creating the
statement."
- Suggest putting the heading "Requirements for updatable cursors
..." above the preceding paragraph.
- Do we need to specify the whole truth here? Maybe it would be
better to focus on the use of CONCUR_UPDATABLE for ResultSets
and mention FOR UPDATE only in the context of SQL cursors?
- "Cursors are read-only by default." I suggest moving this to
the start of the third paragraph.
SQLState and error message reference
- 01J03 "Scroll sensitive and scroll insensitive updatable
ResultSets are not currently implemented." Hopefully, this
error text has been changed.
java.sql.Connection
- "you can only request an updatable ResultSet that has a
TYPE_FORWARD_ONLY scrolling type" !!!
Admin. guide:
--------------------
Updatable Result Sets:
- Any changes to what is supported?
Differences between the embedded client and the network client driver
- Is it still true that scrollable does not support LOBs?
Tools guide:
--------------------
Get Scroll Insensitive Cursor
- "Scroll insensitive cursors are not updatable."
> update doc on updatable cursors
> -------------------------------
>
> Key: DERBY-717
> URL: http://issues.apache.org/jira/browse/DERBY-717
> Project: Derby
> Type: Improvement
> Components: Documentation
> Versions: 10.2.0.0
> Reporter: Andreas Korneliussen
> Assignee: Andreas Korneliussen
> Priority: Minor
> Attachments: DERBY-717.diff, DERBY-717.stat, DERBY-717v2.diff,
> DERBY-717v3.diff, DERBY-717v3.stat, derby-717.tar.gz, derby-717v3.tar.gz
>
> The new features introduced in DERBY-690, DERBY-231, DERBY-775 and DERBY-100
> should be documented in the Derby developer guide.
> This includes:
> * updatable cursors do longer need to "FOR UPDATE" clause
> * Scrollable insensitive resultsets can be updatable
> * ResultSet.insertRow()
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira