oracle support

[deusaquilus]

Fixes #34

Checklist

• Unit test all changes
• Update README.md if applicable
• Add [WIP] to the pull request title if it's work in progress
• Squash commits that aren't meaningful changes
• Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

[fwbrasil]

[deusaquilus]

Heh, don't get exited yet. This is only build stuff.

[fwbrasil]

well... build stuff is the hardest part of this :)

[deusaquilus]

Okay, I'm switching over to use sath89/oracle-12c due to a lack of any other tractable solution.

• If we choose to go with the official oracle image, we need to get dockerhub credentials and call docker-login
• In order to call docker-login, we need to have the username/password as ENV vars or repo encrypted files (info here)
• Neither environment variables, nor encrypted keys work for 'untrusted' PRs which is travis's way of saying any non-maintainer PR (info here).

So all in all, we have two choices:

1. Use a completely public dockerhub account i.e. store the credentials in our source tree. This is bad for multiple reasons including the fact that anyone could go straight into our dockerhub account and push malware.
2. Use a user-created oracle docker account.

I have chosen to do the latter as it solves multiple problems. If Oracle ever decides to actually allow non-logged users to downoad their 'official image' I'll gladly switch back. Alternatively, if docker-hub somehow starts allowing for logins to use api-keys we can go that route too.

For now, I will continue developing the tests on to of this existing build.

[deusaquilus]

Here's an update for anyone interested.
A few days ago I've discovered that Oracle's concat || operator is not ansi compliant that is to say null || "suffix" is "suffix" in Oracle. Since Quill currently relies on the "null fall through" behavior of concatonation operators. When I say "null fall through", I mean the following:

We translate x+"suffix" into the sql x || "sufix" and rely on the fact that if x is null, the whole statement will be null. Take for instance the map operator:

case class Holder(value:Option[String])
query[Holder].map(h => h.value || "suffix")

Will become:

select h.value || suffix from Holder h

With all the databases we have seen so far, the h.value being null will result in null being selected which the Option decoder will turn into None. However with Oracle, the above statement will never be null.

Even outside of Oracle however, this behavior is still broken. Say we have a statement like this:

case class Holder(value:Option[String])
run(query[Holder].map(h => 
  h.value.map(v => if(v == "something") "foo" else "bar")).getOrElse("baz"))
)

This will turn into the following SQL:

SELECT CASE WHEN 
  CASE WHEN h.value = 'something' THEN 'foo' ELSE 'bar' END IS NOT NULL THEN 
  CASE WHEN h.value = 'something' THEN 'foo' ELSE 'bar' END 
  ELSE 'baz' 
END FROM Holder h

Note, how in this case, when value is null, there is no fall through and the value will 'bar' as opposed to 'baz'. This I issue has been outlined in #1053 which I will now attempt to solve prior to continuing the Oracle module. Otherwise Quill's behavior with Oracle will be wildly inconsistent.

[deusaquilus]

Also, I need to note that the Option[T].exists has the same problem with Oracle concatonation. Take something like this:

run(query[Holder].map { holder =>
    holder.value.exists(v => (v+"foo" == "foo"))
}) 

Which will become this:

SELECT (holder.value || 'foo') = 'foo' FROM Holder holder

Since Oracle's concatonation operator || will yield 'foo" when holder.value is null, the above statement will be true when holder.value is null as opposed to false. There needs to be an explicit null check in this operator as well.

[deusaquilus]

In fact, Option[T].forall is broken as well. Take this statement:

run(query[Holder].map { holder =>
  holder.value.forall(v => (v+"notfoo" == "foo"))
}) 

Which becomes:

SELECT holder.value IS NULL OR (holder.value || 'notfoo') = 'foo' FROM Holder holder

Now when holder.value is null, the value will become 'notfoo' which will then be compared to 'foo' and result is false. Whereas, based on the standard Scala behavior, we would expect the statement to yield true.

[deusaquilus]

The Option[T].contains operator however I have not been able to break. Let's say we have something like this:

run(query[Holder].map { holder =>
  if (holder.value.contains("")) "foo" else "bar"
})

Turns into this:

SELECT CASE WHEN holder.value = '' THEN 'foo' ELSE 'bar' END FROM Holder holder

Now this will correctly yeild 'bar' when holder.value is null.

If '' is null is done on the other hand, the Oracle dialect will yield true (and this is not an issue I will address on Quill unless there is are explicit requests since it is very, very difficult to catch in all situations).

[deusaquilus]

Now since #1302 is merged, this work can continue.

[deusaquilus]

It looks like Oracle are being their usual selves, and started issuing takedown DMCA notices for unofficial containers on docker-hub, despite the fact that "allowed for development purposes" is clearly stated in their license agreement (it's like they don't actually want developers to be able to support their DB in their products!).

The Slick people have already been affected by this issue slick/slick#1993 (the original issue here: wnameless/docker-oracle-xe-11g#118). Currently I am using the medgetablelevvel/oracle-12c-base image which is based on sath89/oracle-12c which has also been taken down. I expect medgetablelevvel/oracle-12c-base image will be taken down very soon as well.
Of couse, we can't actually use the official oracle image oracle-database-enterprise-edition because:

• The official Oracle container image requires user credentials. Oracle apparantly wants anyone using their dockerhub image to give them an email and home-address.
• Credentials in environment variables or encrypted files don't work with external user pull-requests (CircleCI has the same problem). This decision was made by Travis and CircleCI because in order to access credentails stored in environment variables, a malicious attacker merely would need to echo the environment variable to get that information.
• Due to this, any build request from a quill-committer would not be able to run with the Oracle docker image unless we publicly expose our user dockerhub user credentials.

The way I see it, if we want to use the official Oracle image, we can go one of two ways:

1. Leave our dockerhub credentials publicly exposed - This means that anyone can do anything they want with our dockerhub account including upload malicious files or even lock us out. If we go this direction, we might as well share our dockerhub credentails in CONTRIBUTING.md and openly state that we take no responsibility for what anyone might do with them.

2. Exclude Oracle builds and tests from external PRs - This would mean if we want to test changes to the Oracle modules before they are comitted, we would have to clone PRs as internal Quill branches and build them outselves to make sure the changes work. Maybe if we could find some way to automatically clone PRs as internal branches the headache associated with this could be mitigated.

Flavio, Quill maintainers, and anyone else. Please let me know your thoughts. I know @ghisvail and @jilen commented on #34. We need some discussion here on the best way to move forward.

@fwbrasil
@getquill/maintainers

[fwbrasil]

I think it's reasonable to use the official message if we disable the oracle tests for PRs and add instructions on CONTIBUTING.md on how to run the tests locally. We can even change the PR template to add a note about it.

We might end up merging something that breaks oracle but it should be fine. We'd just need to revert the PR.

[deusaquilus]

Okay. I'm fine with that. It seems like someone from Oracle is looking into this DMCA issue (wnameless/docker-oracle-xe-11g#118 (comment)) so I'll wait for him to reply before making major structural changes to this build as it is still working for now. If needed I can also make the change in a future PR.

[deusaquilus]

Okay. This is ready for review.

[fwbrasil]

it seems that quill-jdbc-monix has the same code. Would it be possible to reuse?

[fwbrasil]

besides the code duplication on build.sbt LGTM. Feel free to merge :) Thank you for this!

[deusaquilus]

Awesome! I'll resolve the conflicts and merge.