apache / arrow-adbc

Database connectivity API standard and libraries for Apache Arrow
https://arrow.apache.org/adbc/
Apache License 2.0
385 stars 98 forks source link

fix(go/adbc/driver/internal/driverbase): proper unmarshalling for ConstraintColumnNames #2285

Closed zeroshade closed 3 weeks ago

zeroshade commented 4 weeks ago

Fixes #2278

The current UnmarshalJSON function for requiredList ended up unmarshalling into a copy of the underlying slice rather than using the actual slice, so the data wasn't being propagated appropriately. This fixes the UnmarshalJSON function to handle the scenario appropriately.

zeroshade commented 4 weeks ago

CC @davidhcoe Can you confirm that this fixes the issue reported?

davidhcoe commented 3 weeks ago

This doesn't work for me. I put together a sample to test with based on the result I get from Snowflake: sf_results.json

 func (suite *SnowflakeTests) TestJsonUnmarshal() {
    jsonFile, err := os.Open("sf_results.json")
    if err != nil {
        log.Fatal("Failed to open file:", err)
    }
    defer jsonFile.Close()

    byteValue, err := ioutil.ReadAll(jsonFile)

    var getObjectsCatalog driverbase.GetObjectsInfo
    if err := json.Unmarshal(byteValue, &getObjectsCatalog); err != nil {
        log.Fatal("Failed to parse JSON:", err)
    }

    for i, sch := range getObjectsCatalog.CatalogDbSchemas {
        for j, tab := range sch.DbSchemaTables {
            for k, con := range tab.TableConstraints {
                for l, _ := range con.ConstraintColumnNames {
                    fmt.Println(getObjectsCatalog.CatalogDbSchemas[i].DbSchemaTables[j].TableConstraints[k].ConstraintColumnNames[l])
                }
            }
        }
    }
}
zeroshade commented 3 weeks ago

@davidhcoe I've updated the PR and added a test based on your example. Can you confirm it works for you?

davidhcoe commented 3 weeks ago

Yes -- it works for me now. Thank you.