pharo-rdbms / pharo-mysql Goto Github PK

MySQLDriver>>connect: fails in MySQLHelper class>>bytesFromSha1Hash: due to MNU of #bytesCount.

Needs changing to #digitLength.

Would be great if MySQLDriverSpec defaulted the port to the usual MySQL port.

Also, processing result sets for reports is awkward.

MySQLBindParameter>>detectParamType handles Floats (and Fractions) as follows:

((paramValue isKindOf: Float) or:  [paramValue isKindOf: Fraction]) 
		ifTrue: [	
			(paramValue asFloat at: 2) = 0 
				ifTrue: [^ MySQLTypes typeFLOAT]
				ifFalse: [^ MySQLTypes typeDOUBLE] ].

Firstly I'm not sure this is a valid test for floats versus doubles. For example, 4.25 is detected as typeFLOAT whereas 4.2 is detected as typeDOUBLE.

More generally, I think the approach of detecting the parameter type based on the value is incorrect. Test case (where c is a connected MySQLDriver):

c query: 'drop table if exists float_test'.
c query: 'create table float_test (f float)'.
r := c prepare: 'insert into float_test(f) values(?);'.
s := (MySQLDriverStatement onConnection: c) stmtId: r prepareOkay stmtHandlerId; cursoredFetch: false; yourself.
s addBinding: 4.2.
s execute.

This fails at the execute stage with an Out of Range error, since 4.2 is detected as typeDOUBLE, but is being assigned to a FLOAT parameter.

Fundamentally, I think it's flawed to try to deduce the parameter type based on the value being assigned; the type should come from the column definition for the parameter (I don't know if MySQL provides an easy way to do this however).

As a partial aside, running the above test with 4.25 (which is picked up as typeFLOAT) results in a returned value of 2.26562 - this looks to be due to an issue with MySQLBindParameter>>floatBytes which (I'm guessing) is due to a change in the internal representation of Floats in Pharo at some point in the past.

Running the test with a DOUBLE column also results in incorrect results even when using a typeDOUBLE value:

c query: 'drop table if exists double_test'.
c query: 'create table double_test (d double)'.
r := c prepare: 'insert into double_test(d) values(?);'.
s := (MySQLDriverStatement onConnection: c) stmtId: r prepareOkay stmtHandlerId; cursoredFetch: false; yourself.
s addBinding: 4.2.
s execute.
(c query: 'select * from double_test') rows first at: 1.   " '-9.255965342982487e61' "

This issue can be fixed by reversing the order in which the paramValue's bytes are considered in MySQLBindParameter>>doubleBytes:

doubleBytes
	| storable |
	ByteArray
		streamContents: [ :strm | 
			storable := paramValue asFloat at: 2.
			strm
				nextPut: (storable byteAt: 1);
				nextPut: (storable byteAt: 2);
				nextPut: (storable byteAt: 3);
				nextPut: (storable byteAt: 4).
			storable := paramValue asFloat at: 1.
			strm
				nextPut: (storable byteAt: 1);
				nextPut: (storable byteAt: 2);
				nextPut: (storable byteAt: 3);
				nextPut: (storable byteAt: 4).
			^ strm contents ]

All tests carried out with 64bit Pharo 7.0 on OSX, connecting to MySQL (actually MariaDB) on Raspberry Pi 4.

Clone code from pharo-db

Encoding of ScaledDecimal in MySQLBindParameter>>decimalBytes drops first digit of parameter value. Test case (where c is a connected MySQLDriver):

c query: 'drop table if exists decimal_test'.
c query: 'create table decimal_test (d decimal(10,2))'.
r := c prepare: 'insert into decimal_test(d) values(?)'.
s := (MySQLDriverStatement onConnection: c) stmtId: r prepareOkay stmtHandlerId; cursoredFetch: false; yourself.
s addBinding: 123.45s2.
s execute.
(c query: 'select * from decimal_test') rows first at: 1  " '23.45' "

Issue can be resolved as follows:

decimalBytes
	"For scaled decimal"
	ByteArray streamContents: [:strm |
		MySQLHelper encodeLcs: (paramValue abs printString allButLast:2) asByteArray on: strm.
		^ strm contents]

Sending nextRow to a MySQLResultsSet gives a MNU error on the send of removeFirst due to the result of MySQLCommandQuery>>readRowData being an Array. Test case (where c is a connected MySQLDriver):

c query: 'drop table if exists rows_test'.
c query: 'create table rows_test (i integer)'.
r := c prepare: 'insert into rows_test(i) values(?)'.
s := (MySQLDriverStatement onConnection: c) stmtId: r prepareOkay stmtHandlerId; cursoredFetch: false; yourself.
1 to: 5 do: [ :i | s addBinding: i; execute].
results := c query: 'select * from rows_test'.
results nextRow "ERROR: Instance of Array did not understand #removeFirst"

Could be fixed by modifying MySQLCommandQuery>>readRowData to return an OrderedCollection, or having nextRow stream over the results.

Environment:

• Windows 10 64 bit
• Pharo 9
• Pharo-MySQL installed with:
  Metacello new
  repository: 'github://pharo-rdbms/Pharo-MySQL';
  baseline: 'MySQL';
  load

(Note that package 'MySQL-Glorp' did not load, installed it manually.)

• Glorp installed with:
  Metacello new
  repository: 'github://pharo-rdbms/glorp';
  baseline: 'Glorp';
  load.

Evaluated:
login := Login new
database: MySQLPlatform new;
username: 'root';
password: '';
host: '';
port: '3306';
databaseName: 'test'.

accessor := DatabaseAccessor forLogin: login.
accessor login.
accessor isLoggedIn. >>> true

session := GlorpBookDescriptorSystem sessionForLogin: login. *GlorpBookDescriptorSystem is subclass of DescriptorSystem
session login. >>> Instance of MySQLDriver did not understand #queryEncoding

MySQLField contains references to an undeclared shared variable TypeConverters:

MySQLField class>>initializeTypeConverters
MySQLField>>typedValueFrom:

The former is sent during class initialization and brings up a debugger during load in Pharo 12. The latter method is unreferenced.

The unit test MySQLStatementReadTest>>testReadTimeTypes was previously running correctly in GMT timezone but now errors following the move to daylight saving time.

The cause looks to be the mapping of DateAndTime to/from DATETIME values. When converting to DATETIME the DateAndTime representation in the local timezone is used; this implicitly includes the local offset (the DATETIME representation does not include an offset). When converting back to a DateAndTime in MySQLBinaryReader>>dateTimeFrom: a zero offset is applied resulting in a different DateAndTime.

Note that MySQLStatementReadTest>>testReadTimeTypes only fails on the first attempt; subsequent runs succeed due to the test permanently changing the image's localTimeZone to TimeZone default (UTC).

Suggest changing MySQLBinaryReader>>dateTimeFrom: to apply the localTimeZone's offset instead of zero.