snowflakedb / gosnowflake

Go Snowflake Driver
Apache License 2.0
302 stars 125 forks source link

[SNOW-1669514] Honor the Valuer/Stringer methods to resolve #1209 #1211

Open ChronosMasterOfAllTime opened 2 months ago

ChronosMasterOfAllTime commented 2 months ago

Description

SNOW-1669514 - Solve the issue with UUID getting double quoted due to new array function #1209

This adds the ability to check the valuer/stringer interfaces exists

Checklist

Proof test fails.

export SKIP_SETUP=true && go test -run ^TestValueToString$ github.com/snowflakedb/gosnowflake
--- FAIL: TestValueToString (0.00s)
    --- FAIL: TestValueToString/UUID_-_should_return_string (0.00s)
        assert_test.go:103: expected "json" to be empty, but was not. . Thrown from gosnowflake/converter_test.go:290 +0xd4
            testing.tRunner(0xc0000bf520, 0x100e3a478)
        assert_test.go:103: expected ""1414b0f8-e56a-4e14-995b-774d87ea9d73"" to be equal to "1414b0f8-e56a-4e14-995b-774d87ea9d73" but was not. . Thrown from gosnowflake/converter_test.go:291 +0x197
            testing.tRunner(0xc0000bf520, 0x100e3a478)
ChronosMasterOfAllTime commented 6 days ago

@sfc-gh-pfus added tests to structured R/W and Driver. Will attempt to run the tests

sfc-gh-pfus commented 5 days ago

@ChronosMasterOfAllTime good job! But correct me if I'm wrong, but as far as I can see, there are only tests for embedding uuid in structured types and not as inserts into/selects from a regular column? Can you add such a test case as well (probably somewhere in driver_test.go)?

ChronosMasterOfAllTime commented 5 days ago

@ChronosMasterOfAllTime good job! But correct me if I'm wrong, but as far as I can see, there are only tests for embedding uuid in structured types and not as inserts into/selects from a regular column? Can you add such a test case as well (probably somewhere in driver_test.go)?

There is a test added in driver_test.go

ChronosMasterOfAllTime commented 5 days ago

@sfc-gh-pfus need support on this one: (I was running structured_write_test.go from the master branch first to ensure it worked correctly)

Getting the following error:

--- FAIL: TestBindingObjectWithSchema (0.67s)
    driver_test.go:276: error on exec context [[query too large to print]]: 002040 (42601): SQL compilation error:
        Unsupported data type 'OBJECT(s VARCHAR(16777216), b NUMBER(38,0), i16 NUMBER(38,0), i32 NUMBER(38,0), i64 NUMBER(38,0), f32 FLOAT, f64 FLOAT, nfraction NUMBER(38,9), bo BOOLEAN, bi BINARY(8388608), date DATE, time TIME(9), ltz TIMESTAMP_LTZ(9), ntz TIMESTAMP_NTZ(9), tz TIMESTAMP_TZ(9), so OBJECT(s VARCHAR(16777216), i NUMBER(38,0)), sArr ARRAY(VARCHAR(16777216)), f64Arr ARRAY(FLOAT), someMap MAP(VARCHAR(16777216), BOOLEAN))'.
sfc-gh-pfus commented 4 days ago

@sfc-gh-pfus need support on this one: (I was running structured_write_test.go from the master branch first to ensure it worked correctly)

Getting the following error:

--- FAIL: TestBindingObjectWithSchema (0.67s)
    driver_test.go:276: error on exec context [[query too large to print]]: 002040 (42601): SQL compilation error:
        Unsupported data type 'OBJECT(s VARCHAR(16777216), b NUMBER(38,0), i16 NUMBER(38,0), i32 NUMBER(38,0), i64 NUMBER(38,0), f32 FLOAT, f64 FLOAT, nfraction NUMBER(38,9), bo BOOLEAN, bi BINARY(8388608), date DATE, time TIME(9), ltz TIMESTAMP_LTZ(9), ntz TIMESTAMP_NTZ(9), tz TIMESTAMP_TZ(9), so OBJECT(s VARCHAR(16777216), i NUMBER(38,0)), sArr ARRAY(VARCHAR(16777216)), f64Arr ARRAY(FLOAT), someMap MAP(VARCHAR(16777216), BOOLEAN))'.

I tried it locally and it worked. I started build on Github Actions, let's check it. By looking at the message - I believe it because you didn't add this UUID to object definition in table, but in the current version it is present, so let's hope it works.

ChronosMasterOfAllTime commented 4 days ago

What I mean is I tried this on a clean branch and got the same error. Is it something to do with my snowflake instance? (I finally got the ability to test against our QA env)

sfc-gh-pfus commented 4 days ago

So probably yes, probably you don't have access to some features. But it works on CI, so it is good enough.

sfc-gh-pfus commented 16 hours ago

@ChronosMasterOfAllTime I started tests and some of them failed. Mostly because of the syntax (a colon instead of a comma), but some failed on incorrectly reading UUIDs:

=== RUN   TestObjectWithAllTypesSimpleScan/JSON
    assert_test.go:103: expected "00000000-0000-0000-0000-000000000000" to be equal to "d0af8810-9997-42e9-5ff1-8101a9d79732" but was not. . Thrown from /home/runner/work/gosnowflake/gosnowflake/structured_type_read_test.go:600 +0x105c
        github.com/snowflakedb/gosnowflake.forAllStructureTypeFormats.func4(0x0?)

There are still some flaky tests, so ignore them for now, but these one that appear in each result should be fixed :) Also, our linter failed - but only with minor suggestions.

ChronosMasterOfAllTime commented 9 hours ago

@ChronosMasterOfAllTime I started tests and some of them failed. Mostly because of the syntax (a colon instead of a comma), but some failed on incorrectly reading UUIDs:

=== RUN   TestObjectWithAllTypesSimpleScan/JSON
    assert_test.go:103: expected "00000000-0000-0000-0000-000000000000" to be equal to "d0af8810-9997-42e9-5ff1-8101a9d79732" but was not. . Thrown from /home/runner/work/gosnowflake/gosnowflake/structured_type_read_test.go:600 +0x105c
        github.com/snowflakedb/gosnowflake.forAllStructureTypeFormats.func4(0x0?)

There are still some flaky tests, so ignore them for now, but these one that appear in each result should be fixed :) Also, our linter failed - but only with minor suggestions.

Where can I see the results and where can I see the linter checks?