vanstyn / RapidApp

Turnkey ajaxy webapps
http://rapi.io
Other
48 stars 15 forks source link

Tables names aren't qualified with the schema name causing errors on ambiguous tables #164

Closed timbunce closed 8 years ago

timbunce commented 8 years ago

We have a schema called 'amq' which contains a table called 'messages' and there's another table with the same name in the default schema ('public').

I launched rdbic.pl with the --loader-option db_schema=amq option and it correctly displayed the list of tables in the amq schema. When I tried to access the messages table it failed with this error:

DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::Pg::st execute failed:
ERROR:  column me.class_id does not exist
LINE 1: SELECT "me"."id", "me"."class_id", "me"."created_at", "me"."...
                          ^ [for Statement "SELECT "me"."id", "me"."class_id", "me"."created_at", "me"."started_at", "me"."completed_at", "me"."yaml", "me"."stream_id", "me"."in_conditions_met_at", "me"."checksum", "me"."host", "me"."status" FROM "messages" "me" LIMIT ?" with ParamValues: 1='25']
at /home/tim/trunk/comp/rapidapp/cpan/lib/perl5/RapidApp/Module/StorCmp/Role/DbicLnk.pm line 773

I'm believe the cause of this is that the table name gets looked up in the public schema first and then the amq schema. So when referring to just 'messages' it's finding the one in public instead of the one in amq. (This search behaviour is defined by the Postgres SET search_path ... configuration option.)

The right fix would be to always include the schema name with the table name. (At least for tables not in the default schema, but would be best to do all.)

vanstyn commented 8 years ago

@timbunce - two things to try:

  1. Turn on the S::L qualify_objects option, i.e. --loader-option qualify_objects=1
  2. Try placing that statement in on_connect_do, i.e. --connect-option on_connect_do='SET search_path ...'

Let me know if either of those work...

timbunce commented 8 years ago

The first option worked (which is good as the second would be breaking encapsulation). I presume you'll make qualify_objects the default now?

vanstyn commented 8 years ago

Excellent, I was hoping it would... What I think I'll do is make it the default when db_schema is set, the reason being that turning it on across the board would make the schema classes less portable (and this code is shared by rdbic and the normal rapidapp.pl bootstrap). Let me know if you disagree. Otherwise, I'll ship a new release to CPAN today

timbunce commented 8 years ago

Seems like a reasonable approach. Thanks.