Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default search path for SQL*Plus does not match Oracle documentation #747

Closed
vectro opened this issue May 10, 2023 · 10 comments · Fixed by #760
Closed

Default search path for SQL*Plus does not match Oracle documentation #747

vectro opened this issue May 10, 2023 · 10 comments · Fixed by #760
Assignees
Labels

Comments

@vectro
Copy link
Contributor

vectro commented May 10, 2023

According to Oracle documentation,

The SQL*Plus executable is usually installed in $ORACLE_HOME/bin

However, we just look in $ORACLE_HOME:

file( ($ENV{ORACLE_HOME} || ()), 'sqlplus' )->stringify

I guess if this is changed then a change would also be required to docker-sqitch since that seems to work currently.

@theory
Copy link
Collaborator

theory commented May 10, 2023

Yeah if you just unzip the package there is no bin directory:

unzip instantclient-sqlplus-linux.x64-21.9.0.0.0dbru.zip 
Archive:  instantclient-sqlplus-linux.x64-21.9.0.0.0dbru.zip
  inflating: instantclient_21_9/glogin.sql  
  inflating: instantclient_21_9/libsqlplusic.so  
  inflating: instantclient_21_9/libsqlplus.so  
  inflating: instantclient_21_9/sqlplus  
  inflating: instantclient_21_9/SQLPLUS_LICENSE  
  inflating: instantclient_21_9/SQLPLUS_README  

@vectro
Copy link
Contributor Author

vectro commented May 10, 2023

I guess that Instant Client and ORACLE_HOME Client are different things, per https://www.oracle.com/database/technologies/faq-instant-client.html

Definitely not an expert here, but maybe it's possible that docker-sqitch should not be relying on ORACLE_HOME but rather just PATH?

@theory
Copy link
Collaborator

theory commented May 10, 2023

I would defer to people who use Oracle regularly, but TBH I've never seen a standard location for any of this stuff. Hence ORACLE_HOME. That FAQ is confusing af.

@theory
Copy link
Collaborator

theory commented May 20, 2023

Definitely not an expert here, but maybe it's possible that docker-sqitch should not be relying on ORACLE_HOME but rather just PATH?

Not sure how that'd change anything, unless someone is passing ORACLE_HOME to docker run, which seems silly.

@vectro
Copy link
Contributor Author

vectro commented May 31, 2023

Definitely not an expert here, but maybe it's possible that docker-sqitch should not be relying on ORACLE_HOME but rather just PATH?

Not sure how that'd change anything, unless someone is passing ORACLE_HOME to docker run, which seems silly.

Well, I guess that I meant is that if Sqitch were changed to look in the path recommended by Oracle for SQL*Plus ($ORACLE_HOME/bin), then the sqitch-oracle Docker image wouldn't be providing the right ORACLE_HOME.

@theory
Copy link
Collaborator

theory commented Jun 1, 2023

To change it, we'd have to:

  • In the Docker image, move the files output from instantclient-sqlplus-linux.x64-$version.zip into a bin subdirectory of ORACLE_HOME, rather than just in ORACLE_HOME itself.
  • Update paths to point to this location
  • Consider modifying oracle.pm to also look in the bin subdirectory of ORACLE_HOME

After which it would all work pretty much as it did before. I think in the absence of someone complaining that it's wrong and input from an experienced Oracle person declaring what's the right way to do it and what's not, I'm inclined to leave it as-is. In 10+ years I don't recall it being an issue.

@vectro
Copy link
Contributor Author

vectro commented Jun 1, 2023

@theory Or just set PATH to point to the directory where the sqlplus binary is located, and don't depend on ORACLE_HOME to find sqlplus in the Docker image?

The issue that I'm seeing is that in a full Oracle install (i.e., not the Sqitch docker image / not the instant client), Sqitch is not finding SQL*Plus because the sqlplus binary is in $ORACLE_HOME/bin.

@theory
Copy link
Collaborator

theory commented Jun 2, 2023

I think it would make sense to change oracle.pm to default to sqlplus in the path and, if it doesn't exist, to look in ORACLE_HOME and ORACLE_HOME/bin for it.

@vectro
Copy link
Contributor Author

vectro commented Jun 2, 2023

Seems reasonable!

@theory
Copy link
Collaborator

theory commented Jun 2, 2023

Something like this:

if ($ENV{ORACLE_HOME}) {
    my $bin = file $ENV{ORACLE_HOME}, 'bin', 'sqlplus';
    return $bin if -x $bin;
    $bin = file $ENV{ORACLE_HOME}, 'sqlplus';
    return $bin if -x $bin;
}
return file 'sqlplus';

theory added a commit that referenced this issue Jun 4, 2023
Some installations of SQL*Plus live in `$ORACLE_HOME/bin` rather than
just in `$ORACLE_HOME`. So refine `default_client()` to search for an
executable file in `$ORACLE_HOME/bin` and return that value if it
exists, then look for an executable file in `$ORACLE_HOME` and return it
if it exists. Otherwise simply return `sqlplus`. This changes the
previous behavior where it would always return `$ORACLE_HOME/sqlplus` if
`$ORACLE_HOME` is set. Now it will fall back on a path search if
`sqlplus` isn't in `ORACLE_HOME` even when `ORACLE_HOME` is set.
Resolves #747.
@theory theory self-assigned this Jun 4, 2023
@theory theory added engine patched Fixed in a branch but not yet merged. labels Jun 4, 2023
@theory theory closed this as completed in a814ddc Jun 24, 2023
@theory theory removed the patched Fixed in a branch but not yet merged. label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants