Prevent Agate from coercing values in query result sets

[drewbanin]

resolves #2984, #2666
related to #3413

Description

dbt uses Agate to represent internal dataframes (eg. seed file tabular representations, or in-memory query result sets). Agate's type coercions are interesting and helpful for identifying column types in csv files, but they can lead to incorrect/confusing results when applied to query result sets. This PR prevents type coercion from happening when marshaling a query resultset into an Agate table.

Example

I used the following macro to debug / reproduce some type coercion issues locally (using Snowflake):

{% macro debug() %}

    {% set query %}

        select '0005' as stringnum, 'false' as stringbool, 10 as intfield, false as boolfield, '01T00000aabbccdd' as dtstring
        union all
        select '0006' as stringnum, 'true' as stringbool, 20 as intfield, true as boolfield, '01T00000aabbccde' as dtstring

    {% endset %}

    {% set res = run_query(query) %}
    {% do res.print_table() %}

{% endmacro %}

Before this change:

STRINGNUM (bad)	STRINGBOOL (bad)	INTFIELD	BOOLFIELD	DTSTRING (would be funny if it wasn't so bad)
5	False	10	False	0101-01-01 00:00:00
6	True	20	True	0101-01-01 00:00:00

After this change:

STRINGNUM	STRINGBOOL	INTFIELD	BOOLFIELD	DTSTRING
0005	false	10	False	01T00000aabbccdd
0006	true	20	True	01T00000aabbccde

About the implementation

This PR assumes that we never want to coerce types in tables returned by the database, which I think is correct, but would welcome any holes that can be poked in that assumption. I was originally going to try to use the cursor.description provided by dbapi implementation to map columns in a query resultset to Python types and then use those Python types to "force" types for all of the columns in the Agate table. In practice, this would require us to add some logic to every adapter plugin which felt both onerous and unnecessary. Instead, we can just peek at the actual query results and "force" string columns in the resultset to be retained as strings in the Agate table that dbt produces. This works because:

• If the database is returning a string value in a field, we can feel 100% confident that the right type for the column in Agate is a string
• Columns in a database result cannot be of mixed types, so if we find one string value in a column, we know the whole column should be treated as a string
• Agate won't coerce a not-string into an unexpected type (eg. an int returned by the database can't become a bool in Agate)

What else

I tried to make a surgical change here. By putting this logic in the agate_helper client, all adapter plugins should be able to take advantage of the change. Longer term, I think we should consider making a less-surgical change that removes Agate entirely. Let's pick up that discussion in #3413 though :)

Checklist

• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[drewbanin]

I think our existing implementation assumed that the order of keys in a dictionary was guaranteed... but IIRC that's not true for all versions of Python. My Python is rusty, so maybe this change is not necessary. I can revert if need be :)

[jtcohen6]

Not my area, but I do remember hearing that this changed in recent versions of python. Indeed this blog post suggests that dictionaries are ordered by insertion as of python 3.6, and that this is officially guaranteed as of python 3.7. dbt currently requires py36 or higher.

[drewbanin]

I was originally just going to peek at the first row in the result set, but it's possible that the value in a string column returned by the database could be NULL. This implementation looks at all of the values in the result set, which we're actually already doing anyway to json-encode dict/list/tuple types.

[drewbanin]

Is 0.20.0 the right home for this? Or should I slot it into 0.21.0?

[jtcohen6]

I'd prefer 0.21, rather than sneaking it in during RC time. Even though it is technically a bug fix, I imagine it might be breaking behavior for folks who do a lot of custom query-results munging with Jinja + agate.

[gshank]

Looks good to me!

[drewbanin]

Right on, thanks. I updated the changelog to include this fix in the 0.21 section. What's the right base branch for this PR? Is it develop or something else?

[jtcohen6]

develop is right!