[Dbix-class] View Definitions for non-virtual views

[Francisco Obispo]

Any thoughts on the PR ?


On 26 Jul 2017, at 20:55, Francisco Obispo wrote:

> I’ve created a pull request for this feature:
>
> https://github.com/dbsrgits/dbix-class-schema-loader/pull/13
>
> It works by adding: `-o omit_view_definitions=1` to the loader 
> options.
>
> I’ve tested it in my computer, and seems to work correctly,
>
> best regards,
>
>
> On 25 Jul 2017, at 12:24, Matt S Trout wrote:
>
>> On Fri, Jul 21, 2017 at 03:14:33PM -0700, Francisco Obispo wrote:
>>> Yes, but wasn’t sure were to post the request.
>>
>> The Schema-Loader queue would've been ok, here is probably best 
>> anyway.
>>
>>>>> The result is that `.pm`’s derived from a view from `dbicdump` 
>>>>> are
>>>>> larger than they need to be, inherently using more memory, and 
>>>>> with
>>>>> no real good purpose. There should be an option to turn this
>>>>> behavior off.
>>>>
>>>> While I don't see any reason we wouldn't take a patch to supply
>>>> such an
>>>> option, I confess to being surprised that this was noticeable -
>>>> how did
>>>> you measure the memory usage with and without, and how big was the
>>>> difference?
>>>>
>>>
>>> The diffs in git started getting very big all of the sudden, and in
>>> the company that I work, that started raising questions on the QA
>>> team, plus we have some areas where we have fairly large postgres
>>> views whose complexity is completely abstracted in a simple
>>> structure that has no interest to the ORM.
>>
>> Personally, I quite like being able to see when the meaning of one of
>> my result classes just changed, but I can absolutely see "this is
>> beyond our abstraction boundary and therefore we don't want it in the
>> diff" as an argument.
>>
>>> Those fairly large views, force us to store the definition in RAM
>>> (as a perl string) now just for the sake of documentation, but in
>>> reality, at least in our system, is the wrong place to store the
>>> view definition. I don’t want to have to explain a more junior
>>> developer that the real view lives in SQL, and what lives in the .PM
>>> is just a snapshot of how it looked when the schema was dumped.
>>
>> Talking about RAM usage - which I suspect is negligible - is an 
>> incredibly
>> weak argument here and if you don't have numbers I'd suggest just not 
>> making
>> that argument.
>>
>> "This is outside our abstraction boundary" is a good argument.
>>
>> "This is making the diffs when we update the schema ugly" is a good 
>> argument.
>>
>> "This might confuse junior developers" is a good argument.
>>
>> All of those three are IMO each sufficient to justify a patch.
>>
>> So, what I'd suggest is, that you look at putting together a patch 
>> that
>>
>> (a) allows to toggle whether the SQL should be in code, docs, or 
>> nowhere
>> (b) emits a comment immediately above where view_definition normally 
>> is
>>     that mentions it's reflected from the schema and mentions the 
>> option
>>
>> That way junior developers using existing systems shouldn't be 
>> confused
>> anymore because the comment will tell them, and the existence of the 
>> comment
>> should also mean anybody with a use case like yours will find the 
>> option.
>>
>> Plus it can also emit a comment when it *doesn't* reflect the view
>> information, so if somebody later wants to switch to DBIC-controlled
>> deployment they'll also find the option to tweak.
>>
>> I don't think the default should change, though, because I know of 
>> quite a
>> few projects that rely on Schema::Loader'ed schemas being deployable 
>> to set
>> up test databases and similar, and if anybody with needs like yours 
>> can easily
>> find the option anyway, it seems impolite to break their setups.
>>
>> Does that all make sense? Fancy having a go at a patch?
>>
>> -- 
>> Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit 
>> and a clue
>>
>> http://shadowcat.co.uk/blog/matt-s-trout/   
>> http://twitter.com/shadowcat_mst/
>>
>> Email me now on mst (at) shadowcat.co.uk and let's chat about how our 
>> CPAN
>> commercial support, training and consultancy packages could help your 
>> team.
>>
>> _______________________________________________
>> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
>> IRC: irc.perl.org#dbix-class
>> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
>> Searchable Archive: 
>> http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk


> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive: 
> http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20170802/d0e1abcb/attachment.htm>