PhilippSalvisberg / plscope-utils

Utilities for PL/Scope in Oracle Database
Apache License 2.0
35 stars 17 forks source link

The "Compile with PL/Scope" action could use DBA_xxx views if available #56

Closed rvo-cs closed 1 year ago

rvo-cs commented 1 year ago

As of v1.0.0, the PL/SQL code of the "Compile with PL/Scope" action always uses ALL_xxx views.

If using the action with DBA privileges, I'd expect DBA_xxx views to be used, yet SQL Developer does not automatically changes references to ALL_xxx views into references to matching DBA_xxx views in the code of actions, as it does in the queries of editors.

In general, when the DBA role is enabled, ALL_xxx views should return the same results as matching DBA_xxx views [^all_vs_dba]. Yet exceptions to that principle—bugs?—do exist [^all_dependencies], and because of these it might be safer to use DBA_xxx views if possible. [^dba_views_simplicity]

Remark: this is more of a general design principle, for actions intended to be used by users with the DBA role, than an actual issue, as far as v1.0.0 is concerned.

On the other hand, the "Compile with PL/Scope" action should still work for users without SELECT / READ privileges on required DBA_xxx views, by falling back gracefully on matching ALL_xxx views. A simple solution for this would consist in adding a prompt for choosing between using DBA_xxx or ALL_xxx views, with the default choice adequately pre-defined by counting how many of the required DBA_xxx views are listed in ALL_VIEWS. In practice, the user would see the prompt (I'm not aware of an option for an invisible prompt), and could change it if she wishes to, but in practice that would not be necessary.

[^all_vs_dba]: ALL_xxx views are documented using the phrase "describes xxx accessible to the current user", which means that xxx is listed if and only if the current user has enough permissions on it. Therefore, ALL_xxx views include security access checks in their WHERE-clause conditions, and in general system privileges (EXECUTE ANY..., CREATE ANY...) granted through the DBA role are taken into account in these checks.

[^all_dependencies]: Per Oracle 12.2, even with the DBA role enabled, ALL_DEPENDENCIES may omit some rows that DBA_DEPENDENCIES returns. And in 19c, the catprc.sql file (under $ORACLE_HOME/rdbms/admin) mentions the following fix in its heading comments: "Bug 25096570: include Table and Synonym dependencies in ALL_DEPENDENCIES".

[^dba_views_simplicity]: Querying DBA_xxx views guarantees: (1) that what is returned is subject to far simpler security access checks than when querying matching ALL_xxx views; consequently: (2) DBA_xxx views are inherently immune from bugs in the security access checks that may (or may not, depending on version) be present in matching ALL_xxx views.

rvo-cs commented 1 year ago

Remark: this was mostly needed if enabling the "Compile with PL/Scope" action to be used on schemas different from the schema of the session user (issue #55).

PhilippSalvisberg commented 1 year ago

In general, when the DBA role is enabled, ALL_xxx views should return the same results as matching DBA_xxx views. Yet exceptions to that principle—bugs?—do exist, and because of these it might be safer to use DBA_xxx views if possible.

The reason that SQL Developer changes sys.all_ to Dba_ in editors is performance. The all_ views are slower than their dba_ counterparts. It makes the editors unnecessary sluggish when the user has access to the dba_ views. This is an old trick a lot of IDEs are using. This is especially helpful when the performance expectations are in the subseconds.

So in principle I agree that it it would be nice if SQL Developer would apply this logic also in actions. However, I think using simple string replacement functionality is dangerous for queries such in editors but especially in code such in actions. So I can understand why SQL Developer is not doing that. So this leaves the task to us. Anyway, for the Compile with PL/Scope... action the expectations are not in subseconds. Using dba_ instead of all_ views is not necessary from a performance point of view. It's okay when the tasks takes some seconds longer.

So performance is no reason.

How about wrong results by using all_ instead of dba_ views? You mentioned "Bug 25096570: include Table and Synonym dependencies in ALL_DEPENDENCIES". I had a quick look on MOS but could not find it (I wanted to check the status and in which version it is fixed). It's probably not public. Of course there are cases where we have to work around a bug, but in general I think Oracle should fix it and we should install a patch.

Regarding your remark that this is required in combination of your enhancement request #55. I repeat what I've written there: I do not plan to extend the scope of this extension to cover DBA functionalities or extend the scope to multiple schemas.

So, I'm going to reject this enhancement request.