3

This is my original procedure that works beautifully:

create or replace PROCEDURE EXTRACT_0_CPP AS
    CURSOR c_data IS
        SELECT cpp,
               rfu1,
               rfu2,
               mean_rfu,
               charge_ph7_4,
               hydropathy
        FROM   cpp
        ORDER BY LENGTH(cpp);
    F1 UTL_FILE.FILE_TYPE;

BEGIN 
    F1 := UTL_FILE.FOPEN( location => 'EXTRACT_DIR',
                          filename => '0_cpp.TXT',
                          open_mode => 'w',
                          max_linesize => 32767);
    FOR cur_rec IN c_data LOOP 
        UTL_FILE.PUT_LINE (F1, 
                            cur_rec.cpp || ':' ||
                            cur_rec.rfu1 || ':' ||
                            cur_rec.rfu2 || ':' ||
                            cur_rec.mean_rfu || ':' ||
                            cur_rec.charge_ph7_4 || ':' ||
                            cur_rec.hydropathy);                     
    END LOOP;
    UTL_FILE.FCLOSE(F1);
END;

But SQL Developer gives me a squiggly red line under SELECT and suggests I change it to this:

create or replace PROCEDURE EXTRACT_0_CPP AS
    CURSOR c_data IS
        SELECT
    "A1"."CPP"         "CPP",
    "A1"."RFU1"           "RFU1",
    "A1"."RFU2"           "RFU2",
    "A1"."MEAN_RFU"       "MEAN_RFU",
    "A1"."CHARGE_PH7_4"   "CHARGE_PH7_4",
    "A1"."HYDROPATHY"     "HYDROPATHY"
FROM
    "C##ELLIE"."CPP" "A1"
ORDER BY
    length("A1"."CPP");
    F1 UTL_FILE.FILE_TYPE;

BEGIN 
    F1 := UTL_FILE.FOPEN( location => 'EXTRACT_DIR',
                          filename => '0_cpp.TXT',
                          open_mode => 'w',
                          max_linesize => 32767);
    FOR cur_rec IN c_data LOOP 
        UTL_FILE.PUT_LINE (F1, 
                            cur_rec.pk_cpp || ':' ||
                            cur_rec.rfu1 || ':' ||
                            cur_rec.rfu2 || ':' ||
                            cur_rec.mean_rfu || ':' ||
                            cur_rec.charge_ph7_4 || ':' ||
                            cur_rec.hydropathy);                     
    END LOOP;
    UTL_FILE.FCLOSE(F1);
END;

My question is (why) is this better? What is "A1"?

0

4 Answers 4

4

A1 is the alias for the table "C##ELLIE"."CPP"

procedure is the same, but so you and oracle know to which schema the table cpp belongs too.

Additional you should also add a comment what the procedure makes, if you come back in 3 years it is simpler to know, which schema you used and what it is for

5
  • Oh ok, thank you! Is it possible for a table to switch owners and therefore be in a different schema? Would this also be helpful if another person is trying to run this procedure, but they have their own table "cpp"? I'm super new to this, so i'm just trying to understand when this would be helpful. And yes, a comment explaining what this is is a good idea, thank you! Last question. If I decided to rename a table in sql developer, would it be smart enough to know to also adjust all the procedures that reference it? That sounds like it would be too good to be a feature. Aug 20, 2020 at 13:18
  • 1
    a rdms must know where every element is positioned and as you can acess every schema and table and so on that you have rights to, you must identify it, as you notoced. I don't believe that it will change anything automatically this could lead to massive problems otherwise, but feel free to test it
    – nbk
    Aug 20, 2020 at 13:30
  • No, renaming a table will not be reflected in any code that references the table. It will simply make the code invalid until it is corrected and recompiled. Also renaming a table "in sqldeveloper" is no different that renaming it from a command line in sqlplus. SQL Dev is just a gui client, exactly like sqlplus is a cli client. They both send the SQL statement to the server to be processed on the server. The only diff being that SQL Dev, depending on what you are doing, can build the actual SQL for you. But he still sends it to the database for processing.
    – EdStevens
    Aug 20, 2020 at 17:03
  • 1
    All objects referred to in the procedure will be in the same schema as the procedure, unless they are synonyms, or the procedure is declared using invoker's rights i.e. with authid current_user, which is a bit specialised and not the default. So it's not useful to hardcode the schema. Aug 20, 2020 at 17:19
  • 1
    Oh wow, hardcoding schema. So it will be a nightmare reusing these procedures in a different schema. Very bad advice
    – edc65
    Aug 21, 2020 at 6:22
5

On the subject of why it would suggest a different format when the code already works, of course it's possible to have poorly formatted code that works, so in principle there is no reason why you shouldn't consider layout or refactoring improvements that might make the code more robust, efficient, or easier to maintain.

However, double-quoted names are poor practice because they hide naming errors and force you to specify upper/lowercase, and the result is less readable than the original. They are really a portability feature for use with third party applications that have weird table names. Code generators tend to slap double quotes around everything because it's easier to do that than to detect whether they are actually needed in each case. There is no way that

from "C##ELLIE"."CPP" "A1"

is any sort of improvement on

from cpp

except perhaps for the use of a table alias. It's a good idea to give the table an alias (without double-quotes!) and use it when referring to columns. But "A1" (or even a1) is the kind of table alias that only a computer would dream up. It would be far better to use an alias that is some kind of abbreviation of the table name, perhaps in this case c. Imagine if you were joining six tables - now which one is a3? Oh right, that's ORDER_DETAILS. (Though in this case the table name is so short that there is no point aliasing it.) So,

SELECT "A1"."CPP" "CPP"
FROM   "C##ELLIE"."CPP" "A1"

would be far better as just

SELECT c.cpp
FROM   cpp c

(I'd also lowercase it because this isn't 1974 and my editor highlights language keywords using colours and bold, but we'll let that go.)

Hardcoding schema names is poor practice because it's at best redundant (the object is in the schema you are already working in so it adds nothing except needless complication) or worse, it limits portability (if you ever rename the schema or move the code you'll have to go through it cleaning up the hardcoded references).

I'm sure this is a clever feature that means well, but in this instance it's not good advice.

0
4

Here's a demo of what's wrong with using double-quotes around object names. Read each command carefully and pay attention to case-sensitivity of the table names, quoted and un-quoted.

SQL> create table test_table_1 (dob date);

Table created.

SQL> create table "test_table_2" ("dob" date);

Table created.

SQL> select * from test_table_1;

no rows selected

SQL> select * from test_table_2;
select * from test_table_2
              *
ERROR at line 1:
ORA-00942: table or view does not exist


SQL> select * from "test_table_2";

no rows selected

SQL> 
SQL> select table_name
  2  from user_tables
  3  where upper(table_name) like 'TEST_TABLE%'
  4  order by table_name;

TABLE_NAME
--------------------------------------------------------------------------------
TEST_TABLE_1
test_table_2

2 rows selected.

SQL> 
SQL> drop table test_table_1;

Table dropped.

SQL> drop table test_table_2;
drop table test_table_2
           *
ERROR at line 1:
ORA-00942: table or view does not exist


SQL> drop table "test_table_2";

Table dropped.

SQL> --
SQL> spo off
1
  • I've also seen things like "CUSTOMER ID", buried in a long DDL script where everything was uppercase and double-quoted. The missing underscore was unintended but the script worked, and it would have gone into production if I hadn't spotted it. Aug 21, 2020 at 14:27
1

I'd probably make a recommendation of this for your code:

...
    CURSOR c_data IS
        SELECT c.cpp,
               c.rfu1,
               c.rfu2,
               c.mean_rfu,
               c.charge_ph7_4,
               c.hydropathy
        FROM   cpp c
        ORDER BY LENGTH(c.cpp);
...

We're only using one table/view, cpp so aliasing it isn't strictly necessary, but it would be helpful if in future another table is added to this query.

Particularly with 2+ table queries, if the names in the SELECT are not fully qualified (mention a table name or alias before the column name) then queries can break if columns are added to tables, that have the same name as another column in another table:

PersonTable: id, name, status, addressid
AddressTable id, street

--this is ok for now
SELECT name, status, street FROM person INNER JOIN street ON person.addressid = address.id

If we add a Status column to Address, the above query fails because it's now ambiguous. Adding a column that causes this kind of problem isn't going to make the DB announce "I added it, but by the way all these other queries throughout your schema and dependent systems are now gonna fail" - you'll just have to find those failing queries by testing. Or customers complaining ;)

If we'd fully qualified it:

SELECT 
  person.name, 
  person.status, 
  address.street 
FROM 
  person 
  INNER JOIN adress a ON person.addressid = address.id

It would carry on working..

Repeating the table name with the columns is a bit tedious and wordy. Using a shorter name not only improves the readability..

SELECT 
  p.name, 
  p.status, 
  a.street 
FROM 
  person p
  INNER JOIN address a ON p.addressid = a.id

..but psychologically reminds us that we can join tables in multiple times for different reasons:

 SELECT 
  p.name, 
  p.status, 
  a_work.street as workstreet,
  a_home.street as homestreet
FROM 
  person p
  INNER JOIN address a_work ON p.work_addressid = a.id --note, i've silently upgraded Person here to track two addresses
  INNER JOIN address a_home ON p.home_addressid = a.id

All in, SQLDeveloper is trying to do a good thing here in that it's moving towards the good sense of:

  • Give your tables a sensible, readable, contextually relevant alias
  • Always fully qualify your column names with the alias

Where it's falling down is:

  • It's picking a crap alias name A1, that doesn't help you remember anything; it's no accident that I picked p for person and a for address, as I'm sure you can appreciate. If there was a Party, and a Project to join in I might use per, pro and par. I'd avoid pr because Person, Party and Project all have p and r as relevant initial consonants in the word, so pr doesn't shout "it's an alias for PRoject" as obviously as using three letters does (but I would certainly accept you arguing for pe, pa and pr if you want to save a few keystrokes :) )
  • It's blindly ramming double quotes in everywhere, probably "for safety" but also because it's the path of least resistance - It's a lot easier to code a logic of blindly adding quotes like builder.addcolumn( '"' || alias_name || '"."' || col_name || '",') than it is to inspect a column name and see if it might need quoting and only add them if required. This unfortunately means the code ends up an unreadable mess of " everywhere..
  • .. and leading on from that "just blindly quote everything" is "and then make all the identifiers ALL CAPS, because right now the table/col names are not case sensitive. SELECT pErSon.NaME ... is fine; even though the table.column is just PERSON.NAME it isn't case sens.. But when we've blindly added quotes, we then absolutely have to put the names in all caps, because adding the quotes makes them be treated in a case sensitive way! SELECT "pErSon"."NaME" just won't work, so your carefully written out, and beautifully readable* lower case identifiers are gone..

SQLDeveloper really could go to all that introspection and logic of working out what needed to be quoted, whether its because of funky chars, spaces, case sens etc.. But it doesn't - it's taking safe and simple to code approaches like "just quote it", "just uppercase it" and "just create an alias that is some random/incremental unique thing" and that's unfortunately a bad recommendation, though the spirit of some of what it's trying is a good idea

*as kids we learn lowercase before uppercase; we're always faster at reading lowercase sentences than uppercase ones

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.