But schema parsing isn't strictly related to models, is it? I mean, you can request `DB.schema(table_name)` without using models, and the schema is cached on the database object. To me it seems that schema parsing is a Database thing (which the model code uses), and thus it seems to me that it should be fine that a database extension affects this code.
Another thing is that the pg_json extension already affects schema parsing, so isn't the change I'm suggesting still inside the territory that pg_json covers? Regards, Janko > On 09 May 2016, at 22:54, Jeremy Evans <[email protected]> wrote: > > On Sunday, May 8, 2016 at 8:42:58 PM UTC-7, Janko Marohnić wrote: > Hey Jeremy, > > The pg_json and pg_array extensions are awesome. However, I see people often > tripped about the load order, in the sense that they try to load the > extension after model definitions (that is, after the schema has already been > parsed), and then it silently doesn't work for them. I have two improvements > to this problem, which are not mutually exclusive (both can be added). > > First one is that we add a warning when pg_json is loaded after schema has > already been parsed. As far as I know, schema is parsed only once, on first > model definition, am I correct? The documentation of pg_json extension could > also be updated to note that pg_json should be loaded before model > definitions. > > Unfortunately, this mixes layers. pg_json is a database extension, what you > are taking about is model related. Database code can never depend on model > code, as you should always be able to use database code without models. > > FWIW, the schema for the model is requested from the Database every time you > set the model's dataset (generally once when the model class is initially > created). The Database caches schema entries, so future requests for the > same table may not actually reparse the schema, unless you pass the > appropriate option. > > Second one is, when loading the pg_json extension, if the schema is already > parsed, we could reiterate over it and change the :type to :json/:jsonb if > the :db_type is "json"/"jsonb". I'm aware that this is a performance penalty, > but the alternative is that typecasting simply doesn't work. And people can > always avoid the performance penalty by loading the extension before schema > parsing. > > Likewise, this also mixes layers, and as such is not possible. > > Another reason why I would like the second one is that loading the extension > before schema parsing isn't always that easy. In my last job I had a Sinatra > application which lazily set the database connection (accessible via > App.database). And then the model was instantiated via `class Post < > Sequel::Model(App.database)`. This was done because we were communicating > with other databases as well. We couldn't put `App.database.extension > :pg_json` in the Sequel initializer, because at the moment of requiring the > initializer `App` wasn't defined yet. But the `App.database` in model > definitions worked because model files were autoloaded, so the `App` was > defined. After some investigating I somehow found the solution: to use > `Sequel::Database.extension :pg_json` in the Sequel initializer, which was > exactly what I wanted. However, it took me a while to find out about it. If > pg_json extension updated my parsed schema, I'm pretty sure I would have no > problem paying that small performance penalty. And it kind of makes sense, to > be able to do: > > class Post < Sequel::Model(App.database) > db.extension :pg_json > end > > or > > App.database.extension :pg_json > > class Post < Sequel::Model(App.database) > end > > However, now when I wrote all of this, I think I'm more in favour of just > adding the warning and changing documentation, because the above doesn't > actually feel right. I can make the PR for whatever we decide, just wanted to > ask you first what do you think about this. > > If this is really an issue for you, you can set the model's dataset to the > current dataset after loading the extension and reloading the schema: > > class Post < Sequel::Model(App.database) > db.extension :pg_json > db.schema(table_name, :reload=>true) > set_dataset(dataset) > end > > But the recommended way would just be to get more control over the > initialization process in your application so you can load extensions into > your Database object right after it is created. > > Thanks, > Jeremy > > -- > You received this message because you are subscribed to a topic in the Google > Groups "sequel-talk" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/sequel-talk/Ej2E0JJtjh4/unsubscribe > <https://groups.google.com/d/topic/sequel-talk/Ej2E0JJtjh4/unsubscribe>. > To unsubscribe from this group and all its topics, send an email to > [email protected] > <mailto:[email protected]>. > To post to this group, send email to [email protected] > <mailto:[email protected]>. > Visit this group at https://groups.google.com/group/sequel-talk > <https://groups.google.com/group/sequel-talk>. > For more options, visit https://groups.google.com/d/optout > <https://groups.google.com/d/optout>. -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/sequel-talk. For more options, visit https://groups.google.com/d/optout.
