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

Returned values are always []byte for mariadb with mysql driver #1386

Closed
kamalshkeir opened this issue Jan 19, 2023 · 20 comments
Closed

Returned values are always []byte for mariadb with mysql driver #1386

kamalshkeir opened this issue Jan 19, 2023 · 20 comments

Comments

@kamalshkeir
Copy link

Issue description

Scan rows into any should return the underline type if it used with the new fix using Prepared statements

Example code

stmt, err := db.Conn.Prepare(query)
if err != nil {
   return res, err
 }
 defer stmt.Close()
 
rows, err = stmt.Query(args...)

Error log

Error data

Configuration

Go version: 1.18

Server version: MariaDB 10.9

Server OS: , Windows 10

@methane
Copy link
Member

methane commented Jan 19, 2023

Please write a reproducible step to reproduce.

@kamalshkeir
Copy link
Author

kamalshkeir commented Jan 19, 2023

As explained, this is a reproducible step, you can create any table, and try to query it using mysql driver with Mariadb into any, same problem with this issue #407, sqlite driver doesn't behave like that, neither postgres

func getJSON(sqlString string) (string, error) {
  stmt, err := db.Conn.Prepare(query)
if err != nil {
   return res, err
 }
 defer stmt.Close()
  rows, err := stmt.Query()
  if err != nil {
      return "", err
  }
  defer rows.Close()
  columns, err := rows.Columns()
  if err != nil {
      return "", err
  }
  count := len(columns)
  tableData := make([]map[string]interface{}, 0)
  values := make([]interface{}, count)
  valuePtrs := make([]interface{}, count)
  for rows.Next() {
      for i := 0; i < count; i++ {
          valuePtrs[i] = &values[i]
      }
      rows.Scan(valuePtrs...)
      entry := make(map[string]interface{})
      for i, col := range columns {
          var v interface{}
          val := values[i]
          b, ok := val.([]byte)
          if ok {
              v = string(b)
          } else {
              v = val
          }
          entry[col] = v
      }
      tableData = append(tableData, entry)
  }
  jsonData, err := json.Marshal(tableData)
  if err != nil {
      return "", err
  }
  fmt.Println(string(jsonData))
  return string(jsonData), nil 
}
// so i am expecting a result for entry like this 
map[string]any{
    "id":1,
    "is_admin":true,
    "email":"[email protected]"
}

// but i got
map[string]any{
    "id":[]byte("1"),
    "is_admin":[]byte("true"),
    "email":[]byte("[email protected]")
}

@kamalshkeir
Copy link
Author

kamalshkeir commented Jan 19, 2023

so, i have tested everything again with mysql , maria, sqlite and postgres, here is the current behavior:
i didn't notice difference of behavior between mysql and maria, both when scanned to any interface, they return []byte if they are used without a Prepared statement, but also , even with Prepared statements, the time is returned as String not time.Time like postgres

@methane
Copy link
Member

methane commented Jan 19, 2023

I can not reproduce. I can not compile your code because it don't have main() function.
You don't provide schema and data. It is far from "reproducible".

Do not force maintainer to write any code by only guess. "reproducible" means no guess is needed to reproduce.
So I don't run your code. I don't know why binary protocol is not used here.

Anyway, driver is low level library. And driver doesn't know about output type.
It is database/sql design. So do not expect the driver return data type you expect.
It is user's responsibility.

If you don't want to manually convert data, use higher level library.

@methane methane closed this as completed Jan 19, 2023
@kamalshkeir
Copy link
Author

Using a higher level library to build an ORM in Go is not a very good idea for me at least.
You said driver doesn't know about output type , only mysql driver ?, only this implementation of mysql driver ?, because again, postgres and sqlite do know about output type, and without prepared statements required by this driver, and not handled for time.Time
For now, if i need to support mysql for my orm, i need to check if the dialect is mysql on each query, to make a prepared statement required only by it, it is indeed a very weird behavior when comparing it to other drivers

@methane
Copy link
Member

methane commented Jan 19, 2023

You said driver doesn't know about output type , only mysql driver ?,

No. All driver don't know output type. database/sql doesn't pass such information to driver.
MySQL driver just chose data type that have minimum allocation.
That's why output type is depending on protocol level format.

For now, if i need to support mysql for my orm, i need to check if the dialect is mysql on each query, to make a prepared statement required only by it, it is indeed a very weird behavior when comparing it to other drivers

No! We do not guarantee anything. Do not expect mysql driver return data type you expect.
If you are ORM developer, you must read the "database/sql" document first.
Especially, https://pkg.go.dev/database/[email protected]#Rows.ColumnTypes .

Then you know how to get suitable type dynamically.

@kamalshkeir
Copy link
Author

Amazing stuff, Thanks

@elgs
Copy link

elgs commented Mar 6, 2023

Then you know how to get suitable type dynamically.

I think this should be done by the driver, like how every other driver does.

@methane
Copy link
Member

methane commented Mar 6, 2023

You must not read the discussion.

@elgs
Copy link

elgs commented Mar 6, 2023

I have read. But the fact is all other drivers did it properly except yours. I have the feeling you are determined not to make any change. That's fine. We can do the mapping ourselves the hard way, but this is something I don't understand why you don't want to make it to the best.

@methane
Copy link
Member

methane commented Mar 6, 2023

All driver don't know output type is interface{} or not.
All driver just return most lightweight type, not do best for you.
It is the database/sql desigin.

@elgs
Copy link

elgs commented Mar 6, 2023

Can you please take a look at the code I shared in this issue ticket? #1401
The code is runnable. You can compare the output by using this driver and other drivers.

@methane
Copy link
Member

methane commented Mar 6, 2023

So what? I don't want to allow double allocation&convertion for everyone only for your use case.
There is a right way. It is Rows.ColumnTypes.

@elgs
Copy link

elgs commented Mar 6, 2023

Thanks for confirming it. It's ok that I will do the mapping on my end.

@a631807682
Copy link

a631807682 commented Mar 8, 2023

I have a problem related to Rows.ColumnTypes.
Can we handle ScanType of decimal better? I found that mysql will set the ScanType of the decimal type to sql.RawBytes. When we use the interface to receive data, we cannot determine its type.
This does not exist in other drivers. (go-sqlite3pgx)
https://github.com/go-sql-driver/mysql/blob/master/fields.go#L190

@methane
Copy link
Member

methane commented Mar 8, 2023

Which Go type is suitable for decimal?

@a631807682
Copy link

Which Go type is suitable for decimal?

It's a pity that no Go type is fully suitable, but it is still possible to support it using float64 or custom type.

pgx decimal
clickhouse decimal

@methane
Copy link
Member

methane commented Mar 8, 2023

If you need support for customization or non builtin types, you should just ignore ScanType.
You can just use custom mapping.

@a631807682
Copy link

If you need support for customization or non builtin types, you should just ignore ScanType. You can just use custom mapping.

We built an orm, which works well if the user decides the type to receive, but when the user receives using the interface, we only know the real type through ScanType.
We want to see if this behavior can be changed, as it is different from other drivers. If we can, we can avoid the difference between different drivers, if not, we can also remind the user.

@methane
Copy link
Member

methane commented Mar 8, 2023

we only know the real type through ScanType.

ColumnType provides not only ScanType, but also DatabaseTypeName. So you can use custom mapping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants