server: deduplicate Postgres function names with HashSet (close #8888)

[daniel-shuy]

Long Changelog

There was a bug (#8643) where if a function is added as both a root field and computed field at the same time, the migrations would fail.

This was temporarily resolved in b0d2262 by using Data.List#nub in runTxWithMetadataCheck to remove duplicates, however Data.List#nub is really inefficient (O(n^2) time-complexity).

This PR refactors runTxWithMetadataCheck to use a Set instead of Data.List#nub to deduplicate the Postgres function names.

Related Issues

#8888

Solution and Design

I used Data.HashSet.InsOrd instead of Data.HashSet to maintain insertion order, so that the sorting is the same as now (using List), is that desired?

Alternately, should I use Data.Set to sort by alphabetical order instead?

Steps to test and verify

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

• [x] No
• [ ] Yes
  • [ ] Updated docs with SQL for downgrading the catalog

Metadata

Does this PR add a new Metadata feature?

• [x] No
• [ ] Yes
  • Does run_sql auto manages the new metadata through schema diffing?
    • [ ] Yes
    • [ ] Not required
  • Does run_sql auto manages the definitions of metadata on renaming?
    • [ ] Yes
    • [ ] Not required
  • Does export_metadata/replace_metadata supports the new metadata added?
    • [ ] Yes
    • [ ] Not required

GraphQL

• [x] No new GraphQL schema is generated
• [ ] New GraphQL schema is being generated:
  • [ ] New types and typenames are correlated

Breaking changes

• [x] No Breaking changes

• [ ] There are breaking changes:

  1. Metadata API

     Existing query types:

     • [ ] Modify args payload which is not backward compatible
     • [ ] Behavioural change of the API
     • [ ] Change in response JSON schema
     • [ ] Change in error code

  2. GraphQL API

     Schema Generation:

     • [ ] Change in any NamedType
     • [ ] Change in table field names

     Schema Resolve:-

     • [ ] Change in treatment of null value for any input fields

  3. Logging

     • [ ] Log JSON schema has changed
     • [ ] Log type names have changed

[hasura-bot]

Beep boop! :robot:

Hey @daniel-shuy, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! :sunglasses:

[soupi]

@daniel-shuy Thanks for your contribution! As far as I can tell the ordering doesn't matter, so using a regular HashSet is fine (@plcplc please correct me if I'm wrong).

[daniel-shuy]

@plcplc would love to get your feedback on this :)

[daniel-shuy]

@plcplc do you mean changing the return type for all the functions that return Postgres function names (e.g. computedFieldFunctions) to HashSet?

[plcplc]

@plcplc do you mean changing the return type for all the functions that return Postgres function names (e.g. computedFieldFunctions) to HashSet?

That's the gist of it yes. But it's been a while since I looked closely at this, and in certain places duplicates do indicate errors that need to abort execution, so we likely can't just make the change blindly.

[daniel-shuy]

What about the functions that are implemented for other database vendors? Should I change it for all the other databases as well?

[plcplc]

I think a sensible scope for this is just the Postgres backend :-)

[hasura-bot]

Beep boop! :robot:

Awesome work @daniel-shuy!

Your changes were merged successfully. All of us at Hasura :heart: what you did.

Thanks again :hugs:

[Stefmore02]

Hello @daniel-shuy

Thanks so much for your contribution to Hasura OSS during the Hacktoberfest 2022! We'd love to send you some Hasura swag as a token of appreciation and you will be getting an email with the swag form shortly.✨