search

kputnam / piggly

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

Failure to parse a file containing bigint #49

Open davemurp opened 3 years ago

davemurp commented 3 years ago

A function which declares a bigint is throwing an error when executing the trace command. Env: Docker postgres:12.1-alpine

Extract from problematic function CREATE OR REPLACE FUNCTION test.piggly ( param1 INT default 0 ) RETURNS TABLE ( total_rows bigint ) ...

And the output showing the error... Traceback (most recent call last): 10: from /usr/bin/piggly:23:in <main> 9: from /usr/bin/piggly:23:in 'load' 8: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/bin/piggly:8:in '<top (required)>' 7: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/command/base.rb:15:in 'main' 6: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/command/trace.rb:33:in 'main' 4: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/installer.rb:14:in 'install'all' 2: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/installer.rb:16:in 'block in install' 1: from /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/installer.rb:50:in 'trace' /usr/lib/ruby/gems/2.6.0/gems/piggly-2.3.1/lib/piggly/installer.rb:50:in 'exec': ERROR: syntax error at or near ""int8"" (PG::SyntaxError) LINE 1: ...param1" "int4" default 0, t "total_rows" "int8")

kputnam commented 3 years ago

I think I know what's going on. Sometime after the last piggly release, which was a long time back, postgres added support for TABLE arguments to stored procedures. While your procedure returns a table, under the hood I think it works like an output parameter.

If you are able, do you mind editing this source file and adding an entry to the hash "t" => "table"? I don't know if more is needed, but that may fix your issue. If that works, I'll work on creating some test cases and a new release containing that fix. You are also welcome to submit a PR and I'll merge it, but if you don't want to I'll take care of it.

Be sure to clear any cached piggly data so it will recompile your stored proc if needed.

davemurp commented 3 years ago

Hi,

Thank you for getting back to me so quickly. So to set the scene I need to first of all state I have never worked with ruby before and am struggling with the dependencies. I am trying to get this installed on a docker container. 'gem install piggly' installs fine but throws the error reported.

I've tried building from source but here again I encounter other issues. The install instructions from github are probably out of date and I've been playing around using stack overflow/google to try get the dependencies installed. To no success as of yet, hitting this error. 'rake aborted! NoMethodError: undefined method `last_comment' for

'

Attached is the docker file I am using with most non-pertinent code commented out. The aim is to get the db code into docker and run the pgtap test cases against it and capture the output and hopefully code coverage. If you build it you can see yourself where it is bombing out.

I appreciate any help on this knowing you have probably stepped away from looking at this code for quite some time.

Regards,

Dave

On Wed, Sep 30, 2020 at 5:52 AM kputnam notifications@github.com wrote:

Ah, scratch all that. I think I know what's going on. Sometime after the last release, which was a long time back, postgres added support for TABLE arguments to stored procedures. While your procedure returns a table, under the hood I think it looks like an output parameter.

If you are able, do you mind editing this source file https://github.com/kputnam/piggly/blob/15ee96a7dca4187b2e2b70b87df065f33a033260/lib/piggly/dumper/reified_procedure.rb#L56 and adding an entry to the hash "t" => "table"? I don't know if more is needed, but that may fix your issue.

Be sure to clear any cached piggly data so it will recompile your stored proc if needed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kputnam/piggly/issues/49#issuecomment-701155068, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUJ5ANRDANHFNJ5GGTYOHTSIK2RVANCNFSM4R54IKSQ .

-- David Murphy

kputnam commented 3 years ago

Our problems are aligned! Because I've been away for so long, I don't have PostgreSQL setup or available to test my changes. Before your reply I created a ticket to remind myself to make a Docker with Ruby, PostgreSQL, and piggly so I can work locally without having to set it all up.

I also think it would make sense to deploy piggly as a Docker image for users. Ideally someone can just do docker run kputnam/piggly and people won't need to know or care that Ruby is involved. Because I'm not focused on this project I can't promise speedy progress, but I will set some weekend time aside for it.

If you need it sooner, you can work on it and I'll answer any questions you have. The particular error you pasted I think is due to a new(er) version of rake. I think it's actually rspec that depends on that function, and upgrading rspec should address it. You can probably just comment out rspec from the Gemfile for the time being. Or you could specify an older version of rake, I think < 11.0 would work.

davemurp commented 3 years ago

From my perspective it is very much a nice-to-have to include postgres code coverage as I build a new cicd pipeline for our database code, but not essential. If the tooling becomes available at a later date I can always add it in then.

I will keep playing around with the docker image and if I happen to stumble blindly onto a solution I will definitely let you know. Stackoverflow did point to the older version of rake as being a potential solution and so this is something I will try. Like yourself, this is not top of my priority list but the benefits this could bring to our pipeline means I will keep trying in my spare time to get it to work.

You can see the approach I am going for in the docker file, test using pgtap and then report on unit test and code coverage when complete. Ideally in sonarqube but that is a battle for another day. That is the dream anyway :)

Thanks again for a suggested path forward,

Dave

On Sat, Oct 3, 2020 at 6:28 AM kputnam notifications@github.com wrote:

Our problems are aligned! Because I've been away for so long, I don't have PostgreSQL setup or available to test my changes. Before your reply I created a ticket to remind myself to make a Docker with Ruby, PostgreSQL, and piggly so I can work locally without having to set it all up.

I also think it would make sense to deploy piggly as a Docker image for users. Ideally someone can just do docker run kputnam/piggly and people won't need to know or care that Ruby is involved. Because I'm not focused on this project I can't promise speedy progress, but I will set some weekend time aside for it.

If you need it sooner, you can work on it and I'll answer any questions you have. The particular error you pasted I think is due to a new(er) version of rake. I think it's actually rspec that depends on that function, and upgrading rspec should address it. You can probably just comment out rspec from the Gemfile for the time being. Or you could specify an older version of rake, I think < 11.0 would work.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kputnam/piggly/issues/49#issuecomment-703049880, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUJ5APH637NN46JART3WTTSI2Y7TANCNFSM4R54IKSQ .

-- David Murphy