<div dir="ltr"><div><div><div><div>> I don't think that looking in SQL in Java code is clear than looking in a SP code<br><br></div>Looking in SQL isn't the problem I'm trying to solve, but finding that.<br><br>> 1) You will pass more data on the wire instead of calling a SP with parameters<br></div><br>Passing the SQL string shouldn't be a problem unless that is very long one (several K) and then we probably do something wrong.<br><br>> 2) Your data that is passed on the wire is exposed to attacks since you
will have to implement DB security in the engine level (for example
hidden columns).<br><br></div><div>IIUC currently querying tables/views with hidden columns is implemented in a SP that consist of at least 2 SQL's, so those aren't good candidates for my proposal and will stay as is.<br></div>BTW how other projects resolve the security problem? AFAIK usually hidden columns are hidden by defining a proper view that do not include those.<br><br>> 3) Changes in the SQL code done in patches may be more complicated to track <br></div><br>Most of SQL code changes involve changes in a DAO too, so this shouldn't be an issue.<br><br>> 4) SQL Injection <div><br></div><div>Again, how other projects resolve the security problem? Internet is full of articles/blogs of why stored procedures should be avoided ("avoid stored procedures" Google query returned ~6.6M results), so I guess there are some other approaches for the security issue if that exists.<br><br>> 5) I think that SP performs better than SQL inside Java <br><br></div><div>Do we have a measurement of how much better? <br></div><div>One of my intentions for this proposal is that people evidently avoid creating neat SQL queries and re-use the existing ones, which has much bigger performance impact. I guess that the biggest limit here is how complicated that procedure is in the engine code.<br><br> </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 27, 2017 at 11:09 AM, Eli Mesika <span dir="ltr"><<a href="mailto:emesika@redhat.com" target="_blank">emesika@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_default" style="font-size:large"><br></div><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Mon, Mar 27, 2017 at 1:56 AM, Moti Asayag <span dir="ltr"><<a href="mailto:masayag@redhat.com" target="_blank">masayag@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Sun, Mar 26, 2017 at 5:12 PM, Yevgeny Zaspitsky <span dir="ltr"><<a href="mailto:yzaspits@redhat.com" target="_blank">yzaspits@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div>Hi All,<br><br></div>Recently I had a task of performance improvement in one of our network related flows and had some hard time following our DAL code and one of the outcomes of the task was defining a couple of new quite simple, but neat queries.<br>When I came to coding those new queries I remembered how hard was following the existing DAL code, I decided to make my own new methods clearer. So I created [1] and [2] patches. <br><br>Everything is quite standard there beside the fact that they do not use any stored procedure, but use SQL directly, IMHO by that they save time that I spent in trying to follow what a DAO method does. Looking into the method code you get the understanding of what this method is all about:<br><ul><li>no looking for a stored procedure name that is buried in the DAO class hierarchy<br></li><li>no looking for the SP definition</li></ul></div></div></div></div></blockquote><div><br></div></span><div>There are additional pros and cons for the suggestion to consider:<br><br></div><div>Pros:<br></div><div><ol><li>No need to run engine-setup after changing DB related code (in case of SQL inside Java). <br></li></ol></div><div>Cons:<br></div><ol><li>DAO files might become very long.</li><li>If you decide to return the business entity associated with the DAO as a returned object, you won't know as a caller which fields to expect to be populated, which lead to 3:</li><li>An inflation of business entities to represent partial populated business entity or inflation of mappers inflation (this will be required for SP as well).</li><li>SQL code inside of Java: <br></li><ol><li>Beside of the fact that a multi-line concatenated string that cannot be easily copied and run with psql, it means that we should compile the code in order to test the change (vs building with DEV_REBUILD=0 which only package the SQL file).</li><li>No syntax highlighting when performing code review. i.e. I don't think reviewing a patch such as <a href="https://gerrit.ovirt.org/#/c/66729/10/packaging/dbscripts/network_sp.sql" target="_blank">https://gerrit.ovirt.org/#/c/6<wbr>6729/10/packaging/dbscripts/ne<wbr>twork_sp.sql</a> would be more clear inside a java file.</li><li>The user permissions management is implemented on DB level. That means that SQL will be more complex (more concatenated java lines). </li></ol><li>Stored procedure are cached by project's code. See SimpleJdbcCallsHandler.getCall<wbr>(), while the NamedParameterJdbcTemplate are cached by spring's code which seems less optimal (sync all calls using synchronized vs using ConcurrentHashMap as in SP cache).</li><li>With the NamedParametersJdbcTemplate there is no use of the DbEngineDialect. What's the impact of it ? </li></ol><div>So besides 5 and 6, the rest is a matter of style. I'd like to hear more opinions from other members.<br></div></div></div></div></blockquote></span><div><br><div style="font-size:large" class="gmail_default">I agree with all you wrote Moti<br></div><div style="font-size:large" class="gmail_default">I don't think that looking in SQL in Java code is clear than looking in a SP code <br></div><div style="font-size:large" class="gmail_default">Additionally <br></div><div style="font-size:large" class="gmail_default">1) You will pass more data on the wire instead of calling a SP with parameters <br></div><div style="font-size:large" class="gmail_default">2) Your data that is passed on the wire is exposed to attacks since you will have to implement DB security in the engine level (for example hidden columns)<br></div><div style="font-size:large" class="gmail_default">3) Changes in the SQL code done in patches may be more complicated to track <br></div><div style="font-size:large" class="gmail_default">4) SQL Injection <br></div><div style="font-size:large" class="gmail_default">5) I think that SP performs better than SQL inside Java <br></div><div style="font-size:large" class="gmail_default"><br></div><div style="font-size:large" class="gmail_default">I see no real reason to replace the SPs with SQL code , SP is just a container for SQL code <br></div><br> </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Regards,<br></div><div>Moti<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span><div><div><div><div>So I'd like to propose moving towards this approach in general in all cases when nothing beyond a simple SQL is needed (no stored procedure programming language is needed).<br>From my experience with the performance improvement task it looks like people avoid adding new queries for a specific need of a flow, instead they use the existing general ones (e.g. dao.getAllForX()) and do the actual join in the bll code.<br>IMHO the proposed approach would simplify adding new specific queries and by that would prevent a decent part of performance issues in the future.<br><br></div><div>I do not propose changing all existing SP's to inline queries in a once, but to allow adding new queries this way. Also we might consider converting relatively simple SP's to inline SQL statements later in a graduate way.<br></div><br>[1] - <a href="https://gerrit.ovirt.org/#/c/74456" target="_blank">https://gerrit.ovirt.org/#/c/7<wbr>4456</a><br>[2] - <a href="https://gerrit.ovirt.org/#/c/74458" target="_blank">https://gerrit.ovirt.org/#/c/7<wbr>4458</a><br><br></div>Regards,<br></div>Yevgeny<br></div>
<br></span><span>______________________________<wbr>_________________<br>
Devel mailing list<br>
<a href="mailto:Devel@ovirt.org" target="_blank">Devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/mailman<wbr>/listinfo/devel</a><br></span></blockquote></div><span class="m_4651098172073966859HOEnZb"><font color="#888888"><br></font></span></div><span class="m_4651098172073966859HOEnZb"><font color="#888888"><div class="gmail_extra"><br clear="all"><br>-- <br><div class="m_4651098172073966859m_4110374462987635130gmail-m_1038891330517396707gmail_signature"><div dir="ltr"><div>Regards,<br></div>Moti<br></div></div>
</div></font></span></div>
<br>______________________________<wbr>_________________<br>
Devel mailing list<br>
<a href="mailto:Devel@ovirt.org" target="_blank">Devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/mailman<wbr>/listinfo/devel</a><br></blockquote></span></div><br></div></div>
</blockquote></div><br></div>