zim32 / mysql.dart

MySQL client for Dart written in Dart
BSD 3-Clause "New" or "Revised" License
64 stars 17 forks source link

map SQL dates and times to DateTime #42

Closed fsw closed 1 year ago

fsw commented 1 year ago

I have tested this just for my simple use case. @zim32 I can add some more tests for column types in this PR if general approach in this change is OK. This change is not backward compatible (typedAssoc will return different types) fixes #12

zim32 commented 1 year ago

Hi fsw. ThI think because this library is still not in stable semver we can just bump minor version and write changelog.

I will check your PR in a few days

fsw commented 1 year ago

Yes, I guess minor bump would be OK.

I am not sure what to do with sql TIME type, but I guess this one should be passed as String and not converted to DateTime. I will fix this and add some tests.

zim32 commented 1 year ago

Yes, TIME should be just String

fsw commented 1 year ago

added some changes:

Few things I noticed while adding types mapping test:

zim32 commented 1 year ago

I think if we convert year to INT now, users will expect that numbers will be converted to numbers or at least to BigInt. So yeah it is logical to convert them to BigInt.

zim32 commented 1 year ago

I am merging this PR, but we need to implement BigInt mapping to be consistent.

zim32 commented 1 year ago

You changed something in the tests, so now they are not working.

dart test test/mysql_client.dart

00:00 +0 -1: loading test/mysql_client.dart [E]                                                                                                                           
  Failed to load "test/mysql_client.dart":
  /tmp/dart_test.TYGLEF/test.dart:9:42: Error: Undefined name 'main'.
        internalBootstrapVmTest(() => test.main, sendPort);
                                           ^^^^
00:00 +0 -1: Some tests failed.  
zim32 commented 1 year ago

Ah I see. You refactor and move it to separate files for TCP and Unix socket tests. Ok

fsw commented 1 year ago

@zim32 yes,

Maybe the file "test/mysql_client.dart" should be moved to "test/lib/mysql_client.dart" or something like this to make it more clear.

And yes the BigInt change can be separate PR.

thanks.