sql: pgwire behavior for string arrays is partially incorrect

[jordanlewis]

Sequelize queries various pg_catalog tables to assemble an array of column names, using the following query:

SELECT i.relname AS name,
       ix.indisprimary AS PRIMARY,
       ix.indisunique AS UNIQUE,
       ix.indkey AS indkey,
       ARRAY_AGG(a.attnum) AS column_indexes,
       ARRAY_AGG(a.attname) AS column_names,
       pg_get_indexdef(ix.indexrelid) AS definition
FROM pg_class t,
     pg_class i,
     pg_index ix,
     pg_attribute a
WHERE t.oid = ix.indrelid
AND   i.oid = ix.indexrelid
AND   a.attrelid = t.oid
AND   t.relkind = 'r'
AND   t.relname = 'customers' -- this query is run once for each table
GROUP BY i.relname,
         ix.indexrelid,
         ix.indisprimary,
         ix.indisunique,
         ix.indkey
ORDER BY i.relname

One of the return values is the result of ARRAY_AGG(a.attname), which we treat as an array of strings. We return arrays of strings as strings that look like this: {'foo','bar','baz'}.

Sequelize fails to parse the representation of this array that we send over the pg wire protocol for two separate reasons.

1. Sequelize expects that the strings inside the array are not quoted, and in fact postgres returns string arrays that look like {foo,bar,baz} unless a string has a , character in it in which case it'll be {foo,"bar,",baz}.
2. Sequelize expects that the type we report for this array over pgwire is 1003, corresponding to an array of name types. We report that this array is of type 1009, or an array of text types. If we report 1009, Sequelize actually blows up while trying to parse the returned array.

It shouldn't be too hard to solve the first one. The second one is a little bit more involved - we'll need to make a Datum type for column name literal values, as well as a corresponding type for arrays of those. Then we'll need to change the relevant column definitions in pg_catalog to use this new type.

[jordanlewis]

There's another similar issue with this query: the indkey field is returned over pgwire as a field of type 22 or int2vector. We return it as an int8 array, which sequelize helpfully fails to parse.

[cuongdo]

When running a Sequelize app against the latest master code I still get this:

Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'user' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
Unhandled rejection TypeError: text.replace is not a function
    at Object.fromArray (/Users/cdocr/Dropbox/sequelize/node_modules/sequelize/lib/dialects/postgres/query-generator.js:848:17)
    at /Users/cdocr/Dropbox/sequelize/node_modules/sequelize/lib/dialects/postgres/query.js:144:56
    at Array.forEach (native)
    at /Users/cdocr/Dropbox/sequelize/node_modules/sequelize/lib/dialects/postgres/query.js:135:15
    at tryCatcher (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:507:35)
    at Promise._settlePromise (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:567:18)
    at Promise._settlePromise0 (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:612:10)
    at Promise._settlePromises (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:691:18)
    at Promise._fulfill (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:636:18)
    at PromiseArray._resolve (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise_array.js:125:19)
    at PromiseArray._promiseFulfilled (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise_array.js:143:14)
    at PromiseArray._iterate (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise_array.js:113:31)
    at PromiseArray.init [as _init] (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise_array.js:77:10)
    at Promise._settlePromise (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:564:21)
    at Promise._settlePromise0 (/Users/cdocr/Dropbox/sequelize/node_modules/bluebird/js/release/promise.js:612:10)

@jordanlewis can you take a look?

[jordanlewis]

Thanks for the report @cuongdo. Indeed @nvanbenschoten's PR didn't quite get us to the finish line for this issue.

The problem with this query is the following part of the SELECT: array_agg(a.attname) as column_names). As mentioned in the issue description, Sequelize expects the result type of this to be __name a.k.a. an array of names. We still output it as __text a.k.a. an array of strings.

I believe we just need to add a new override for array_agg that handles name types. I'll look into that.

[nvanbenschoten]

My last PR alone wasn't meant to solve this issue fully. I have another PR out (#12644) that will fix this. I haven't pushed an update since knz's review yet because I'm reconsidering a part of it, but I'll make sure to update it later today.

[jordanlewis]

Ah, okay. Thanks, Nathan!

[nvanbenschoten]

Sorry for not catching that the __name issue was the root cause of @cuongdo's stack trace and saving you the effort, but thanks for investigating @jordanlewis!