0

I'm working with an ERP database that stores a lot of data in multi-year tables. (Each new year a new table is created to hold the data for that year) We have a requirement to be able to query and report on some of these tables. I am currently using views but the views are just getting bigger and bigger year after year and slower. I created a pipelined table function that executes some dynamic sql and queries the appropriate tables based on a from and to date passed in as parameters. I can call the pipelined function from normal SQL and it works fine. However, the goal is to be able to re-use the table function(s) in many different stored procedures and join with other data. The reporting system we use requires that we use stored procedures returning ref cursors.

I created a test function and test stored procedure (simplified versions for brevity) to attempt to return the table function as a cursor but i'm getting an error when I execute the procedure (PLS-00382: expression is of wrong type). I'm not even sure if a pipeline function can be accessed by a procedure but i've done similar things in SQL Server so there's got to be some way. I've searched hi and low but can't really find anyone having the exact same situation. Please see my code.

Following is the user defined types i've created in my schema:

CREATE OR REPLACE TYPE PUCCONNECT.wo_trans_type AS OBJECT
       (GL_YEAR             INT,
        SUBSYSTEM           VARCHAR2(2)
        );

CREATE OR REPLACE TYPE PUCCONNECT.wo_trans_table_test AS TABLE OF PUCCONNECT.wo_trans_type;

Following is the function and procedure declarations. The tables i'm selecting from in the function GL101Txx have many columns so i'm only selecting the first 2 to keep things simple. Those first 2 columns have the same definition as the columns defined in my user-defined object "wo_trans_type "

CREATE OR REPLACE FUNCTION PUCCONNECT.WO_MULTIYEAR_TEST(fromdate date, todate date) 
         RETURN WO_TRANS_TABLE_TEST PIPELINED IS
TYPE            ref0 IS REF CURSOR;
cur0            ref0;
v_year_start    int;
v_year_end      int;
out_rec         wo_trans_type
            := wo_trans_type(NULL,NULL);

BEGIN

    v_year_start := EXTRACT(year FROM fromdate);
    v_year_end := EXTRACT(year FROM todate);

    FOR yearNumber in v_year_start..v_year_end LOOP

        OPEN cur0 FOR

             'SELECT ' || yearNumber || ' "gl_year", GL.SUBSYSTEM

                 FROM fmsdata.GL101T' || SUBSTR(to_char(yearNumber), 3,2) || ' GL

                WHERE (GL.transaction_date BETWEEN ''' || fromdate || ''' AND ''' || todate || ''')';                          

                LOOP
                    FETCH cur0 INTO out_rec.gl_year, out_rec.subsystem;

                    EXIT WHEN cur0 %NOTFOUND;
                    PIPE ROW(out_rec);
                END LOOP;
                CLOSE cur0;

    END LOOP;

    RETURN;
END WO_MULTIYEAR_TEST;

And here is the procedure where I attempt to consume the function:

CREATE OR REPLACE PROCEDURE PUCCONNECT."SP_WO_TRANS_PA" (
    --table_out out wo_trans_table,
    wo_trans_cursor out sys_refcursor
 )
 AS 

BEGIN
           OPEN wo_trans_cursor FOR

           SELECT   gl_year, subsystem
           FROM     TABLE( PUCCONNECT.WO_MULTIYEAR_TEST('01-jan-2019', '05-may-2019'));

END;

Does anyone know if this is possible through a stored procedure? Is it possible with a non-pipelined table function? Any suggestions or advice for the best method of achieving this and maintaining performance is appreciated.

Here is the full error returned by TOAD when I try and execute the procedure:

[Error] ORA-06550: line 12, column 12:
PLS-00382: expression is of wrong type
ORA-06550: line 12, column 6:
PL/SQL: Statement ignored
 (1: 0): >> DECLARE
    -- Declarations
    l_WO_TRANS_CURSOR   SYS_REFCURSOR;
BEGIN
    -- Call
    PUCCONNECT.SP_WO_TRANS_PA (WO_TRANS_CURSOR => l_WO_TRANS_CURSOR);

    -- Transaction Control
    COMMIT;

    -- Output values, do not modify
     :1 := l_WO_TRANS_CURSOR;
END;
Error at line 1
ORA-06550: line 12, column 12:
PLS-00382: expression is of wrong type
ORA-06550: line 12, column 6:
PL/SQL: Statement ignored

Here is how i'm calling it in TOAD:

DECLARE
    -- Declarations
    l_WO_TRANS_CURSOR   SYS_REFCURSOR;
BEGIN
    -- Call
    PUCCONNECT.SP_WO_TRANS_PA (WO_TRANS_CURSOR => l_WO_TRANS_CURSOR);

    -- Transaction Control
    COMMIT;

    -- Output values, do not modify
     :1 := l_WO_TRANS_CURSOR;
END;
PlasmaX
  • 15
  • 5
  • 1
    Could you edit your question and add table definitions (ddl if possible) of the table involved and also some sample rows from that table? It becomes easier for us to simulate your problem that way rather than guessing. – Kaushik Nayak May 10 '19 at 13:40
  • Also, do add definitions of the type `WO_TRANS_TABLE_TEST` – Kaushik Nayak May 10 '19 at 13:46
  • What you're doing is allowed, and [works with guessed object definitions](https://dbfiddle.uk/?rdbms=oracle_11.2&fiddle=fe84335c0a1dc127d6c3bfd09b3f90d1). So either your objects aren't defined how I've guessed (which is quite likely), or by simplifying the code you've hidden the actual problem. As well as all the DDL, please include the full error stack you get, and clarify whether you get that on compilation of the procedure or when you call the procedure (and if the latter, *how* you call it). – Alex Poole May 10 '19 at 14:35
  • Ok i've added the definitions of the the user-defined types. Alex Poole - I will add that the function works perfectly when I call it from an SQL editor window in TOAD like this: `SELECT gl_year, subsystem FROM TABLE( PUCCONNECT.WO_MULTIYEAR_TEST('01-jan-2019', '05-may-2019'));` The stored procedure compiles successfully. I get the error while executing the procedure from TOAD. – PlasmaX May 10 '19 at 14:37
  • Again, show how you're executing it, and the full error stack. If the problem is coming from the call then we kind of need to see that. (It works [with your type definitions](https://dbfiddle.uk/?rdbms=oracle_11.2&fiddle=65b0180a7850aaae6c7861038c4af95d) too; so I guess you're calling it differently?) – Alex Poole May 10 '19 at 14:44
  • Added the error code to the bottom of the original post. I'm just executing it (stored procedure) through TOAD Script Runner. – PlasmaX May 10 '19 at 14:47
  • The error is pointing to the `:1 := l_WO_TRANS_CURSOR;` in the final anonymous block; which means it is nothing to do with the rest of the code you've shown really. How is Toad defining that bind variable - what data type is it? It needs to also be a ref cursor for this to work - the error means it is something else (e.g. `varchar2`, which I think the default). – Alex Poole May 10 '19 at 14:53
  • You're passing in strings but the function has date parameters. Now I suspect the strings would attempt to be implicitly converted, but regardless, passing strings that should really be dates is not a good thing. Try using `to_date(...)` to convert the string into a date with the appropriate format mask. – Boneist May 10 '19 at 14:58
  • Nothing to do with the error, but completely agree *8-) – Alex Poole May 10 '19 at 14:59
  • You're right it's something to do with the way toad is calling the procedure. I just tested the procedure from Visual Studio and it works - although it's very slow. I'm not sure why - the function runs very quickly when called by itself (returning 54,000 rows) – PlasmaX May 10 '19 at 15:07
  • Ok I've added to_date to convert the strings to dates - it was working before that though. I'm just wondering if a pipelined function is the best way to achieve what I want. Is their a better way of doing this? – PlasmaX May 10 '19 at 15:29
  • I wonder why you've got a table for each year rather than partitions in a single table? Is it because your database is XE, or do you not have the licence for partitions? (or maybe, as it's ERP, it's a design thing forced upon you?) – Boneist May 10 '19 at 15:37
  • The database is not created by be ... it's from an ERP that was created in the 80s and they just haven't updated their DB schema to be more modern. – PlasmaX May 10 '19 at 15:55

1 Answers1

0

You can rewrite the whole thing in one procedure, something like:

CREATE OR REPLACE PROCEDURE PUCCONNECT."SP_WO_TRANS_PA"(fromdate        IN DATE,
                                                        todate          IN DATE,
                                                        wo_trans_cursor OUT SYS_REFCURSOR) AS
  v_year_start INT;
  v_year_end   INT;

  v_sql CLOB;
  c_union_all CONSTANT VARCHAR2(9) := 'union all';
  v_table_append VARCHAR2(38);
  v_column_append VARCHAR2(10);
BEGIN

  v_year_start := extract(YEAR FROM fromdate);
  v_year_end   := extract(YEAR FROM todate);

  FOR yearnumber IN v_year_start .. v_year_end
  LOOP
    IF yearnumber != v_year_start
    THEN
      v_sql := v_sql || chr(10) || c_union_all || chr(10) ||;
    END IF;

    -- to avoid any sql injection due to the to_char (just in case)
    v_table_append := dbms_assert.qualified_sql_name('fmsdata.GL101T' || substr(to_char(yearnumber), 3, 2));
    v_column_append := dbms_assert.enquote_literal(to_char(yearnumber));

    v_sql := v_sql || 'select to_number(' || v_column_append || ') gl_year, gl.subsystem ' || chr(10)
                   || 'from   v_table_append' || chr(10)
                   || 'where  gl.transaction_date between :fromdate and :todate';

  END LOOP;

  v_sql := v_sql || chr(10) || 'order by 1, 2';

  OPEN wo_trans_cursor FOR v_sql
    USING fromdate, todate;

END sp_wo_trans_pa;
/

This loops through the years and generates the sql to run in the cursor, before opening the cursor with the relevant bind variables.

I have used bind variables where possible and, where it wasn't possible, dbms_assert to sanitize the concatenated values to avoid any SQL injection vulnerabilities.

Boneist
  • 22,910
  • 1
  • 25
  • 40
  • I like that approach a lot. I didn't think of doing it that way - just build your sql query with unions for all the relevant years and open as a cursor. I'll try this and see what the performance gains are. The only think I liked about the table functions is that they can be reused in many procedures and joined to other data. – PlasmaX May 10 '19 at 15:53
  • You might be able to create a view with the union all’s and and table pruning may be able to take place when you query the view – Boneist May 10 '19 at 15:58
  • Or you could throw a pipelined function around the procedure passing. – Boneist May 10 '19 at 15:59
  • I've already been using views with union all and the views are getting large and need to be updated every year. If I query the views a certain way the performance isn't horrible but not great either. I wanted to get away from doing it that way. – PlasmaX May 10 '19 at 16:45