Skip to content

plscope_identifiers: fix procedure_scope in case of forward decl. + other fixes/chg#38

Merged
PhilippSalvisberg merged 11 commits intoPhilippSalvisberg:bugfix/various-fixes-pr-38from
rvo-cs:rvocs_d
Jul 9, 2022
Merged

plscope_identifiers: fix procedure_scope in case of forward decl. + other fixes/chg#38
PhilippSalvisberg merged 11 commits intoPhilippSalvisberg:bugfix/various-fixes-pr-38from
rvo-cs:rvocs_d

Conversation

@rvo-cs
Copy link
Copy Markdown
Contributor

@rvo-cs rvo-cs commented Jul 4, 2022

Hi,

Couple of proposed changes to the PLSCOPE_IDENTIFIERS view:

(This involves rewriting the hierarchical tree subquery using recursive-with.)

Other changes:

  • Added columns:
    • name_usage: name + type + usage (+ left indentation), in a single column for conveniently viewing the hierarchy, as a complement to name_path
    • is_fixed_context_id: YES if the value of usage_context_id was fixed in order to repair the broken hierarchy (for the current identifier), null otherwise
    • procedure_signature: the signature of the current top-level procedure (as in procedure_name, procedure_scope)
    • ref_line, ref_col: line and column where the identifier was originally declared, in the object identified by ref_owner, ref_object_type, and ref_object_name
    • proc_ends_before_line, proc_ends_before_col: upper bound of the position in the source code where a top-level procedure ends
  • Added the following condition to filters against DBA_SOURCE, DBA_IDENTIFIERS, DBA_STATEMENTS: origin_con_id = sys_context('USERENV', 'CON_ID')

Unsure about the last one (commit: 06655d4) : perhaps it would be better to leave that to the new CONTAINER_DATA parameter (MOS Doc. id 2745111.1), but on the other hand not every DB has that patch.

In any case, the above are just suggestions; feel free to pick any or none of them, as you see fit.

Regards,

rvo-cs added 10 commits June 28, 2022 00:51
This refactors the PLSCOPE_IDENTIFIERS view in order to correctly
determine the procedure_scope of private forward-declared procedures
in package bodies. The recursive subquery "tree" is refactored using
a recursive-with hierarchical query.

This also adds the following columns:
    . name_usage: name + type + usage (+ left indentation)
        in a single column, for conveniently viewing the hierarchy
    . is_fixed_context_id: YES if the value of usage_context_id
        was fixed in order to repair the broken hierarchy, NO
        otherwise
    . procedure_signature: the signature of the current top-level
        procedure, as in procedure_name, procedure_scope
    . ref_line, ref_col: line and column where the identifier was
        originally declared, in the object identified by ref_owner,
        ref_object_type, and ref_object_name
This limits the search for identifiers, statements, and source code
to the current container, by adding the following condition to the
corresponding sub-queries:

    origin_con_id = sys_context('USERENV', 'CON_ID')

This may make the query faster in multitenant environments, by querying
only the partitions for the current container, as opposed to all the
partitions when this condition is not used.
This fixes issue PhilippSalvisberg#34:

    ORA-01489: result of string concatenation is too long

by ensuring that the length of name_path remains <= 4000 bytes,
replacing as many levels as necessary by .../ starting from
level 4, so the first 3 levels are always kept, and the tailmost
levels should be preserved too.
These 2 additional columns might help in cases when it would
be useful to known on which line of code a given top-level
function/procedure ends.
plscope_identifiers: in the tree_plus subquery, ensure that
the is_new_proc column applies only to functions or procedures.

(This amends commit 5bf7d93.)
In 12.2, upon creating the dependent view plscope_tab_usage
the following exception would be raised:

  ORA-00600: internal error code, arguments: [17090], [], ...

Joining directly with dba_statements, and removing the stmt
materialized subquery as a result, fixes it.

Also coerced the procedure_scope and is_fixed_context_id columns
back into varchar2.
Reason: that column is cool and "nice-to-have", but...

1) It takes an additional outer join with the dba_statements view,
   which could be costly, and further that join cannot be eliminated
   even if that column is not used in the end: that join would always
   be there.

2) End-users may easily add that join if and when they need it.

To summarize, it's non-core functionality, and potentially costly,
so we'd better not have it in the core view.

(The same could be said of the "text" column BTW, which requires a
rather costly join with dba_source... But that column was there before,
so we'll keep it for now.)
Reason: required for pull-request.

I'd argue that EXECUTE is not the best choice of a pseudo-usage
name for SQL statements--we're not executing anything, this is
static code analysis, and all about usages of identifiers/SQL
statements... But it's been EXECUTE so far, and that change is
not ready to be merged yet.
@PhilippSalvisberg PhilippSalvisberg changed the base branch from main to bugfix/various-fixes-pr-38 July 5, 2022 05:53
@PhilippSalvisberg
Copy link
Copy Markdown
Owner

Thank you @rvo-cs . This is really helpful! I changed the base branch since I plan to merge it into this branch. This will simplify testing and applying small changes before I merge it into the main branch.

@PhilippSalvisberg PhilippSalvisberg self-requested a review July 5, 2022 05:58
@PhilippSalvisberg PhilippSalvisberg self-assigned this Jul 5, 2022
This limits the length of the name_usage column to at most 250
characters, wrapping the value to the left if necessary. Without
this there remained a small possibility to hit the ORA-01489
"result of string concatenation is too long" exception when
computing that value, at least in principle.
@PhilippSalvisberg PhilippSalvisberg merged commit 66c33fc into PhilippSalvisberg:bugfix/various-fixes-pr-38 Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants