nakagami / firebirdsql

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

char(1) vs varchar(1) #164

Closed McFris closed 4 months ago

McFris commented 4 months ago

Firebird 4.0 In version 0.9.7 compared to version 0.9.4 when returning a field in char(1) in WIN1251 encoding, and connecting without specifying the encoding (Unicode by default) returns a broken character. If return varchar(1) then everything is ok. Example stored proc:

create procedure CHAR_VARCHAR
returns (
    TEXT_CHAR char(1),
    TEXT_VARCHAR varchar(1))
as
begin
   TEXT_CHAR = 'А'; --in 0.9.7 returns wrong char. In 0.9.4 correct char
   TEXT_VARCHAR = 'А'; --correct char in 0.9.7 and 0.9.4
   suspend;
end
nakagami commented 4 months ago

Something seems wrong here https://github.com/nakagami/firebirdsql/blob/master/xsqlvar.go#L400-L411 that was fixed in #117. Not sure how to fix it yet.

nakagami commented 4 months ago

Can you run go test in your environment?

Does this test fail in your environment? https://github.com/nakagami/firebirdsql/pull/165

It passes in my environment and seems to work correctly. Is it only for stored procedures that incorrect values are returned?

I would appreciate it if you could send me a pull request, even if it's just an additional test code that FAILs.

McFris commented 4 months ago

Can you run go test in your environment?

Yeah, a little later. But for now I can show in my version of the test (using sqlx directly in the project):

a := Aaa{}
_ = connn.Get(&a, "select cast('А' as char(1)) achar, cast('А' as varchar(1)) avar from rdb$database")

a.AcharHex = []byte(a.Achar)
a.AvarHex = []byte(a.Avar)

log.Info(fmt.Sprintf("Achar is: '%s', hex: %v", a.Achar, a.AcharHex))
log.Info(fmt.Sprintf(" Avar is: '%s', hex: %v", a.Avar, a.AvarHex))

and result for diff version: 0.9.4: Achar is: 'А ', len: 4, hex: [208 144 32 32] Avar is: 'А', len: 2, hex: [208 144]

0.9.7: Achar is: '\xd0', hex: [208] Avar is: 'А', hex: [208 144]

nakagami commented 4 months ago

So the upper letter is Cyrillic A! https://en.wikipedia.org/wiki/A_(Cyrillic)

nakagami commented 4 months ago

Slightly modified test data. https://github.com/nakagami/firebirdsql/commit/29e3f7a4f3519045c8c89d2fdf26c7cb9da14724

But this would result in another error.

sysdba:masterkey@localhost:3050/tmp/test_issue164_48272ab79cb2c00edb7ad5bf13106d8e.fdb?charset=WIN1251
--- FAIL: TestGoIssue164 (0.21s)
    driver_test.go:1136:
            Error Trace:    /home/nakagami/firebirdsql/driver_test.go:1136
            Error:          Received unexpected error:
                            arithmetic exception, numeric overflow, or string truncation
                            string right truncation
                            expected length 1, actual 2
            Test:           TestGoIssue164

I think char(2) is correct as follows, is it the same in your environment?

query := `CREATE TABLE t (text CHAR(2))`
McFris commented 4 months ago

So the upper letter is Cyrillic A!

I apologize for not clarifying this earlier, I thought I mentioned it, but.... :(

I think char(2) is correct as follows, is it the same in your environment?

rows, err := conn.Query("select cast('Б' as char(1)) from rdb$database")
require.NoError(t, err)

var text string
require.True(t, rows.Next())
require.NoError(t, rows.Scan(&text))
assert.Equal(t, "Б", text)

Error Trace: c:\Users\user\go\pkg\mod\github.com\nakagami\firebirdsql@v0.9.7\driver_test.go:1136 Error: Should be true

nakagami commented 4 months ago

Thank you.

The fix in issue #117 assumed the same number of bytes per character. However, it did not work with character sets that had a mix of 1 and 2 bytes per character.

The behavior has changed, but the char(n) type has been modified to remove the spaces to the right of string.

This should prevent strings from being corrupted. Is it fixed? https://github.com/nakagami/firebirdsql/pull/165

McFris commented 4 months ago

Is it fixed?

Yes, thank you! Achar is: 'Б', hex: [208 145] Avar is: 'Б', hex: [208 145]

One more question, is there way to implement trim(char) in any text fields? As an option? Now absolutely all fields have to be trimmed manually. Or is this also an error and should not be so?

nakagami commented 4 months ago

With this fix, in the char(n) field for data that already exists, the spaces on the right side are trimmed. I am trying to merge this fix into master for v0.9.8. The spaces on the left side will not be trimmed. Does this answer answer your question?

I have the same behavior in the Python driver. https://github.com/nakagami/pyfirebirdsql/blob/master/firebirdsql/xsqlvar.py#L157

nakagami commented 4 months ago

merge and v0.9.8 tagged

McFris commented 4 months ago

Example (0.9.8): select cast('Б ' as char(5)) achar, trim(cast('Б ' as char(5))) atchar, <<<<! cast('Б ' as varchar(5)) avar, trim(cast('Б ' as varchar(5))) atvar

result: Achar is: 'Б', hex: [208 145] Atchar is: 'Б ', hex: [208 145 32 32] <<<<! Avar is: 'Б', hex: [208 145] Atvar is: 'Б', hex: [208 145]

nakagami commented 4 months ago

Hmmm, it doesn't seem to happen in my environment. If you can reproduce it, please send me a separate issue with test code to reproduce it.

McFris commented 4 months ago

Thanks, close this bug, I will create a new one.