[
https://issues.apache.org/jira/browse/DERBY-1520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12483260
]
A B commented on DERBY-1520:
----------------------------
Hi Laura,
Thank you for addressing my first set of comments--the second version of the
new page is looking a lot better. I know it's late, but I finally got around
to doing a more extensive review of the latest changes (which you've already
committed) and I have the following suggestions.
Is it possible for you to address these in a follow-up patch?
----
#1)
-- Suggested change:
Reword
There are two types of table expressions in Derby:
to be
There are two kinds of diagnostic table expressions in Derby:
-- Why?
First, the word "type" often has specific connotations in the context of SQL,
so I think replacing it with "kind" is a cosmetic but still useful change.
More importantly, the original makes it sound like there are only two kinds
of table expressions in *all* of Derby and that both are related to
diagnostics--but of course that's not true. There are a variety of table
expressions available to a user and most of them have nothing to do with the
built-in diagnostic expressions. The latter are simply specialized table
expressions with specific built-in functionality.
#2)
-- Suggested change:
Reword
The diagnostic table functions require one or more arguments.
to be
Diagnostic table functions can accept zero or more arguments,
depending on the table function in question.
-- Why?
It is not true that all table functions require one or more arguments. For
example, as the document itself says later on, the following is valid:
SELECT *
FROM TABLE (SYSCS_DIAG.ERROR_LOG_READER())
AS T1
As we can see, the table function in this example does not have an argument.
So it is incorrect to say that one or more arguments is required. Instead, the
unifying characteristic of the table functions is that they all *can* accept
one or more arguments--but it's not a requirement.
#3)
-- Suggested change:
Reword
The following table shows the types and names of the Derby table
expressions.
to be
The following table shows the types and names of the Derby diagnostic
table expressions.
i.e. add the word "diagnostic".
-- Why?
I think it's clearer to include the word "diagnostic" for reasons similar to
those outlined in #1 above: namely, the sentence as it is currently makes it
sound like we're talking about *all* of Derby, but that is not the case.
#4)
-- Suggested change:
Reword
Table 1. System table expressions provided by Derby
to be
Table 1. System diagnostic table expressions provided by Derby
i.e. add the word "diagnostic".
-- Why?
Without the word "diagnostic" it sounds like we're listing *all* of the
available system table expressions in Derby, but that's not true. There are
other non-diagnostic system table expressions--esp. system tables in the SYS
schema--that are not listed here (and should not be).
#5)
-- Suggested change:
I wonder if it would be better to either separate the table into two
different tables (one for "tables", one for "table functions") or else make the
table two-columned as follows:
Diagnostic table expression | Table or Table Function
----------------------------------------------------------
SYSCS_DIAG.ERROR_MESSAGES | table
... ...
SYSCS_DIAG.ERROR_LOG_READER | table function
-- Why?
I realize it's a bit picky, but with the two-column table as it is right now,
I think it gives the impression that there is some kind of correlation between
the "table" on the left and the "table function" on the right (esp. given the
way the names line up), but that's not actually the case.
#6)
-- Suggested change:
Reword
To use SYSCS_DIAG.ERROR_LOG_READER diagnostic table function, you
specify the name of the function in the table function syntax.
to be
To access the SYSCS_DIAG.ERROR_LOG_READER diagnostic table function,
you must use the SQL table function syntax.
-- Why?
Just personal opinion: the phrasing of "specify the name of the function in
the table function syntax" sounds awkward to me. Note that a similar change
would be good for all other table functions, as well...
#7)
-- Suggested change:
Reword
To specify a log file name, the file name must ...
to be
You can specify a log file name by passing it in as an argument to
the SYSCS_DIAG.ERROR_LOG_READER table function. When specifying such
an argument, the file name must ...
-- Why?
I think it is a bit clearer to explicitly mention that the log file name is
an (optional) argument to the table function...
#8)
-- Suggested change:
Delete
Use single quotation marks when you specify the log file name to ensure
that the argument is read as string, otherwise a syntax error is thrown.
-- Why?
String literals are just one example of "an expression whose data type maps
to Java string." String literals always have single quotation marks around
them. In the example you give you are specifying a string literal, so the
single quotations are required. But if you were to specify some other,
non-literal expression, then the single quotes may not be necessary. Thus it
is not correct to say that the quotation marks are required.
I thought about rewriting the sentence so that it only applies to string
literals, but in the end my feeling is that the use of single quotes is implied
for string literals--and is emphasized by the example itself. So I don't think
we really need to include this sentence.
#9)
-- Suggested change:
Reword
You can use the SYSCS_DIAG.SPACE_TABLE diagnostics table function to
determine
if space might be saved by compressing the table and indexes.
to be
You can use this diagnostic table function to determine if space might be
saved by compressing the table and indexes.
i.e. change "SYSCS_DIAG.SPACE_TABLE diagnostics" with "this diagnostic".
-- Why?
Just being picky: the above sentence is the second of three sentences in a
row that explicitly spell out "SYSCS_DIAG.SPACE_TABLE". That seems rather
awkward to me..
#10)
-- Suggested change:
Same as #6 above, except for SYSCS_DIAG.SPACE_TABLE.
#11)
-- Suggested change:
Same as #8 above, except for SYSCS_DIAG.SPACE_TABLE.
#12)
-- Suggested change:
Remove the first example in the "SYSCS_DIAG.SAPCE_TABLE" section.
-- Why?
The example is not entirely correct (it throws an error) and does not show
anything more than the second example does. I think it's best to delete the
first example, describe the possible arguments (see #13 below), mention that
they (the arguments) must be expressions whose data types map to Java strings,
and *then* give the second example.
#13)
-- Suggested change:
Instead of the first example in the "SYSCS_DIAG.SPACE_TABLE" section I think
it would be good to mention that there there are two variations of this method:
1. Single argument: the argument specifies the table name.
2. Two arguments: first argument specifies the schema name, the
second argument specifies the table name.
Then, after mentioning that the arguments must be expressions whose data
types map to Java strings, we should give an example of each (one of those
examples would be the second example that is already there).
-- Why?
It is not currently clear that this table function can be called with one
*or* two arguments. There is one sentence saying "If you do not specify the
schemaName, the current schema is is used", but I don't think that in itself is
clear enough. I think this needs to be made explicit.
#14)
-- Suggested change:
Reword
You can use the SYSCS_DIAG.STATEMENT_DURATION diagnostic table function
to get a indication of where the bottlenecks are in the JDBC code for
an application.
to be
You can also use this diagnostic table function to get an indication of
where the bottlenecks are in the JDBC code for an application.
i.e.:
a - add the word "also" after "you can".
b - replace SYSCS_DIAG.STATEMENT_DURATION with "this" (similar to #9 above)
c - change "a" to "an" before the word "indication".
-- Why?
Fix some typos, plus argument for #9 above.
#15)
-- Suggested change:
Same as #6 above, except for SYSCS_DIAG.STATEMENT_DURATION.
#16)
-- Suggested change:
Same as #7 above, except for SYSCS_DIAG.STATEMENT_DURATION.
#17)
-- Suggested change:
Same as #8 above, except for SYSCS_DIAG.STATEMENT_DURATION.
----
Thanks a ton for your patience and thoroughness here...
> Document new SYSCS_DIAG tables
> ------------------------------
>
> Key: DERBY-1520
> URL: https://issues.apache.org/jira/browse/DERBY-1520
> Project: Derby
> Issue Type: Sub-task
> Components: Documentation
> Affects Versions: 10.2.1.6
> Reporter: Stan Bradbury
> Assigned To: Laura Stewart
> Attachments: derby1520_1.diff, derby1520_2.diff, derby1520_3.diff,
> derby1520_4.diff, refderby.ditamap, rrefsyscsdiagtables.html,
> rrefsyscsdiagtables.html
>
>
> See comments for DERBY-571 for initial documentation discussion. The new
> tables (mapped to the old Diagnostic VTIs) are:
> The old style syntax will remain in place for 10.2, but become deprecated.
> The tables to be implemented in this change are:
> SYSCS_DIAG.LOCK_TABLE replaces org.apache.derby.diag.LockTable
> SYSCS_DIAG.STATEMENT_CACHE replaces org.apache.derby.diag.StatementCache
> SYSCS_DIAG.TRANSACTION_TABLE replaces org.apache.derby.diag.TransactionTable
> SYSCS_DIAG.ERROR_MESSAGES replaces org.apache.derby.diag.ErrorMessages
> The information about the tables can be found in the javadoc for the class
> listed above.
> That can be found at:
> http://db.apache.org/derby/javadoc/engine/
> click on the org.apache.derby.diag link in the Packages table, then select
> each class, e.g. LockTable to see the info.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
