Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nested catListTable (by representing nested arrays as text) #242

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

shane-circuithub
Copy link
Contributor

@shane-circuithub shane-circuithub commented Jun 18, 2023

This is a possible fix to #168. With this we can catListTable arbitrarily deep trees of ListTables.

It comes at a relatively high cost, however.

Currently we represent nested arrays with anonymous records. This works reasonably well, except that we can't extract the field from the anonymous record when we need it (PostgreSQL theoretically suports .f1 syntax since PG13 but it only works in very limited situations). But it does mean we can decode the results using Hasql's binary decoders, and ordering works how we expect to (array[row(array[9])] < array[row(array[10])]).

What this PR does is instead represent nested arrays as text. To be able to decode this, we need each DBType to supply a text parser in addition to a binary decoder. It also means that ordering is no longer intuitive, because array[array[9]::text] > array[array[10]::text]. However, it does mean we can nest catListTables to our heart's content and it will always just work.

@shane-circuithub shane-circuithub changed the title Support nested catListTable (by represented nested arrays as text) Support nested catListTable (by representing nested arrays as text) Jun 18, 2023
shane-circuithub added a commit that referenced this pull request Jun 18, 2023
This is another possible "fix" to #168 (as opposed to #242). It doesn't really fix the problem, but it allows us to use two levels of `catListTable` instead of only one. Instead of trying to use Postgres's broken `.f1` syntax, we cast the anonymous record to text, remove the parentheses and quotes and unescape any escaped quotes or backslashes, and then cast the resulting text back to the appropriate type. The reason this only works one level deep is that if the type we cast the text back to is itself an anonymous record, then PostgreSQL doesn't know how to parse the text.

It's kind of ugly and hacky but it does work and otherwise maintains the status quo. Comparison operators on nested lists continue to work as before and we don't need to burden `DBType` with parsing nonsense.
@ocharles
Copy link
Contributor

Is this basically a variant on what we used to do when encoding to JSON?

@shane-circuithub
Copy link
Contributor Author

I don't recall that we ever did actually use JSON for this but we did talk about it. As far as I know PostgreSQL doesn't have a way to reliably convert arbitrary things from JSON so I don't think JSON could really be used for this. There's also no guarantee that the FromJSON instance on the Haskell side will line up with the JSON that Postgres would convert the array to. At least the text encoding is stable and we can convert things to and from it.

@shane-circuithub shane-circuithub force-pushed the profunctor-fold-aggregator branch from 144366d to e28cc31 Compare June 18, 2023 19:57
Base automatically changed from profunctor-fold-aggregator to master June 18, 2023 20:05
shane-circuithub added a commit that referenced this pull request Jun 18, 2023
This is another possible "fix" to #168 (as opposed to #242). It doesn't really fix the problem, but it allows us to use two levels of `catListTable` instead of only one. Instead of trying to use Postgres's broken `.f1` syntax, we cast the anonymous record to text, remove the parentheses and quotes and unescape any escaped quotes or backslashes, and then cast the resulting text back to the appropriate type. The reason this only works one level deep is that if the type we cast the text back to is itself an anonymous record, then PostgreSQL doesn't know how to parse the text.

It's kind of ugly and hacky but it does work and otherwise maintains the status quo. Comparison operators on nested lists continue to work as before and we don't need to burden `DBType` with parsing nonsense.
shane-circuithub added a commit that referenced this pull request Jun 18, 2023
This is another possible "fix" to #168 (as opposed to #242). It doesn't really fix the problem, but it allows us to use two levels of `catListTable` instead of only one. Instead of trying to use Postgres's broken `.f1` syntax, we cast the anonymous record to text, remove the parentheses and quotes and unescape any escaped quotes or backslashes, and then cast the resulting text back to the appropriate type. The reason this only works one level deep is that if the type we cast the text back to is itself an anonymous record, then PostgreSQL doesn't know how to parse the text.

It's kind of ugly and hacky but it does work and otherwise maintains the status quo. Comparison operators on nested lists continue to work as before and we don't need to burden `DBType` with parsing nonsense.
shane-circuithub added a commit that referenced this pull request Jun 18, 2023
This is another possible "fix" to #168 (as opposed to #242). It doesn't really fix the problem, but it allows us to use two levels of `catListTable` instead of only one. Instead of trying to use Postgres's broken `.f1` syntax, we cast the anonymous record to text, remove the parentheses and quotes and unescape any escaped quotes or backslashes, and then cast the resulting text back to the appropriate type. The reason this only works one level deep is that if the type we cast the text back to is itself an anonymous record, then PostgreSQL doesn't know how to parse the text.

It's kind of ugly and hacky but it does work and otherwise maintains the status quo. Comparison operators on nested lists continue to work as before and we don't need to burden `DBType` with parsing nonsense.
@shane-circuithub shane-circuithub force-pushed the nested-array-text branch 2 times, most recently from 971d4d6 to fcbab82 Compare August 16, 2023 01:06
@ocharles
Copy link
Contributor

I don't recall that we ever did actually use JSON for this but we did talk about it.

Yea, I had it working but couldn't find anything in the old rel8 branches. Oh well. Would have been around Mar 2021.

As far as I know PostgreSQL doesn't have a way to reliably convert arbitrary things from JSON so I don't think JSON could really be used for this.

I think what I was doing previously was (for non-JSON things): casting to ::text, then casting to ::jsonb. This way you're basically always using the text encoding, but building structure in JSON.

There's also no guarantee that the FromJSON instance on the Haskell side will line up with the JSON that Postgres would convert the array to. At least the text encoding is stable and we can convert things to and from it.

FromJSON isn't involved with this. The TypeInformation would know that we have our own internal JSON structure that's used by various things, just like your Decoder. That said, Decoder is basically what I had before, and I didn't like it because we've now got to maintain two parsers - one for binary and one for text. This seems like a lot of complexity. We're also duplicating all of the parsing that postgresql-simple does. This all makes me a little uneasy, and I think is what you're alluding to when you say "comes at a high cost"

This is one possible "fix" to #168. With this we can `catListTable` arbitrarily deep trees of `ListTable`s.

It comes at a relatively high cost, however.

Currently we represent nested arrays with anonymous records. This works reasonably well, except that we can't extract the field from the anonymous record when we need it (PostgreSQL [theoretically](https://www.postgresql.org/docs/13/release-13.html#id-1.11.6.16.5.6) suports `.f1` syntax since PG13 but it only works in very limited situations). But it does mean we can decode the results using Hasql's binary decoders, and ordering works how we expect ('array[row(array[9])] < array[row(array[10])]'.

What this PR does is instead represent nested arrays as text. To be able to decode this, we need each 'DBType' to supply a text parser in addition to a binary decoder. It also means that ordering is no longer intuitive, because `array[array[9]::text] > array[array[10]::text]`. However, it does mean we can nest `catListTable`s to our heart's content and it will always just work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants