kputnam / piggly

PL/pgSQL stored procedure code coverage tool
Other
69 stars 14 forks source link

Parser issue - DML in WITH query #31

Closed rtaseen closed 7 years ago

rtaseen commented 7 years ago

Got the error below. Initially thought I resolved by placing a space between "--" and "transfer" in the comment "--transfer last to past" but its just that piggly compiles the functions in different orders on different executions, and the error remains. Might be due to "%rowtype" datatype. See below

C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/parser.rb:23:in 'block in parse': Expected one of /*, --, :=, = at line 6, column 8 (byte 144) after declare new_rev     int; (Piggly::Parser::Failure)
        past_record past_statement_value%rowtype;
begin
  --transfer last to past
  with
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/thunk.rb:23:in 'call'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/thunk.rb:23:in 'force!'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/compiler/trace_compiler.rb:27:in 'compile'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:49:in 'block (2 levels) in trace'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:59:in 'call'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:59:in 'serially'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:49:in 'execute'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:53:in 'trace'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:32:in 'main'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/base.rb:15:in 'main'
        from C:/Ruby22-x64/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/bin/piggly:8:in ''
        from C:/Ruby22-x64/bin/piggly:23:in 'load'
        from C:/Ruby22-x64/bin/piggly:23:in '
'
rtaseen commented 7 years ago

So the problem seems to be the WITH query: virtual relations generated by UPDATE or DELETE don't seem to work. e.g. WITH upd AS( UPDATE foo SET bar = 2 WHERE bar = 1 returning bar ) SELECT bar FROM upd; Changing the procedure so that the UPDATE returns into a variable, then subsequently using that variable resolves the issue, although suboptimal.

born-in-brooklyn commented 7 years ago

Hey, running into a similar issue. Was wodering when this might be worked on. As it stands, I'm unable to use piggly against my production code.

kputnam commented 7 years ago

Hi @born-in-brooklyn, if you can provide some sample code that doesn't parse, I could take a closer look at this issue. I don't recall implementing the parsing grammar for WITH or CTEs, so adding it might be straightforward.

born-in-brooklyn commented 7 years ago

sure! here's the function definition:

CREATE FUNCTION risk.func_agrt_firm(p_firm_id INTEGER)
  RETURNS TEXT AS $$
DECLARE
  v_state   TEXT;
  v_message TEXT;
BEGIN
  IF p_firm_id IS NULL THEN
    RAISE EXCEPTION 'Can not recalculate aggregate risk for firm if firm id passed is NULL';
  END IF;

  PERFORM risk.func_agrt_exct_firm_aplbl(p_firm_id);
  PERFORM risk.func_agrt_exct_firm_likelihood(p_firm_id);
  PERFORM risk.func_cmpst_exct_firm_score(p_firm_id);
  RETURN '';

  EXCEPTION
  WHEN OTHERS
    THEN
      GET STACKED DIAGNOSTICS v_message = MESSAGE_TEXT,
      v_state = RETURNED_SQLSTATE;
      RAISE WARNING 'Exception - % -ERROR- %', v_state, v_message;
      RETURN v_message;
END;
$$ LANGUAGE plpgsql;

And here's the error:

C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/parser.rb:24:in `block in parse': Expected one of /*, --, perform at line 17, column 11 (byte 407) after declare (Piggly::Parser::Failure)
  v_state   text;
  v_message text;
begin
  if p_firm_id is null then
    raise exception 'can not recalculate aggregate risk for firm if firm id passed is null';
  end if;

  perform risk.func_agrt_exct_firm_aplbl(p_firm_id);
  perform risk.func_agrt_exct_firm_likelihood(p_firm_id);
  perform risk.func_cmpst_exct_firm_score(p_firm_id);
  return '';

  exception
  when others
    then
      get
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/thunk.rb:23:in `call'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/thunk.rb:23:in `force!'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/compiler/trace_compiler.rb:27:in `compile'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:49:in `block (2 levels) in trace'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:59:in `call'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:59:in `serially'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/util/process_queue.rb:49:in `execute'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:53:in `trace'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:32:in `main'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/base.rb:15:in `main'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/bin/piggly:8:in `<top (required)>'
        from C:/Ruby22/bin/piggly:23:in `load'
        from C:/Ruby22/bin/piggly:23:in `<main>'

Thanks

born-in-brooklyn commented 7 years ago

I think the error might be related to the presence of a call to raise warning. strictly speaking these are probably not necessary in order to make the application work, but I would rather not remove them

kputnam commented 7 years ago

Hi @born-in-brooklyn, I'm not able to reproduce this. Did you install it from RubyGems or from source? My guess is the last RubyGems release is outdated (and doesn't support GET STACKED DIAGNOSTICS from #28), so you could try installing from source.

Let me know if that does or doesn't work! I'll release a new gem once we figure it out.

born-in-brooklyn commented 7 years ago

Hi @kputnam Thanks for this. It took me a while to get around to it, but I was able to test, and the new code you mentioned above certainly helps with the GET STACKED DIAGNOSTICS issue. I was unable to build from source (having windows/ruby problems, and I'm not a terribly adept ruby dev) so I overlayed the changes in the above mentioned PR in the appropriate ruby lib directory, and it worked. I do however have another problem. there are a number of the procs I have to test that use custom types as input parameters. These Procs fail to be analyzed, throwing the following error:

C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/installer.rb:50:in `exec': ERROR:  type risk.type_dmn does not exist (PG::UndefinedObject)

Error installing traced procedure risk.func_cmpst_firm_params from C:/Users/K26962/RAW/comp-workspace/src/test/resources/sql/piggly/cache/Dumper/fd21a1392c8f27328f6cf730d46f459a.plpgsql
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/installer.rb:50:in `trace'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/installer.rb:16:in `block in install'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/installer.rb:14:in `each'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/installer.rb:14:in `install'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:58:in `install'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/trace.rb:33:in `main'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/lib/piggly/command/base.rb:15:in `main'
        from C:/Ruby22/lib/ruby/gems/2.2.0/gems/piggly-2.0.0/bin/piggly:8:in `<top (required)>'
        from C:/Ruby22/bin/piggly:23:in `load'
        from C:/Ruby22/bin/piggly:23:in `<main>'

Does piggly support custom types as input parameters to functions/procs? the type I seem to be having problems with types defined like so:

DROP TYPE IF EXISTS risk.type_dmn;
CREATE TYPE risk.type_dmn AS ENUM ('foo', 'bar');

DROP TYPE IF EXISTS risk.type_cmpst_firm_score;
CREATE TYPE risk.type_cmpst_firm_score AS (
  firm_id INTEGER, cmpst_score NUMERIC(7,4));

and is used in a function like this:

CREATE FUNCTION risk.func_cmpst_store_firm_score(p_dmn risk.type_dmn, p_firm_cmpst_score risk.type_cmpst_firm_score[])
  RETURNS INTEGER AS $$
DECLARE
  v_cnt INTEGER := 0;
BEGIN
/* some magic here */
  RETURN v_cnt;
END;
$$ LANGUAGE plpgsql;

please let me know if you need more info. Thanks again for all your help!

kputnam commented 7 years ago

Hi, @born-in-brooklyn, sorry for the delay. It looks like the problem might be that piggly doesn't recognize the argument types are from the risk schema instead of the default schema.

I changed lib/piggly/command/trace.rb to pretty-print the list of procedures (after I only installed the one you gave).

      def main(argv)
...
        elsif config.dry_run?
          require 'pp'
          pp procedures
          puts procedures.map{|p| p.signature }
          exit 0
        end
...

Which printed this when running ./bin/piggly trace -t -d config/database.yml

[#<Piggly::Dumper::ReifiedProcedure:0x00000101306770
  @arg_defaults=[nil, nil],
  @arg_modes=[],
  @arg_names=
   [#<Piggly::Dumper::QualifiedName:0x00000101306ef0
     @name="p_dmn",
     @schema=nil>,
    #<Piggly::Dumper::QualifiedName:0x00000101306e28
     @name="p_firm_cmpst_score",
     @schema=nil>],
  @arg_types=
   [#<Piggly::Dumper::QualifiedType:0x00000101306bf8
     @name="risk.type_dmn",
     @schema=nil>, #<= HERE
    #<Piggly::Dumper::QualifiedType:0x00000101306b08
     @name="risk.type_cmpst_firm_score[]",
     @schema=nil>], #<= HERE
  @identifier="9d72330dfcac0942efc7c7686a63cc24",
  @name=
   #<Piggly::Dumper::QualifiedName:0x00000101316490
    @name="func_cmpst_store_firm_score",
    @schema="risk">,

Now that I have PostgreSQL installed locally I can work on confirming this and fixing it. Hopefully it'll be a quick fix and I can update the ticket tonight or tomorrow.

born-in-brooklyn commented 7 years ago

@kputnam no worries. Thanks for taking the time to research this. I'm sure this will solve a host of issues for me. Could you build a new Gem once this fix is done? thanks!

kputnam commented 7 years ago

Ok, looks like the issue was that parameters with array types weren't correctly quoted: it should be "foo_type"[] but was "foo_type[]". I also fixed the issue with parameter types' schemas, though it might have worked without that change. Your example seems to be working now, and I've published a new gem (version 2.1.0).

kputnam commented 7 years ago

Seems I was too quick to publish! I just noticed PostgreSQL complains if "integer" is quoted since this is some kind of alias rather than the actual name of the type ("int4"). I'll fix this and publish a new version shortly, 2.2.0.

kputnam commented 7 years ago

Ok, all set now

born-in-brooklyn commented 7 years ago

thank you SO much! Will try first thing tomorrow