array update/inserts malformed

[nbrustein]

I'm not sure if this is an issue in sql-bricks or an issue in sql-bricks-postgres, so I'm creating issues in both projects.

When I try to insert (or update) into an array column of type uuid[], then value is malformed. There should be quotes around it, but there are not.

sql-bricks generates this:

 INSERT INTO my_table (empty_array, array_with_something) VALUES ({}, {'aee828d5-6615-4fbd-bc58-41ccd39470ae'})

but should generate

 INSERT INTO my_table (empty_array, array_with_something) VALUES ('{}','{\'aee828d5-6615-4fbd-bc58-41ccd39470ae\'}')

I am able to get the query to work with the following regex, though this would not work in all cases:

    sql = sql.replace(/(\{)([^\}]*)(\})/g, function(ignore1, ignore2, content) {
        return '\'{' + content.replace(/\'/g, '"') + '}\'';
    });

[prust]

@nbrustein sorry for the delay responding. Thanks for posting follow-up example code in https://github.com/Suor/sql-bricks-postgres/issues/7. I think the problem is in this line (line 836 on v1.2.3):

    'Array': function(arr) { return '{' + arr.map(sql.convert).join(', ') + '}'; }

Your work-around (replacing sql-bricks' output via regex) could be made slightly more precise by replacing the sql.conversions.Array function like this:

sql.conversions.Array = function(arr) {
  return '\'{' + arr.map(sql.convert).join(', ').replace(/'/g, '"') + '}\'';
};

That said, I'm not sure this implementation is 100% correct (it seems to me like it would need to be a little smarter than this in order to avoid touching embedded single quotes that are already escaped -- or maybe the code above would double-escape these which is what we want? I'm not sure).

My intention is for Sql-Bricks to only support the functionality in the SQL-92 standard and for other people to write & maintain extension libraries (like sql-bricks-postgres) to add support for postgres/sqlite extensions and features added in SQL-99, SQL-2003, etc. This keeps the sql-bricks library small enough for one person to maintain. Supporting everything in the (much larger) later SQL standards would be a huge amount of work which I have no desire to do. Since Arrays were first introduced in SQL-99, I'll remove the existing buggy Array support (see #81) from sql-bricks and encourage sql-bricks-postgres and/or other extensions to implement it, if desired.

Aside: the SQL-99 array syntax (ARRAY['aee828d5']) is supported by Postgres, but my hunch is that the other syntax that Postgres supports ('{"aee828d5"}') was introduced first and is probably still the preferred syntax of Postgres users.

Incidentally, the SQL-99 syntax would be simpler/easier for sql-bricks-postgres or other sql-bricks extensions to implement correctly, since it doesn't change the rendering of strings within the array (the postgres syntax requires that strings within the array be escaped or use double-quotes). I would think that SQL-99 arrays could be implemented correctly as easily as this:

sql.conversions.Array = function(arr) {
  return 'ARRAY[' + arr.map(sql.convert).join(', ') + ']';
};

[prust]

cc @Suor

[nbrustein]

Thanks for you suggestion. I finally got around to giving it a try, but I ran into troubles because js has no way of knowing what the type is supposed to be on the array. I'm not sure there is a clean fix, but I plan to revisit it at some point.

[Suor]

Not sure what you mean by js not knowing type of an array. Why do you need it?

[nbrustein]

There are certain cases where postgresql gives errors if you do something like Array[] because it does not know if that is an array of text elements or integers or uuids or what. It wants Array[]::uuid or something like that (I'd have to check the syntax again)

[Suor]

First, does using array literal not constructor works for you or you need to cast type anyway? Second, why are you using .toString() at all? One should use .toParams() when actually making queries.

[Suor]

P.S. I suppose you don't use pg-bricks, this issue should not arise there.

[nbrustein]

We don't. Didn't know about it. I'll give it a try.