search

PhilippSalvisberg / plscope-utils

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

Minor enhancement/fix to the installation script #46

Closed rvo-cs closed 2 years ago

rvo-cs commented 2 years ago

Hi Philipp,

Here is a pull request for a small enhancement, and a small fix, to the installation script of the core database objects:

  1. Minor fix: SQL*Plus 18.5 (*) would not parse the C-style comment embedded in the PLSCOPE_NAMING view create statement correctly, resulting in that view not being created.

    (*) Tested under Windows 7; SQL*Plus from the Instant Client 18.5 bundle for Windows x64.

    Of course the installation runs fine under SQLcl, and SQL Developer, and (most likely) other versions of SQL*Plus; this may even be platform-specific. Nevertheless, if one version/platform is affected, more might be, so...

    The fix consists in using a SQL*Plus substitution variable; it's not terribly elegant, but it does the trick without sacrificing readability too much (in my opinion) for those reading it directly from the source file.

  2. Minor enhancement; have the install.sql script fail if started as SYSDBA, or if the current schema is SYS or SYSTEM (for completeness). Because the utils/user/plscope.sql script must be run as SYSDBA first, it's extremely easy to omit switching to the PLSCOPE account, and call the install.sql script while being still connected as SYSDBA. And being a single command away from a disaster, even a small one, is unpleasant. Whereas putting a safeguard in the installation script is not difficult.

    Note that, in order to have the script fail, the whenever sqlerror directive must be used. So far, that directive was not present in any of the installation scripts, so the installation would run with whatever setting was effective in the client. But now we need to change it, and therefore we have to explicitly decide how to use it throughout the entire installation procedure. If it was only for SQL*Plus, or even SQLcl, we could take the more sophisticated approach of using the STORE SET command to save the current settings, change the whenever sqlerror directive, then restore it later as it was initially. Unfortunately, the STORE SET command has never been ported to SQL Developer, so if we use it the behaviour of the installation procedure will be different in SQL Developer as compared to SQL*Plus or SQLcl, which doesn't look good at all. So we should keep things very simple, and just decide on what the whenever sqlerror directive should be during the installation.

    In this pull-request, I have set it to the default SQL*Plus behaviour, whenever sqlerror continue none (except during the above-mentioned sanity check), but you might have a different opinion about that, of course.

Best regards,

PhilippSalvisberg commented 2 years ago

Thank you, @rvo-cs for this PR. I've opened #47 to handle it. I will merge this PR and then apply some cosmetic changes.