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 Jul 9, 2022
Conversation
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.
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. |
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.
66c33fc
into
PhilippSalvisberg:bugfix/various-fixes-pr-38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
Couple of proposed changes to the
PLSCOPE_IDENTIFIERSview:procedure_scopenot correctly taking into account forward-declared procedures/functions(This involves rewriting the hierarchical
treesubquery using recursive-with.)Other changes:
name_usage: name + type + usage (+ left indentation), in a single column for conveniently viewing the hierarchy, as a complement toname_pathis_fixed_context_id:YESif the value ofusage_context_idwas fixed in order to repair the broken hierarchy (for the current identifier), null otherwiseprocedure_signature: the signature of the current top-level procedure (as inprocedure_name,procedure_scope)ref_line,ref_col: line and column where the identifier was originally declared, in the object identified byref_owner,ref_object_type, andref_object_nameproc_ends_before_line,proc_ends_before_col: upper bound of the position in the source code where a top-level procedure endsDBA_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_DATAparameter (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,