Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser issue - DML in WITH query #31

Closed
rtaseen opened this issue Apr 3, 2017 · 13 comments
Closed

Parser issue - DML in WITH query #31

rtaseen opened this issue Apr 3, 2017 · 13 comments

Comments

@rtaseen
Copy link

rtaseen commented Apr 3, 2017

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 rtaseen changed the title Parser bug: comments starting with "--" without spacing Parser issue - rowtype? Apr 3, 2017
@rtaseen
Copy link
Author

rtaseen commented Apr 3, 2017

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.

@rtaseen rtaseen changed the title Parser issue - rowtype? Parser issue - DML in WITH query Apr 3, 2017
@born-in-brooklyn
Copy link

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
Copy link
Owner

kputnam commented Jul 28, 2017

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
Copy link

born-in-brooklyn commented Aug 3, 2017

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
Copy link

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
Copy link
Owner

kputnam commented Aug 8, 2017

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
Copy link

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
Copy link
Owner

kputnam commented Sep 7, 2017

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
Copy link

born-in-brooklyn commented Sep 8, 2017

@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 kputnam closed this as completed in d8ea3d9 Sep 8, 2017
@kputnam
Copy link
Owner

kputnam commented Sep 8, 2017

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
Copy link
Owner

kputnam commented Sep 8, 2017

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 kputnam reopened this Sep 8, 2017
@kputnam
Copy link
Owner

kputnam commented Sep 8, 2017

Ok, all set now

@kputnam kputnam closed this as completed Sep 8, 2017
@born-in-brooklyn
Copy link

thank you SO much! Will try first thing tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants