0

Trying to write a procedure which takes no values in, adds a sale price column to my existing product table, then loops through to calculate a sale price and insert that into the new column.

I haven't been able to get anything to work, I think it's something to do with Oracle not liking ALTER TABLE to be run from inside a procedure, but I don't know, and I don't know enough to direct my attempts anywhere else.

This is my attempt

CREATE or REPLACE PROCEDURE ProductLineSale as
BEGIN
    DECLARE
       NewSalePrice NUMBER(6,2):=0; 
    EXECUTE IMMEDIATE 'alter table ' || Product || 'add or replace column' || 'SalePrice NUMBER(6,2);'
    FOR p in (SELECT ProductStandardPrice FROM Product
            group by ProductStandardPrice)
        LOOP
            CASE WHEN p.ProductStandardPrice>=400 THEN NewSalePrice:=.9*price
                 WHEN p.ProductStandardPrice<400 THEN NewSalePrice:=.85*price
            INSERT INTO Product(SalePrice)
            VALUES(NewSalePrice)
        END LOOP;
END ProductLineSale

Product is the literal name of the Product table in my database. SalePrice is what I would like the new column to be named.

SQLDeveloper won't compile the procedure. The error I get is fairly cryptic as well:

Error(2,10): PLS-00103: Encountered the symbol "=" when expecting one of the following: constant exception table long double ref char time timestamp interval date binary national character nchar.

4
  • I don't think that the issue is with ALTER TABLE, as there is no = in that statement. Look immediately above there, where there is an equals sign. Why do you need to ALTER TABLE at all in a stored procedure, though? That should be a one-off operation that you would do outside the stored procedure, not something that has to happen every time the stored procedure is executed. I also don't see any need for you to be using string concatenation at all, as your product table name will never change, so it can be hard-coded directly into the SQL itself. I think this is an XY problem. Commented Nov 7, 2020 at 23:51
  • I agree that placing ALTER TABLE inside a procedure is bad form.....however this is an assignment for class and one of the stipulations for our function/procedure is that it adds the new column as part of its operation. This same teacher stipulates that we comment every SQL command we run. I'm certainly in a bit over my head with this task, but SQL seems like a language that documents itself very well when it comes to simple stuff like creating tables and inserting values into rows. Commented Nov 8, 2020 at 0:14
  • Honestly I don't know what sql developer thinks the issue is. I have tried variations of the above incantation, sql developer moves the red error line around, but the compiler spits out the same error message. Every example I can find on various websites uses the = sign as I am as well so I dunno. Commented Nov 8, 2020 at 0:18
  • Do they use := in a DECLARE statement, though? And did you check the Oracle documentation for the proper syntax for that? Commented Nov 8, 2020 at 0:19

1 Answer 1

1

There are a host of errors... The ones that jump out at me on a first pass.

  • The requirement doesn't make sense. Adding a column in a procedure doesn't make sense. You create a procedure because you want code to be reusable. Adding a column can only be done once, hence it is by definition not reusable.
  • A procedure has to be compiled before it can be executed. If there is a reference to a column that doesn't exist, the procedure will fail to compile. Thus, if you want to add a column to the table using dynamic SQL, all subsequent references to the column (i.e. your insert statement) would need to use dynamic SQL as well.
  • Your DDL statement is incorrect. There is no add or replace clause, it's alter table product add SalePrice NUMBER(6,2). Note that when you're building your string, you also have to ensure that there is a space between the clause add and the column name SalesPrice-- one of the two strings you're concatenating together would need that.
  • It doesn't make sense to have a declare where you do. You can declare variables between the as and the begin one line above. You are allowed to create a nested PL/SQL block there with the declare but then you'd need a matching begin and end that you don't have.
  • If you're going to use a case statement in PL/SQL, you'd need an end case. You would also need to have a semicolon ; after each expression.
  • Your insert statement is also missing a semicolon.
  • Logically, I am hard pressed to imagine that you really want have an insert here. It doesn't make logical sense to create a bunch of new rows in the table when you add a new column. I would assume that you want to update the value of the new column in existing rows. Which, presumably, requires that your cursor selects the primary key column(s) and potentially changes whether and what you're grouping by.
  • Product and price are being used as local variables in the execute immediate statement and in the case statement but aren't defined. I'm guessing that you just want to hard code the name of the table you're altering and that price is supposed to reference the name of a column in the table that you need to select in your cursor but I'm not sure.

This case statement is syntactically valid (or would be if price resolves to something valid). Many of the other corrections are less obvious because of the reasons I detailed above.

case when p.ProductStandardPrice>=400 
     then NewSalePrice:=.9*price;
     when p.ProductStandardPrice<400 
     THEN NewSalePrice:=.85*price;
  end case;

If I was to speculate at what you actually want (given that this is a homework assignment with requirements that don't actually make sense), I'd guess something like

CREATE or REPLACE PROCEDURE ProductLineSale 
as
begin
  execute immediate 'alter table Product add SalePrice NUMBER(6,2)';
  execute immediate 'update product ' ||
                    '   set SalePrice = (case when ProductStandardPrice >= 400 ' ||
                    '                         then 0.9 * Price ' ||
                    '                         else 0.85 * Price ' ||
                    '                      end) ';
end ProductLineSale;

If you're going to use dynamic SQL, it almost always makes sense to declare a local variable, build the SQL statement in that variable, and then execute it so that you can debug things by printing out the statement you've build to debug it.

Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.