[jira] Commented: (DERBY-2152) Support diagnostic vti tables that take parameters, such as SpaceTable

["A B (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DERBY-2152?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12462267 ] 

A B commented on DERBY-2152:
----------------------------

Attached d2152_vtiMappingCleanup_v1.patch, which does some reorganization of the code added for this issue and for DERBY-571 to make them more consistent with each other.  In particular, the patch addresses the following review comment from Dan:

> The lack of consistency between getVTIClassForTableFunction() and
> getVTIClassForTable() seems strange, though I can see why you did
> it that way. Once the code has been committed maybe some cleanup
> could be done, could be the old code to match the new code, or
> some other common ground. The differences are:
>
> - getVTIClassFor*() methods take different argument styles to
>   represent the same information
>
> - one resolves the vti class outside the NewInvocationNode, one inside.

In an attempt to resolve these differences, the patch does the following:

  1. Creates a new "init()" method for NewInvocationNode that is specifically targeted for mapping VTI "tables" and VTI "table functions" to their corresponding class names.  The new init() method takes both a TableDescriptor and a TableName, exactly one of which must be null.  If the TableDescriptor is null then we will resolve the TableName to a VTI "table function"; if the TableName is null then we will resolve the TableDescriptor to a VTI "table".  Thus VTI classes are now consistently resolved inside the NewInvocationNode class.

  2. Updates NodeFactory.mapTableAsVTI() to make use of the new "init()" method in NewInvocationNode.  mapTableAsVTI() used to take as a parameter the resolved VTI class name; now it just passes the received TableDescriptor to the new init() method in NewInvocationNode and the latter resolves the VTI class.

  3. Combines the getVTIClassForTable() and getVTIClassForTableFunction() methods of DataDictionary into a new method, "getVTIClass()", that takes a TableDescriptor and a boolean value.  The boolean indicates the kind of mapping to do--"table" or "table function"--while the TableDescriptor holds the schema and name information used for resolving the class.  This resolves the inconsistency between the argument styles used in the old getVTIClassFor*() methods.

I ran suites.All on Windows 2000 with ibm142 and there were no failures.  I also ran derbyall on Red Hat Linux with ibm142 and all was clean.  And as a sanity check I ran SysDiagVTIMappingTest against jdk16 (after building JDBC 4.0) and the test ran without problem.

Review comments/suggestions are appreciated, as always.

> Support diagnostic vti tables that take parameters, such as SpaceTable
> ----------------------------------------------------------------------
>
>                 Key: DERBY-2152
>                 URL: https://issues.apache.org/jira/browse/DERBY-2152
>             Project: Derby
>          Issue Type: Improvement
>          Components: SQL
>            Reporter: Daniel John Debrunner
>         Assigned To: A B
>         Attachments: d2152_engine_v1.patch, d2152_engine_v2.patch, d2152_junit_jdk16_fix.patch, d2152_testing_v1.patch, d2152_testing_v2.patch, d2152_v1.stat, d2152_v2.stat, d2152_vtiMappingCleanup_v1.patch, d2152_vtiMappingCleanup_v1.stat, junit-errors.txt
>
>
> Expand the work of DERBY-571 to support the remaining diagnostic tables that take parameters.
> Syntax would use the table constructor, like (not sure if an 'AS' clause will be required:
> select * from TABLE(SYSCS_DIAG.SPACE_TABLE(?, ?))
> Diagnostic VTIs that could be handled this way are:
> ErrorLogReader(String log file name)
> SpaceTable(String tableName)
> SpaceTable(String schemaName, String tableName)
> StatementDuration(String inputFileName)
> This is the second stage mentioned in DERBY-571

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira