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

PG::SyntaxError with Apartment gem #1316

Open
2 of 7 tasks
cedm opened this issue Mar 11, 2020 · 6 comments
Open
2 of 7 tasks

PG::SyntaxError with Apartment gem #1316

cedm opened this issue Mar 11, 2020 · 6 comments

Comments

@cedm
Copy link

cedm commented Mar 11, 2020

This issue is a (choose one):

  • Problem/bug report.
  • Feature request.
  • Request for support. Note: Please try to avoid submitting issues for support requests. Use Gitter instead.

Checklist before submitting:

  • I've searched for an existing issue.
  • I've asked my question on Gitter and have not received a satisfactory answer.
  • I've included a complete bug report template. This step helps us and allows us to see the bug without trying to reproduce the problem from your description. It helps you because you will frequently detect if it's a problem specific to your project.
  • The feature I'm asking for is compliant with the JSON:API spec.

Description

  • Ruby 2.5.5
  • Rails 5.2
  • jsonapi-resources 0.10.2

Bug reports:

When using Apartment (multi-tenancy gem), any model that is shared among all tenants (i.e. in the 'public' schema) return an error 500 (PG::SyntaxError) when trying to query the API. This issue only happens with shared models, and not with tenant-specific models. The issue lies in the way jsonapi-resources writes its queries, e.g:

Internal Server Error: PG::SyntaxError: ERROR:  syntax error at or near "."
LINE 1: SELECT public.accounts.id AS public.accounts_id FROM "public...

The whole query is:

SELECT public.accounts.id AS public.accounts_id FROM "public"."accounts" WHERE "public"."accounts" = $1

SELECT public.accounts.id AS public.accounts_id FROM "public"."accounts" WHERE "public"."accounts" = $1

The bold part is the issue. There should be no dot in the AS statement. public.accounts_id should be public_accounts_id or just accounts_id.

This should be a trivial fix, but I just don't know where in the gem code queries are generated.

@cedm
Copy link
Author

cedm commented Mar 11, 2020

Just noticed this is the same issue reported 20 days ago by Albertjan90: #1310

But no one replied to his report.

@cedm
Copy link
Author

cedm commented Mar 11, 2020

Besides the AS statement, the WHERE clause is also erroneous as it is missing the column name:

SELECT public.accounts.id AS public.accounts_id FROM "public"."accounts" WHERE "public"."accounts" = $1

The proper query should be:

SELECT public.accounts.id AS accounts_id FROM public.accounts WHERE public.accounts.id = $1

@cedm
Copy link
Author

cedm commented Mar 12, 2020

I've got a solution by modifying the _table_name method found in lib/jsonapi/basic_resource.rb with:

def _table_name
  @_table_name ||= _model_class.respond_to?(:table_name) ? _model_class.table_name.partition('.').last : _model_name.tableize
end

.partition('.').last gets rid of the schema name, e.g. public.accounts -> accounts.

Works well with and without schema for me, but this solution would probably truncate table names if they include a 'dot' (apparently that's allowed in SQL...). Haven't tested the later scenario though.

@lgebhardt
Copy link
Member

@cedm Could you test out the #1318 PR and add comments there? I don't have the test framework setup to handle multi tenancy, and unfortunately I'm a bit short on time at the moment to work on that.

Rather than your fix using partition to extract the table portion I'm converting the '.' to '_' to handle the same table name in both databases. Not sure if that's likely to happen, but without it I think we could have issues. Hopefully that doesn't create too long of aliases.

@cedm
Copy link
Author

cedm commented Mar 16, 2020

@lgebhardt Your PR fixed the AS statement, but didn't help with the WHERE clause issue. I've commented on the PR.

@cedm
Copy link
Author

cedm commented Mar 18, 2020

Improving on my hack-ish solution with:

def _table_name
  @_table_name ||= _model_class.respond_to?(:table_name) ? _model_class.table_name.split('.').last : _model_name.tableize
end

split rather than partition, as partition broke related links.

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

No branches or pull requests

2 participants