Null support for inserts

oerdogmus commented 1 year ago

Client version: v0.312.0

I have a use case where I insert multiple records with different fields are being nullable. For example, my table schema is as follows:

id: int
name: string
order: int (nullable)
url: string (nullable)

order and url fields are nullable so when I have 2 records:

(1, "first record", nil, "url") (2, "second record", 1, nil)

I expected to execute the following SQL query (with arguments):

INSERT INTO my_table (id, name, order, url)
VALUES
(1, 'first record', NULL, 'url'),
(2, 'second record', 1, NULL);

However, when ExecContext function is called with driver.NamedValue that has nil as a value, it throws exception in type conversion function here.

To support nil values, we can change the type conversion for nil to:

case nil:
    return "NULL", nil

I am curious what your thoughts are. If this is not a desirable behaviour, is there another workaround for the problem?

mosabua commented 1 year ago

I think this is a bug and your suggestion a potential bug fix. Could you maybe send a pull request that fixes this and also test the fix?

Also wdyt @nineinchnick and @losipiuk ?

losipiuk commented 1 year ago

Yeah - it looks like a bug to me. We currently do not have any DML test coverage for go client - so there is a chance that there are other bugs in code, similar to this one. @oerdogmus if you are up for a PR would you also be able to add some test coverage for DML in trino_test.go?

@nineinchnick does proposed fix look ok to you?

nineinchnick commented 1 year ago

Yes, it looks good. We definitely need more tests.

trinodb / trino-go-client

Null support for inserts #84