Usability/flexibility of API

[jgebal]

I have few observations, nice to have features for the API.

1. the insert API should return whole row (rowtype), not just PK column. This way we allow API to be generated even if there is no PK or the PK is multi-column. When used with default values to insert record the default values, it gives additional benefit - you get the whole inserted row, so you don't need to cal the get_row API separately
2. what is the reason/value for using procedure with out %rowtype parameter? Having the function seems sufficient and much more readable when you use it in your code.
3. there is API created: get_pk_by_unique_cols, not sure what will happen if i have more than one set of unique cols. Can we have control over generation of those functions?
4. if i call the api with option p_col_prefix_in_method_names => FALSE causes the generator to with error:

Got the following error: ORA-20000: The prefix of your column names (example: prefix_rest_of_column_name) is not unique and you requested to cut off the prefix for method names. Please ensure either your column names have a unique prefix or switch the parameter p_col_prefix_in_method_names to true (SQL Developer oddgen integration: check option "Keep column prefix in method names").
ORA-06512: at "A571293.OM_TAPIGEN", line 1606
ORA-06512: at "A571293.OM_TAPIGEN", line 2310
ORA-06512: at "A571293.OM_TAPIGEN", line 2350
ORA-06512: at "A571293.OM_TAPIGEN", line 2566
ORA-06512: at "A571293.OM_TAPIGEN_ODDGEN_WRAPPER", line 124
ORA-06512: at line 13

You can reference procedure parameters with procedure name, that way you don't need to prefix column_name parameters. This will also let you avoid the common issue of variable name length > 30 char, when column name length = 30 char.

  PROCEDURE set_JOB_ID( EMPLOYEE_ID IN EMPLOYEES."EMPLOYEE_ID"%TYPE, JOB_ID IN EMPLOYEES."JOB_ID"%TYPE )
  IS
    v_row EMPLOYEES%ROWTYPE;
  BEGIN
    v_row := read_row ( EMPLOYEE_ID => set_JOB_ID.EMPLOYEE_ID );
    IF v_row."EMPLOYEE_ID" IS NOT NULL THEN
      -- update only, if the column value really differs
      IF COALESCE( v_row."JOB_ID", '@@@@@@@@@@@@@@@' ) <> COALESCE( set_JOB_ID.JOB_ID, '@@@@@@@@@@@@@@@' ) THEN
        UPDATE EMPLOYEES x
           SET x."JOB_ID" = set_JOB_ID.JOB_ID
         WHERE x."EMPLOYEE_ID" = set_JOB_ID.EMPLOYEE_ID;
      END IF;
    END IF;
  END set_JOB_ID;

[jgebal]

In order to enable generation of random rows something like the below could be used.
It enables creation of a row in table only by specifying the relevant attributes.

There is very little usability coming with procedures using out parameters. They actually require explicit declaration of each attribute returned. It would be great to be able to disable generation of those. Maybe that should be default or disabled at all?

Likewise, the get_[column_name] are of little value as the read_row gives you access to both whole row as well as individual attribute so you can write employees_api.read_row(1).job_id

Please let me know what you think about implementing something like below.
Thanks

CREATE OR REPLACE PACKAGE employees_api
IS
   /**
    * This is the API for the table EMPLOYEES.
    *
    * GENERATION OPTIONS
    * - must be in the lines 5-25 to be reusable by the generator
    * - DO NOT TOUCH THIS until you know what you do - read the
    *   docs under github.com/OraMUC/table-api-generator ;-)
    * <options
    *   generator=OM_TAPIGEN
    *   generator_version=0.4.0
    *   generator_action=GET_CODE
    *   generated_at=2017-05-27 13:05:19
    *   generated_by=HR
    *   p_table_name=EMPLOYEES
    *   p_reuse_existing_api_params=TRUE
    *   p_col_prefix_in_method_names=TRUE
    *   p_enable_insertion_of_rows=TRUE
    *   p_enable_update_of_rows=FALSE
    *   p_enable_deletion_of_rows=TRUE
    *   p_enable_generic_change_log=FALSE
    *   p_enable_dml_view=FALSE
    *   p_sequence_name=EMPLOYEES_SEQ/>
    *
    * This API provides DML functionality that can be easily called from APEX.
    * Target of the table API is to encapsulate the table DML source code for
    * security (UI schema needs only the execute right for the API and the
    * read/write right for the EMPLOYEES_dml_v, tables can be hidden in
    * extra data schema) and easy readability of the business logic (all DML is
    * then written in the same style). For APEX automatic row processing like
    * tabular forms you can optionally use the EMPLOYEES_dml_v, which has
    * an instead of trigger who is also calling the EMPLOYEES_api.
    */
   ----------------------------------------
   FUNCTION row_exists (p_employee_id IN employees.employee_id%TYPE)
      RETURN BOOLEAN;

   ----------------------------------------
   FUNCTION row_exists_yn (p_employee_id IN employees.employee_id%TYPE)
      RETURN VARCHAR2;

   ----------------------------------------
   FUNCTION get_a_row RETURN employees%ROWTYPE;
   ----------------------------------------
   FUNCTION create_row (
      p_employee_id    IN employees.employee_id%TYPE DEFAULT get_a_row().employee_id,
      p_first_name     IN employees.first_name%TYPE DEFAULT get_a_row().first_name,
      p_last_name      IN employees.last_name%TYPE DEFAULT get_a_row().last_name,
      p_email          IN employees.email%TYPE DEFAULT get_a_row().email,
      p_phone_number   IN employees.phone_number%TYPE DEFAULT get_a_row().phone_number,
      p_hire_date      IN employees.hire_date%TYPE DEFAULT get_a_row().hire_date,
      p_job_id         IN employees.job_id%TYPE DEFAULT get_a_row().job_id,
      p_salary         IN employees.salary%TYPE DEFAULT get_a_row().salary,
      p_commission_pct IN employees.commission_pct%TYPE DEFAULT get_a_row().commission_pct,
      p_manager_id     IN employees.manager_id%TYPE DEFAULT get_a_row().manager_id,
      p_department_id  IN employees.department_id%TYPE DEFAULT get_a_row().department_id
   )
      RETURN employees%ROWTYPE;

   ----------------------------------------
   PROCEDURE create_row (
      p_employee_id    IN employees.employee_id%TYPE DEFAULT get_a_row().employee_id,
      p_first_name     IN employees.first_name%TYPE DEFAULT get_a_row().first_name,
      p_last_name      IN employees.last_name%TYPE DEFAULT get_a_row().last_name,
      p_email          IN employees.email%TYPE DEFAULT get_a_row().email,
      p_phone_number   IN employees.phone_number%TYPE DEFAULT get_a_row().phone_number,
      p_hire_date      IN employees.hire_date%TYPE DEFAULT get_a_row().hire_date,
      p_job_id         IN employees.job_id%TYPE DEFAULT get_a_row().job_id,
      p_salary         IN employees.salary%TYPE DEFAULT get_a_row().salary,
      p_commission_pct IN employees.commission_pct%TYPE DEFAULT get_a_row().commission_pct,
      p_manager_id     IN employees.manager_id%TYPE DEFAULT get_a_row().manager_id,
      p_department_id  IN employees.department_id%TYPE DEFAULT get_a_row().department_id
   );

   ----------------------------------------
   FUNCTION create_row (p_row IN employees%ROWTYPE)
      RETURN employees%ROWTYPE;

   ----------------------------------------
   PROCEDURE create_row (p_row IN employees%ROWTYPE);

   ----------------------------------------
   FUNCTION read_row (p_employee_id IN employees.employee_id%TYPE DEFAULT NULL)
      RETURN employees%ROWTYPE;

   ----------------------------------------
   PROCEDURE delete_row (p_employee_id IN employees.employee_id%TYPE);

----------------------------------------
END employees_api;
/

CREATE OR REPLACE PACKAGE BODY employees_api
IS
   ----------------------------------------
   FUNCTION row_exists (p_employee_id IN employees.employee_id%TYPE)
      RETURN BOOLEAN
   IS
      v_return BOOLEAN := FALSE;
   BEGIN
      FOR i IN (
                  SELECT 1
                    FROM employees
                   WHERE employee_id = p_employee_id
               ) LOOP
         v_return := TRUE;
      END LOOP;

      RETURN v_return;
   END;

   ----------------------------------------
   FUNCTION row_exists_yn (p_employee_id IN employees.employee_id%TYPE)
      RETURN VARCHAR2
   IS
   BEGIN
      RETURN CASE WHEN row_exists (p_employee_id => p_employee_id) THEN 'Y' ELSE 'N' END;
   END;

   ----------------------------------------
   FUNCTION create_row (
      p_employee_id    IN employees.employee_id%TYPE DEFAULT get_a_row().employee_id,
      p_first_name     IN employees.first_name%TYPE DEFAULT get_a_row().first_name,
      p_last_name      IN employees.last_name%TYPE DEFAULT get_a_row().last_name,
      p_email          IN employees.email%TYPE DEFAULT get_a_row().email,
      p_phone_number   IN employees.phone_number%TYPE DEFAULT get_a_row().phone_number,
      p_hire_date      IN employees.hire_date%TYPE DEFAULT get_a_row().hire_date,
      p_job_id         IN employees.job_id%TYPE DEFAULT get_a_row().job_id,
      p_salary         IN employees.salary%TYPE DEFAULT get_a_row().salary,
      p_commission_pct IN employees.commission_pct%TYPE DEFAULT get_a_row().commission_pct,
      p_manager_id     IN employees.manager_id%TYPE DEFAULT get_a_row().manager_id,
      p_department_id  IN employees.department_id%TYPE DEFAULT get_a_row().department_id
   )
      RETURN employees%ROWTYPE
   IS
      v_pk  employees.employee_id%TYPE;
      v_row employees%ROWTYPE;
   BEGIN
      v_pk := COALESCE (p_employee_id, employees_seq.NEXTVAL);

      INSERT INTO employees (
                     employee_id,
                     first_name,
                     last_name,
                     email,
                     phone_number,
                     hire_date,
                     job_id,
                     salary,
                     commission_pct,
                     manager_id,
                     department_id
                  )
           VALUES (
                     v_pk,
                     p_first_name,
                     p_last_name,
                     p_email,
                     p_phone_number,
                     p_hire_date,
                     p_job_id,
                     p_salary,
                     p_commission_pct,
                     p_manager_id,
                     p_department_id
                  )
        RETURNING employee_id,
                  first_name,
                  last_name,
                  email,
                  phone_number,
                  hire_date,
                  job_id,
                  salary,
                  commission_pct,
                  manager_id,
                  department_id
             INTO v_row;

      RETURN v_row;
   END create_row;

   ----------------------------------------
   PROCEDURE create_row (
      p_employee_id    IN employees.employee_id%TYPE DEFAULT get_a_row().employee_id,
      p_first_name     IN employees.first_name%TYPE DEFAULT get_a_row().first_name,
      p_last_name      IN employees.last_name%TYPE DEFAULT get_a_row().last_name,
      p_email          IN employees.email%TYPE DEFAULT get_a_row().email,
      p_phone_number   IN employees.phone_number%TYPE DEFAULT get_a_row().phone_number,
      p_hire_date      IN employees.hire_date%TYPE DEFAULT get_a_row().hire_date,
      p_job_id         IN employees.job_id%TYPE DEFAULT get_a_row().job_id,
      p_salary         IN employees.salary%TYPE DEFAULT get_a_row().salary,
      p_commission_pct IN employees.commission_pct%TYPE DEFAULT get_a_row().commission_pct,
      p_manager_id     IN employees.manager_id%TYPE DEFAULT get_a_row().manager_id,
      p_department_id  IN employees.department_id%TYPE DEFAULT get_a_row().department_id
   )
   IS
      v_row employees%ROWTYPE;
   BEGIN
      v_row      :=
         create_row (
            p_employee_id    => p_employee_id,
            p_first_name     => p_first_name,
            p_last_name      => p_last_name,
            p_email          => p_email,
            p_phone_number   => p_phone_number,
            p_hire_date      => p_hire_date,
            p_job_id         => p_job_id,
            p_salary         => p_salary,
            p_commission_pct => p_commission_pct,
            p_manager_id     => p_manager_id,
            p_department_id  => p_department_id
         );
   END create_row;

   ----------------------------------------
   FUNCTION create_row (p_row IN employees%ROWTYPE)
      RETURN employees%ROWTYPE
   IS
      v_row employees%ROWTYPE;
   BEGIN
      v_row      :=
         create_row (
            p_employee_id    => p_row.employee_id,
            p_first_name     => p_row.first_name,
            p_last_name      => p_row.last_name,
            p_email          => p_row.email,
            p_phone_number   => p_row.phone_number,
            p_hire_date      => p_row.hire_date,
            p_job_id         => p_row.job_id,
            p_salary         => p_row.salary,
            p_commission_pct => p_row.commission_pct,
            p_manager_id     => p_row.manager_id,
            p_department_id  => p_row.department_id
         );
      RETURN v_row;
   END create_row;

   ----------------------------------------
   PROCEDURE create_row (p_row IN employees%ROWTYPE)
   IS
      v_row employees%ROWTYPE;
   BEGIN
      v_row      :=
         create_row (
            p_employee_id    => p_row.employee_id,
            p_first_name     => p_row.first_name,
            p_last_name      => p_row.last_name,
            p_email          => p_row.email,
            p_phone_number   => p_row.phone_number,
            p_hire_date      => p_row.hire_date,
            p_job_id         => p_row.job_id,
            p_salary         => p_row.salary,
            p_commission_pct => p_row.commission_pct,
            p_manager_id     => p_row.manager_id,
            p_department_id  => p_row.department_id
         );
   END create_row;

   ----------------------------------------
   FUNCTION read_row (p_employee_id IN employees.employee_id%TYPE DEFAULT NULL)
      RETURN employees%ROWTYPE
   IS
      v_row employees%ROWTYPE;
   BEGIN
      IF p_employee_id IS NOT NULL THEN
         SELECT *
           INTO v_row
           FROM employees
          WHERE employee_id = p_employee_id;
      ELSE
         SELECT *
           INTO v_row
           FROM employees
          WHERE ROWNUM = 1;
      END IF;

      RETURN v_row;
   END read_row;

   ----------------------------------------
   PROCEDURE delete_row (p_employee_id IN employees.employee_id%TYPE)
   IS
   BEGIN
      DELETE FROM employees
            WHERE employee_id = p_employee_id;
   END delete_row;

   ----------------------------------------
   FUNCTION get_a_row RETURN employees%ROWTYPE
   IS
      v_row employees%ROWTYPE;
   BEGIN
      v_row.employee_id := employees_seq.nextval; --generated from SEQ
      v_row.first_name :=  'Chuck';
      v_row.last_name := 'Norris';
      v_row.email := substr(sys_guid(),1,25); --generged as there is a UK on that column
      v_row.phone_number := '123';
      v_row.hire_date := to_date('2017-05-27','YYYY-MM-DD');
      v_row.job_id := 'FI_MGR';
      v_row.salary := 123;
      v_row.commission_pct := 0.01;
      v_row.manager_id := null;
      v_row.department_id := null;
      RETURN v_row;
  END;
END employees_api;
/

[ogobrecht]

Hi Jacek,

to answer some of your questions I edited the issue #6 (change questions from checkmark to ordered list)

Regarding number 1 - the insert API should return whole row...

I personally think this is a good idea. I had no idea that something like this is possible: employees_api.create_row(v.row).job_id. So there should only small changes in existing code. The only thing is, that we must document this change very clear for the users of our generator.

Regarding number 2 - what is the reason/value for using procedure with out %rowtype parameter?

I can't find a generated procedure with such an out type in the example-api. Can you please make clear your question?

Regarding number 3 - there is API created: get_pk_by_unique_cols, not sure what will happen if i have more than one set of unique cols. Can we have control over generation of those functions?

This function is created per unique key (overloeded) - I have such tables in some of my applications ;-)

Regarding number 4 - if i call the api with option p_col_prefix_in_method_names => FALSE causes the generator to throw an error: The prefix of your column names (example: prefix_rest_of_column_name) is not unique and you requested to cut off the prefix for method names. Please ensure either your column names have a unique prefix or switch the parameter p_col_prefix_in_method_names to true (SQL Developer oddgen integration: check option "Keep column prefix in method names").

I answered the behaviour a little bit in your issue #8. The column prefix is derived automatically - in the example below the it would be EMP. If not all columns have the same prefix then it would be null. If you tell the generator to cut of the prefix (p_col_prefix_in_method_names => false) and you have not the same prefix in all columns then the generator throws an error to enforce you to rethink your decision. Otherwise it could be that the generator cuts of the the first text of the name until the first underscore (PHONE_NUMBER would become SET_NUMBER or get GET_NUMBER) on some method names and others are untouched because they have no underscore in it (SET_SALARY or GET_SALERY for example). At the end your API looks not like you expected.

I know - you don't want the getter and setter at all, but this is another story ;-)

CREATE TABLE "HR"."EMPLOYEES" (
    "EMP_ID"               NUMBER(6,0),
    "EMP_FIRST_NAME"       VARCHAR2(20 BYTE),
    "EMP_LAST_NAME"        VARCHAR2(25 BYTE),
    "EMP_EMAIL"            VARCHAR2(25 BYTE),
    "EMP_PHONE_NUMBER"     VARCHAR2(20 BYTE),
    "EMP_HIRE_DATE"        DATE,
    "EMP_JOB_ID"           VARCHAR2(10 BYTE),
    "EMP_SALARY"           NUMBER(8,2),
    "EMP_COMMISSION_PCT"   NUMBER(2,2),
    "EMP_MANAGER_ID"       NUMBER(6,0),
    "EMP_DEPARTMENT_ID"    NUMBER(4,0)
);

Your second comment regarding random row_exists

I personally prefer to have a special function called read_random_row instead of a read_row function, which reads a random row, if the input parameter is null. The reason for this is, that I have also logic in my applications where it's not clear, if we could find a row with get_pk_by_unique_cols function, which is used for the input parameter of the read_row function (v_row := read_row(get_pk_by_unique_cols(...))). In that case I expect to get an empty record and not a random one.

I find the idea to have a read_random_row function nice. Could be useful in other cases.

And yes, you are right, the getter and setter are not that useful, but in some cases...

... in short, parameterizing this is a good idea and may be omitting this stuff is also ok because there is a alternative as you mentioned already with the row type and the dot notation...

Good, that we currently in a early stage with the v0.4.1 ;-)

I have to discuss many things with André now...

Best regards

[jgebal]

Thanks for your answer,
On the dot notation, you would not believe what is possible with Oracle PL/SQL when you really try :)
for example the syntax like:
ut.expect( 1 ).to_( equal(1) );
or
ut.expect( lv_refcursor ).not_to( be_empty );

I've discovered some magic of PLSQL within last 2 years or so.

[ogobrecht]

Hi Jacek,

as already noted in issue #3 the new version 0.5.0 is out. I close this issue now - returning the rowtype is implemented as an option and the questions are answered. If you have any further comments or questions please use the remaining issue #3.

Best regards
Ottmar