{"id":9023,"date":"2016-10-06T15:32:03","date_gmt":"2016-10-06T13:32:03","guid":{"rendered":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/"},"modified":"2016-10-06T15:32:03","modified_gmt":"2016-10-06T13:32:03","slug":"fun-with-plsql-code-reviews-part-1","status":"publish","type":"post","link":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/","title":{"rendered":"Fun with PL\/SQL code reviews &#8211; Part 1"},"content":{"rendered":"<p>For quite a long time I did not work anymore with <a href=\"https:\/\/docs.oracle.com\/cloud\/latest\/db112\/LNPLS\/toc.htm\" target=\"_&quot;blank&quot;\" rel=\"noopener\">PL\/SQL<\/a> and was quite happy when I had the chance to review some code at a customer. The status today: I am not that happy anymore \ud83d\ude41 Let me explain why and show you some examples on what was discovered. Of course all of the identifiers have been obfuscated and this is not to blame anyone. It is more to make people aware of what you should not do and that you have to be as exact as possible when you do programming. Even more important: Code that you write must be maintainable and understandable by others.<\/p>\n<p><!--more--><\/p>\n<p>We start with a simple &#8220;if-then-else-end if&#8221; block:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n         IF c &lt;= d\n         THEN\n            IF c = d\n            THEN\n               l2 := l2 || l3;\n            ELSE\n               l2 := l2 || l3 || ',';\n            END IF;\n         END IF;\n<\/pre>\n<p>So what is wrong with this? The code does what it is supposed to do, true. But this is more complicated than it needs to be. Lets look at the first line:<\/p>\n<pre class=\"brush: bash; gutter: true; first-line: 1\">\n         IF c &lt;= d\n<\/pre>\n<p>If we enter this &#8220;IF&#8221; then we know that c is either less than d or equal to d, correct? On line 3 we know that c is d if we enter that &#8220;IF&#8221;. What does this imply for line 6 (the &#8220;ELSE&#8221;)?<br \/>\nIt implies that c is less than d when we enter the &#8220;ELSE&#8221;. But then we could also write it like this:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n            IF c = d\n            THEN\n               l2 := l2 || l3;\n            ELSIF c &lt; d THEN\n               l2 := l2 || l3 || &#039;,&#039;;\n            END IF;\n<\/pre>\n<p>This is much more clear and less code. We are only interested if c is equal to d or less than d, that&#8217;s it. Then we should design the code exactly for that use case. <\/p>\n<p>The next example is about the usage of a record. This is the code block: <\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n         ...\n\n         ll_col1 SCHEMA.TABLE.COLUMN1%TYPE;\n         ll_col2 SCHEMA.TABLE.COLUMN2%TYPE;\n         ll_col3 SCHEMA.TABLE.COLUMN3%TYPE;\n         ll_col4 SCHEMA.TABLE.COLUMN4%TYPE;\n         ...\n         DECLARE\n            TYPE MyRecord IS RECORD\n            (\n               l_col1    SCHEMA.TABLE.COLUMN1%TYPE,\n               l_col2    SCHEMA.TABLE.COLUMN2%TYPE,\n               l_col3    SCHEMA.TABLE.COLUMN3%TYPE,\n               l_col4    SCHEMA.TABLE.COLUMN4%TYPE\n            );\n            rec1   MyRecord;\n         BEGIN\n            v_sql := 'SELECT col1, col2, col3, col4 FROM SCHEMA.TABLE WHERE col3 = '''\n               || ll_col3\n               || '''';\n \n            EXECUTE IMMEDIATE v_sql INTO rec1;\n\n            ll_col1 := rec1.l_col1;\n            ll_col2 := rec1.l_col2;\n            ll_col3 := rec1.l_col3;\n            ll_col4 := rec1.l_col4;\n         EXCEPTION\n            WHEN NO_DATA_FOUND\n            THEN\n               ll_col3 := 0;\n         END;\n<\/pre>\n<p>Beside that declaring a new block inside an existing block is really ugly what is wrong here? There is the definition of a new record which is then used to fetch the data from the dynamic sql statement. Nothing wrong here. But then the fields of this record are written back to four variables which are valid in and outside of that block, why that? Probably because the record is only valid in the &#8220;declare-begin-end&#8221; block. But for what do I need the record then at all? Really confusing \ud83d\ude42<\/p>\n<p>The next one is more about how easy it is to understand code written by someone else. This is the code block:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n      SELECT COUNT (*)\n        INTO l_count\n        FROM USER_INDEXES\n       WHERE INDEX_NAME = '' || l_index_name || '';\n\n      IF l_count &gt; 0\n      THEN\n         EXECUTE IMMEDIATE 'DROP INDEX ' || '' || l_index_name || '';\n      END IF;\n<\/pre>\n<p>I really had to read this carefully until I understood why it was written this way. Probably the developer had this intension: &#8220;I want to know if a specific index exists and if yes, then I want to drop it&#8221;.<br \/>\nBut what he did actually code is: I want to know if there are multiple indexes with the same name. As the query is against user_tables the answer can not be more than 1, correct? But then the correct and much more easy to understand way of writing this would be:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n      SELECT COUNT (*)\n        INTO l_count\n        FROM USER_INDEXES\n       WHERE INDEX_NAME = '' || l_index_name || '';\n\n      IF l_count = 1\n      THEN\n         EXECUTE IMMEDIATE 'DROP INDEX ' || '' || l_index_name || '';\n      END IF;\n<\/pre>\n<p>This is just a small change and does not affect the functionality at all but it is much more clear. Even more clear would be something like this:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n      BEGIN\n        SELECT 'index_exists'\n          INTO l_exists\n          FROM USER_INDEXES\n         WHERE INDEX_NAME = '' || l_index_name || '';\n      EXCEPTION\n        WHEN NO_DATA_FOUND THEN l_exists := 'index_does_not_exist';\n      END;\n      IF l_exists = 'index_exists'\n      THEN\n         EXECUTE IMMEDIATE 'DROP INDEX ' || '' || l_index_name || '';\n      END IF;\n<\/pre>\n<p>Using our language we can make the code much more easy to understand. Beside that the concatenation in the where clause and in the execute immediate statement is not required so the final code could look like this:<\/p>\n<pre class=\"brush: sql; gutter: true; first-line: 1\">\n      BEGIN\n        SELECT 'index_exists'\n          INTO l_exists\n          FROM USER_INDEXES\n         WHERE INDEX_NAME = l_index_name;\n      EXCEPTION\n        WHEN NO_DATA_FOUND THEN l_exists := 'index_does_not_exist';\n      END;\n      IF l_exists = 'index_exists'\n      THEN\n         EXECUTE IMMEDIATE 'DROP INDEX ' || l_index_name;\n      END IF;\n<\/pre>\n<p>In the next post we&#8217;ll look at some other examples which can be improved by changing the way we think about what we code and how others may interpret it.<\/p>\n","protected":false},"excerpt":{"rendered":"<p>For quite a long time I did not work anymore with PL\/SQL and was quite happy when I had the chance to review some code at a customer. The status today: I am not that happy anymore \ud83d\ude41 Let me explain why and show you some examples on what was discovered. Of course all of [&hellip;]<\/p>\n","protected":false},"author":29,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[229],"tags":[96,24],"type_dbi":[],"class_list":["post-9023","post","type-post","status-publish","format-standard","hentry","category-database-administration-monitoring","tag-oracle","tag-pl-sql"],"acf":[],"yoast_head":"<!-- This site is optimized with the Yoast SEO Premium plugin v27.2 (Yoast SEO v27.2) - https:\/\/yoast.com\/product\/yoast-seo-premium-wordpress\/ -->\n<title>Fun with PL\/SQL code reviews - Part 1 - dbi Blog<\/title>\n<meta name=\"robots\" content=\"index, follow, max-snippet:-1, max-image-preview:large, max-video-preview:-1\" \/>\n<link rel=\"canonical\" href=\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\" \/>\n<meta property=\"og:locale\" content=\"en_US\" \/>\n<meta property=\"og:type\" content=\"article\" \/>\n<meta property=\"og:title\" content=\"Fun with PL\/SQL code reviews - Part 1\" \/>\n<meta property=\"og:description\" content=\"For quite a long time I did not work anymore with PL\/SQL and was quite happy when I had the chance to review some code at a customer. The status today: I am not that happy anymore \ud83d\ude41 Let me explain why and show you some examples on what was discovered. Of course all of [&hellip;]\" \/>\n<meta property=\"og:url\" content=\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\" \/>\n<meta property=\"og:site_name\" content=\"dbi Blog\" \/>\n<meta property=\"article:published_time\" content=\"2016-10-06T13:32:03+00:00\" \/>\n<meta name=\"author\" content=\"Daniel Westermann\" \/>\n<meta name=\"twitter:card\" content=\"summary_large_image\" \/>\n<meta name=\"twitter:creator\" content=\"@westermanndanie\" \/>\n<meta name=\"twitter:label1\" content=\"Written by\" \/>\n\t<meta name=\"twitter:data1\" content=\"Daniel Westermann\" \/>\n\t<meta name=\"twitter:label2\" content=\"Est. reading time\" \/>\n\t<meta name=\"twitter:data2\" content=\"4 minutes\" \/>\n<script type=\"application\/ld+json\" class=\"yoast-schema-graph\">{\"@context\":\"https:\/\/schema.org\",\"@graph\":[{\"@type\":\"Article\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#article\",\"isPartOf\":{\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\"},\"author\":{\"name\":\"Daniel Westermann\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66\"},\"headline\":\"Fun with PL\/SQL code reviews &#8211; Part 1\",\"datePublished\":\"2016-10-06T13:32:03+00:00\",\"mainEntityOfPage\":{\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\"},\"wordCount\":569,\"commentCount\":0,\"keywords\":[\"Oracle\",\"PL\/SQL\"],\"articleSection\":[\"Database Administration &amp; Monitoring\"],\"inLanguage\":\"en-US\",\"potentialAction\":[{\"@type\":\"CommentAction\",\"name\":\"Comment\",\"target\":[\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#respond\"]}]},{\"@type\":\"WebPage\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\",\"url\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\",\"name\":\"Fun with PL\/SQL code reviews - Part 1 - dbi Blog\",\"isPartOf\":{\"@id\":\"https:\/\/www.dbi-services.com\/blog\/#website\"},\"datePublished\":\"2016-10-06T13:32:03+00:00\",\"author\":{\"@id\":\"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66\"},\"breadcrumb\":{\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#breadcrumb\"},\"inLanguage\":\"en-US\",\"potentialAction\":[{\"@type\":\"ReadAction\",\"target\":[\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/\"]}]},{\"@type\":\"BreadcrumbList\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#breadcrumb\",\"itemListElement\":[{\"@type\":\"ListItem\",\"position\":1,\"name\":\"Accueil\",\"item\":\"https:\/\/www.dbi-services.com\/blog\/\"},{\"@type\":\"ListItem\",\"position\":2,\"name\":\"Fun with PL\/SQL code reviews &#8211; Part 1\"}]},{\"@type\":\"WebSite\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/#website\",\"url\":\"https:\/\/www.dbi-services.com\/blog\/\",\"name\":\"dbi Blog\",\"description\":\"\",\"potentialAction\":[{\"@type\":\"SearchAction\",\"target\":{\"@type\":\"EntryPoint\",\"urlTemplate\":\"https:\/\/www.dbi-services.com\/blog\/?s={search_term_string}\"},\"query-input\":{\"@type\":\"PropertyValueSpecification\",\"valueRequired\":true,\"valueName\":\"search_term_string\"}}],\"inLanguage\":\"en-US\"},{\"@type\":\"Person\",\"@id\":\"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66\",\"name\":\"Daniel Westermann\",\"image\":{\"@type\":\"ImageObject\",\"inLanguage\":\"en-US\",\"@id\":\"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g\",\"url\":\"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g\",\"contentUrl\":\"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g\",\"caption\":\"Daniel Westermann\"},\"description\":\"Daniel Westermann is Principal Consultant and Technology Leader Open Infrastructure at dbi services. He has more than 15 years of experience in management, engineering and optimization of databases and infrastructures, especially on Oracle and PostgreSQL. Since the beginning of his career, he has specialized in Oracle Technologies and is Oracle Certified Professional 12c and Oracle Certified Expert RAC\/GridInfra. Over time, Daniel has become increasingly interested in open source technologies, becoming \u201cTechnology Leader Open Infrastructure\u201d and PostgreSQL expert. \u00a0Based on community or EnterpriseDB tools, he develops and installs complex high available solutions with PostgreSQL. He is also a certified PostgreSQL Plus 9.0 Professional and a Postgres Advanced Server 9.4 Professional. He is a regular speaker at PostgreSQL conferences in Switzerland and Europe. Today Daniel is also supporting our customers on AWS services such as AWS RDS, database migrations into the cloud, EC2 and automated infrastructure management with AWS SSM (System Manager). He is a certified AWS Solutions Architect Professional. Prior to dbi services, Daniel was Management System Engineer at LC SYSTEMS-Engineering AG in Basel. Before that, he worked as Oracle Developper &amp;\u00a0Project Manager at Delta Energy Solutions AG in Basel (today Powel AG). Daniel holds a diploma in Business Informatics (DHBW, Germany). His branch-related experience mainly covers the pharma industry, the financial sector, energy, lottery and telecommunications.\",\"sameAs\":[\"https:\/\/x.com\/westermanndanie\"],\"url\":\"https:\/\/www.dbi-services.com\/blog\/author\/daniel-westermann\/\"}]}<\/script>\n<!-- \/ Yoast SEO Premium plugin. -->","yoast_head_json":{"title":"Fun with PL\/SQL code reviews - Part 1 - dbi Blog","robots":{"index":"index","follow":"follow","max-snippet":"max-snippet:-1","max-image-preview":"max-image-preview:large","max-video-preview":"max-video-preview:-1"},"canonical":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/","og_locale":"en_US","og_type":"article","og_title":"Fun with PL\/SQL code reviews - Part 1","og_description":"For quite a long time I did not work anymore with PL\/SQL and was quite happy when I had the chance to review some code at a customer. The status today: I am not that happy anymore \ud83d\ude41 Let me explain why and show you some examples on what was discovered. Of course all of [&hellip;]","og_url":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/","og_site_name":"dbi Blog","article_published_time":"2016-10-06T13:32:03+00:00","author":"Daniel Westermann","twitter_card":"summary_large_image","twitter_creator":"@westermanndanie","twitter_misc":{"Written by":"Daniel Westermann","Est. reading time":"4 minutes"},"schema":{"@context":"https:\/\/schema.org","@graph":[{"@type":"Article","@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#article","isPartOf":{"@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/"},"author":{"name":"Daniel Westermann","@id":"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66"},"headline":"Fun with PL\/SQL code reviews &#8211; Part 1","datePublished":"2016-10-06T13:32:03+00:00","mainEntityOfPage":{"@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/"},"wordCount":569,"commentCount":0,"keywords":["Oracle","PL\/SQL"],"articleSection":["Database Administration &amp; Monitoring"],"inLanguage":"en-US","potentialAction":[{"@type":"CommentAction","name":"Comment","target":["https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#respond"]}]},{"@type":"WebPage","@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/","url":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/","name":"Fun with PL\/SQL code reviews - Part 1 - dbi Blog","isPartOf":{"@id":"https:\/\/www.dbi-services.com\/blog\/#website"},"datePublished":"2016-10-06T13:32:03+00:00","author":{"@id":"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66"},"breadcrumb":{"@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#breadcrumb"},"inLanguage":"en-US","potentialAction":[{"@type":"ReadAction","target":["https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/"]}]},{"@type":"BreadcrumbList","@id":"https:\/\/www.dbi-services.com\/blog\/fun-with-plsql-code-reviews-part-1\/#breadcrumb","itemListElement":[{"@type":"ListItem","position":1,"name":"Accueil","item":"https:\/\/www.dbi-services.com\/blog\/"},{"@type":"ListItem","position":2,"name":"Fun with PL\/SQL code reviews &#8211; Part 1"}]},{"@type":"WebSite","@id":"https:\/\/www.dbi-services.com\/blog\/#website","url":"https:\/\/www.dbi-services.com\/blog\/","name":"dbi Blog","description":"","potentialAction":[{"@type":"SearchAction","target":{"@type":"EntryPoint","urlTemplate":"https:\/\/www.dbi-services.com\/blog\/?s={search_term_string}"},"query-input":{"@type":"PropertyValueSpecification","valueRequired":true,"valueName":"search_term_string"}}],"inLanguage":"en-US"},{"@type":"Person","@id":"https:\/\/www.dbi-services.com\/blog\/#\/schema\/person\/8d08e9bd996a89bd75c0286cbabf3c66","name":"Daniel Westermann","image":{"@type":"ImageObject","inLanguage":"en-US","@id":"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g","url":"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g","contentUrl":"https:\/\/secure.gravatar.com\/avatar\/31350ceeecb1dd8986339a29bf040d4cd3cd087d410deccd8f55234466d6c317?s=96&d=mm&r=g","caption":"Daniel Westermann"},"description":"Daniel Westermann is Principal Consultant and Technology Leader Open Infrastructure at dbi services. He has more than 15 years of experience in management, engineering and optimization of databases and infrastructures, especially on Oracle and PostgreSQL. Since the beginning of his career, he has specialized in Oracle Technologies and is Oracle Certified Professional 12c and Oracle Certified Expert RAC\/GridInfra. Over time, Daniel has become increasingly interested in open source technologies, becoming \u201cTechnology Leader Open Infrastructure\u201d and PostgreSQL expert. \u00a0Based on community or EnterpriseDB tools, he develops and installs complex high available solutions with PostgreSQL. He is also a certified PostgreSQL Plus 9.0 Professional and a Postgres Advanced Server 9.4 Professional. He is a regular speaker at PostgreSQL conferences in Switzerland and Europe. Today Daniel is also supporting our customers on AWS services such as AWS RDS, database migrations into the cloud, EC2 and automated infrastructure management with AWS SSM (System Manager). He is a certified AWS Solutions Architect Professional. Prior to dbi services, Daniel was Management System Engineer at LC SYSTEMS-Engineering AG in Basel. Before that, he worked as Oracle Developper &amp;\u00a0Project Manager at Delta Energy Solutions AG in Basel (today Powel AG). Daniel holds a diploma in Business Informatics (DHBW, Germany). His branch-related experience mainly covers the pharma industry, the financial sector, energy, lottery and telecommunications.","sameAs":["https:\/\/x.com\/westermanndanie"],"url":"https:\/\/www.dbi-services.com\/blog\/author\/daniel-westermann\/"}]}},"_links":{"self":[{"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/posts\/9023","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/users\/29"}],"replies":[{"embeddable":true,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/comments?post=9023"}],"version-history":[{"count":0,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/posts\/9023\/revisions"}],"wp:attachment":[{"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/media?parent=9023"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/categories?post=9023"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/tags?post=9023"},{"taxonomy":"type","embeddable":true,"href":"https:\/\/www.dbi-services.com\/blog\/wp-json\/wp\/v2\/type_dbi?post=9023"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}