graphiti-api / graphiti

Stylish Graph APIs
https://www.graphiti.dev/
MIT License
960 stars 138 forks source link

Missing next page link when the resource is a postgres table has json columns #439

Open MattFenelon opened 1 year ago

MattFenelon commented 1 year ago

The next link isn't included (and the last page link does not have a page number) when the resource has json columns because of the use of distinct.count(:all) in:

https://github.com/graphiti-api/graphiti/blob/14e84ef7e10c347ec391b96948365b5195074632/lib/graphiti/adapters/active_record.rb#L195-L201

In postgresql, json columns can’t be included in a DISTINCT because postgresql does not have an equality operator for json columns, only jsonb.

In addition, the usual comparison operators shown in Table 9.1 are available for jsonb, though not for json. — PostgreSQL: Documentation: 15: 9.16. JSON Functions and Operators

Digging into this a bit more, I believe the Graphiti::Delegates::Pagination.links method is responsible for building the pagination links. That method calls has_next_page?:

https://github.com/graphiti-api/graphiti/blob/14e84ef7e10c347ec391b96948365b5195074632/lib/graphiti/delegates/pagination.rb#L12-L20

In a rails console I ran:

> Graphiti::Delegates::Pagination.new(ModelWithJsonColumnsResource.all).has_next_page?
=> false

The return value of false is odd because my development environment has 18k records for that resource:

> ModelWithJsonColumns.count
=> 18000

has_next_page? calls the private methods current_page and last_page. current_page returns 1, as expected (it's the first page). last_page, on the other hand, returns nil. I expected it to return a high number (18,000 / page_size).

last_page calls item_count and calling that method returns 0, not the expected actual count of 18,000.

So it looks like there is something wrong with item_count.

item_count is assigned using:

https://github.com/graphiti-api/graphiti/blob/14e84ef7e10c347ec391b96948365b5195074632/lib/graphiti/delegates/pagination.rb#L71

item_count_from_proxy returns nil. I believe that's expected.

item_count_from_stats however returns an exception:

> Graphiti::Delegates::Pagination.new(ModelWithJsonColumnsResource).send(:item_count_from_stats)
…
   (0.8ms)  SELECT COUNT(*) FROM (SELECT DISTINCT … FROM (SELECT DISTINCT ON (…) "model_with_json_columns".* FROM "model_with_json_columns" WHERE … ORDER BY …) model_with_json_columns ORDER BY "model_with_json_columns"."id" ASC) subquery_for_count
ActiveRecord::StatementInvalid: PG::UndefinedFunction: ERROR:  could not identify an equality operator for type json
LINE 1: ..."another_column", "model_with_json_columns"."some_json_column", "model_with_json_columns...
                                                                           ^

from /usr/local/bundle/gems/activerecord-6.1.6.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'
Caused by PG::UndefinedFunction: ERROR:  could not identify an equality operator for type json
LINE 1: ..."another_column", "model_with_json_columns"."some_json_column", "model_with_json_columns...
                                                                           ^

from /usr/local/bundle/gems/activerecord-6.1.6.1/lib/active_record/connection_adapters/postgresql_adapter.rb:672:in `exec_params'

And what does item_count do when it sees an exception? Return 0:

https://github.com/graphiti-api/graphiti/blob/14e84ef7e10c347ec391b96948365b5195074632/lib/graphiti/delegates/pagination.rb#L75-L81

That's why has_next_page? is returning false, and why the :next link is not being included in the output.

I'm not sure what the fix could be. Is there a reason why distinct is the default over a simple count? Could the distinct be dropped? Could the count(:all) be changed to count(:id)?

Zeneixe commented 1 month ago

BUMP on this issue