Support for Oracle

[AlekSi]

4 steps:

1. Decide on the drivers. The two most popular choices are https://github.com/mattn/go-oci8 and https://github.com/rana/ora. Are we going to support both? Which one is easier to install and test?
2. Add Drone CI configuration with Docker images from https://github.com/oracle/docker-images.
3. Add dialect for Oracle.
4. Fix code and/or tests.

Some experience with Oracle and Go drivers for it is pretty much required.

[xpbliss]

https://github.com/mattn/go-oci8

[dddpaul]

Seems like driver from @mattn doesn't support LastInsertId, see https://github.com/mattn/go-oci8/issues/78. I've achieved to fetch LastInsertId by using second driver. It looks like this:

stmt, _ := db.Prepare("INSERT INTO person_project (project_id, person_id) VALUES (:1, :2) RETURNING person_id /*LastInsertId*/ INTO :3")
res, _ := stmt.Exec("traveler", 100, 0)
fmt.Println(res.LastInsertId())

So, you have to append RETURNING col /*LastInsertId*/ INTO :n to inserts and pass zero value to this :n placeholder. Is reform ready for such improvements?

[dddpaul]

There is another problem regarding Oracle Schema Object Names and Qualifiers:

• A quoted identifier begins and ends with double quotation marks ("). If you name a schema object using a quoted identifier, then you must use the double quotation marks whenever you refer to that object.
• Nonquoted identifiers are not case sensitive. Oracle interprets them as uppercase. Quoted identifiers are case sensitive.
• Nonquoted identifiers cannot be Oracle Database reserved words. Quoted identifiers can be reserved words, although this is not recommended.

For example, "employees", "Employees", "EMPLOYEES" are not the same. But employees, EMPLOYEES, "EMPLOYEES" are the same.

And here Reform beats you in a hard way, because it uses "start" as a column name in test data. And "start" is a reserved word in Oracle. So you cannot name your column as start unquoted, you must use "start".

But Reform cannot use quotes selectively, it will generate queries with "start", "email", "project", or start, email, project. And all of them will fail because of aforementioned Oracle schema name rules.

I see two ways to handle this:

1. Use only quoted identifiers for Oracle. The drawback - plenty of tests must be rewritten because of hardcode like "INSERT INTO people (id, name, created_at) ..." (unquoted colums here). How this will affect other databases compatibility - who knows.
2. Do not use quotes for Oracle at all. Rename "start" column to "start_time" (and "end" to "end_time" just in cases). Some tests must be rewritten (like "INSERT INTO projects (id, name, start) VALUES ...") but it will not break compatibility.

@AlekSi, I need a feedback from you for this matter.

[AlekSi]

I'm not familiar with Oracle at all (I guess not yet… I was unfamiliar with MS SQL too just recently), so I'm not sure I fully understand the current state of last ID problem. Looks like it works with go-oci8 now? In any case, we can extend LastInsertIdMethod with one more type just for Oracle. I think that RETURNING syntax is preferred if it works with non-integer primary keys.

[AlekSi]

But Reform cannot use quotes selectively, it will generate queries with "start", "email", "project", or start, email, project. And all of them will fail because of aforementioned Oracle schema name rules.

It always generates queries with "start", "email", "project" (at least it should). You mean if a column is named project, it can be accessed as project or "PROJECT", but not as "project"?

A second way is a no-go: it will not allow to use reform with columns named with reserved words. So let's go the first way. Yeah, tests should be changed, but you can change them to work with Oracle, I then change them to work for all systems.

[dddpaul]

@AlekSi, thanks for reply. I observe some problems here.

1. LastInsertId - solved.

mattn/go-oci8 driver do not suit for that because it implements returning of ROWID (not arbitrary ID field). It can be used as record ID but it shouldn't. For example, if you delete and reinsert a row with the Import and Export utilities, then its rowid may change. If you delete a row, then Oracle may reassign its rowid to a new row inserted later.

rana/oracle driver implements some workaround I've already described. I've add new LastInsertId method "ReturningInto" with some special treatment and it works fine.

2. Quoted columns - solved.

All identifiers are quoted now. Not email, EMAIL or "EMAIL" - just "email". Tests are rewritten from s.q.DeleteFrom(PersonTable, "WHERE email IS NULL") to s.q.DeleteFrom(PersonTable, "WHERE "+s.q.QuoteIdentifier("email")+" IS NULL").

3. Null and empty columns - not solved.

The root of the problem, I think, is that Oracle Database treats a character value with a length of zero as NULL. Current Oracle implementation internally changes empty string to NULL values.

This is a slightly "oraclized" people table (with nullable "email" and "name" columns):

CREATE TABLE "people" (
  "id" integer GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY NOT NULL,
  "group_id" integer DEFAULT 65534,
  "name" varchar2(40),
  "email" varchar2(40),
  "created_at" timestamp with time zone NOT NULL,
  "updated_at" timestamp with time zone
);

It's being filled with non-null and null email:

INSERT INTO "people" ("id", "name", "email", "created_at") VALUES (102, 'Elfrieda Abbott', '[email protected]', TO_TIMESTAMP_TZ('2014-01-01 00:00:00 00:00', 'yyyy-mm-dd hh24:mi:ss tzh:tzm'));
INSERT INTO "people" ("id", "name", "email", "created_at") VALUES (103, 'Elfrieda Abbott', NULL, TO_TIMESTAMP_TZ('2014-01-01 00:00:00 00:00', 'yyyy-mm-dd hh24:mi:ss tzh:tzm'));

And this test will fail:

func (s *ReformSuite) TestFindAllFrom() {
    structs, err := s.q.FindAllFrom(PersonTable, "name", "Elfrieda Abbott")
    s.NoError(err)
    s.Len(structs, 2)
    s.Equal([]reform.Struct{
        &Person{ID: 102, GroupID: pointer.ToInt32(65534), Name: "Elfrieda Abbott", Email: pointer.ToString("[email protected]"), CreatedAt: personCreated},
        &Person{ID: 103, GroupID: pointer.ToInt32(65534), Name: "Elfrieda Abbott", CreatedAt: personCreated},
    }, structs)

Seems like driver returns empty string on NULL "email" value:

- (*models.Person)(0xc420017e50)(ID: 102 (int32), GroupID: 65534 (*int32), Name: `Elfrieda Abbott` (string), Email: `[email protected]` (*string), CreatedAt: 2014-01-01 00:00:00 +0000 UTC (time.Time), UpdatedAt: <nil> (*time.Time)),
- (*models.Person)(0xc420017ea0)(ID: 103 (int32), GroupID: 65534 (*int32), Name: `Elfrieda Abbott` (string), Email: <nil> (*string), CreatedAt: 2014-01-01 00:00:00 +0000 UTC (time.Time), UpdatedAt: <nil> (*time.Time))
+ (*models.Person)(0xc420017d60)(ID: 102 (int32), GroupID: 65534 (*int32), Name: `Elfrieda Abbott` (string), Email: `[email protected]` (*string), CreatedAt: 2014-01-01 00:00:00 +0000 UTC (time.Time), UpdatedAt: <nil> (*time.Time)),
+ (*models.Person)(0xc420017db0)(ID: 103 (int32), GroupID: 65534 (*int32), Name: `Elfrieda Abbott` (string), Email: `` (*string), CreatedAt: 2014-01-01 00:00:00 +0000 UTC (time.Time), UpdatedAt: <nil> (*time.Time))

To be more precise, driver returns NullString or empty string, it's depends on receiver type, see https://github.com/rana/ora/blob/master/defString.go:

func (def *defString) value(offset int) (value interface{}, err error) {
    if def.nullInds[offset] < 0 {
        if def.isNullable {
            return String{IsNull: true}, nil
        }
        return "", nil
    }

There is some discussion and corresponding commits to return nil in this case. So I've just replace return "", nil with return nil, nil. Then I've got this:

--- FAIL: TestInsert (0.02s)
    base_test.go:131: >>> BEGIN
    base_test.go:131: <<< BEGIN 82.476µs
    base_test.go:131: >>> INSERT INTO "people" ("group_id", "name", "email", "created_at", "updated_at") VALUES (:1, :2, :3, :4, :5) RETURNING "id" /*LastInsertId*/ INTO :6 [<nil> (*int32), `` (string), `[email protected]` (*string), 2016-09-06 08:01:23 +0000 UTC (time.Time), <nil> (*time.Time), 0 (int)]
    base_test.go:131: <<< INSERT INTO "people" ("group_id", "name", "email", "created_at", "updated_at") VALUES (:1, :2, :3, :4, :5) RETURNING "id" /*LastInsertId*/ INTO :6 [<nil> (*int32), `` (string), `[email protected]` (*string), 2016-09-06 08:01:23 +0000 UTC (time.Time), <nil> (*time.Time), 0 (int)] 3.084333ms
    base_test.go:131: >>> SELECT "people"."id", "people"."group_id", "people"."name", "people"."email", "people"."created_at", "people"."updated_at" FROM "people" WHERE "people"."id" = :1 AND ROWNUM = 1 [5 (int32)]
    base_test.go:131: <<< SELECT "people"."id", "people"."group_id", "people"."name", "people"."email", "people"."created_at", "people"."updated_at" FROM "people" WHERE "people"."id" = :1 AND ROWNUM = 1 [5 (int32)] 540.537µs
    assertions.go:225: 

    Error Trace:    querier_commands_test.go:27

    Error:      Received unexpected error "sql: Scan error on column index 2: unsupported Scan, storing driver.Value type <nil> into type *string"

So, as far as I can understand this driver forces to use NullXYZ types for proper NULL handling. Any ideas how we can cope with that?

[AlekSi]

mattn/go-oci8 driver do not suit for that because it implements returning of ROWID (not arbitrary ID field).

Absolutely agree, we should not use ROWID as PK.

Are you using RETURNING clause instead of sql.Result.LastInsertId()?

[AlekSi]

2. Great. Also you may want to use fmt.Sprintf() instead of + when there are many columns.

[AlekSi]

Oracle Database treats a character value with a length of zero as NULL. Current Oracle implementation internally changes empty string to NULL values.

That's nuts. I will look into this.

[AlekSi]

In the meantime:

• have you looked into adding Oracle to Travis CI?
• what is the easiest way to install Oracle to investigate 3?
• may you publish you code?

[dddpaul]

I didn't examine Travis CI integration yet. Right now I use this docker-compose.yml:

oracle12c:
  image: sath89/oracle-12c
  ports: ['1521:1521']

Code is published in https://github.com/dddpaul/reform/tree/oracle.

[dddpaul]

Forget to mention. To run tests locally you also need sqlplus binary which will be available after Oracle Instant Client install.

[dddpaul]

One more :) To compile both Go drivers against Oracle Instant Client you need to put oci8.pc to directory included in PKG_CONFIG_PATH environment variable. Or simple to /usr/lib/pkgconfig.

Links:

• Making connections to the Oracle Database from Golang - details for Linux
• Accessing an Oracle DB in Go - details for MacOS

[AlekSi]

@dddpaul We moved to self-hosted Drone CI (#63), so testing with Oracle in Docker is now easier. May you please create a pull request?

[dddpaul]

@AlekSi I've achived some success with running builds on Travis for another Go & Oracle project. But then I've stumbled with the real show-stopper for Oracle integration. The point is that Oracle drivers require binary Oracle client dynamically. So application user will have to install this Oracle client to run application even if he doesn't need to work with Oracle database at all.

I did not figure out how to deal with that. Maybe distribute multiple binaries or use custom build rules for "go get ...". Do you have any clues about this?

[AlekSi]

How about build tags?

[dddpaul]

I think it's reasonable to suggest that Oracle client is installed if ORACLE_HOME environment variable is set. So if this variable is set then build Oracle-enabled reform binary. If not then build binary without Oracle support. I didn't find how to implement this logic with build tags.

[AlekSi]

I understand correctly there is no pure Go driver for Oracle?

Neither reform library package, nor reform command depends on drivers. Even dialects are driver-independent. reform-db does import drivers, but we can import them only if some build tag is specified. Tags is the only way to configure build if we want to keep using go get (and we want it). For example, we can do the following for sqlite3 which also requires cgo and may cause problems:

// +build sqlite3

package main

import (
	_ "github.com/mattn/go-sqlite3"
)

So if user needs sqlite3, he/she should install it with go get -u -tags sqlite3 gopkg.in/reform.v1/reform-db.

We can do the same for Oracle.

[AlekSi]

We now have Docker images from Oracle: https://github.com/oracle/docker-images