tag:blogger.com,1999:blog-7849367040589270673.post6899302976157616124..comments2024-03-21T22:50:39.997-07:00Comments on Obsessed with Oracle PL/SQL: Code cleanup: post-authentication in Application ExpressSteven Feuersteinhttp://www.blogger.com/profile/18405765731886460622noreply@blogger.comBlogger10125tag:blogger.com,1999:blog-7849367040589270673.post-36755441766330257382015-04-22T06:57:38.711-07:002015-04-22T06:57:38.711-07:00Great idea, Chris - and not only because I thought...Great idea, Chris - and not only because I thought of it first. :-) I like how it offers a neat implementation in this example. I was thinking (and did bring this up with Bryn Llewellyn as an enhancement) of adding an INDEX BY clause similar to the TYPE statement for creating a collation type as in:<br /><br />SELECT <br />BULK COLLECT INTO <br />INDEX BY <br /><br />I believe this is an elegant extension from the current syntax and would simplify any number of scenarios for PL/SQL programmers. <br /><br />Having said that, I cannot speak to when or if this might actually appear in the PL/SQL language.<br />Steven Feuersteinhttps://www.blogger.com/profile/18405765731886460622noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-25072692449106030702015-04-22T05:53:16.314-07:002015-04-22T05:53:16.314-07:00Steven
When we discussed this at the Collaborate ...Steven<br /><br />When we discussed this at the Collaborate CodeTalk session someone mentioned using bulk collect instead of the for loop.<br /><br />Thinking about this a bit more, I realised it would be cool if bulk collect allowed you to define your own indicies for the target collection. In this case you could index by varchar2, with the role names as the indicies. You could then assign the variables by referencing the role name.<br /><br />The code for this would look something like this:<br /><br />select case when au.role_name is null then 'FALSE' else 'TRUE' end<br />bulk collect into user_roles<br />indicies rs.r<br />from (select 'ADMINISTRATOR' r from dual union all<br /> select 'CONTRIBUTOR' r from dual union all<br /> select 'TESTER' r from dual union all<br /> select 'REPORTING' r from dual union all<br /> select 'SUPER ADMINISTRATOR' r from dual) roles rs<br />left join app_users au<br />on rs.r = au.role_name;<br /><br />admin_out := user_roles('ADMINISTRATOR');<br />contributor_out := user_roles('CONTRIBUTOR');<br />tester_out := user_roles('TESTER');<br />reporting_out := user_roles('REPORTING');<br />superadmin_out := user_roles('SUPER ADMINISTRATOR');<br /><br />What do you think? Does being able to define your own indicies for bulk collect have value?Chris Saxonhttps://www.blogger.com/profile/09485645419920814104noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-84716880737259212732015-03-27T06:50:19.531-07:002015-03-27T06:50:19.531-07:00Thanks, Kim. Very thoughtful and balanced response...Thanks, Kim. Very thoughtful and balanced response. And, for the record, I am perfectly fine with not having even a single nested subprogram if it's already quite understandable (and lots of the logic has been pushed into SQL).Steven Feuersteinhttps://www.blogger.com/profile/18405765731886460622noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-28341775761380503772015-03-27T06:21:10.291-07:002015-03-27T06:21:10.291-07:00Actually, no, not in this case.
In this case the ...Actually, no, not in this case.<br /><br />In this case the extremely minor benefit of 1 row with 5 columns instead of 5 rows of 1 column does *not* justify the complexity of pivoting the SQL. That one I just wrote as learning example to give a method that might be useful for larger cases :-) <br /><br />It even just might (haven't tested it, though) make the SQL engine work more than the work saved for the PL/SQL engine - in this simple case. There can easily be more complex cases where it makes more sense to move logic from PL/SQL to SQL, but this one isn't one of them.<br /><br />But the solution with one FOR loop and a CASE statement I believe is fully justified as it turns 5 SQL calls into 1 - even though it does not use a self-documenting local procedure ;-) It's quite understandable as is.<br /><br />I'm a SQL guy so I'll always at least *think* about how I could possibly do *anything* in a single SQL statement. But yes, I can easily fall into the trap of adding unnecessary complexity in order to shave milliseconds of a mixed SQL + PL/SQL solution. Plus the complex SQL has a higher risk of plan *in*stability due to being more difficult to optimize.<br /><br />It's always a tradeoff and it's a matter of finding the best balance between SQL and PL/SQL :-)Kim Berg Hansenhttps://www.blogger.com/profile/06491635470794828550noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-40943591436111758362015-03-26T12:59:55.062-07:002015-03-26T12:59:55.062-07:00So, Kim and others: do you feel that the additiona...So, Kim and others: do you feel that the additional complexity of your second solution - to remove the CASE from PL/SQL is justified? Seems like at a minimum one would want to turn such solutions into a "learning moment" by adding comments so that anyone working on it later would (a) know what the heck is going on and (b) be inspired to use it elsewhere.Steven Feuersteinhttps://www.blogger.com/profile/18405765731886460622noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-24610340275940889702015-03-26T05:20:07.644-07:002015-03-26T05:20:07.644-07:00Steven, here is some code + an add. comment :
If...Steven, here is some code + an add. comment : <br />If the APEX application is to be used frequently & that db is 11+, I will maybe choose to keep your function with app_user in its parameters and cache-enabled the function. <br />But if we start using caching (result cache, smart flash cache or even in-memory to drastically speed things) for all little things we will completely stop taking care of our code, don't we ? ;)<br /><br />Code:<br /><br />PROCEDURE post_auth_7048783549465082709<br />IS<br />BEGIN<br /><br /> :is_admin:='FALSE';<br /> :is_contributor:='FALSE';<br /> :is_tester:='FALSE';<br /> :is_reporting:='FALSE';<br /> :is_superadmin:='FALSE';<br /><br />for c1 in (SELECT role_name<br /> FROM app_users<br /> WHERE username = :app_user)<br /> loop<br /> CASE <br /> WHEN c1.role_name='ADMINISTRATOR' THEN :is_admin:='TRUE';<br /> WHEN c1.role_name='CONTRIBUTOR' THEN :is_contributor:='TRUE';<br /> WHEN c1.role_name='TESTER' THEN :is_tester:='TRUE';<br /> WHEN c1.role_name='REPORTING' THEN :is_reporting:='TRUE';<br /> ELSE c1.role_name='SUPER ADMINISTRATOR' THEN :is_superadmin:='TRUE';<br /> END;<br /> end loop;<br /><br /> is_employee :=<br /> CASE<br /> WHEN SUBSTR (LOWER ( :app_user), INSTR ( :app_user, '@') + 1) =<br /> 'our_domain.com'<br /> THEN<br /> 'TRUE'<br /> ELSE<br /> 'FALSE'<br /> END;<br />END;Jean-Marc Desvauxhttps://www.blogger.com/profile/02800246730586030258noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-91092203280393248482015-03-26T01:55:31.722-07:002015-03-26T01:55:31.722-07:00(Part 2:)
An alternative eliminates the need for ...(Part 2:)<br /><br />An alternative eliminates the need for the CASE logic in PL/SQL and moves all logic into SQL. It has the advantage of only fetching a single row of 5 columns instead of 5 rows of 1 column, but at the cost of a more complex and less readable SQL:<br /><br /><br />CREATE OR REPLACE PROCEDURE app_post_authenticate (<br /> app_user_in IN VARCHAR2,<br /> admin_out OUT VARCHAR2,<br /> contributor_out OUT VARCHAR2,<br /> tester_out OUT VARCHAR2,<br /> reporting_out OUT VARCHAR2,<br /> super_admin_out OUT VARCHAR2,<br /> is_employee_out OUT VARCHAR2)<br />IS<br />BEGIN<br /> APEX_UTIL.RESET_AUTHORIZATIONS;<br /><br /> begin<br /> select case admin_exist when 0 then 'FALSE' else 'TRUE' end<br /> , case contributor_exist when 0 then 'FALSE' else 'TRUE' end<br /> , case tester_exist when 0 then 'FALSE' else 'TRUE' end<br /> , case reporting_exist when 0 then 'FALSE' else 'TRUE' end<br /> , case superadmin_exist when 0 then 'FALSE' else 'TRUE' end<br /> into admin_out<br /> , contributor_out<br /> , tester_out<br /> , reporting_out<br /> , superadmin_out<br /> from (<br /> select role_name<br /> from app_users<br /> where username = app_user_in<br /> and role_name in (<br /> 'ADMINISTRATOR'<br /> , 'CONTRIBUTOR'<br /> , 'TESTER'<br /> , 'REPORTING'<br /> , 'SUPER ADMINISTRATOR'<br /> )<br /> )<br /> pivot (<br /> count(*) as exist<br /> for role_name in (<br /> 'ADMINISTRATOR' as admin<br /> , 'CONTRIBUTOR' as contributor<br /> , 'TESTER' as tester<br /> , 'REPORTING' as reporting<br /> , 'SUPER ADMINISTRATOR' as superadmin<br /> )<br /> ) <br /> ;<br /> exception<br /> when no_data_found then<br /> admin_out := 'FALSE';<br /> contributor_out := 'FALSE';<br /> tester_out := 'FALSE';<br /> reporting_out := 'FALSE';<br /> superadmin_out := 'FALSE';<br /> end;<br /> <br /> is_employee_out :=<br /> CASE<br /> WHEN SUBSTR (LOWER (app_user_in), INSTR (app_user_in, '@') + 1) =<br /> 'our_domain.com'<br /> THEN<br /> 'TRUE'<br /> ELSE<br /> 'FALSE'<br /> END;<br />END;<br /><br /><br />(An aggregate function is needed in order to PIVOT, hence the count(*) - but as we know we have a unique role_name here, it hardly matters ;-)<br /><br />/KimKim Berg Hansenhttps://www.blogger.com/profile/06491635470794828550noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-44754816160157724292015-03-26T01:54:36.514-07:002015-03-26T01:54:36.514-07:00Hi, Steven
Very good points you make in this post...Hi, Steven<br /><br />Very good points you make in this post. But like Jean-Marc I'll also add that when you have code that have to repeatedly call the same SQL, I'll try to look for ways to avoid repeated SQL calls and replace it with single SQL calls.<br /><br /><br />I'll have to assume that (username, role_name) is unique in the table? Otherwise your revised code would have to worry about TOO_MANY_ROWS or add a AND ROWNUM <= 1.<br /><br />So given that assumption, a couple ways to have only one SELECT statement.<br /><br />First like Jean-Marc describes:<br /><br /><br />CREATE OR REPLACE PROCEDURE app_post_authenticate (<br /> app_user_in IN VARCHAR2,<br /> admin_out OUT VARCHAR2,<br /> contributor_out OUT VARCHAR2,<br /> tester_out OUT VARCHAR2,<br /> reporting_out OUT VARCHAR2,<br /> super_admin_out OUT VARCHAR2,<br /> is_employee_out OUT VARCHAR2)<br />IS<br />BEGIN<br /> APEX_UTIL.RESET_AUTHORIZATIONS;<br /><br /> admin_out := 'FALSE';<br /> contributor_out := 'FALSE';<br /> tester_out := 'FALSE';<br /> reporting_out := 'FALSE';<br /> superadmin_out := 'FALSE';<br /><br /> for role in (<br /> select role_name<br /> from app_users<br /> where username = app_user_in<br /> and role_name in (<br /> 'ADMINISTRATOR'<br /> , 'CONTRIBUTOR'<br /> , 'TESTER'<br /> , 'REPORTING'<br /> , 'SUPER ADMINISTRATOR'<br /> )<br /> ) loop<br /> case role.role_name<br /> when 'ADMINISTRATOR' then<br /> admin_out := 'TRUE';<br /> when 'CONTRIBUTOR' then<br /> contributor_out := 'TRUE';<br /> when 'TESTER' then<br /> tester_out := 'TRUE';<br /> when 'REPORTING' then<br /> reporting_out := 'TRUE';<br /> when 'SUPER ADMINISTRATOR' then<br /> superadmin_out := 'TRUE';<br /> end case;<br /> end loop;<br /><br /> is_employee_out :=<br /> CASE<br /> WHEN SUBSTR (LOWER (app_user_in), INSTR (app_user_in, '@') + 1) =<br /> 'our_domain.com'<br /> THEN<br /> 'TRUE'<br /> ELSE<br /> 'FALSE'<br /> END;<br />END;<br /><br /><br />Nice and simple and readable :-)<br /><br />(Alternative comes in a seperate reply due to 4096 char limit.)<br /><br /><br />/Kim<br />Kim Berg Hansenhttps://www.blogger.com/profile/06491635470794828550noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-84849819739749574432015-03-25T13:07:31.813-07:002015-03-25T13:07:31.813-07:00I like that, Jean-Marc. Here's the only proble...I like that, Jean-Marc. Here's the only problem I see: we have a hard-coded list of application items. What happens if there is a role that does not match up to one of those? And how would associate the value fetched from the cursor FOR loop to the item? Care to post some code? :-) Steven Feuersteinhttps://www.blogger.com/profile/18405765731886460622noreply@blogger.comtag:blogger.com,1999:blog-7849367040589270673.post-48196515159531984632015-03-25T12:59:53.819-07:002015-03-25T12:59:53.819-07:00Steven, to optimise furher : Instead of calling a ...Steven, to optimise furher : Instead of calling a function for each role I would set all to FALSE and use a cursor for all roles for app user and in the loop use "case" to set TRUE for roles returned. This way you have only one select statement executed instead of one per role.Jean-Marc Desvauxhttps://www.blogger.com/profile/02800246730586030258noreply@blogger.com