go-gorm / gorm

The fantastic ORM library for Golang, aims to be developer friendly
https://gorm.io
MIT License
36.81k stars 3.93k forks source link

Oracle Driver v2 POC #3156

Closed stevefan1999-personal closed 3 years ago

stevefan1999-personal commented 4 years ago

Your Question

Hello, just to let you know that I started converting the work in #2891 to some success. I have copypasted from the MySQL dialector plugin and also copied some code from sqlserver dialector. (the LIMIT clause translator is working exactly right)

So I need some help to convert the INSERT ... RETURNING ... INTO thingy, and I see no direct way of doing this from dialector point of view. Source code change in upstream codebase is a must.

Also I had some weird issue that uint8 as column field type is unsupported for some reason. This works in sqlite and that obviously mean something is wrong in the Oracle dialector. But I'm not sure why since the panic backtrace didn't tell me shit.

The POC driver repo is available at stevefan1999-personal/gorm-driver-oracle

My Oracle Autonomous DB is running the 19c version (and yes I'm running it off in a remote server with a wallet offered by Oracle...). Not sure if that had some compatibility issue to some people had for reproduction.

I wanted to argue on how do you design the boolean field because using 8-bit for a single boolean is obviously silly. Maybe we could use bitmap or to an extreme, a CHAR(1) with CHECK (<COLNAME> IN '0', '1')?

jinzhu commented 4 years ago

Hello @stevefan1999-personal

You can define a customized create func for Oracle driver, check out the SQL server one as example.

I wanted to argue on how do you design the boolean field because using 8-bit for a single boolean is obviously silly. Maybe we could use bitmap or to an extreme, a CHAR(1) with CHECK ( IN '0', '1')?

Which driver? I think I am not using a 8-bit for bool?

stevefan1999-personal commented 4 years ago

@jinzhu Hello I wonder how do I make a temporary variable to a binding variable that matches a certain field type? e.g. ID => uint, but it seems like I cannot make a NamedArg for that right?

The code I'm trying to address is here: https://github.com/stevefan1999-personal/gorm-driver-oracle/blob/e28ffbd5cd9b72e2b333c8603c0276f79185c4ac/create.go#L208-L217

jinzhu commented 4 years ago

Hi @stevefan1999-personal

I don't get why use NamedArg here, could you let me know the SQL that you want to generate when creating?

stevefan1999-personal commented 4 years ago

@jinzhu

ODPI [03804] 2020-07-17 19:56:34.939: SQL INSERT INTO users (created_at,updated_at,deleted_at,name,email,age,birthday,member_number) VALUES (:1,:2,:3,:4,:5,:6,:7,:8) RETURNING id INTO :9
time="2020-07-17T19:56:34+08:00" level=trace msg="gorm Trace" error="9. arg: unknown type sql.NamedArg" rows=0 sql="INSERT INTO users (created_at,updated_at,deleted_at,name,email,age,birthday,member_number) VALUES ('2020-07-17 19:56
:34.939','2020-07-17 19:56:34.939',NULL,'foo',NULL,10,NULL,NULL) RETURNING id INTO '{{} id <nil>}'"
time="2020-07-17T19:56:35+08:00" level=panic msg="9. arg: unknown type sql.NamedArg" unsupported type="unknown type sql.NamedArg"
stevefan1999-personal commented 4 years ago

@jinzhu Oracle DB uses RETURNING <var| ',' var> INTO <bind-var| ',' bind-var> for this (yes, it doesn't return a row, fucking bullshit right?), so I need to arbitrarily bind a value

stevefan1999-personal commented 4 years ago

@jinzhu of course the other way of doing this is select the bound values from DUAL which is the dummy reflective table and that will obviously return a row, but it honest I rather be parsing the bound values returned instead.

stevefan1999-personal commented 4 years ago

@jinzhu More details: https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpls/RETURNING-INTO-clause.html#GUID-38F735B9-1100-45AF-AE71-18FB74A899BE

stevefan1999-personal commented 4 years ago

alright, i think this is basically ok, it passed my own CRUD workflow. Next target: pass official test suite

stevefan1999-personal commented 4 years ago

@jinzhu if you or someone else would like to add the oracle dialect to the test drive files for me?

stevefan1999-personal commented 4 years ago

@jinzhu oh wait now i have some issue regarding multivalue inserts, that INSERT ALL INTO doesn't seems to support RETURNING INTO. What should I do in this case😅

stevefan1999-personal commented 4 years ago

I wonder if anyone can help me find out why the preloading doesn't work in my driver, thank you! It has something to do with reflection and I see that a certain field.Value(elem) doesn't actually return the keys for some reason. It's so weird but I had no clues on it.

Edit: got it, it is about column naming. I changed the namer so that it will capitalize all the time

stevefan1999-personal commented 4 years ago

@jinzhu would you like to follow up on a seeming bug here: https://github.com/go-gorm/gorm/blob/b4166d9515c3a86da2a1c7a695bc73d83861737d/schema/schema.go#L164

I have hacked it so that now the playground test passes, but I'm not sure if this is intended behavior either.

func (n Namer) ColumnName(table, column string) (name string) {
    // HACK: fix GORM not detecting primary key field due to it only recognizing lower-case "id" column name
    if column == "ID" {
        return "id"
    }
    return ConvertNonReservedWordToCap(n.NamingStrategy.ColumnName(table, column))
}
jinzhu commented 4 years ago

would you like to follow up on a seeming bug here:

Fixed that issue

stevefan1999-personal commented 4 years ago

@jinzhu Now I have to fix the row scanner, I want you to help figure out how to deal with the row column name mismatch problem, as this I can't really tell by words, so see for yourself: image Relevant source code: https://github.com/go-gorm/gorm/blob/53f8c9fc1c5d24324308673cc9ae3afd4442516a/scan.go#L102-L103 Relevant log and SQL code:

time="2020-08-30T22:46:56+08:00" level=trace msg="gorm Trace" error="<nil>" rows=3 sql="SELECT USERS.ID,USERS.CREATED_AT,USERS.UPDATED_AT,USERS.DELETED_AT,CreditCards.ID AS CreditCards__ID,CreditCards.CREATED_AT AS CreditCards__CREATED_AT,CreditCards.UPDATED_AT AS CreditCards__UPDATED_AT,CreditCards.DELETED_AT AS CreditCards__DELETED_AT,CreditCards.\"number\" AS CreditCards__number,CreditCards.USER_ID AS CreditCards__USER_ID FROM USERS LEFT JOIN CREDIT_CARDS CreditCards ON USERS.ID = CreditCards.USER_ID WHERE USERS.DELETED_AT IS NULL"

As we see here, the schema name is almost correct, but the result set column names are for some reason no matter what, capitalized, even if you set a selection alias. This means field reflection and thus table joins cannot work properly (it still can parse the ID, but the rest of the sub-structure is zeroed out).

PS: I actually copy pasted the example struct from GORM docs...

So for some brainstorm I think we can have more than 2 ways of dealing with this:

  1. replace all Schema.Relationships.Relations lookup by case insensitive check, this is the most convenient but also the slowest (linear search for each key so O(n))
  2. Hack https://github.com/go-gorm/gorm/blob/53f8c9fc1c5d24324308673cc9ae3afd4442516a/schema/relationship.go#L108 or the upper chain so that we can 1-1 match the relation name to the table schema, most dangerous because it breaks previous behaviors probably
  3. normalize both the column names and the keys of Schema.Relationships.Relations. Still O(n) initially but mapping Schema.Relationships.Relations can be done once so perhaps O(1) afterwards
  4. As stated in https://stackoverflow.com/a/41138144, we can turn the alias into an identifier (in Oracle's term), this however IIRC will also slate double quotes into the column names, so we need to sanitize that

sample for 1.

value := funk.Find(funk.Keys(Schema.Relationships.Relations), func(x string) bool {
    return strings.EqualFold(x, names[0])
})

if value != nil {
    rel := Schema.Relationships.Relations[value.(string)]
    ...

I did approach 4 because it can be directly hacked in my source code: https://github.com/stevefan1999-personal/gorm-driver-oracle/blob/da7a71b7196d38f0ba3f61c3377f9eb82ebde989/oracle.go#L131

I wonder if this is enough?

stevefan1999-personal commented 4 years ago

a little panic trace:

panic: reflect: call of reflect.Value.Field on slice Value

goroutine 1 [running]:
reflect.Value.Field(0x6c6a40, 0xc0000a19a8, 0x197, 0x0, 0xc000118aa8, 0xc000118aa8, 0x415436)
    C:/Go/src/reflect/value.go:834 +0x12a
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func6(0x6c6a40, 0xc0000a19a8, 0x197, 0xc0002a6d80, 0xc0002a6d68, 0xc0002a6d80)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:420 +0x84
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func11(0x6c6a40, 0xc0000a19a8, 0x197, 0x6cbb20, 0xc0002a6d80, 0xc0002a6d80, 0x725de0)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:592 +0x7ee
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func8(0x6c6a40, 0xc0000a19a8, 0x197, 0x6c4480, 0xc0002a6d68, 0xc0000a6580, 0x6c4480, 0xc0002a6d68)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:485 +0x25f
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func11(0x6c6a40, 0xc0000a19a8, 0x197, 0x6c4480, 0xc0002a6d68, 0xc0002a6d68, 0xc0002e6120)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:622 +0xe0
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func8(0x6c6a40, 0xc0000a19a8, 0x197, 0xc0002cc900, 0xc00009ca88, 0xc0000a6580, 0x0, 0x0)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:485 +0x25f
gorm.io/gorm/schema.(*Field).setupValuerAndSetter.func11(0x6c6a40, 0xc0000a19a8, 0x197, 0xc0002cc900, 0xc00009ca88, 0x197, 0x0)
    E:/Git/github.com/stevefan1999-personal/gorm/schema/field.go:622 +0xe0
gorm.io/gorm.Scan(0xc0002e2000, 0xc0002b7bf0, 0xc00000a000)
    E:/Git/github.com/stevefan1999-personal/gorm/scan.go:146 +0xc6d
gorm.io/gorm/callbacks.Query(0xc0002b7bf0)
    E:/Git/github.com/stevefan1999-personal/gorm/callbacks/query.go:34 +0x1e5
gorm.io/gorm.(*processor).Execute(0xc0000615c0, 0xc0002b7bf0)
    E:/Git/github.com/stevefan1999-personal/gorm/callbacks.go:101 +0x224
gorm.io/gorm.(*DB).Find(0xc0002b7bf0, 0x6bc300, 0xc0002b9da0, 0x0, 0x0, 0x0, 0x0)
    E:/Git/github.com/stevefan1999-personal/gorm/finisher_api.go:111 +0xbf
main.main()
    E:/go/oracle/main.go:110 +0xa69
type User struct {
    gorm.Model
    CreditCards []CreditCard
    Languages []*Language `gorm:"many2many:user_languages;"`
}

type CreditCard struct {
    gorm.Model
    Number string
    UserID uint32
}
type Language struct {
    gorm.Model
    Name string
    Users []*User `gorm:"many2many:user_languages;"`
}
...
    var users []User
    if err = db.Model(&User{}).Joins("CreditCards").Find(&users).Error; err != nil {
        logrus.Panic(err)
    }

    logrus.WithField("users", users).Info("got users")
...
stevefan1999-personal commented 4 years ago

@jinzhu I think you can also sign up to Oracle Cloud haha, I test and develop mostly on their always free autonomous database and it works well, and you can put the secret as a Runner credential. This way we do not need to wait 20 minutes for the Oracle XE database service to load.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days