nakagami / firebirdsql

Firebird RDBMS sql driver for Go (golang)
MIT License
220 stars 60 forks source link

Result truncated on row with VARCHAR field using max length and non UTF8 charset #134

Closed tprouza closed 2 years ago

tprouza commented 2 years ago

example table contains 10 rows the NAME field is VARCHAR 30 with WIN1250 charset second row has 30 characters in the NAME field with one non ASCII char

` rows, errQ := db.Query("select ID, NAME from TABLE") if errQ != nil { t.Fatal(errQ) }

defer rows.Close()
var id, name string
for rows.Next() {
    errScan := rows.Scan(&id, &name)
    if errScan != nil {
        t.Fatal(errScan)
    }
    t.Log(id, name)
}

`

returns only first row, rest is droped no error is returned

bat22 commented 2 years ago

https://pkg.go.dev/database/sql#Rows.Next

Next prepares the next result row for reading with the Scan method. It returns true on success, or false if there is no next result row or an error happened while preparing it. Err should be consulted to distinguish between the two cases.

tprouza commented 2 years ago

Ok, my bad on error handling, it indeed returns error in rows.Err() on the second row: arithmetic exception, numeric overflow, or string truncation string right truncation expected length 30, actual 120

But the error is a bug. Same code, different golang driver returns all rows no problem.

jacobalberty commented 2 years ago

Does the error happen for you if you use v0.9.0 of the driver? I've been having a similar error message show up ever since v0.9.1. For now i've rolled back to v0.9.0 I've been trying to get together a test case for it but I believe #123 might have the same issue and they have a test case there, havent had a chance to try their test case agains 0.9.0 vs 0.9.1+ though

tprouza commented 2 years ago

The error does not occur with v0.9.0, It starts showing up with v0.9.1

tprouza commented 2 years ago

On problematic row, when I remove non ascii char (ý), or when I change it from ý to y, or when I shorten the string to 29 characters, the error does not occur. But when the original string is converted to UTF8 from WIN1250, it actually takes 31 bytes. Error always happens when there are more UTF8 bytes than defined varchar length of the column. So string in column of varchar(30) can have 25 characters, but if more than 5 of those are later converted to two-byte UTF8 chars, then the error occurs

bat22 commented 2 years ago

@tprouza is there error or not if you execute this query in isql?

tprouza commented 2 years ago

@tprouza is there error or not if you execute this query in isql?

No, there is not. All rows are returned as expected.

jacobalberty commented 2 years ago

I've narrowed down the source of the issue to a specific pull request. It appears #118 introduced the behavior, this can be confirmed by cloning this repo and using replace in your go.mod to use your local clone, then run git revert 48bc4d0 -m 1 to back out the pull request.

We need to come up with a decent test case to reproduce the issue still so that the fix for #117 that is in #118 can be correctly implemented.

xhit commented 2 years ago

Hi. PR author here.

Sorry for that.

@tprouza are you setting the correct charset of db in connection string?

The character length is 4 bytes by default.

For WIN1250 is 1 byte.

tprouza commented 2 years ago

@tprouza are you setting the correct charset of db in connection string?

The character length is 4 bytes by default.

For WIN1250 is 1 byte.

I am not setting charset in connection string. I want connection to be using UTF-8, which I supposed was default for charset param. WIN1250 is DB internal charset, which I do not have control over and I am now thinking it is not relevant. Server responses are encoded and returned in UTF8 correctly. It is just when returned field has e.g. max length 30, contains 30 chars but some characters are multibyte (UTF-8 has dynamic 1-4 bytes chars, right?) so number of bytes > column max length, only then errors ocurs.

jacobalberty commented 2 years ago

I'm testing with a database with NONE as the charset. What I don't get is if i pass charset=NONE into my connection string it absolutely should run the same as it did without #118 right? my connstring looks like user:pass@ip/test.fdb?auth_plugin_name=Legacy_Auth&charset=NONE but it still behaves exactly the same as user:pass@ip/test.fdb?auth_plugin_name=Legacy_Auth

Edit: nevermind figured it out i was putting my conn string for testing in a bash script and forgot to escape the &, setting charset does stop the error for me.

iyudincev commented 2 years ago

Experiencing the same problem with UTF8 charset. FB 3.0.1, nakagami/firebirdsql 0.9.3

DB schema:

SET SQL DIALECT 3;
SET NAMES UTF8;

CREATE DATABASE 'localhost:/var/lib/firebird/3.0/data/test.fdb' PAGE_SIZE 8192 DEFAULT CHARACTER SET UTF8 COLLATION UNICODE_CI_AI;

CREATE TABLE T (
    TEXT  VARCHAR(4) NOT NULL
);

CREATE TABLE T1 (
    TEXT  VARCHAR(4) NOT NULL
);

data:

$ isql-fb test.fdb 
Database: test.fdb, User: SYSDBA
SQL> select * from t;

TEXT             
================ 
café             

SQL> select * from t1;

TEXT             
================ 
cafe             

SQL>

Go code:

    db, err := sql.Open("firebirdsql", "SYSDBA:1@192.168.65.129//var/lib/firebird/3.0/data/test.fdb")
    if err != nil {
        log.Fatal(err)
    }
    defer db.Close()

    rows, err := db.Query("select text from t")
    if err != nil {
        log.Fatal(err)
    }
    defer rows.Close()

    for rows.Next() {
        var text string
        err = rows.Scan(&text)
        if err != nil {
            log.Fatal(err)
        }
        log.Print(text)
    }
    if err = rows.Err(); err != nil {
        log.Fatal(err)
    }

Output for the table T:

2022/02/10 17:02:51 Dynamic SQL Error
SQL error code = -303
arithmetic exception, numeric overflow, or string truncation
string right truncation
expected length 4, actual 16
exit status 1

Output for the table T1:

2022/02/10 17:04:32 cafe
nakagami commented 2 years ago

thanks @iyudincev @tprouza It's too late now, but I know it's happening.

nakagami commented 2 years ago

Fixed and tagged v0.9.4 Is it really fixed?

iyudincev commented 2 years ago

Is it really fixed?

Yes, queries run ok for both Unicode and one-byte codepages.

nakagami commented 2 years ago

Thanks!